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 000000000..f2c72b0ec --- /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 fcc2ed21e..690d99d95 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1304,6 +1304,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, 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 40c22f53b..4c0920f39 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -206,7 +206,11 @@ cli-rsa-sha256.crt.der: cli-rsa-sha256.crt $(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 000000000..c40ba2a44 Binary files /dev/null and b/tests/data_files/cli-rsa-sha256-badalg.crt.der differ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 1b0316eb8..d368ef4f4 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -2644,6 +2644,10 @@ X509 File parse (trailing spaces, OK) 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