Generate Authenticode for the entire PE file

[esnowberg]

The Authenticode is a hash calculation that excludes parts of the PE file that are altered in the signing process itself. The Authenticode for a PE file should be the same for both the signed and unsigned versions. Both sbsign and pesign have chosen to modify a portion of the PE file that falls outside the actual digital signature. This is done by zero padding the end of the PE file to align the signature table that is later added. After doing this padding, the new zero padded data is included within the Authenticode hash calculation.

Both pesign and sbsign can display the Authenticode for an unsigned binary with the padding included within the calculation. Adding this hash to the MOK does not allow the program to run. The pesign program also has an option to generate the Authenticode without the padding included. Adding this hash to the MOK also does NOT allow the program to run. When shim finds a PE file without a digital signature, it completely stops calculating the hash towards the end of the file. Part of the file is excluded. Testing has shown that the last 3K of the file can be omitted from the calculation.

If the Authenticode is generated using Shim’s MokManager, it will calculate a hash without the last part and allow the program to run. Since the end of the file is not included within the calculation, other things could be added.

Fix all this by hashing the entire file that is outside the digital signature to calculate the Authenticode. Also add zero padding when necessary to do the Authenticode calculation. If the program is signed, this code should never be referenced. However, if this code is entered by a signed PE file, there is potentially something nefarious going on.

link: https://blog.hansenpartnership.com/problems-with-tianocore-after-multi-sign-r14141-fixed/
link: osresearch/sbsigntools@370abb7
link: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/sbsigntools.git/commit/?id=592ec2188f7b9cf003fe7cb0835e93559f19156f

[esnowberg]

I’m sending this initially as an RFC. As probably everyone here already knows, the Authenticode contains a SHA-256 hash of all parts of the code outside of the embedded signature table. From my testing, I would expect to see the same Authenticode for the same PE/COFF program, whether signed or not. When using shim, pesign, sbsign, and MokManager, there can be three different Authenticodes for the same executable.

When using either pesign or sbsign to sign a program, padding is added at the end of the file before the signature table. This zero padding is then included in the Authenticode calculation. When using either pesign or sbsign to display the Authenticode for an unsigned program, the Authenticode returned is not valid. Enrolling this into the MOK, will not allow this program to run. The unsigned program doesn’t contain this padding, the tooling is calculating a hash with padding that doesn’t exist in the file.

The pesign program has “-p” option to not pad the binary. This will produce a different Authenticode. Enrolling this Authenticode into the MOK will also NOT allow the program to run.

From within shim, the MokManager can be loaded. Within MokManager there is an option to generate an Authenticode for a binary and enroll it into the MOK. Doing this will generate yet a third Authenticode and allow the program to run from shim. From my testing, shim is skipping up to 3K of the end of the file when doing the Authenticode calculation for unsigned programs.

This patch adds the padding when calculating the Authenticode for the unsigned program. It is also fixing and hashing the end of the file that was previously being skipped.

It needs to be understood that this padded Authenticode is not compatible with TianoCore. If an Authenticode is generated using TianoCore for an unsigned binary, padding is not included in the calculation. Another option would be to accept the fact that there can be two Authenticodes for the same binary. Then only the first portion of this patch would be necessary. If this were to happen, the default setting for pesign should probably change to not display the padded hash by default.

[jsetje]

Without compatibility this does represent an epoch event. My choice would be to match both the old and new hash, especially for a revocation and print a message if the old hash matches to encourage folks to update. Then at a future time we could deprecate the old algorithm. I do accept that the old algorithm has a substantial gap, so it could also be hidden behind another mok variable or even removed entirely with a substantial heads-up period.

Naively I would have expected firmware and shim to use the same algorithm to determine which part of a file to hash so that the same hashes could be used in db/dbx no matter how a file is loaded. Why isn't that a goal?

This is however not a requirement: Since we have determined that all signed files are padded, this only applies to unsigned files. Unsigned files would have been added to db via hash, and adding that same hash to dbx will revoke that file and it couldn't have been loaded via the other mechanism anyway since the db hash won't match.

Are any of sbsign or pesign capable of producing hashes that match what firmware uses?

[esnowberg]

Naively I would have expected firmware and shim to use the same algorithm to determine which part of a file to hash so that the same hashes could be used in db/dbx no matter how a file is loaded. Why isn't that a goal?

This could be done if the first half of the patch was taken. As I brought up above, we would need to accept the fact that there can be two Authenticodes for the same binary.

Are any of sbsign or pesign capable of producing hashes that match what firmware uses?

Yes, you can use pesign without default padding.

