diff --git a/library/asn1parse.c b/library/asn1parse.c index 4764ca4cb..412259e35 100644 --- a/library/asn1parse.c +++ b/library/asn1parse.c @@ -149,16 +149,26 @@ int mbedtls_asn1_get_int( unsigned char **p, if( ( ret = mbedtls_asn1_get_tag( p, end, &len, MBEDTLS_ASN1_INTEGER ) ) != 0 ) return( ret ); - if( len == 0 || ( **p & 0x80 ) != 0 ) + /* len==0 is malformed (0 must be represented as 020100). */ + if( len == 0 ) + return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); + /* This is a cryptography library. Reject negative integers. */ + if( ( **p & 0x80 ) != 0 ) return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); + /* Skip leading zeros. */ while( len > 0 && **p == 0 ) { ++( *p ); --len; } + + /* Reject integers that don't fit in an int. This code assumes that + * the int type has no padding bit. */ if( len > sizeof( int ) ) return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); + if( len == sizeof( int ) && ( **p & 0x80 ) != 0 ) + return( MBEDTLS_ERR_ASN1_INVALID_LENGTH ); *val = 0; while( len-- > 0 ) diff --git a/tests/suites/test_suite_asn1parse.data b/tests/suites/test_suite_asn1parse.data index c5d9136b7..4abae0bb4 100644 --- a/tests/suites/test_suite_asn1parse.data +++ b/tests/suites/test_suite_asn1parse.data @@ -164,8 +164,7 @@ Not BOOLEAN get_boolean:"020101":0:MBEDTLS_ERR_ASN1_UNEXPECTED_TAG Empty INTEGER -depends_on:SUPPORT_NEGATIVE_INTEGERS -get_integer:"0200":"":MBEDTLS_ERR_ASN1_INVALID_LENGTH +empty_integer:"0200" INTEGER 0 get_integer:"020100":"0":0 @@ -173,27 +172,15 @@ get_integer:"020100":"0":0 INTEGER 0, extra leading 0 get_integer:"02020000":"0":0 -INTEGER -0 -depends_on:SUPPORT_NEGATIVE_INTEGERS -get_integer:"020180":"0":0 - INTEGER 1 get_integer:"020101":"1":0: INTEGER 1, extra leading 0 get_integer:"02020001":"1":0: -INTEGER -1 -depends_on:SUPPORT_NEGATIVE_INTEGERS -get_integer:"020181":"-1":0 - INTEGER 0x7f get_integer:"02017f":"7f":0 -INTEGER -0x7f -depends_on:SUPPORT_NEGATIVE_INTEGERS -get_integer:"0201ff":"-7f":0 - INTEGER 0x80 get_integer:"02020080":"80":0 @@ -212,9 +199,30 @@ get_integer:"020412345678":"12345678":0 INTEGER 0x12345678, extra leading 0 get_integer:"02050012345678":"12345678":0 +INTEGER 0x7fffffff +get_integer:"02047fffffff":"7fffffff":0 + +INTEGER 0x7fffffff, extra leading 0 +get_integer:"0205007fffffff":"7fffffff":0 + +INTEGER 0x80000000 +get_integer:"02050080000000":"80000000":0 + +INTEGER 0xffffffff +get_integer:"020500ffffffff":"ffffffff":0 + +INTEGER 0x100000000 +get_integer:"02050100000000":"0100000000":0 + INTEGER 0x123456789abcdef0 get_integer:"0208123456789abcdef0":"123456789abcdef0":0 +INTEGER 0xfedcab9876543210 +get_integer:"020900fedcab9876543210":"fedcab9876543210":0 + +INTEGER 0x1fedcab9876543210 +get_integer:"020901fedcab9876543210":"1fedcab9876543210":0 + INTEGER with 127 value octets get_integer:"027f0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcd":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcd":0 @@ -227,6 +235,51 @@ get_integer:"0281800123456789abcdef0123456789abcdef0123456789abcdef0123456789abc INTEGER with 128 value octets (leading 0 in length) get_integer:"028200800123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":0 +INTEGER -1 +get_integer:"0201ff":"-1":0 + +INTEGER -1, extra leading ff +get_integer:"0202ffff":"-1":0 + +INTEGER -0x7f +get_integer:"020181":"-7f":0 + +INTEGER -0x80 +get_integer:"020180":"-80":0 + +INTEGER -0x81 +get_integer:"0202ff7f":"-81":0 + +INTEGER -0xff +get_integer:"0202ff01":"-ff":0 + +INTEGER -0x100 +get_integer:"0202ff00":"-100":0 + +INTEGER -0x7fffffff +get_integer:"020480000001":"-7fffffff":0 + +INTEGER -0x80000000 +get_integer:"020480000000":"-80000000":0 + +INTEGER -0x80000001 +get_integer:"0205ff7fffffff":"-80000001":0 + +INTEGER -0xffffffff +get_integer:"0205ff00000001":"-ffffffff":0 + +INTEGER -0x100000000 +get_integer:"0205ff00000000":"-100000000":0 + +INTEGER -0x123456789abcdef0 +get_integer:"0208edcba98765432110":"-123456789abcdef0":0 + +INTEGER -0xfedcba9876543210 +get_integer:"0209ff0123456789abcdf0":"-fedcba9876543210":0 + +INTEGER -0x1fedcab9876543210 +get_integer:"0209fe0123546789abcdf0":"-1fedcab9876543210":0 + Not INTEGER get_integer:"010101":"":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG diff --git a/tests/suites/test_suite_asn1parse.function b/tests/suites/test_suite_asn1parse.function index 3bfb1c703..defbd01bb 100644 --- a/tests/suites/test_suite_asn1parse.function +++ b/tests/suites/test_suite_asn1parse.function @@ -59,6 +59,10 @@ static int nested_parse( unsigned char **const p, *p = start; ret = mbedtls_asn1_get_mpi( p, end, &mpi ); mbedtls_mpi_free( &mpi ); +#else + *p = start + 1; + ret = mbedtls_asn1_get_len( p, end, &len ); + *p += len; #endif /* If we're sure that the number fits in an int, also * call mbedtls_asn1_get_int(). */ @@ -246,6 +250,41 @@ void get_boolean( const data_t *input, } /* END_CASE */ +/* BEGIN_CASE */ +void empty_integer( const data_t *input ) +{ + unsigned char *p; +#if defined(MBEDTLS_BIGNUM_C) + mbedtls_mpi actual_mpi; +#endif + int val; + +#if defined(MBEDTLS_BIGNUM_C) + mbedtls_mpi_init( & actual_mpi ); +#endif + + /* An INTEGER with no content is not valid. */ + p = input->x; + TEST_EQUAL( mbedtls_asn1_get_int( &p, input->x + input->len, &val ), + MBEDTLS_ERR_ASN1_INVALID_LENGTH ); + +#if defined(MBEDTLS_BIGNUM_C) + /* INTEGERs are sometimes abused as bitstrings, so the library accepts + * an INTEGER with empty content and gives it the value 0. */ + p = input->x; + TEST_EQUAL( mbedtls_asn1_get_mpi( &p, input->x + input->len, &actual_mpi ), + 0 ); + TEST_EQUAL( mbedtls_mpi_cmp_int( &actual_mpi, 0 ), 0 ); +#endif + +exit: +#if defined(MBEDTLS_BIGNUM_C) + mbedtls_mpi_free( &actual_mpi ); +#endif + /*empty cleanup in some configurations*/ ; +} +/* END_CASE */ + /* BEGIN_CASE */ void get_integer( const data_t *input, const char *expected_hex, int expected_result ) @@ -254,16 +293,18 @@ void get_integer( const data_t *input, #if defined(MBEDTLS_BIGNUM_C) mbedtls_mpi expected_mpi; mbedtls_mpi actual_mpi; + mbedtls_mpi complement; + int expected_result_for_mpi = expected_result; #endif long expected_value; int expected_result_for_int = expected_result; - int expected_result_for_mpi = expected_result; int val; int ret; #if defined(MBEDTLS_BIGNUM_C) mbedtls_mpi_init( &expected_mpi ); mbedtls_mpi_init( &actual_mpi ); + mbedtls_mpi_init( &complement ); #endif errno = 0; @@ -275,6 +316,16 @@ void get_integer( const data_t *input, #endif ) ) { + /* The library returns the dubious error code INVALID_LENGTH + * for integers that are out of range. */ + expected_result_for_int = MBEDTLS_ERR_ASN1_INVALID_LENGTH; + } + if( expected_result == 0 && expected_value < 0 ) + { + /* The library does not support negative INTEGERs and + * returns the dubious error code INVALID_LENGTH. + * Test that we preserve the historical behavior. If we + * decide to change the behavior, we'll also change this test. */ expected_result_for_int = MBEDTLS_ERR_ASN1_INVALID_LENGTH; } @@ -300,7 +351,34 @@ void get_integer( const data_t *input, TEST_EQUAL( ret, expected_result_for_mpi ); if( ret == 0 ) { - TEST_ASSERT( mbedtls_mpi_cmp_mpi( &actual_mpi , &expected_mpi ) == 0 ); + if( expected_value >= 0 ) + { + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &actual_mpi, + &expected_mpi ) == 0 ); + } + else + { + /* The library ignores the sign bit in ASN.1 INTEGERs + * (which makes sense insofar as INTEGERs are sometimes + * abused as bit strings), so the result of parsing them + * is a positive integer such that expected_mpi + + * actual_mpi = 2^n where n is the length of the content + * of the INTEGER. (Leading ff octets don't matter for the + * expected value, but they matter for the actual value.) + * Test that we don't change from this behavior. If we + * decide to fix the library to change the behavior on + * negative INTEGERs, we'll fix this test code. */ + unsigned char *q = input->x + 1; + size_t len; + TEST_ASSERT( mbedtls_asn1_get_len( &q, input->x + input->len, + &len ) == 0 ); + TEST_ASSERT( mbedtls_mpi_lset( &complement, 1 ) == 0 ); + TEST_ASSERT( mbedtls_mpi_shift_l( &complement, len * 8 ) == 0 ); + TEST_ASSERT( mbedtls_mpi_add_mpi( &complement, &complement, + &expected_mpi ) == 0 ); + TEST_ASSERT( mbedtls_mpi_cmp_mpi( &complement, + &actual_mpi ) == 0 ); + } TEST_ASSERT( p == input->x + input->len ); } #endif @@ -309,7 +387,9 @@ exit: #if defined(MBEDTLS_BIGNUM_C) mbedtls_mpi_free( &expected_mpi ); mbedtls_mpi_free( &actual_mpi ); + mbedtls_mpi_free( &complement ); #endif + /*empty cleanup in some configurations*/ ; } /* END_CASE */