Add tag check to cert algorithm check
Add missing tag check for algorithm parameters when comparing the
signature in the description part of the cert against the actual
signature whilst loading a certificate. This was found by a
certificate (created by fuzzing) that openssl would not verify, but
mbedtls would.
Regression test added (one of the client certs modified accordingly)
Signed-off-by: Paul Elliott <paul.elliott@arm.com>
diff --git a/ChangeLog.d/x509-add-tag-check-to-algorithm-params b/ChangeLog.d/x509-add-tag-check-to-algorithm-params
new file mode 100644
index 0000000..f2c72b0
--- /dev/null
+++ b/ChangeLog.d/x509-add-tag-check-to-algorithm-params
@@ -0,0 +1,11 @@
+Security
+ * Fix a compliance issue whereby we were not checking the tag on the
+ algorithm parameters (only the size) when comparing the signature in the
+ description part of the cert to the real signature. This meant that a
+ NULL algorithm parameters entry would look identical to an array of REAL
+ (size zero) to the library and thus the certificate would be considered
+ valid. However, if the parameters do not match in *any* way then the
+ certificate should be considered invalid, and indeed OpenSSL marks these
+ certs as invalid when mbedtls did not.
+ Many thanks to guidovranken who found this issue via differential fuzzing
+ and reported it in #3629.
diff --git a/library/x509_crt.c b/library/x509_crt.c
index fcc2ed2..690d99d 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -1304,6 +1304,7 @@
if( crt->sig_oid.len != sig_oid2.len ||
memcmp( crt->sig_oid.p, sig_oid2.p, crt->sig_oid.len ) != 0 ||
+ sig_params1.tag != sig_params2.tag ||
sig_params1.len != sig_params2.len ||
( sig_params1.len != 0 &&
memcmp( sig_params1.p, sig_params2.p, sig_params1.len ) != 0 ) )
diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile
index 40c22f5..4c0920f 100644
--- a/tests/data_files/Makefile
+++ b/tests/data_files/Makefile
@@ -206,7 +206,11 @@
$(OPENSSL) x509 -in $< -out $@ -inform PEM -outform DER
all_final += cli-rsa-sha256.crt.der
- cli-rsa.key.der: $(cli_crt_key_file_rsa)
+cli-rsa-sha256-badalg.crt.der: cli-rsa-sha256.crt.der
+ hexdump -ve '1/1 "%.2X"' $< | sed "s/06092A864886F70D01010B0500/06092A864886F70D01010B0900/2" | xxd -r -p > $@
+all_final += cli-rsa-sha256-badalg.crt.der
+
+cli-rsa.key.der: $(cli_crt_key_file_rsa)
$(OPENSSL) pkey -in $< -out $@ -inform PEM -outform DER
all_final += cli-rsa.key.der
diff --git a/tests/data_files/cli-rsa-sha256-badalg.crt.der b/tests/data_files/cli-rsa-sha256-badalg.crt.der
new file mode 100644
index 0000000..c40ba2a
--- /dev/null
+++ b/tests/data_files/cli-rsa-sha256-badalg.crt.der
Binary files differ
diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data
index 1b0316e..d368ef4 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -2644,6 +2644,10 @@
depends_on:MBEDTLS_ECDSA_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
x509parse_crt_file:"data_files/server7_trailing_space.crt":0
+X509 File parse (Algorithm Params Tag mismatch)
+depends_on:MBEDTLS_SHA256_C:MBEDTLS_RSA_C
+x509parse_crt_file:"data_files/cli-rsa-sha256-badalg.crt.der":MBEDTLS_ERR_X509_SIG_MISMATCH
+
X509 Get time (UTC no issues)
depends_on:MBEDTLS_X509_USE_C
x509_get_time:MBEDTLS_ASN1_UTC_TIME:"500101000000Z":0:1950:1:1:0:0:0
