From 5246ee5c59906fc4b021ceab63f09afdedbc66b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Wed, 19 Mar 2014 16:18:38 +0100 Subject: [PATCH] Work around compressed EC public key in some cases --- include/polarssl/ecp.h | 6 ++-- library/ecp.c | 8 +++-- library/pkparse.c | 82 +++++++++++++++++++----------------------- 3 files changed, 46 insertions(+), 50 deletions(-) diff --git a/include/polarssl/ecp.h b/include/polarssl/ecp.h index b35b0d1d0..61d6a1dd4 100644 --- a/include/polarssl/ecp.h +++ b/include/polarssl/ecp.h @@ -376,8 +376,10 @@ int ecp_point_write_binary( const ecp_group *grp, const ecp_point *P, * \param ilen Actual length of input * * \return 0 if successful, - * POLARSSL_ERR_ECP_BAD_INPUT_DATA if input is invalid - * POLARSSL_ERR_MPI_MALLOC_FAILED if memory allocation failed + * POLARSSL_ERR_ECP_BAD_INPUT_DATA if input is invalid, + * POLARSSL_ERR_MPI_MALLOC_FAILED if memory allocation failed, + * POLARSSL_ERR_ECP_FEATURE_UNAVAILABLE if the point format + * is not implemented. * * \note This function does NOT check that the point actually * belongs to the given group, see ecp_check_pubkey() for diff --git a/library/ecp.c b/library/ecp.c index f4f2d1c13..7cfb44f2c 100644 --- a/library/ecp.c +++ b/library/ecp.c @@ -478,7 +478,8 @@ cleanup: * Import a point from unsigned binary data (SEC1 2.3.4) */ int ecp_point_read_binary( const ecp_group *grp, ecp_point *pt, - const unsigned char *buf, size_t ilen ) { + const unsigned char *buf, size_t ilen ) +{ int ret; size_t plen; @@ -487,7 +488,10 @@ int ecp_point_read_binary( const ecp_group *grp, ecp_point *pt, plen = mpi_size( &grp->P ); - if( ilen != 2 * plen + 1 || buf[0] != 0x04 ) + if( buf[0] != 0x04 ) + return( POLARSSL_ERR_ECP_FEATURE_UNAVAILABLE ); + + if( ilen != 2 * plen + 1 ) return( POLARSSL_ERR_ECP_BAD_INPUT_DATA ); MPI_CHK( mpi_read_binary( &pt->X, buf + 1, plen ) ); diff --git a/library/pkparse.c b/library/pkparse.c index 19078493d..aed50d1b1 100644 --- a/library/pkparse.c +++ b/library/pkparse.c @@ -185,43 +185,6 @@ static int pk_get_ecparams( unsigned char **p, const unsigned char *end, return( 0 ); } -/* - * Import a point from unsigned binary data (SEC1 2.3.4), sloppy version: - * accepts compressed point, but then don't fill the Y coordinate, only its - * parity bit. This is a hack to be able to read SpecifiedECDomain to the - * degree that we can check if it is one of the known groups, - * see pk_group_from_specified() and ultimately pk_group_id_from_specified(). - */ -static int pk_ecp_point_read_sloppy( const ecp_group *grp, ecp_point *pt, - const unsigned char *buf, size_t ilen ) -{ - int ret; - size_t plen; - - if( ilen == 1 && buf[0] == 0x00 ) - return( ecp_set_zero( pt ) ); - - plen = mpi_size( &grp->P ); - - if( ilen == 2 * plen + 1 && buf[0] == 0x04 ) - { - MPI_CHK( mpi_read_binary( &pt->X, buf + 1, plen ) ); - MPI_CHK( mpi_read_binary( &pt->Y, buf + 1 + plen, plen ) ); - MPI_CHK( mpi_lset( &pt->Z, 1 ) ); - } - else if( ilen == plen + 1 && ( buf[0] == 0x02 || buf[0] == 0x03 ) ) - { - MPI_CHK( mpi_read_binary( &pt->X, buf + 1, plen ) ); - MPI_CHK( mpi_lset( &pt->Y, buf[0] - 2 ) ); - MPI_CHK( mpi_lset( &pt->Z, 1 ) ); - } - else - return( POLARSSL_ERR_ECP_BAD_INPUT_DATA ); - -cleanup: - return( ret ); -} - /* * Parse a SpecifiedECDomain (SEC 1 C.2) and (mostly) fill the group with it. * WARNING: the resulting group should only be used with @@ -344,11 +307,27 @@ static int pk_group_from_specified( const asn1_buf *params, ecp_group *grp ) /* * ECPoint ::= OCTET STRING */ - if( ( ret = asn1_get_tag( &p, end, &len, ASN1_OCTET_STRING ) ) != 0 || - ( ret = pk_ecp_point_read_sloppy( grp, &grp->G, - ( const unsigned char *) p, len ) ) != 0 ) + if( ( ret = asn1_get_tag( &p, end, &len, ASN1_OCTET_STRING ) ) != 0 ) return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT + ret ); + if( ( ret = ecp_point_read_binary( grp, &grp->G, + ( const unsigned char *) p, len ) ) != 0 ) + { + /* + * If we can't read the point because it's compressed, cheat by + * reading only the X coordinate and the parity bit of Y. + */ + if( ret != POLARSSL_ERR_ECP_FEATURE_UNAVAILABLE || + ( p[0] != 0x02 && p[0] != 0x03 ) || + len != mpi_size( &grp->P ) + 1 || + mpi_read_binary( &grp->G.X, p + 1, len - 1 ) != 0 || + mpi_lset( &grp->G.Y, p[0] - 2 ) != 0 || + mpi_lset( &grp->G.Z, 1 ) != 0 ) + { + return( POLARSSL_ERR_PK_KEY_INVALID_FORMAT ); + } + } + p += len; /* @@ -471,6 +450,10 @@ static int pk_use_ecparams( const asn1_buf *params, ecp_group *grp ) /* * EC public key is an EC point + * + * The caller is responsible for clearing the structure upon failure if + * desired. Take care to pass along the possible ECP_FEATURE_UNAVAILABLE + * return code of ecp_point_read_binary() and leave p in a usable state. */ static int pk_get_ecpubkey( unsigned char **p, const unsigned char *end, ecp_keypair *key ) @@ -478,19 +461,17 @@ static int pk_get_ecpubkey( unsigned char **p, const unsigned char *end, int ret; if( ( ret = ecp_point_read_binary( &key->grp, &key->Q, - (const unsigned char *) *p, end - *p ) ) != 0 || - ( ret = ecp_check_pubkey( &key->grp, &key->Q ) ) != 0 ) + (const unsigned char *) *p, end - *p ) ) == 0 ) { - ecp_keypair_free( key ); - return( POLARSSL_ERR_PK_INVALID_PUBKEY ); + ret = ecp_check_pubkey( &key->grp, &key->Q ); } /* - * We know ecp_point_read_binary consumed all bytes + * We know ecp_point_read_binary consumed all bytes or failed */ *p = (unsigned char *) end; - return( 0 ); + return( ret ); } #endif /* POLARSSL_ECP_C */ @@ -801,6 +782,15 @@ static int pk_parse_key_sec1_der( ecp_keypair *eck, 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 ) {