diff --git a/ChangeLog.d/x509-ec-algorithm-identifier-fix.txt b/ChangeLog.d/x509-ec-algorithm-identifier-fix.txt new file mode 100644 index 000000000..c1de491e6 --- /dev/null +++ b/ChangeLog.d/x509-ec-algorithm-identifier-fix.txt @@ -0,0 +1,4 @@ +Bugfix + * Fix x509 certificate generation to conform to RFC 5480 / RFC 5758 when + using ECC key. The certificate was rejected by some crypto frameworks. + Fixes #2924. diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index 6e1f5b630..e21356f64 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -503,7 +503,8 @@ int mbedtls_x509_write_names(unsigned char **p, unsigned char *start, mbedtls_asn1_named_data *first); int mbedtls_x509_write_sig(unsigned char **p, unsigned char *start, const char *oid, size_t oid_len, - unsigned char *sig, size_t size); + unsigned char *sig, size_t size, + mbedtls_pk_type_t pk_alg); int mbedtls_x509_get_ns_cert_type(unsigned char **p, const unsigned char *end, unsigned char *ns_cert_type); diff --git a/library/x509_create.c b/library/x509_create.c index cdfc82aa5..bd772d3ac 100644 --- a/library/x509_create.c +++ b/library/x509_create.c @@ -285,9 +285,11 @@ int mbedtls_x509_write_names(unsigned char **p, unsigned char *start, int mbedtls_x509_write_sig(unsigned char **p, unsigned char *start, const char *oid, size_t oid_len, - unsigned char *sig, size_t size) + unsigned char *sig, size_t size, + mbedtls_pk_type_t pk_alg) { int ret = MBEDTLS_ERR_ERROR_CORRUPTION_DETECTED; + int write_null_par; size_t len = 0; if (*p < start || (size_t) (*p - start) < size) { @@ -310,8 +312,19 @@ int mbedtls_x509_write_sig(unsigned char **p, unsigned char *start, // Write OID // - MBEDTLS_ASN1_CHK_ADD(len, mbedtls_asn1_write_algorithm_identifier(p, start, oid, - oid_len, 0)); + if (pk_alg == MBEDTLS_PK_ECDSA) { + /* + * The AlgorithmIdentifier's parameters field must be absent for DSA/ECDSA signature + * algorithms, see https://www.rfc-editor.org/rfc/rfc5480#page-17 and + * https://www.rfc-editor.org/rfc/rfc5758#section-3. + */ + write_null_par = 0; + } else { + write_null_par = 1; + } + MBEDTLS_ASN1_CHK_ADD(len, + mbedtls_asn1_write_algorithm_identifier_ext(p, start, oid, oid_len, + 0, write_null_par)); return (int) len; } diff --git a/library/x509write_crt.c b/library/x509write_crt.c index bcee4dcca..3586a3c96 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -577,6 +577,7 @@ int mbedtls_x509write_crt_der(mbedtls_x509write_cert *ctx, size_t sub_len = 0, pub_len = 0, sig_and_oid_len = 0, sig_len; size_t len = 0; mbedtls_pk_type_t pk_alg; + int write_sig_null_par; /* * Prepare data to be signed at the end of the target buffer @@ -668,9 +669,20 @@ int mbedtls_x509write_crt_der(mbedtls_x509write_cert *ctx, /* * Signature ::= AlgorithmIdentifier */ + if (pk_alg == MBEDTLS_PK_ECDSA) { + /* + * The AlgorithmIdentifier's parameters field must be absent for DSA/ECDSA signature + * algorithms, see https://www.rfc-editor.org/rfc/rfc5480#page-17 and + * https://www.rfc-editor.org/rfc/rfc5758#section-3. + */ + write_sig_null_par = 0; + } else { + write_sig_null_par = 1; + } MBEDTLS_ASN1_CHK_ADD(len, - mbedtls_asn1_write_algorithm_identifier(&c, buf, - sig_oid, strlen(sig_oid), 0)); + mbedtls_asn1_write_algorithm_identifier_ext(&c, buf, + sig_oid, strlen(sig_oid), + 0, write_sig_null_par)); /* * Serial ::= INTEGER @@ -762,8 +774,8 @@ int mbedtls_x509write_crt_der(mbedtls_x509write_cert *ctx, * into the CRT buffer. */ c2 = buf + size; MBEDTLS_ASN1_CHK_ADD(sig_and_oid_len, mbedtls_x509_write_sig(&c2, c, - sig_oid, sig_oid_len, sig, - sig_len)); + sig_oid, sig_oid_len, + sig, sig_len, pk_alg)); /* * Memory layout after this step: diff --git a/library/x509write_csr.c b/library/x509write_csr.c index b67cdde28..5d3d17658 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -363,7 +363,7 @@ static int x509write_csr_der_internal(mbedtls_x509write_csr *ctx, c2 = buf + size; MBEDTLS_ASN1_CHK_ADD(sig_and_oid_len, mbedtls_x509_write_sig(&c2, buf + len, sig_oid, sig_oid_len, - sig, sig_len)); + sig, sig_len, pk_alg)); /* * Compact the space between the CSR data and signature by moving the diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile index eff44d8ac..5230a30d9 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -1385,7 +1385,7 @@ all_final += server5-badsign.crt # The use of 'Server 1' in the DN is intentional here, as the DN is hardcoded in the x509_write test suite.' server5.req.ku.sha1: server5.key - $(MBEDTLS_CERT_REQ) output_file=$@ filename=$< key_usage=digital_signature,non_repudiation subject_name="C=NL,O=PolarSSL,CN=PolarSSL Server 1" md=SHA1 + $(OPENSSL) req -key $< -out $@ -new -nodes -subj "/C=NL/O=PolarSSL/CN=PolarSSL Server 1" -sha1 -addext keyUsage=digitalSignature,nonRepudiation all_final += server5.req.ku.sha1 # server6* diff --git a/tests/data_files/Readme-x509.txt b/tests/data_files/Readme-x509.txt index 84c775fd9..82f93d230 100644 --- a/tests/data_files/Readme-x509.txt +++ b/tests/data_files/Readme-x509.txt @@ -76,6 +76,10 @@ List of certificates: -badsign.crt: S5 with corrupted signature -expired.crt: S5 with "not after" date in the past -future.crt: S5 with "not before" date in the future + -non-compliant.crt: S5, RFC non-compliant + (with forbidden EC algorithm identifier NULL parameter) + generated by (before fix): + cert_write subject_key=server5.key subject_name="CN=Test EC RFC non-compliant" issuer_crt=test-ca2.crt issuer_key=test-ca2.key -selfsigned.crt: Self-signed cert with S5 key -ss-expired.crt: Self-signed cert with S5 key, expired -ss-forgeca.crt: Copy of test-int-ca3 self-signed with S5 key diff --git a/tests/data_files/parse_input/server5-non-compliant.crt b/tests/data_files/parse_input/server5-non-compliant.crt new file mode 100644 index 000000000..abea17ddc --- /dev/null +++ b/tests/data_files/parse_input/server5-non-compliant.crt @@ -0,0 +1,12 @@ +-----BEGIN CERTIFICATE----- +MIIBwjCCAUagAwIBAgIBATAMBggqhkjOPQQDAgUAMD4xCzAJBgNVBAYTAk5MMREw +DwYDVQQKDAhQb2xhclNTTDEcMBoGA1UEAwwTUG9sYXJzc2wgVGVzdCBFQyBDQTAe +Fw0wMTAxMDEwMDAwMDBaFw0zMDEyMzEyMzU5NTlaMCQxIjAgBgNVBAMMGVRlc3Qg +RUMgUkZDIG5vbi1jb21wbGlhbnQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAAQ3 +zFbZdgkeWnI+x1kt/yBu7nz5BpF00K0UtfdoIllikk7lANgjEf/qL9I0XV0WvYqI +wmt3DVXNiioO+gHItO3/o00wSzAJBgNVHRMEAjAAMB0GA1UdDgQWBBRQYaWP1AfZ +14IBDOVlf4xjRqcTvjAfBgNVHSMEGDAWgBSdbSAkSQE/K8t4tRm8fiTJ2/s2fDAM +BggqhkjOPQQDAgUAA2gAMGUCMAJ3J/DooFSaBG2OhzyWai32q6INDZfoS2bToSKf +gy6hbJiIX/G9eFts5+BJQ3QpjgIxALRmIgdR91BDdqpeF5JCmhgjbfbgMQ7mrMeS +ZGfNyFyjS75QnIA6nKryQmgPXo+sCQ== +-----END CERTIFICATE----- diff --git a/tests/data_files/server5.req.ku.sha1 b/tests/data_files/server5.req.ku.sha1 index 3281c9460..c73a0e27d 100644 --- a/tests/data_files/server5.req.ku.sha1 +++ b/tests/data_files/server5.req.ku.sha1 @@ -1,8 +1,8 @@ -----BEGIN CERTIFICATE REQUEST----- -MIIBFjCBvAIBADA8MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9sYXJTU0wxGjAY +MIIBFDCBvAIBADA8MQswCQYDVQQGEwJOTDERMA8GA1UECgwIUG9sYXJTU0wxGjAY BgNVBAMMEVBvbGFyU1NMIFNlcnZlciAxMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcD QgAEN8xW2XYJHlpyPsdZLf8gbu58+QaRdNCtFLX3aCJZYpJO5QDYIxH/6i/SNF1d Fr2KiMJrdw1VzYoqDvoByLTt/6AeMBwGCSqGSIb3DQEJDjEPMA0wCwYDVR0PBAQD -AgbAMAsGByqGSM49BAEFAANIADBFAiEAnIKF+xKk0iEuN4MHd4FZWNvrznLQgkeg -2n8ejjreTzcCIAH34z2TycuMpWQRhpV+YT988pBWR67LAg7REyZnjSAB +AgbAMAkGByqGSM49BAEDSAAwRQIhAJyChfsSpNIhLjeDB3eBWVjb685y0IJHoNp/ +Ho463k83AiAB9+M9k8nLjKVkEYaVfmE/fPKQVkeuywIO0RMmZ40gAQ== -----END CERTIFICATE REQUEST----- diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 3d092db6e..7af9de9cf 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -3115,6 +3115,14 @@ X509 File parse (Algorithm Params Tag mismatch) depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C mbedtls_x509_crt_parse_file:"data_files/parse_input/cli-rsa-sha256-badalg.crt.der":MBEDTLS_ERR_X509_SIG_MISMATCH:0 +X509 File parse (does not conform to RFC 5480 / RFC 5758 - AlgorithmIdentifier's parameters field is present, mbedTLS generated before bugfix, OK) +depends_on:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_MD_CAN_SHA256 +x509parse_crt_file:"data_files/parse_input/server5-non-compliant.crt":0 + +X509 File parse (conforms to RFC 5480 / RFC 5758 - AlgorithmIdentifier's parameters field must be absent for ECDSA) +depends_on:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_MD_CAN_SHA256 +x509parse_crt_file:"data_files/parse_input/server5.crt":0 + 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