commit ed9fb78739bc798873f31975b963f40310b003ac
author Przemek Stekiel <przemyslaw.stekiel@mobica.com> Wed May 03 16:27:25 2023 +0200
committer Przemek Stekiel <przemyslaw.stekiel@mobica.com> Mon May 08 11:15:54 2023 +0200
tree 40d0d34cd35026ea4445af61e79ee945330cefe1
parent 8194285cf1f2d5384f180e26b1924b299a56c09d

Fix parsing of KeyIdentifier (tag length error case) + test

Signed-off-by: Przemek Stekiel <przemyslaw.stekiel@mobica.com>

library/x509_crt.c

diff --git a/library/x509_crt.c b/library/x509_crt.c
index 59a6946..c6d7358 100644
--- a/library/x509_crt.c
+++ b/library/x509_crt.c
@@ -641,10 +641,13 @@
                                  MBEDTLS_ERR_ASN1_LENGTH_MISMATCH);
     }
 
-    if ((ret = mbedtls_asn1_get_tag(p, end, &len,
-                                    MBEDTLS_ASN1_CONTEXT_SPECIFIC)) != 0) {
-        /* KeyIdentifier is an OPTIONAL field */
-    } else {
+    ret = mbedtls_asn1_get_tag(p, end, &len,
+                               MBEDTLS_ASN1_CONTEXT_SPECIFIC);
+
+    /* KeyIdentifier is an OPTIONAL field */
+    if (ret != 0 && ret != MBEDTLS_ERR_ASN1_UNEXPECTED_TAG) {
+        return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret);
+    } else if (ret == 0) {
         authority_key_id->keyIdentifier.len = len;
         authority_key_id->keyIdentifier.p = *p;
         /* Setting tag of the keyIdentfier intentionally to 0x04.
@@ -663,26 +666,24 @@
             /* authorityCertIssuer and authorityCertSerialNumber MUST both
                be present or both be absent. At this point we expect to have both. */
             return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret);
-        } else {
-            /* "end" also includes the CertSerialNumber field so "len" shall be used */
-            ret = mbedtls_x509_get_subject_alt_name_ext(p,
-                                                        (*p+len),
-                                                        &authority_key_id->authorityCertIssuer);
-            if (ret != 0) {
-                return ret;
-            }
-
-            /* Getting authorityCertSerialNumber using the required specific class tag [2] */
-            if ((ret = mbedtls_asn1_get_tag(p, end, &len,
-                                            MBEDTLS_ASN1_CONTEXT_SPECIFIC | 2)) != 0) {
-                return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret);
-            } else {
-                authority_key_id->authorityCertSerialNumber.len = len;
-                authority_key_id->authorityCertSerialNumber.p = *p;
-                authority_key_id->authorityCertSerialNumber.tag = MBEDTLS_ASN1_INTEGER;
-                *p += len;
-            }
         }
+        /* "end" also includes the CertSerialNumber field so "len" shall be used */
+        ret = mbedtls_x509_get_subject_alt_name_ext(p,
+                                                    (*p+len),
+                                                    &authority_key_id->authorityCertIssuer);
+        if (ret != 0) {
+            return ret;
+        }
+
+        /* Getting authorityCertSerialNumber using the required specific class tag [2] */
+        if ((ret = mbedtls_asn1_get_tag(p, end, &len,
+                                        MBEDTLS_ASN1_CONTEXT_SPECIFIC | 2)) != 0) {
+            return MBEDTLS_ERROR_ADD(MBEDTLS_ERR_X509_INVALID_EXTENSIONS, ret);
+        }
+        authority_key_id->authorityCertSerialNumber.len = len;
+        authority_key_id->authorityCertSerialNumber.p = *p;
+        authority_key_id->authorityCertSerialNumber.tag = MBEDTLS_ASN1_INTEGER;
+        *p += len;
     }
 
     if (*p != end) {