[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 <mcb30@ipxe.org>

4 files changed, 81 insertions, 103 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
@@ -301,6 +301,65 @@ int asn1_shrink_any ( struct asn1_cursor *cursor ) {
 }
 
 /**
+ * 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
  *
  * @v cursor		ASN.1 object cursor
@@ -363,87 +422,6 @@ int asn1_integer ( const struct asn1_cursor *cursor, int *value ) {
 }
 
 /**
- * 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
  *
  * @v cursor1		ASN.1 object cursor
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;