From d2df936e67e395e5f9ab0bcf059a8c0040f0a6da Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Fri, 16 Feb 2018 13:11:04 -0800 Subject: [PATCH] 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. --- ChangeLog | 7 ++ library/pkparse.c | 3 + tests/data_files/Makefile | 79 ++++++++++++++++++ .../{ec_prv.noopt.der => ec_prv.pk8nopub.der} | Bin tests/data_files/ec_prv.pk8nopub.pem | 4 + tests/data_files/ec_prv.pk8nopubparam.der | Bin 0 -> 79 bytes tests/data_files/ec_prv.pk8nopubparam.pem | 4 + tests/data_files/ec_prv.pk8param.der | Bin 0 -> 150 bytes tests/data_files/ec_prv.pk8param.pem | 5 ++ tests/suites/test_suite_pkparse.data | 28 ++++++- 10 files changed, 126 insertions(+), 4 deletions(-) rename tests/data_files/{ec_prv.noopt.der => ec_prv.pk8nopub.der} (100%) create mode 100644 tests/data_files/ec_prv.pk8nopub.pem create mode 100644 tests/data_files/ec_prv.pk8nopubparam.der create mode 100644 tests/data_files/ec_prv.pk8nopubparam.pem create mode 100644 tests/data_files/ec_prv.pk8param.der create mode 100644 tests/data_files/ec_prv.pk8param.pem diff --git a/ChangeLog b/ChangeLog index e0b016dfb..b2bee2bae 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 9022db2f9..5ad5edf84 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -861,7 +861,10 @@ static int pk_parse_key_sec1_der( mbedtls_ecp_keypair *eck, 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 59516bab8..f9832a014 100644 --- a/tests/data_files/Makefile +++ b/tests/data_files/Makefile @@ -578,7 +578,86 @@ keys_rsa_enc_pkcs8_v2: keys_rsa_enc_pkcs8_v2_1024 keys_rsa_enc_pkcs8_v2_2048 key ### 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 diff --git a/tests/data_files/ec_prv.pk8nopub.pem b/tests/data_files/ec_prv.pk8nopub.pem new file mode 100644 index 000000000..0ec527205 --- /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 0000000000000000000000000000000000000000..70d30fb81a8ccf94a72a1bcb48a35fc455eae481 GIT binary patch literal 79 zcmXr;WnyG75N2c7YV$Z}%f!gW0cJ2Wva=W)7&0+3vM3yXfAmWHbHka&7p{q>B<|iG Z_95o$c{5?fu8iy_Ouhb-7jPjo0RVjE8Gir( literal 0 HcmV?d00001 diff --git a/tests/data_files/ec_prv.pk8nopubparam.pem b/tests/data_files/ec_prv.pk8nopubparam.pem new file mode 100644 index 000000000..5c910c9ad --- /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 0000000000000000000000000000000000000000..8bbaa3a8b3a0ebf0a0839d4bf9498f7163600c25 GIT binary patch literal 150 zcmXqLoXo_?U?9xKuGQvo&X$RhjRVYJW@KlnG$?0cWMok|{{HBd_~(W*jW1jiO-bCn zJ?ulw*Yjq=id`AmPndfBCokYaXjul;z{G0p=sHBxz%dQ`KI7`k*H u-(Pz+_Jzqdxs#SpO;_sQD|c{WHb0UU?VDO