[tls] Accept certificates without a version number

The version field of an X.509 certificate appears to be optional.

Reported-by: Sebastiano Manusia <Sebastiano.Manusia@chuv.ch>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

src/crypto/asn1.c

@@ -29,6 +29,20 @@ FILE_LICENCE ( GPL2_OR_LATER );
  *
  */
 
+/* Disambiguate the various error causes */
+#define EINVAL_ASN1_EMPTY \
+	__einfo_error ( EINFO_EINVAL_ASN1_EMPTY )
+#define EINFO_EINVAL_ASN1_EMPTY \
+	__einfo_uniqify ( EINFO_EINVAL, 0x01, "Empty or underlength cursor" )
+#define EINVAL_ASN1_LEN_LEN \
+	__einfo_error ( EINFO_EINVAL_ASN1_LEN_LEN )
+#define EINFO_EINVAL_ASN1_LEN_LEN \
+	__einfo_uniqify ( EINFO_EINVAL, 0x02, "Length field overruns cursor" )
+#define EINVAL_ASN1_LEN \
+	__einfo_error ( EINFO_EINVAL_ASN1_LEN )
+#define EINFO_EINVAL_ASN1_LEN \
+	__einfo_uniqify ( EINFO_EINVAL, 0x03, "Field overruns cursor" )
+
 /**
  * Start parsing ASN.1 object
  *
@@ -40,32 +54,23 @@ FILE_LICENCE ( GPL2_OR_LATER );
  * object body (i.e. the first byte following the length byte(s)), and
  * the length of the object body (i.e. the number of bytes until the
  * following object tag, if any) is returned.
- *
- * If any error occurs (i.e. if the object is not of the expected
- * type, or if we overflow beyond the end of the ASN.1 object), then
- * the cursor will be invalidated and a negative value will be
- * returned.
  */
-static int asn1_start ( struct asn1_cursor *cursor,
+static int asn1_start ( struct asn1_cursor *cursor, unsigned int type ) {
-			       unsigned int type ) {
 	unsigned int len_len;
 	unsigned int len;
-	int rc;
 
 	/* Sanity check */
 	if ( cursor->len < 2 /* Tag byte and first length byte */ ) {
 		if ( cursor->len )
 			DBGC ( cursor, "ASN1 %p too short\n", cursor );
-		rc = -EINVAL;
+		return -EINVAL_ASN1_EMPTY;
-		goto notfound;
 	}
 
 	/* Check the tag byte */
 	if ( *( ( uint8_t * ) cursor->data ) != type ) {
 		DBGC ( cursor, "ASN1 %p type mismatch (expected %d, got %d)\n",
 		       cursor, type, *( ( uint8_t * ) cursor->data ) );
-		rc = -ENXIO;
+		return -ENXIO;
-		goto notfound;
 	}
 	cursor->data++;
 	cursor->len--;
@@ -82,8 +87,7 @@ static int asn1_start ( struct asn1_cursor *cursor,
 	if ( cursor->len < len_len ) {
 		DBGC ( cursor, "ASN1 %p bad length field length %d (max "
 		       "%zd)\n", cursor, len_len, cursor->len );
-		rc = -EINVAL;
+		return -EINVAL_ASN1_LEN_LEN;
-		goto notfound;
 	}
 
 	/* Extract the length and sanity check */
@@ -96,16 +100,10 @@ static int asn1_start ( struct asn1_cursor *cursor,
 	if ( cursor->len < len ) {
 		DBGC ( cursor, "ASN1 %p bad length %d (max %zd)\n",
 		       cursor, len, cursor->len );
-		rc = -EINVAL;
+		return -EINVAL_ASN1_LEN;
-		goto notfound;
 	}
 
 	return len;
-
- notfound:
-	cursor->data = NULL;
-	cursor->len = 0;
-	return rc;
 }
 
 /**
@@ -123,8 +121,10 @@ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
 	int len;
 
 	len = asn1_start ( cursor, type );
-	if ( len < 0 )
+	if ( len < 0 ) {
+		asn1_invalidate_cursor ( cursor );
 		return len;
+	}
 
 	cursor->len = len;
 	DBGC ( cursor, "ASN1 %p entered object type %02x (len %x)\n",
@@ -133,6 +133,37 @@ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
 	return 0;
 }
 
+/**
+ * Skip ASN.1 object if present
+ *
+ * @v cursor		ASN.1 object cursor
+ * @v type		Expected type
+ * @ret rc		Return status code
+ *
+ * The object cursor will be updated to point to the next ASN.1
+ * object.  If any error occurs, the object cursor will not be
+ * modified.
+ */
+int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) {
+	int len;
+
+	len = asn1_start ( cursor, type );
+	if ( len < 0 )
+		return len;
+
+	cursor->data += len;
+	cursor->len -= len;
+	DBGC ( cursor, "ASN1 %p skipped object type %02x (len %x)\n",
+	       cursor, type, len );
+
+	if ( ! cursor->len ) {
+		DBGC ( cursor, "ASN1 %p reached end of object\n", cursor );
+		return -ENOENT;
+	}
+
+	return 0;
+}
+
 /**
  * Skip ASN.1 object
  *
@@ -145,21 +176,11 @@ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
  * invalidated.
  */
 int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) {
-	int len;
+	int rc;
 
-	len = asn1_start ( cursor, type );
+	if ( ( rc = asn1_skip_if_exists ( cursor, type ) ) < 0 ) {
-	if ( len < 0 )
+		asn1_invalidate_cursor ( cursor );
-		return len;
+		return rc;
-
-	cursor->data += len;
-	cursor->len -= len;
-	DBGC ( cursor, "ASN1 %p skipped object type %02x (len %x)\n",
-	       cursor, type, len );
-
-	if ( ! cursor->len ) {
-		DBGC ( cursor, "ASN1 %p reached end of object\n", cursor );
-		cursor->data = NULL;
-		return -ENOENT;
 	}
 
 	return 0;

src/crypto/x509.c

@@ -55,7 +55,7 @@ static int x509_public_key ( const struct asn1_cursor *certificate,
 	memcpy ( &cursor, certificate, sizeof ( cursor ) );
 	rc = ( asn1_enter ( &cursor, ASN1_SEQUENCE ), /* Certificate */
 	       asn1_enter ( &cursor, ASN1_SEQUENCE ), /* tbsCertificate */
-	       asn1_skip ( &cursor, ASN1_EXPLICIT_TAG ), /* version */
+	       asn1_skip_if_exists ( &cursor, ASN1_EXPLICIT_TAG ), /* version */
 	       asn1_skip ( &cursor, ASN1_INTEGER ), /* serialNumber */
 	       asn1_skip ( &cursor, ASN1_SEQUENCE ), /* signature */
 	       asn1_skip ( &cursor, ASN1_SEQUENCE ), /* issuer */

src/include/ipxe/asn1.h

@@ -28,7 +28,19 @@ struct asn1_cursor {
 	size_t len;
 };
 
+/**
+ * Invalidate ASN.1 object cursor
+ *
+ * @v cursor		ASN.1 object cursor
+ */
+static inline __attribute__ (( always_inline )) void
+asn1_invalidate_cursor ( struct asn1_cursor *cursor ) {
+	cursor->len = 0;
+}
+
 extern int asn1_enter ( struct asn1_cursor *cursor, unsigned int type );
+extern int asn1_skip_if_exists ( struct asn1_cursor *cursor,
+				 unsigned int type );
 extern int asn1_skip ( struct asn1_cursor *cursor, unsigned int type );
 
 #endif /* _IPXE_ASN1_H */