| Message ID | 20210525033425.972519-4-dja@axtens.net |
|---|---|
| State | Superseded |
| Headers | show |
| Series | secvar cleanups and fixes | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch master (1cdde9466ab658fb5b5a53af8c4e6a8929eef698) |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot | success | Test snowpatch/job/snowpatch-skiboot on branch master |
| snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco | success | Signed-off-by present |
On 5/25/21 05:34, Daniel Axtens wrote: > This code isn't directly used by skiboot, but it is wrong and potentially > insecure so I'm fixing it in case it's used in the future. > > We pass sizeof(hash) into mbedtls_pk_verify(). However, hash is a pointer, > not an array, so rather than passing the length of the hash to verify we'll > pass in 8, and only compare the first 8 bytes of the hash rather than all 32. > > Pass in 0 instead. That tells mbedtls to work out the length based on the > hash type. We allocated enough memory for whatever hash type the PKCS#7 > message declared so this will be safe. > > Signed-off-by: Daniel Axtens <dja@axtens.net> Applied to master. Thanks, C.
diff --git a/libstb/crypto/pkcs7/pkcs7.c b/libstb/crypto/pkcs7/pkcs7.c index 4407e201a4cc..3f41ba7acb2e 100644 --- a/libstb/crypto/pkcs7/pkcs7.c +++ b/libstb/crypto/pkcs7/pkcs7.c @@ -538,7 +538,7 @@ int mbedtls_pkcs7_signed_data_verify( mbedtls_pkcs7 *pkcs7, mbedtls_md( md_info, data, datalen, hash ); - ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, sizeof(hash), + ret = mbedtls_pk_verify( &pk_cxt, md_alg, hash, 0, pkcs7->signed_data.signers.sig.p, pkcs7->signed_data.signers.sig.len );
This code isn't directly used by skiboot, but it is wrong and potentially insecure so I'm fixing it in case it's used in the future. We pass sizeof(hash) into mbedtls_pk_verify(). However, hash is a pointer, not an array, so rather than passing the length of the hash to verify we'll pass in 8, and only compare the first 8 bytes of the hash rather than all 32. Pass in 0 instead. That tells mbedtls to work out the length based on the hash type. We allocated enough memory for whatever hash type the PKCS#7 message declared so this will be safe. Signed-off-by: Daniel Axtens <dja@axtens.net> --- libstb/crypto/pkcs7/pkcs7.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)