Fix parsing of PKCS#8 encoded Elliptic Curve keys.
The relevant ASN.1 definitions for a PKCS#8 encoded Elliptic Curve key are:
PrivateKeyInfo ::= SEQUENCE {
version Version,
privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
privateKey PrivateKey,
attributes [0] IMPLICIT Attributes OPTIONAL
}
AlgorithmIdentifier ::= SEQUENCE {
algorithm OBJECT IDENTIFIER,
parameters ANY DEFINED BY algorithm OPTIONAL
}
ECParameters ::= CHOICE {
namedCurve OBJECT IDENTIFIER
-- implicitCurve NULL
-- specifiedCurve SpecifiedECDomain
}
ECPrivateKey ::= SEQUENCE {
version INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
privateKey OCTET STRING,
parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
publicKey [1] BIT STRING OPTIONAL
}
Because of the two optional fields, there are 4 possible variants that need to
be parsed: no optional fields, only parameters, only public key, and both
optional fields. Previously mbedTLS was unable to parse keys with "only
parameters". Also, only "only public key" was tested. There was a test for "no
optional fields", but it was labelled incorrectly as SEC.1 and not run because
of a great renaming mixup.
diff --git a/ChangeLog b/ChangeLog
index e0b016d..b2bee2b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,12 @@
mbed TLS ChangeLog (Sorted per branch, date)
+= mbed TLS x.x.x branch released xxx-xx-xx
+
+Bugfix
+ * Fix parsing of PKCS#8 encoded Elliptic Curve keys. Previously Mbed TLS was
+ unable to parse keys with only the optional parameters field of the
+ ECPrivateKey structure. Found by jethrogb, fixed in #1379.
+
= mbed TLS 2.8.0 branch released 2018-03-16
Default behavior changes
diff --git a/library/pkparse.c b/library/pkparse.c
index 9022db2..5ad5edf 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -861,7 +861,10 @@
mbedtls_ecp_keypair_free( eck );
return( MBEDTLS_ERR_PK_KEY_INVALID_FORMAT + ret );
}
+ }
+ if( p != end )
+ {
/*
* Is 'publickey' present? If not, or if we can't read it (eg because it
* is compressed), create it from the private key.
diff --git a/tests/data_files/Makefile b/tests/data_files/Makefile
index 59516ba..f9832a0 100644
--- a/tests/data_files/Makefile
+++ b/tests/data_files/Makefile
@@ -578,7 +578,86 @@
### Generate all RSA keys
keys_rsa_all: keys_rsa_unenc keys_rsa_enc_basic keys_rsa_enc_pkcs8_v1 keys_rsa_enc_pkcs8_v2
+################################################################
+#### Generate various EC keys
+################################################################
+###
+### PKCS8 encoded
+###
+
+ec_prv.pk8.der:
+ $(OPENSSL) genpkey -algorithm EC -pkeyopt ec_paramgen_curve:prime192v1 -pkeyopt ec_param_enc:named_curve -out $@ -outform DER
+all_final += ec_prv.pk8.der
+
+# ### Instructions for creating `ec_prv.pk8nopub.der`,
+# ### `ec_prv.pk8nopubparam.der`, and `ec_prv.pk8param.der` by hand from
+# ### `ec_prv.pk8.der`.
+#
+# These instructions assume you are familiar with ASN.1 DER encoding and can
+# use a hex editor to manipulate DER.
+#
+# The relevant ASN.1 definitions for a PKCS#8 encoded Elliptic Curve key are:
+#
+# PrivateKeyInfo ::= SEQUENCE {
+# version Version,
+# privateKeyAlgorithm PrivateKeyAlgorithmIdentifier,
+# privateKey PrivateKey,
+# attributes [0] IMPLICIT Attributes OPTIONAL
+# }
+#
+# AlgorithmIdentifier ::= SEQUENCE {
+# algorithm OBJECT IDENTIFIER,
+# parameters ANY DEFINED BY algorithm OPTIONAL
+# }
+#
+# ECParameters ::= CHOICE {
+# namedCurve OBJECT IDENTIFIER
+# -- implicitCurve NULL
+# -- specifiedCurve SpecifiedECDomain
+# }
+#
+# ECPrivateKey ::= SEQUENCE {
+# version INTEGER { ecPrivkeyVer1(1) } (ecPrivkeyVer1),
+# privateKey OCTET STRING,
+# parameters [0] ECParameters {{ NamedCurve }} OPTIONAL,
+# publicKey [1] BIT STRING OPTIONAL
+# }
+#
+# `ec_prv.pk8.der` as generatde above by OpenSSL should have the following
+# fields:
+#
+# * privateKeyAlgorithm namedCurve
+# * privateKey.parameters NOT PRESENT
+# * privateKey.publicKey PRESENT
+# * attributes NOT PRESENT
+#
+# # ec_prv.pk8nopub.der
+#
+# Take `ec_prv.pk8.der` and remove `privateKey.publicKey`.
+#
+# # ec_prv.pk8nopubparam.der
+#
+# Take `ec_prv.pk8nopub.der` and add `privateKey.parameters`, the same value as
+# `privateKeyAlgorithm.namedCurve`. Don't forget to add the explicit tag.
+#
+# # ec_prv.pk8param.der
+#
+# Take `ec_prv.pk8.der` and add `privateKey.parameters`, the same value as
+# `privateKeyAlgorithm.namedCurve`. Don't forget to add the explicit tag.
+
+ec_prv.pk8.pem: ec_prv.pk8.der
+ $(OPENSSL) pkey -in $< -inform DER -out $@
+all_final += ec_prv.pk8.pem
+ec_prv.pk8nopub.pem: ec_prv.pk8nopub.der
+ $(OPENSSL) pkey -in $< -inform DER -out $@
+all_final += ec_prv.pk8nopub.pem
+ec_prv.pk8nopubparam.pem: ec_prv.pk8nopubparam.der
+ $(OPENSSL) pkey -in $< -inform DER -out $@
+all_final += ec_prv.pk8nopubparam.pem
+ec_prv.pk8param.pem: ec_prv.pk8param.der
+ $(OPENSSL) pkey -in $< -inform DER -out $@
+all_final += ec_prv.pk8param.pem
################################################################
### Generate certificates for CRT write check tests
diff --git a/tests/data_files/ec_prv.noopt.der b/tests/data_files/ec_prv.pk8nopub.der
similarity index 100%
rename from tests/data_files/ec_prv.noopt.der
rename to tests/data_files/ec_prv.pk8nopub.der
Binary files differ
diff --git a/tests/data_files/ec_prv.pk8nopub.pem b/tests/data_files/ec_prv.pk8nopub.pem
new file mode 100644
index 0000000..0ec5272
--- /dev/null
+++ b/tests/data_files/ec_prv.pk8nopub.pem
@@ -0,0 +1,4 @@
+-----BEGIN PRIVATE KEY-----
+MEECAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEJzAlAgEBBCDH78XUX+cxmTPQ1hVkYbu3VvBc9c82
+EyGKaGvkAo1Pkw==
+-----END PRIVATE KEY-----
diff --git a/tests/data_files/ec_prv.pk8nopubparam.der b/tests/data_files/ec_prv.pk8nopubparam.der
new file mode 100644
index 0000000..70d30fb
--- /dev/null
+++ b/tests/data_files/ec_prv.pk8nopubparam.der
Binary files differ
diff --git a/tests/data_files/ec_prv.pk8nopubparam.pem b/tests/data_files/ec_prv.pk8nopubparam.pem
new file mode 100644
index 0000000..5c910c9
--- /dev/null
+++ b/tests/data_files/ec_prv.pk8nopubparam.pem
@@ -0,0 +1,4 @@
+-----BEGIN PRIVATE KEY-----
+ME0CAQAwEwYHKoZIzj0CAQYIKoZIzj0DAQcEMzAxAgEBBCDH78XUX+cxmTPQ1hVkYbu3VvBc9c82
+EyGKaGvkAo1Pk6AKBggqhkjOPQMBBw==
+-----END PRIVATE KEY-----
diff --git a/tests/data_files/ec_prv.pk8param.der b/tests/data_files/ec_prv.pk8param.der
new file mode 100644
index 0000000..8bbaa3a
--- /dev/null
+++ b/tests/data_files/ec_prv.pk8param.der
Binary files differ
diff --git a/tests/data_files/ec_prv.pk8param.pem b/tests/data_files/ec_prv.pk8param.pem
new file mode 100644
index 0000000..978beca
--- /dev/null
+++ b/tests/data_files/ec_prv.pk8param.pem
@@ -0,0 +1,5 @@
+-----BEGIN PRIVATE KEY-----
+MIGTAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBHkwdwIBAQQgx+/F1F/nMZkz0NYVZGG7t1bwXPXP
+NhMhimhr5AKNT5OgCgYIKoZIzj0DAQehRANCAARkJXH1LofHesYJwJkoZQ0ijCVrxDFEi8e/fc1d
+6DS2Hsk55TWpL953QEIDN8RmW01lejceK3jQWs0uGDenGCcM
+-----END PRIVATE KEY-----
diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data
index 91d2408..cbbbd5b7 100644
--- a/tests/suites/test_suite_pkparse.data
+++ b/tests/suites/test_suite_pkparse.data
@@ -992,10 +992,6 @@
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_parse_keyfile_ec:"data_files/ec_prv.sec1.der":"NULL":0
-Parse EC Key #1a (SEC1 DER, no optional part)
-depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECP_C:POLARSSL_ECP_DP_SECP256R1_ENABLED
-pk_parse_keyfile_ec:"data_files/ec_prv.noopt.der":"NULL":0
-
Parse EC Key #2 (SEC1 PEM)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_parse_keyfile_ec:"data_files/ec_prv.sec1.pem":"NULL":0
@@ -1008,10 +1004,34 @@
depends_on:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_parse_keyfile_ec:"data_files/ec_prv.pk8.der":"NULL":0
+Parse EC Key #4a (PKCS8 DER, no public key)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopub.der":"NULL":0
+
+Parse EC Key #4b (PKCS8 DER, no public key, with parameters)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopubparam.der":"NULL":0
+
+Parse EC Key #4c (PKCS8 DER, with parameters)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8param.der":"NULL":0
+
Parse EC Key #5 (PKCS8 PEM)
depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_parse_keyfile_ec:"data_files/ec_prv.pk8.pem":"NULL":0
+Parse EC Key #5a (PKCS8 PEM, no public key)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopub.pem":"NULL":0
+
+Parse EC Key #5b (PKCS8 PEM, no public key, with parameters)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8nopubparam.pem":"NULL":0
+
+Parse EC Key #5c (PKCS8 PEM, with parameters)
+depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED
+pk_parse_keyfile_ec:"data_files/ec_prv.pk8param.pem":"NULL":0
+
Parse EC Key #6 (PKCS8 encrypted DER)
depends_on:MBEDTLS_DES_C:MBEDTLS_SHA1_C:MBEDTLS_ECP_C:MBEDTLS_ECP_DP_SECP192R1_ENABLED
pk_parse_keyfile_ec:"data_files/ec_prv.pk8.pw.der":"polar":0