Fix bug in pk_parse_key()

ChangeLog

@@ -25,6 +25,8 @@ Features
      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

@@ -761,58 +761,61 @@ static int pk_parse_key_sec1_der( ecp_keypair *eck,
 
     p += len;
 
-    /*
+    pubkey_done = 0;
-     * Is 'parameters' present?
+    if( p != end )
-     */
-    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 )
+         * 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( 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( 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 );
     }
 
     if( ! pubkey_done &&

tests/suites/test_suite_pkparse.data

@@ -146,6 +146,10 @@ Parse EC Key #1 (SEC1 DER)
 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