commit 924cd100a6dd6d5738e7816457efb3e40579f19d
author Manuel Pégourié-Gonnard <mpg@elzevir.fr> Tue Apr 14 11:18:04 2015 +0200
committer Manuel Pégourié-Gonnard <mpg@elzevir.fr> Tue Apr 14 11:18:04 2015 +0200
tree 48f8ab41b8b8d62343efec2ed806c69d1b322ab4
parent dbd60f72b12765823f0572b4b6f89e24afbd7ed8

Fix bug in pk_parse_key()

ChangeLog

diff --git a/ChangeLog b/ChangeLog
index 3697f53..671df64 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -25,6 +25,8 @@
      errors on use of deprecated functions.
 
 Bugfix
+   * Fix bug in pk_parse_key() that caused some valid private EC keys to be
+     rejected.
    * Fix bug in Via Padlock support (found by Nikos Mavrogiannopoulos).
    * Fix thread safety bug in RSA operations (found by Fredrik Axelsson).
    * Fix hardclock() (only used in the benchmarking program) with some

library/pkparse.c

diff --git a/library/pkparse.c b/library/pkparse.c
index 06fb292..39c51f6 100644
--- a/library/pkparse.c
+++ b/library/pkparse.c
@@ -761,58 +761,61 @@
 
     p += len;
 
-    /*
-     * Is 'parameters' present?
-     */
-    if( ( ret = asn1_get_tag( &p, end, &len,
-                    ASN1_CONTEXT_SPECIFIC | ASN1_CONSTRUCTED | 0 ) ) == 0 )
+    pubkey_done = 0;
+    if( p != end )
     {
-        if( ( ret = pk_get_ecparams( &p, p + len, &params) ) != 0 ||
-            ( ret = pk_use_ecparams( &params, &eck->grp )  ) != 0 )
+        /*
+         * Is 'parameters' present?
+         */
+        if( ( ret = asn1_get_tag( &p, end, &len,
+                        ASN1_CONTEXT_SPECIFIC | ASN1_CONSTRUCTED | 0 ) ) == 0 )
+        {
+            if( ( ret = pk_get_ecparams( &p, p + len, &params) ) != 0 ||
+                ( ret = pk_use_ecparams( &params, &eck->grp )  ) != 0 )
+            {
+                ecp_keypair_free( eck );
+                return( ret );
+            }
+        }
+        else if( ret != POLARSSL_ERR_ASN1_UNEXPECTED_TAG )
         {
             ecp_keypair_free( eck );
-            return( ret );
-        }
-    }
-    else if( ret != POLARSSL_ERR_ASN1_UNEXPECTED_TAG )
-    {
-        ecp_keypair_free( eck );
-        return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret );
-    }
-
-    /*
-     * Is 'publickey' present? If not, or if we can't read it (eg because it
-     * is compressed), create it from the private key.
-     */
-    pubkey_done = 0;
-    if( ( ret = asn1_get_tag( &p, end, &len,
-                    ASN1_CONTEXT_SPECIFIC | ASN1_CONSTRUCTED | 1 ) ) == 0 )
-    {
-        end2 = p + len;
-
-        if( ( ret = asn1_get_bitstring_null( &p, end2, &len ) ) != 0 )
             return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret );
-
-        if( p + len != end2 )
-            return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT +
-                    POLARSSL_ERR_ASN1_LENGTH_MISMATCH );
-
-        if( ( ret = pk_get_ecpubkey( &p, end2, eck ) ) == 0 )
-            pubkey_done = 1;
-        else
-        {
-            /*
-             * The only acceptable failure mode of pk_get_ecpubkey() above
-             * is if the point format is not recognized.
-             */
-            if( ret != POLARSSL_ERR_ECP_FEATURE_UNAVAILABLE )
-                return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT );
         }
-    }
-    else if( ret != POLARSSL_ERR_ASN1_UNEXPECTED_TAG )
-    {
-        ecp_keypair_free( eck );
-        return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret );
+
+        /*
+         * Is 'publickey' present? If not, or if we can't read it (eg because it
+         * is compressed), create it from the private key.
+         */
+        if( ( ret = asn1_get_tag( &p, end, &len,
+                        ASN1_CONTEXT_SPECIFIC | ASN1_CONSTRUCTED | 1 ) ) == 0 )
+        {
+            end2 = p + len;
+
+            if( ( ret = asn1_get_bitstring_null( &p, end2, &len ) ) != 0 )
+                return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret );
+
+            if( p + len != end2 )
+                return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT +
+                        POLARSSL_ERR_ASN1_LENGTH_MISMATCH );
+
+            if( ( ret = pk_get_ecpubkey( &p, end2, eck ) ) == 0 )
+                pubkey_done = 1;
+            else
+            {
+                /*
+                 * The only acceptable failure mode of pk_get_ecpubkey() above
+                 * is if the point format is not recognized.
+                 */
+                if( ret != POLARSSL_ERR_ECP_FEATURE_UNAVAILABLE )
+                    return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT );
+            }
+        }
+        else if( ret != POLARSSL_ERR_ASN1_UNEXPECTED_TAG )
+        {
+            ecp_keypair_free( eck );
+            return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret );
+        }
     }
 
     if( ! pubkey_done &&

tests/data_files/ec_prv.noopt.der

diff --git a/tests/data_files/ec_prv.noopt.der b/tests/data_files/ec_prv.noopt.der
new file mode 100644
index 0000000..fde16a1
--- /dev/null
+++ b/tests/data_files/ec_prv.noopt.der
Binary files differ

tests/suites/test_suite_pkparse.data

diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data
index b502017..aab568d 100644
--- a/tests/suites/test_suite_pkparse.data
+++ b/tests/suites/test_suite_pkparse.data
@@ -146,6 +146,10 @@
 depends_on:POLARSSL_PEM_PARSE_C:POLARSSL_ECP_C:POLARSSL_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:POLARSSL_PEM_PARSE_C:POLARSSL_ECP_C:POLARSSL_ECP_DP_SECP192R1_ENABLED
 pk_parse_keyfile_ec:"data_files/ec_prv.sec1.pem":"NULL":0