[Skiboot] [PATCH v2] secvar/backend: require sha256 in our PKCS#7 messages

[Daniel Axtens]

We only handle sha256 hashes in auth structures.

In the process of verifying an auth structure, we extract the pkcs7
message and we calculate the hopefully-matching hash, which is
sha256(name || vendor guid || attributes || timestamp || newcontent)
We then verify that the PKCS#7 signature matches that calculated hash.

However, at no point do we check that the PKCS#7 hash algorithm is
sha256. So if the PKCS#7 message says that it is a signature on a sha512,
mbedtls will compare 64 bytes of hash from the signature with 64 bytes
from our hash, resulting in a 32 byte overread.

Verify that the hash algorithm in the PKCS#7 message is sha256.

Signed-off-by: Daniel Axtens <dja at axtens.net>

---

This is the minimal fix for the underlying bug. It should probably
go in ahead of any potential future reworking of the area.

v2: thanks Nick and Nayna for your feedback. Added error messages
    and properly cleaned up the pkcs7 structure.

As always, compile tested only because I don't have access to a box
set up to test this.
---
 libstb/secvar/backend/edk2-compat-process.c | 22 +++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/libstb/secvar/backend/edk2-compat-process.c b/libstb/secvar/backend/edk2-compat-process.c
index 244f23403fe0..df9753245014 100644
--- a/libstb/secvar/backend/edk2-compat-process.c
+++ b/libstb/secvar/backend/edk2-compat-process.c
@@ -11,6 +11,7 @@
 #include <stdint.h>
 #include <ccan/endian/endian.h>
 #include <mbedtls/error.h>
+#include <mbedtls/oid.h>
 #include <device.h>
 #include <assert.h>
 #include "libstb/crypto/pkcs7/pkcs7.h"
@@ -460,6 +461,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 {
 	mbedtls_pkcs7 *pkcs7 = NULL;
 	mbedtls_x509_crt x509;
+	mbedtls_md_type_t md_alg;
 	char *signing_cert = NULL;
 	char *x509_buf = NULL;
 	int signing_cert_size;
@@ -478,6 +480,25 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 	if (!pkcs7)
 		return OPAL_PARAMETER;
 
+	/*
+	 * We only support sha256, which has a hash length of 32.
+	 * If the alg is not sha256, then we should bail now.
+	 */
+	rc = mbedtls_oid_get_md_alg(&pkcs7->signed_data.digest_alg_identifiers,
+				    &md_alg);
+	if (rc != 0) {
+		prlog(PR_ERR, "Failed to get the Digest Algorithm Identifier: %d\n", rc);
+		rc = OPAL_PARAMETER;
+		goto err_pkcs7;
+	}
+
+	if (md_alg != MBEDTLS_MD_SHA256) {
+		prlog(PR_ERR, "Unexpected digest algorithm: expected %d (SHA-256), got %d\n",
+		      MBEDTLS_MD_SHA256, md_alg);
+		rc = OPAL_PARAMETER;
+		goto err_pkcs7;
+	}
+
 	prlog(PR_INFO, "Load the signing certificate from the keystore");
 
 	eslvarsize = avar->data_size;
@@ -562,6 +583,7 @@ static int verify_signature(const struct efi_variable_authentication_2 *auth,
 	}
 
 	free(signing_cert);
+err_pkcs7:
 	mbedtls_pkcs7_free(pkcs7);
 	free(pkcs7);
 
-- 
2.30.2