[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>

diff --git a/src/crypto/asn1.c b/src/crypto/asn1.c
index a54d31d0c36e982ca4b4e2f8387821377aada87d..40b87533dda4f6a9ed5beb32abd4346517973457 100644 (file)
--- a/src/crypto/asn1.c
+++ b/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,
-                              unsigned int type ) {
+static int asn1_start ( struct asn1_cursor *cursor, 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;
-               goto notfound;
+               return -EINVAL_ASN1_EMPTY;
        }
 
        /* 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;
-               goto notfound;
+               return -ENXIO;
        }
        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;
-               goto notfound;
+               return -EINVAL_ASN1_LEN_LEN;
        }
 
        /* 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;
-               goto notfound;
+               return -EINVAL_ASN1_LEN;
        }
 
        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",
@@ -134,17 +134,17 @@ int asn1_enter ( struct asn1_cursor *cursor, unsigned int type ) {
 }
 
 /**
- * Skip ASN.1 object
+ * 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 be
- * invalidated.
+ * object.  If any error occurs, the object cursor will not be
+ * modified.
  */
-int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) {
+int asn1_skip_if_exists ( struct asn1_cursor *cursor, unsigned int type ) {
        int len;
 
        len = asn1_start ( cursor, type );
@@ -158,9 +158,30 @@ int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) {
 
        if ( ! cursor->len ) {
                DBGC ( cursor, "ASN1 %p reached end of object\n", cursor );
-               cursor->data = NULL;
                return -ENOENT;
        }
 
        return 0;
 }
+
+/**
+ * Skip ASN.1 object
+ *
+ * @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 be
+ * invalidated.
+ */
+int asn1_skip ( struct asn1_cursor *cursor, unsigned int type ) {
+       int rc;
+
+       if ( ( rc = asn1_skip_if_exists ( cursor, type ) ) < 0 ) {
+               asn1_invalidate_cursor ( cursor );
+               return rc;
+       }
+
+       return 0;
+}

diff --git a/src/crypto/x509.c b/src/crypto/x509.c
index 49bc2b8676dbd00c03ae9cc2ce579b390f118b57..28d28fc14ffbea617fc9a0006195c20a9fb704a0 100644 (file)
--- a/src/crypto/x509.c
+++ b/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 */

diff --git a/src/include/ipxe/asn1.h b/src/include/ipxe/asn1.h
index 85e480e87c692cc049f386dbe0372f895f9f28e7..7ac10862666fcfd950d35a4da71b67985853c1a6 100644 (file)
--- a/src/include/ipxe/asn1.h
+++ b/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 */