mbox series

[v2,0/5] secvar cleanups and fixes

Message ID 20210621082641.26476-1-dja@axtens.net
Headers show
Series secvar cleanups and fixes | expand

Message

Daniel Axtens June 21, 2021, 8:26 a.m. UTC 
We recently found that our mbedtls PKCS#7 implementation has a potentially
nasty bug. It turns out we don't use the affected function in skiboot,
but we want to fix it anyway in case we use it in future.

v1: https://lists.ozlabs.org/pipermail/skiboot/2021-May/017534.html

Nayna suggested that we might want to do a more thorough job while we're
at it, and the more I look at things the more I want to change!

The first 3 patches are fairly trivial cleanups. Patch 5 is the mbedtls
fix: it's dead code but is included to prevent any future issues.

The bulk of my changes are in patch 4, where I attempt to make sure we
only handle PKCS#7 messages that embed a sha256 signature. My main
concern is that if we ever were passed an auth structure which embedded
a PKCS#7 message with a sha512 signature, we would end up overreading
our hash buffer because mbedtls has a curious disregard for the supplied
hash length. I also try - not entirely successfully - to future-proof us
against future mbedtls updates.

As usual, I can't test this.

Daniel Axtens (5):
  secvar/backend: rename verify_signature parameters
  secvar/backend: clarify variables in process_update
  secvar/backend: fix comment of get_hash_to_verify
  secvar/backend: redo hash algorithm handling for auth structures
  secvar/pkcs7: fix a wrong sizeof()

 libstb/crypto/pkcs7/pkcs7.c                 |  2 +-
 libstb/secvar/backend/edk2-compat-process.c | 35 ++++++++++++++-------
 2 files changed, 24 insertions(+), 13 deletions(-)

Comments

Vasant Hegde July 27, 2021, 11:19 a.m. UTC | #1 
On 6/21/21 1:56 PM, Daniel Axtens wrote:
> We recently found that our mbedtls PKCS#7 implementation has a potentially
> nasty bug. It turns out we don't use the affected function in skiboot,
> but we want to fix it anyway in case we use it in future.
> 
> v1: https://lists.ozlabs.org/pipermail/skiboot/2021-May/017534.html
> 
> Nayna suggested that we might want to do a more thorough job while we're
> at it, and the more I look at things the more I want to change!

I have merged 1-3 patches from this series to master as of 54f52f163.
Patch 4 is superseded.
Patch 5, I hope you will address review comments and respin this patch.


-Vasant