From d60e3780163174677150a89dcc4516ce90708dcf Mon Sep 17 00:00:00 2001 From: Andres Amaya Garcia Date: Wed, 26 Sep 2018 10:48:24 +0100 Subject: [PATCH] Fix ASN1 bitstring writing Refactor the function mbedtls_asn1_write_bitstring() that removes trailing 0s at the end of DER encoded bitstrings. The function is implemented according to Hanno Becker's suggestions. This commit also changes the functions x509write_crt_set_ns_cert_type and crt_set_key_usage to call the new function as the use named bitstrings instead of the regular bitstrings. --- library/asn1write.c | 28 +++++++++++++++---------- library/x509write_crt.c | 45 ++++++++++++++++++++++++++++++++++------- library/x509write_csr.c | 36 +++++++++++++++++++++++++++++---- 3 files changed, 87 insertions(+), 22 deletions(-) diff --git a/library/asn1write.c b/library/asn1write.c index a4d23f619..c0b4622d5 100644 --- a/library/asn1write.c +++ b/library/asn1write.c @@ -294,22 +294,28 @@ int mbedtls_asn1_write_bitstring( unsigned char **p, unsigned char *start, const unsigned char *buf, size_t bits ) { int ret; - size_t len = 0, size; + size_t len = 0; + size_t unused_bits, byte_len; - size = ( bits / 8 ) + ( ( bits % 8 ) ? 1 : 0 ); + byte_len = ( bits + 7 ) / 8; + unused_bits = ( byte_len * 8 ) - bits; - // Calculate byte length - // - if( *p < start || (size_t)( *p - start ) < size + 1 ) + if( *p < start || (size_t)( *p - start ) < byte_len + 1 ) return( MBEDTLS_ERR_ASN1_BUF_TOO_SMALL ); - len = size + 1; - (*p) -= size; - memcpy( *p, buf, size ); + len = byte_len + 1; - // Write unused bits - // - *--(*p) = (unsigned char) (size * 8 - bits); + /* Write the bitstring. Ensure the unused bits are zeroed */ + if( byte_len > 0 ) + { + byte_len--; + *--( *p ) = buf[byte_len] & ~( ( 0x1 << unused_bits ) - 1 ); + ( *p ) -= byte_len; + memcpy( *p, buf, byte_len ); + } + + /* Write unused bits */ + *--( *p ) = (unsigned char)unused_bits; MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_len( p, start, len ) ); MBEDTLS_ASN1_CHK_ADD( len, mbedtls_asn1_write_tag( p, start, MBEDTLS_ASN1_BIT_STRING ) ); diff --git a/library/x509write_crt.c b/library/x509write_crt.c index b1ef216c9..10497e752 100644 --- a/library/x509write_crt.c +++ b/library/x509write_crt.c @@ -218,26 +218,51 @@ int mbedtls_x509write_crt_set_authority_key_identifier( mbedtls_x509write_cert * } #endif /* MBEDTLS_SHA1_C */ +static size_t crt_get_unused_bits_for_named_bitstring( unsigned char bitstring, + size_t bit_offset ) +{ + size_t unused_bits; + + /* Count the unused bits removing trailing 0s */ + for( unused_bits = bit_offset; unused_bits < 8; unused_bits++ ) + if( ( ( bitstring >> unused_bits ) & 0x1 ) != 0 ) + break; + + return( unused_bits ); +} + int mbedtls_x509write_crt_set_key_usage( mbedtls_x509write_cert *ctx, unsigned int key_usage ) { unsigned char buf[4], ku; unsigned char *c; int ret; + size_t unused_bits; + const unsigned int allowed_bits = MBEDTLS_X509_KU_DIGITAL_SIGNATURE | + MBEDTLS_X509_KU_NON_REPUDIATION | + MBEDTLS_X509_KU_KEY_ENCIPHERMENT | + MBEDTLS_X509_KU_DATA_ENCIPHERMENT | + MBEDTLS_X509_KU_KEY_AGREEMENT | + MBEDTLS_X509_KU_KEY_CERT_SIGN | + MBEDTLS_X509_KU_CRL_SIGN; - /* We currently only support 7 bits, from 0x80 to 0x02 */ - if( ( key_usage & ~0xfe ) != 0 ) + /* Check that nothing other than the allowed flags is set */ + if( ( key_usage & ~allowed_bits ) != 0 ) return( MBEDTLS_ERR_X509_FEATURE_UNAVAILABLE ); c = buf + 4; - ku = (unsigned char) key_usage; + ku = (unsigned char)key_usage; + unused_bits = crt_get_unused_bits_for_named_bitstring( ku, 1 ); + ret = mbedtls_asn1_write_bitstring( &c, buf, &ku, 8 - unused_bits ); - if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ku, 7 ) ) != 4 ) + if( ret < 0 ) return( ret ); + else if( ret < 3 || ret > 4 ) + return( MBEDTLS_ERR_X509_INVALID_FORMAT ); ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_KEY_USAGE, MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ), - 1, buf, 4 ); + 1, c, (size_t)ret ); if( ret != 0 ) return( ret ); @@ -249,16 +274,22 @@ int mbedtls_x509write_crt_set_ns_cert_type( mbedtls_x509write_cert *ctx, { unsigned char buf[4]; unsigned char *c; + size_t unused_bits; int ret; c = buf + 4; - if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 ) + unused_bits = crt_get_unused_bits_for_named_bitstring( ns_cert_type, 0 ); + ret = mbedtls_asn1_write_bitstring( &c, + buf, + &ns_cert_type, + 8 - unused_bits ); + if( ret < 3 || ret > 4 ) return( ret ); ret = mbedtls_x509write_crt_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE, MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ), - 0, buf, 4 ); + 0, c, (size_t)ret ); if( ret != 0 ) return( ret ); diff --git a/library/x509write_csr.c b/library/x509write_csr.c index 66cee5601..d70ba0ed9 100644 --- a/library/x509write_csr.c +++ b/library/x509write_csr.c @@ -81,20 +81,39 @@ int mbedtls_x509write_csr_set_extension( mbedtls_x509write_csr *ctx, 0, val, val_len ); } +static size_t csr_get_unused_bits_for_named_bitstring( unsigned char bitstring, + size_t bit_offset ) +{ + size_t unused_bits; + + /* Count the unused bits removing trailing 0s */ + for( unused_bits = bit_offset; unused_bits < 8; unused_bits++ ) + if( ( ( bitstring >> unused_bits ) & 0x1 ) != 0 ) + break; + + return( unused_bits ); +} + int mbedtls_x509write_csr_set_key_usage( mbedtls_x509write_csr *ctx, unsigned char key_usage ) { unsigned char buf[4]; unsigned char *c; + size_t unused_bits; int ret; c = buf + 4; - if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &key_usage, 7 ) ) != 4 ) + unused_bits = csr_get_unused_bits_for_named_bitstring( key_usage, 0 ); + ret = mbedtls_asn1_write_bitstring( &c, buf, &key_usage, 8 - unused_bits ); + + if( ret < 0 ) return( ret ); + else if( ret < 3 || ret > 4 ) + return( MBEDTLS_ERR_X509_INVALID_FORMAT ); ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_KEY_USAGE, MBEDTLS_OID_SIZE( MBEDTLS_OID_KEY_USAGE ), - buf, 4 ); + c, (size_t)ret ); if( ret != 0 ) return( ret ); @@ -106,16 +125,25 @@ int mbedtls_x509write_csr_set_ns_cert_type( mbedtls_x509write_csr *ctx, { unsigned char buf[4]; unsigned char *c; + size_t unused_bits; int ret; c = buf + 4; - if( ( ret = mbedtls_asn1_write_bitstring( &c, buf, &ns_cert_type, 8 ) ) != 4 ) + unused_bits = csr_get_unused_bits_for_named_bitstring( ns_cert_type, 0 ); + ret = mbedtls_asn1_write_bitstring( &c, + buf, + &ns_cert_type, + 8 - unused_bits ); + + if( ret < 0 ) + return( ret ); + else if( ret < 3 || ret > 4 ) return( ret ); ret = mbedtls_x509write_csr_set_extension( ctx, MBEDTLS_OID_NS_CERT_TYPE, MBEDTLS_OID_SIZE( MBEDTLS_OID_NS_CERT_TYPE ), - buf, 4 ); + c, (size_t)ret ); if( ret != 0 ) return( ret );