Do not Fatal in ReadSignatureList

[Itxaka]

Running a Fatal in here means that the program calling this will fail inmediatly, which can break anything using this as a lib as there is no way of catching a call to os.Exit

Returning an error seems to be a much nicer way of going, so the caller can deal with that error directly and decide if it wants to stop execution or continue

[Foxboron]

Why are you reaching the "Fatal" clause?

[Itxaka]

Why are you reaching the "Fatal" clause?

Seems to be triggered when running under VMWARE VMs with EFI. And it triggers the default clause, so probably the signatures are not x509/SHA256 format?

Or its not fully reading the signs and failing to identify them?

I think it should not matter though, Fataling there means that whatever process is using this function will immediately exit with no way to recover. Even a panic would be better as someone can call recover and deal with it internally. In our case, because the installer is gathering all the machine state before doing the install, the installer gets exited with no way of dealing with it somehow.

[Foxboron]

Seems to be triggered when running under VMWARE VMs with EFI. And it triggers the default clause, so probably the signatures are not x509/SHA256 format?

Okay, but it would be nice to figure this out and support this.

I think it should not matter though, Fataling there means that whatever process is using this function will immediately exit with no way to recover. Even a panic would be better as someone can call recover and deal with it internally. In our case, because the installer is gathering all the machine state before doing the install, the installer gets exited with no way of dealing with it somehow.

This change is not really going to solve anything as the state collection you are doing is just going to be an empty array? So even if you want the panic removed, returning nil is going to be problematic regardless I reckon?

[Itxaka]

Okay, but it would be nice to figure this out and support this.

Yep indeed, no idea how can we figure it out how its happening. From what I could gather the VMWARE bundled caerts ARE SHA256 and they ship the usual Microsoft ones plus a custom one from VMware so there should be no reason for them to fail? Any ideas what to looks for?

This change is not really going to solve anything as the state collection you are doing is just going to be an empty array? So even if you want the panic removed, returning nil is going to be problematic regardless I reckon?

Not really. If an error is returned and you check the error then you can decide what to do based on that, either panic or skip. In our case, because we iterate over the certs found, if the object its empty we just dont do anything, as its optional info not critical to our install or anything. So we dont even need to check if there is an error :D

[Itxaka]

Got more info, this is the signature type of the signs that fails:

{1160678637 57343 19340 [174 1 81 24 134 46 104 44]}

This is not in the ValidEFISignatureSchemes so the sig is empty, hence why it fails to parse it and goes to the default case

[Itxaka]

And this is the full SignatureList

signature.SignatureList{
  SignatureType: util.EFIGUID{
    Data1: 1160678637,
    Data2: 57343,
    Data3: 19340,
    Data4: [8]uint8{
      174,
      1,
      81,
      24,
      134,
      46,
      104,
      44,
    },
  },
  ListSize: 45,
  HeaderSize: 0,
  Size: 17,
  SignatureHeader: nil,
  Signatures: nil,
}

[Itxaka]

Im thikning that something went wrong and PK is broken somehow, while KEK and db looks good checking the efivars, the PK looks terribly small

[Itxaka]

yeah the core for this issue may be a wrong system with a wrong or empty PK.

Still the issue with running a Fatal from a potential consumed lib is there IMHO. So Im guessing an empty db or kek might also trigger this?

[Foxboron]

Still the issue with running a Fatal from a potential consumed lib is there IMHO. So Im guessing an empty db or kek might also trigger this?

It shouldn't. It would be nice to get a copy of the file, it would be easier to test.

[Itxaka]

Still the issue with running a Fatal from a potential consumed lib is there IMHO. So Im guessing an empty db or kek might also trigger this?

It shouldn't. It would be nice to get a copy of the file, it would be easier to test.

Im trying to get copies of the full db/kek/pk and its default from the efivars but I cant access the machine directly. As soon as I get them I;ll attache them here.

[Itxaka]

@Foxboron here are the db/kek/pk files and their defaults extracted directly from the efivars (with a simple cp)

certs.zip

[Foxboron]

It turns out the pkdefault has a CERT_EXTERNAL_MANAGEMENT_GUID which is effectivelly a zero sized certificate we can ignore. I've implemented support for that by just reading and skipping the element in the list.

Added the certs as testdata in the project as well.

Thanks!