ASN1 tests: Match negative INTEGERs with the actual library behavior

mbedtls_asn1_get_int() and mbedtls_asn1_get_mpi() behave differently
on negative INTEGERs (0200). Don't change the library behavior for now
because this might break interoperability in some applications. Change
the test function to the library behavior.

Fix the test data with negative INTEGERs. These test cases were
previously not run (they were introduced but deliberately deactivated
in 27d806fab4). The test data was
actually wrong: ASN.1 uses two's complement, which has no negative 0,
and some encodings were wrong. Now the tests have correct data, and
the test code rectifies the expected data to match the library
behavior.
This commit is contained in:
Gilles Peskine 2019-10-10 19:21:12 +02:00
parent 321adb297c
commit 970dcbf453
2 changed files with 47 additions and 13 deletions

View File

@ -172,27 +172,15 @@ get_integer:"020100":"0":0
INTEGER 0, extra leading 0 INTEGER 0, extra leading 0
get_integer:"02020000":"0":0 get_integer:"02020000":"0":0
INTEGER -0
depends_on:SUPPORT_NEGATIVE_INTEGERS
get_integer:"020180":"0":0
INTEGER 1 INTEGER 1
get_integer:"020101":"1":0: get_integer:"020101":"1":0:
INTEGER 1, extra leading 0 INTEGER 1, extra leading 0
get_integer:"02020001":"1":0: get_integer:"02020001":"1":0:
INTEGER -1
depends_on:SUPPORT_NEGATIVE_INTEGERS
get_integer:"020181":"-1":0
INTEGER 0x7f INTEGER 0x7f
get_integer:"02017f":"7f":0 get_integer:"02017f":"7f":0
INTEGER -0x7f
depends_on:SUPPORT_NEGATIVE_INTEGERS
get_integer:"0201ff":"-7f":0
INTEGER 0x80 INTEGER 0x80
get_integer:"02020080":"80":0 get_integer:"02020080":"80":0
@ -226,6 +214,12 @@ get_integer:"0281800123456789abcdef0123456789abcdef0123456789abcdef0123456789abc
INTEGER with 128 value octets (leading 0 in length) INTEGER with 128 value octets (leading 0 in length)
get_integer:"028200800123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":0 get_integer:"028200800123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef":0
INTEGER -1
get_integer:"0201ff":"-1":0
INTEGER -0x7f
get_integer:"020181":"-7f":0
Not INTEGER Not INTEGER
get_integer:"010101":"":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG get_integer:"010101":"":MBEDTLS_ERR_ASN1_UNEXPECTED_TAG

View File

@ -293,6 +293,7 @@ void get_integer( const data_t *input,
#if defined(MBEDTLS_BIGNUM_C) #if defined(MBEDTLS_BIGNUM_C)
mbedtls_mpi expected_mpi; mbedtls_mpi expected_mpi;
mbedtls_mpi actual_mpi; mbedtls_mpi actual_mpi;
mbedtls_mpi complement;
int expected_result_for_mpi = expected_result; int expected_result_for_mpi = expected_result;
#endif #endif
long expected_value; long expected_value;
@ -303,6 +304,7 @@ void get_integer( const data_t *input,
#if defined(MBEDTLS_BIGNUM_C) #if defined(MBEDTLS_BIGNUM_C)
mbedtls_mpi_init( &expected_mpi ); mbedtls_mpi_init( &expected_mpi );
mbedtls_mpi_init( &actual_mpi ); mbedtls_mpi_init( &actual_mpi );
mbedtls_mpi_init( &complement );
#endif #endif
errno = 0; errno = 0;
@ -314,6 +316,16 @@ void get_integer( const data_t *input,
#endif #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; expected_result_for_int = MBEDTLS_ERR_ASN1_INVALID_LENGTH;
} }
@ -339,7 +351,34 @@ void get_integer( const data_t *input,
TEST_EQUAL( ret, expected_result_for_mpi ); TEST_EQUAL( ret, expected_result_for_mpi );
if( ret == 0 ) 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 ); TEST_ASSERT( p == input->x + input->len );
} }
#endif #endif
@ -348,6 +387,7 @@ exit:
#if defined(MBEDTLS_BIGNUM_C) #if defined(MBEDTLS_BIGNUM_C)
mbedtls_mpi_free( &expected_mpi ); mbedtls_mpi_free( &expected_mpi );
mbedtls_mpi_free( &actual_mpi ); mbedtls_mpi_free( &actual_mpi );
mbedtls_mpi_free( &complement );
#endif #endif
/*empty cleanup in some configurations*/ ; /*empty cleanup in some configurations*/ ;
} }