pesign -h -p -i <filename>

This hash could be loaded directly into the db. Tianocore could then boot it directly. Currently shim would not allow it to load with or without this current patch.

I could change the patch to remove the padding calculation. But pesign should be modified to not add the pad within the calculation by default. The added padding pesign does when calculating the hash will only happen with unsigned programs. When the file is actually signed, the zero padding is written to the file. Therefore having it reversed in pesign will show the correct Authenticode in both cases. What may trip up some people is the Authenticodes will not match.

[jsetje]

Thanks!

I suspect @chrisccoulson and/or @xnox will have some opinions here since they've been down this road as well.

[xnox]

Similar issues are being addressed elsewhere too with slightly different approaches. I think kernel arm64 & riscv64 zimg format tries to intentionally pad the image to ensure that it is always aligned and thus it is never needed to pad things. It is sort of a backwards compatible approach.

I wish we could for example, prohibit unpadded things to be ever verified or loaded. And refuse signing thing that require padding. Such that unsigned, signed, stip signature binaries must be round trip safe. And apply this across the board for all binaries (shim, mm, FB, grub, Linux kernel, UKI, fwupd, fwupd capsules, etc).

This is a general observation that can be implemented "backwards" compatible way. On my phone now, so will look into this in more detail when back on my laptop.

[jsetje]

I had the same thought to just enforce padding, but really that ship has sailed. We could start adding that to the builds for various projects that produce artifacts and might get there in some amount of time, but it's not a quick fix.

[vathpela]

I pretty strongly feel (not saying I can't be convinced otherwise) that these things are all true:

• shim should be able to match the padded or unpadded binary (especially in dbx/vendor_dbx/mokx/etc)
• that should be true regardless of the presence of signatutes
• that should be true regardless of the presence of padding
• we can ignore "unpadded but signed", it doesn't appear to happen and it doesn't make sense
• the pesign output should default to being the padded version, because people get confused when signing it changes the digest, which it's fundamentally not supposed to do.

Honestly I think edk2 got this wrong, but mostly because Authenticode failed to specify it well. So I think we have to accept that there are two correct hashes for every binary.

[vathpela]

I wish we could for example, prohibit unpadded things to be ever verified or loaded. And refuse signing thing that require padding. Such that unsigned, signed, stip signature binaries must be round trip safe. And apply this across the board for all binaries (shim, mm, FB, grub, Linux kernel, UKI, fwupd, fwupd capsules, etc).

I wish we could too, but the PE spec and Authenticode just plain got this wrong.

[esnowberg]

I pretty strongly feel (not saying I can't be convinced otherwise) that these things are all true:

• shim should be able to match the padded or unpadded binary (especially in dbx/vendor_dbx/mokx/etc)
• that should be true regardless of the presence of signatutes
• that should be true regardless of the presence of padding

I can see your point here.

• we can ignore "unpadded but signed", it doesn't appear to happen and it doesn't make sense
• the pesign output should default to being the padded version, because people get confused when signing it changes the digest, which it's fundamentally not supposed to do.

I completely agree that fundamentally the Authenticode should match. I guess it comes down to if the Authenticode generated in pesign is only useful when enrolling things into mok or mokx. For me, what is confusing, is the output of pesign can only be used with signed binaries in EDK2. Using pesign with an unsigned binary and enrolling it directly into the db or dbx is not going to match up with the generated Authenticode.

Honestly I think edk2 got this wrong, but mostly because Authenticode failed to specify it well. So I think we have to accept that there are two correct hashes for every binary.

Today, with unsigned binaries, we have a shim that doesn't match up with either of the two Authenticode's being discussed. It skips the end of the file. With that said, if shim worked the same way as EDK2. There wouldn't need to be two Authenticodes being tracked and we wouldn't be breaking anything that shouldn't already be fixed. But this would require pesign's default behavior to change and the zero padding to be removed from this patch.

If there is a strong feeling to track both Authenticodes, I can modify this patch to make that work. It certainly would be more invasive. The Authenticode's pesign displays would only work with shim.

With the recent popularity of UKIs, I believe Authenticode usage is going to become more common.

[jsetje]

It certainly seems unreasonable to change this in edk2 without providing compatibility there, but it may be worth engaging someone there so that we can come up with a combined patch to a less confusing future.

I'm fundamentally in favor of providing compatibility by supporting both methods, at least for a release or two. If the plan is phase out one of the methods, then we should print a warning when it is seen in use.

[jsetje]

@vincentjzimmer we really should get this aligned with edk2. I just heard a rumor about an updated authenticode spec that will hopefully be pushed that may provide some answers.