From c9c0282594feef0811aac4311e579f75b0eac82c Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Fri, 28 Nov 2025 13:21:00 +0000 Subject: [PATCH] [crypto] Restructure handling of ASN.1 bit strings Signature values in ASN.1 tend to be encoded as bit strings rather than octet strings. In practice, no existent signature scheme uses a non-integral number of bytes. Switch to using a standard ASN.1 cursor to hold signature values, to simplify consuming code. Restructure the API to treat entering an ASN.1 bit string in the same way as entering any other ASN.1 type. Signed-off-by: Michael Brown --- src/crypto/asn1.c | 140 +++++++++++++++++----------------------- src/crypto/ocsp.c | 13 ++-- src/crypto/rsa.c | 6 +- src/crypto/x509.c | 25 +++---- src/include/ipxe/asn1.h | 16 +---- src/include/ipxe/ocsp.h | 2 +- src/include/ipxe/x509.h | 6 +- 7 files changed, 87 insertions(+), 121 deletions(-) diff --git a/src/crypto/asn1.c b/src/crypto/asn1.c index 7b84f6fc3..802553cab 100644 --- a/src/crypto/asn1.c +++ b/src/crypto/asn1.c @@ -300,6 +300,65 @@ int asn1_shrink_any ( struct asn1_cursor *cursor ) { return asn1_shrink ( cursor, ASN1_ANY ); } +/** + * Enter ASN.1 bit string + * + * @v cursor ASN.1 cursor + * @v unused Unused bits to fill in (or NULL to require all used) + * @ret rc Return status code + */ +int asn1_enter_bits ( struct asn1_cursor *cursor, unsigned int *unused ) { + const struct { + uint8_t unused; + uint8_t data[0]; + } __attribute__ (( packed )) *bit_string; + const uint8_t *last; + unsigned int unused_bits; + uint8_t unused_mask; + int rc; + + /* Enter bit string */ + if ( ( rc = asn1_enter ( cursor, ASN1_BIT_STRING ) ) != 0 ) + return rc; + + /* Check that bit string header exists */ + if ( cursor->len < sizeof ( *bit_string ) ) { + DBGC ( cursor, "ASN1 %p invalid bit string:\n", cursor ); + DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); + asn1_invalidate_cursor ( cursor ); + return -EINVAL_BIT_STRING; + } + bit_string = cursor->data; + cursor->data = &bit_string->data; + cursor->len -= offsetof ( typeof ( *bit_string ), data ); + unused_bits = bit_string->unused; + + /* Check validity of unused bits */ + unused_mask = ( 0xff >> ( 8 - unused_bits ) ); + last = ( cursor->data + cursor->len - 1 ); + if ( ( unused_bits >= 8 ) || + ( ( unused_bits > 0 ) && ( cursor->len == 0 ) ) || + ( ( *last & unused_mask ) != 0 ) ) { + DBGC ( cursor, "ASN1 %p invalid bit string:\n", cursor ); + DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); + asn1_invalidate_cursor ( cursor ); + return -EINVAL_BIT_STRING; + } + + /* Record or check number of unused bits, as applicable */ + if ( unused ) { + *unused = unused_bits; + } else if ( unused_bits ) { + DBGC ( cursor, "ASN1 %p invalid integral bit string:\n", + cursor ); + DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); + asn1_invalidate_cursor ( cursor ); + return -EINVAL_BIT_STRING; + } + + return 0; +} + /** * Parse value of ASN.1 boolean * @@ -362,87 +421,6 @@ int asn1_integer ( const struct asn1_cursor *cursor, int *value ) { return 0; } -/** - * Parse ASN.1 bit string - * - * @v cursor ASN.1 cursor - * @v bits Bit string to fill in - * @ret rc Return status code - */ -int asn1_bit_string ( const struct asn1_cursor *cursor, - struct asn1_bit_string *bits ) { - struct asn1_cursor contents; - const struct { - uint8_t unused; - uint8_t data[0]; - } __attribute__ (( packed )) *bit_string; - size_t len; - unsigned int unused; - uint8_t unused_mask; - const uint8_t *last; - int rc; - - /* Enter bit string */ - memcpy ( &contents, cursor, sizeof ( contents ) ); - if ( ( rc = asn1_enter ( &contents, ASN1_BIT_STRING ) ) != 0 ) { - DBGC ( cursor, "ASN1 %p cannot locate bit string:\n", cursor ); - DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); - return rc; - } - - /* Validity checks */ - if ( contents.len < sizeof ( *bit_string ) ) { - DBGC ( cursor, "ASN1 %p invalid bit string:\n", cursor ); - DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); - return -EINVAL_BIT_STRING; - } - bit_string = contents.data; - len = ( contents.len - offsetof ( typeof ( *bit_string ), data ) ); - unused = bit_string->unused; - unused_mask = ( 0xff >> ( 8 - unused ) ); - last = ( bit_string->data + len - 1 ); - if ( ( unused >= 8 ) || - ( ( unused > 0 ) && ( len == 0 ) ) || - ( ( *last & unused_mask ) != 0 ) ) { - DBGC ( cursor, "ASN1 %p invalid bit string:\n", cursor ); - DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); - return -EINVAL_BIT_STRING; - } - - /* Populate bit string */ - bits->data = &bit_string->data; - bits->len = len; - bits->unused = unused; - - return 0; -} - -/** - * Parse ASN.1 bit string that must be an integral number of bytes - * - * @v cursor ASN.1 cursor - * @v bits Bit string to fill in - * @ret rc Return status code - */ -int asn1_integral_bit_string ( const struct asn1_cursor *cursor, - struct asn1_bit_string *bits ) { - int rc; - - /* Parse bit string */ - if ( ( rc = asn1_bit_string ( cursor, bits ) ) != 0 ) - return rc; - - /* Check that there are no unused bits at end of string */ - if ( bits->unused ) { - DBGC ( cursor, "ASN1 %p invalid integral bit string:\n", - cursor ); - DBGC_HDA ( cursor, 0, cursor->data, cursor->len ); - return -EINVAL_BIT_STRING; - } - - return 0; -} - /** * Compare two ASN.1 objects * diff --git a/src/crypto/ocsp.c b/src/crypto/ocsp.c index e65f7180a..ae70f320c 100644 --- a/src/crypto/ocsp.c +++ b/src/crypto/ocsp.c @@ -158,8 +158,8 @@ static int ocsp_request ( struct ocsp_check *ocsp ) { digest_final ( digest, digest_ctx, name_digest ); digest_init ( digest, digest_ctx ); digest_update ( digest, digest_ctx, - ocsp->issuer->subject.public_key.raw_bits.data, - ocsp->issuer->subject.public_key.raw_bits.len ); + ocsp->issuer->subject.public_key.value.data, + ocsp->issuer->subject.public_key.value.len ); digest_final ( digest, digest_ctx, pubkey_digest ); /* Construct request */ @@ -422,8 +422,8 @@ static int ocsp_compare_responder_key_hash ( struct ocsp_check *ocsp, /* Generate SHA1 hash of certificate's public key */ digest_init ( &sha1_algorithm, ctx ); digest_update ( &sha1_algorithm, ctx, - cert->subject.public_key.raw_bits.data, - cert->subject.public_key.raw_bits.len ); + cert->subject.public_key.value.data, + cert->subject.public_key.value.len ); digest_final ( &sha1_algorithm, ctx, digest ); /* Compare responder key hash with hash of certificate's public key */ @@ -701,7 +701,7 @@ static int ocsp_parse_basic_response ( struct ocsp_check *ocsp, const struct asn1_cursor *raw ) { struct ocsp_response *response = &ocsp->response; struct asn1_algorithm **algorithm = &response->algorithm; - struct asn1_bit_string *signature = &response->signature; + struct asn1_cursor *signature = &response->signature; struct asn1_cursor cursor; int rc; @@ -726,7 +726,8 @@ static int ocsp_parse_basic_response ( struct ocsp_check *ocsp, asn1_skip_any ( &cursor ); /* Parse signature */ - if ( ( rc = asn1_integral_bit_string ( &cursor, signature ) ) != 0 ) { + memcpy ( signature, &cursor, sizeof ( *signature ) ); + if ( ( rc = asn1_enter_bits ( signature, NULL ) ) != 0 ) { DBGC ( ocsp, "OCSP %p \"%s\" cannot parse signature: %s\n", ocsp, x509_name ( ocsp->cert ), strerror ( rc ) ); return rc; diff --git a/src/crypto/rsa.c b/src/crypto/rsa.c index 44041da3e..f9041eede 100644 --- a/src/crypto/rsa.c +++ b/src/crypto/rsa.c @@ -176,7 +176,6 @@ static int rsa_parse_integer ( struct asn1_cursor *integer, static int rsa_parse_mod_exp ( struct asn1_cursor *modulus, struct asn1_cursor *exponent, const struct asn1_cursor *raw ) { - struct asn1_bit_string bits; struct asn1_cursor cursor; int is_private; int rc; @@ -220,10 +219,7 @@ static int rsa_parse_mod_exp ( struct asn1_cursor *modulus, asn1_skip ( &cursor, ASN1_SEQUENCE ); /* Enter subjectPublicKey */ - if ( ( rc = asn1_integral_bit_string ( &cursor, &bits ) ) != 0 ) - return rc; - cursor.data = bits.data; - cursor.len = bits.len; + asn1_enter_bits ( &cursor, NULL ); /* Enter RSAPublicKey */ asn1_enter ( &cursor, ASN1_SEQUENCE ); diff --git a/src/crypto/x509.c b/src/crypto/x509.c index 10bc6369a..0b01171b6 100644 --- a/src/crypto/x509.c +++ b/src/crypto/x509.c @@ -392,7 +392,7 @@ static int x509_parse_public_key ( struct x509_certificate *cert, const struct asn1_cursor *raw ) { struct x509_public_key *public_key = &cert->subject.public_key; struct asn1_algorithm **algorithm = &public_key->algorithm; - struct asn1_bit_string *raw_bits = &public_key->raw_bits; + struct asn1_cursor *value = &public_key->value; struct asn1_cursor cursor; int rc; @@ -416,8 +416,9 @@ static int x509_parse_public_key ( struct x509_certificate *cert, cert, (*algorithm)->name ); asn1_skip_any ( &cursor ); - /* Parse bit string */ - if ( ( rc = asn1_bit_string ( &cursor, raw_bits ) ) != 0 ) { + /* Parse subjectPublicKey */ + memcpy ( value, &cursor, sizeof ( *value ) ); + if ( ( rc = asn1_enter_bits ( value, NULL ) ) != 0 ) { DBGC ( cert, "X509 %p could not parse public key bits: %s\n", cert, strerror ( rc ) ); return rc; @@ -498,8 +499,9 @@ static int x509_parse_basic_constraints ( struct x509_certificate *cert, static int x509_parse_key_usage ( struct x509_certificate *cert, const struct asn1_cursor *raw ) { struct x509_key_usage *usage = &cert->extensions.usage; - struct asn1_bit_string bit_string; + struct asn1_cursor cursor; const uint8_t *bytes; + unsigned int unused; size_t len; unsigned int i; int rc; @@ -507,16 +509,17 @@ static int x509_parse_key_usage ( struct x509_certificate *cert, /* Mark extension as present */ usage->present = 1; - /* Parse bit string */ - if ( ( rc = asn1_bit_string ( raw, &bit_string ) ) != 0 ) { + /* Enter bit string */ + memcpy ( &cursor, raw, sizeof ( cursor ) ); + if ( ( rc = asn1_enter_bits ( &cursor, &unused ) ) != 0 ) { DBGC ( cert, "X509 %p could not parse key usage: %s\n", cert, strerror ( rc ) ); return rc; } /* Parse key usage bits */ - bytes = bit_string.data; - len = bit_string.len; + bytes = cursor.data; + len = cursor.len; if ( len > sizeof ( usage->bits ) ) len = sizeof ( usage->bits ); for ( i = 0 ; i < len ; i++ ) { @@ -1005,7 +1008,7 @@ int x509_parse ( struct x509_certificate *cert, const struct asn1_cursor *raw ) { struct x509_signature *signature = &cert->signature; struct asn1_algorithm **signature_algorithm = &signature->algorithm; - struct asn1_bit_string *signature_value = &signature->value; + struct asn1_cursor *signature_value = &signature->value; struct asn1_cursor cursor; int rc; @@ -1033,8 +1036,8 @@ int x509_parse ( struct x509_certificate *cert, asn1_skip_any ( &cursor ); /* Parse signatureValue */ - if ( ( rc = asn1_integral_bit_string ( &cursor, - signature_value ) ) != 0 ) { + memcpy ( signature_value, &cursor, sizeof ( *signature_value ) ); + if ( ( rc = asn1_enter_bits ( signature_value, NULL ) ) != 0 ) { DBGC ( cert, "X509 %p could not parse signature value: %s\n", cert, strerror ( rc ) ); return rc; diff --git a/src/include/ipxe/asn1.h b/src/include/ipxe/asn1.h index 9528c40f5..e3f48e8f7 100644 --- a/src/include/ipxe/asn1.h +++ b/src/include/ipxe/asn1.h @@ -427,16 +427,6 @@ extern struct asn1_algorithm oid_sha224_algorithm __asn1_algorithm; extern struct asn1_algorithm oid_sha512_224_algorithm __asn1_algorithm; extern struct asn1_algorithm oid_sha512_256_algorithm __asn1_algorithm; -/** An ASN.1 bit string */ -struct asn1_bit_string { - /** Data */ - const void *data; - /** Length */ - size_t len; - /** Unused bits at end of data */ - unsigned int unused; -} __attribute__ (( packed )); - /** * Invalidate ASN.1 object cursor * @@ -489,12 +479,10 @@ extern int asn1_shrink ( struct asn1_cursor *cursor, unsigned int type ); extern int asn1_enter_any ( struct asn1_cursor *cursor ); extern int asn1_skip_any ( struct asn1_cursor *cursor ); extern int asn1_shrink_any ( struct asn1_cursor *cursor ); +extern int asn1_enter_bits ( struct asn1_cursor *cursor, + unsigned int *unused ); extern int asn1_boolean ( const struct asn1_cursor *cursor ); extern int asn1_integer ( const struct asn1_cursor *cursor, int *value ); -extern int asn1_bit_string ( const struct asn1_cursor *cursor, - struct asn1_bit_string *bits ); -extern int asn1_integral_bit_string ( const struct asn1_cursor *cursor, - struct asn1_bit_string *bits ); extern int asn1_compare ( const struct asn1_cursor *cursor1, const struct asn1_cursor *cursor2 ); extern int asn1_algorithm ( const struct asn1_cursor *cursor, diff --git a/src/include/ipxe/ocsp.h b/src/include/ipxe/ocsp.h index 9eb70b2cc..a973f6f5e 100644 --- a/src/include/ipxe/ocsp.h +++ b/src/include/ipxe/ocsp.h @@ -76,7 +76,7 @@ struct ocsp_response { /** Signature algorithm */ struct asn1_algorithm *algorithm; /** Signature value */ - struct asn1_bit_string signature; + struct asn1_cursor signature; /** Signing certificate */ struct x509_certificate *signer; }; diff --git a/src/include/ipxe/x509.h b/src/include/ipxe/x509.h index e8cd0f303..4903eb656 100644 --- a/src/include/ipxe/x509.h +++ b/src/include/ipxe/x509.h @@ -51,8 +51,8 @@ struct x509_public_key { struct asn1_cursor raw; /** Public key algorithm */ struct asn1_algorithm *algorithm; - /** Raw public key bit string */ - struct asn1_bit_string raw_bits; + /** Public key value */ + struct asn1_cursor value; }; /** An X.509 certificate subject */ @@ -70,7 +70,7 @@ struct x509_signature { /** Signature algorithm */ struct asn1_algorithm *algorithm; /** Signature value */ - struct asn1_bit_string value; + struct asn1_cursor value; }; /** An X.509 certificate basic constraints set */ -- 2.47.3