diff --git a/ChangeLog b/ChangeLog index 3697f5310..671df6477 100644 --- a/ChangeLog +++ b/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 diff --git a/library/pkparse.c b/library/pkparse.c index 06fb2929f..39c51f648 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -761,58 +761,61 @@ static int pk_parse_key_sec1_der( ecp_keypair *eck, 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, ¶ms) ) != 0 || - ( ret = pk_use_ecparams( ¶ms, &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, ¶ms) ) != 0 || + ( ret = pk_use_ecparams( ¶ms, &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 && diff --git a/tests/data_files/ec_prv.noopt.der b/tests/data_files/ec_prv.noopt.der new file mode 100644 index 000000000..fde16a17a Binary files /dev/null and b/tests/data_files/ec_prv.noopt.der differ diff --git a/tests/suites/test_suite_pkparse.data b/tests/suites/test_suite_pkparse.data index b502017b0..aab568d18 100644 --- a/tests/suites/test_suite_pkparse.data +++ b/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