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 acda1f22c..2334a35dc 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -929,6 +929,7 @@ static int x509_crt_parse_der_core( mbedtls_x509_crt *crt, const unsigned char * 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 61edad1d4..d76aa5d7f 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -129,6 +129,14 @@ cli-rsa-sha256.crt: $(cli_crt_key_file_rsa) test-ca-sha256.crt cli-rsa.csr $(OPENSSL) x509 -req -extfile $(cli_crt_extensions_file) -extensions cli-rsa -CA test-ca-sha256.crt -CAkey $(test_ca_key_file_rsa) -passin "pass:$(test_ca_pwd_rsa)" -set_serial 4 -days 3653 -sha256 -in cli-rsa.csr -out $@ all_final += cli-rsa-sha256.crt +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-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 + server2-rsa.csr: server2.key $(OPENSSL) req -new -key server2.key -passin "pass:$(test_ca_pwd_rsa)" -subj "/C=NL/O=PolarSSL/CN=localhost" -out $@ all_intermediate += server2-rsa.csr 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..be75255ac 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 b87859ec6..97eeac87f 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1864,6 +1864,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