Fixed AlgorithmIdentifier parameters when used with ECDSA signature algorithm in x509 certificate
Signed-off-by: Marek Jansta <jansta@2n.cz>
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 0000000..cd216da
--- /dev/null
+++ b/ChangeLog.d/x509-ec-algorithm-identifier-fix.txt
@@ -0,0 +1,4 @@
+Bugfix
+ * Fix x509 certificate generation to conform to RFCs 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 df6d762..fffa475 100644
--- a/include/mbedtls/x509.h
+++ b/include/mbedtls/x509.h
@@ -478,7 +478,8 @@
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 50db956..da10c39 100644
--- a/library/x509_create.c
+++ b/library/x509_create.c
@@ -282,9 +282,11 @@
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) {
@@ -307,8 +309,19 @@
// 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 59fd589..04ff624 100644
--- a/library/x509write_crt.c
+++ b/library/x509write_crt.c
@@ -577,6 +577,7 @@
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 @@
/*
* 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 @@
* 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 d792d34..4622d7a 100644
--- a/library/x509write_csr.c
+++ b/library/x509write_csr.c
@@ -363,7 +363,7 @@
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 0c2fa141..89a61c2 100644
--- a/tests/data_files/Makefile
+++ b/tests/data_files/Makefile
@@ -1395,7 +1395,7 @@
# 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 84c775f..82f93d2 100644
--- a/tests/data_files/Readme-x509.txt
+++ b/tests/data_files/Readme-x509.txt
@@ -76,6 +76,10 @@
-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 0000000..abea17d
--- /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 3281c94..c73a0e2 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 edb7824..cbc4648 100644
--- a/tests/suites/test_suite_x509parse.data
+++ b/tests/suites/test_suite_x509parse.data
@@ -3101,6 +3101,14 @@
depends_on:MBEDTLS_MD_CAN_SHA256:MBEDTLS_RSA_C
x509parse_crt_file:"data_files/parse_input/cli-rsa-sha256-badalg.crt.der":MBEDTLS_ERR_X509_SIG_MISMATCH
+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