Add zero-cost abstraction layer for CRT verification chain

When verifying an X.509 certificate, the current verification logic
maintains an instance of the internal mbedtls_x509_crt_verify_chain
structure representing the state of the verification process. This
instance references the list of certificates that comprise the chain
built so far together with their verification flags. This information
must be stored during verification because it's being passed to the
verification callback at the end of verification - if the user has
specified those.

If the user hasn't specified a verification callback, it is not
necessary to maintain the list of CRTs, and it is also not necessary
to maintain verification flags for each CRT individually, as they're
merged at the end of the verification process.

To allow a readable simplification of the code in case no verification
callbacks are used, this commit introduces a zero-cost abstraction layer
for the functionality that's required from the verification chain structure:
- init/reset
- add a new CRT to the chain
- get pointer to current CRT flags
- add flags to EE certificate
- get current chain length
- trigger callbacks and get final (merged) flags
This gives flexibility for re-implementing the verification chain
structure, e.g. in the case where no verification callbacks are
provided, and there's hence no need to store CRTs and flags
individually. This will be done in a later commit.

include/mbedtls/x509_crt.h

@@ -249,6 +249,9 @@ typedef struct
     /* for find_parent_in() */
     mbedtls_x509_crt *parent; /* non-null iff parent_in in progress */
 
+    /* current child CRT */
+    mbedtls_x509_crt *cur_crt;
+
 #if defined(MBEDTLS_HAVE_TIME_DATE)
     mbedtls_x509_crt *fallback_parent;
     int fallback_signature_is_good;

library/x509_crt.c

@@ -669,23 +669,6 @@ static int x509_check_wildcard( char const *cn,
 }
 #endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */
 
-/*
- * Reset (init or clear) a verify_chain
- */
-static void x509_crt_verify_chain_reset(
-    mbedtls_x509_crt_verify_chain *ver_chain )
-{
-    size_t i;
-
-    for( i = 0; i < MBEDTLS_X509_MAX_VERIFY_CHAIN_SIZE; i++ )
-    {
-        ver_chain->items[i].crt = NULL;
-        ver_chain->items[i].flags = (uint32_t) -1;
-    }
-
-    ver_chain->len = 0;
-}
-
 /*
  *  Version  ::=  INTEGER  {  v1(0), v2(1), v3(2)  }
  */
@@ -3202,6 +3185,82 @@ static int x509_crt_check_ee_locally_trusted(
     return( -1 );
 }
 
+/*
+ * Reset (init or clear) a verify_chain
+ */
+static void x509_crt_verify_chain_reset(
+    mbedtls_x509_crt_verify_chain *ver_chain )
+{
+    size_t i;
+
+    for( i = 0; i < MBEDTLS_X509_MAX_VERIFY_CHAIN_SIZE; i++ )
+    {
+        ver_chain->items[i].crt = NULL;
+        ver_chain->items[i].flags = (uint32_t) -1;
+    }
+
+    ver_chain->len = 0;
+}
+
+/*
+ * Merge the flags for all certs in the chain, after calling callback
+ */
+static int x509_crt_verify_chain_get_flags(
+           const mbedtls_x509_crt_verify_chain *ver_chain,
+           uint32_t *flags,
+           int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *),
+           void *p_vrfy )
+{
+    int ret;
+    unsigned i;
+    uint32_t cur_flags;
+    const mbedtls_x509_crt_verify_chain_item *cur;
+
+    for( i = ver_chain->len; i != 0; --i )
+    {
+        cur = &ver_chain->items[i-1];
+        cur_flags = cur->flags;
+
+        if( NULL != f_vrfy )
+            if( ( ret = f_vrfy( p_vrfy, cur->crt, (int) i-1, &cur_flags ) ) != 0 )
+                return( ret );
+
+        *flags |= cur_flags;
+    }
+
+    return( 0 );
+}
+
+static void x509_crt_verify_chain_add_ee_flags(
+    mbedtls_x509_crt_verify_chain *chain,
+    uint32_t ee_flags )
+{
+    chain->items[0].flags |= ee_flags;
+}
+
+static void x509_crt_verify_chain_add_crt(
+    mbedtls_x509_crt_verify_chain *chain,
+    mbedtls_x509_crt *crt )
+{
+    mbedtls_x509_crt_verify_chain_item *cur;
+    cur = &chain->items[chain->len];
+    cur->crt = crt;
+    cur->flags = 0;
+    chain->len++;
+}
+
+static uint32_t* x509_crt_verify_chain_get_cur_flags(
+    mbedtls_x509_crt_verify_chain *chain )
+{
+    return( &chain->items[chain->len - 1].flags );
+}
+
+static unsigned x509_crt_verify_chain_len(
+    mbedtls_x509_crt_verify_chain const *chain )
+{
+    return( chain->len );
+}
+
 /*
  * Build and verify a certificate chain
  *
@@ -3254,7 +3313,6 @@ static int x509_crt_verify_chain(
      * catch potential issues with jumping ahead when restarting */
     int ret;
     uint32_t *flags;
-    mbedtls_x509_crt_verify_chain_item *cur;
     mbedtls_x509_crt *child_crt;
     mbedtls_x509_crt *parent_crt;
     int parent_is_trusted;
@@ -3269,10 +3327,7 @@ static int x509_crt_verify_chain(
         /* restore saved state */
         *ver_chain = rs_ctx->ver_chain; /* struct copy */
         self_cnt = rs_ctx->self_cnt;
-
+        child_crt = rs_ctx->cur_crt;
-        /* restore derived state */
-        cur = &ver_chain->items[ver_chain->len - 1];
-        child_crt = cur->crt;
 
         child_is_trusted = 0;
         goto find_parent;
@@ -3291,16 +3346,13 @@ static int x509_crt_verify_chain(
         int self_issued;
 
         /* Add certificate to the verification chain */
-        cur = &ver_chain->items[ver_chain->len];
+        x509_crt_verify_chain_add_crt( ver_chain, child_crt );
-        cur->crt = child_crt;
-        cur->flags = 0;
-        ver_chain->len++;
 
 #if defined(MBEDTLS_ECDSA_C) && defined(MBEDTLS_ECP_RESTARTABLE)
 find_parent:
 #endif
 
-        flags = &cur->flags;
+        flags = x509_crt_verify_chain_get_cur_flags( ver_chain );
 
         {
             mbedtls_x509_crt_sig_info child_sig;
@@ -3342,7 +3394,7 @@ find_parent:
                     *flags |= MBEDTLS_X509_BADCERT_BAD_PK;
 
                 /* Special case: EE certs that are locally trusted */
-                if( ver_chain->len == 1 && self_issued &&
+                if( x509_crt_verify_chain_len( ver_chain ) == 1 && self_issued &&
                     x509_crt_check_ee_locally_trusted( child, trust_ca ) == 0 )
                 {
                     mbedtls_x509_crt_frame_release( child_crt );
@@ -3364,7 +3416,8 @@ find_parent:
             ret = x509_crt_find_parent( &child_sig, child_crt->next,
                                         trust_ca, &parent_crt,
                                         &parent_is_trusted, &signature_is_good,
-                                        ver_chain->len - 1, self_cnt, rs_ctx );
+                                        x509_crt_verify_chain_len( ver_chain ) - 1,
+                                        self_cnt, rs_ctx );
 
             x509_crt_free_sig_info( &child_sig );
         }
@@ -3376,6 +3429,7 @@ find_parent:
             rs_ctx->in_progress = x509_crt_rs_find_parent;
             rs_ctx->self_cnt = self_cnt;
             rs_ctx->ver_chain = *ver_chain; /* struct copy */
+            rs_ctx->cur_crt = child_crt;
             return( ret );
         }
 #else
@@ -3392,13 +3446,14 @@ find_parent:
         /* Count intermediate self-issued (not necessarily self-signed) certs.
          * These can occur with some strategies for key rollover, see [SIRO],
          * and should be excluded from max_pathlen checks. */
-        if( ver_chain->len != 1 && self_issued )
+        if( x509_crt_verify_chain_len( ver_chain ) != 1 && self_issued )
             self_cnt++;
 
         /* path_cnt is 0 for the first intermediate CA,
          * and if parent is trusted it's not an intermediate CA */
         if( ! parent_is_trusted &&
-            ver_chain->len > MBEDTLS_X509_MAX_INTERMEDIATE_CA )
+            x509_crt_verify_chain_len( ver_chain ) >
+            MBEDTLS_X509_MAX_INTERMEDIATE_CA )
         {
             /* return immediately to avoid overflow the chain array */
             return( MBEDTLS_ERR_X509_FATAL_ERROR );
@@ -3552,35 +3607,6 @@ static int x509_crt_verify_name( const mbedtls_x509_crt *crt,
 }
 #endif /* !MBEDTLS_X509_REMOVE_HOSTNAME_VERIFICATION */
 
-/*
- * Merge the flags for all certs in the chain, after calling callback
- */
-static int x509_crt_merge_flags_with_cb(
-           uint32_t *flags,
-           const mbedtls_x509_crt_verify_chain *ver_chain,
-           int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *),
-           void *p_vrfy )
-{
-    int ret;
-    unsigned i;
-    uint32_t cur_flags;
-    const mbedtls_x509_crt_verify_chain_item *cur;
-
-    for( i = ver_chain->len; i != 0; --i )
-    {
-        cur = &ver_chain->items[i-1];
-        cur_flags = cur->flags;
-
-        if( NULL != f_vrfy )
-            if( ( ret = f_vrfy( p_vrfy, cur->crt, (int) i-1, &cur_flags ) ) != 0 )
-                return( ret );
-
-        *flags |= cur_flags;
-    }
-
-    return( 0 );
-}
-
 /*
  * Verify the certificate validity (default profile, not restartable)
  */
@@ -3714,13 +3740,13 @@ int mbedtls_x509_crt_verify_restartable( mbedtls_x509_crt *crt,
         goto exit;
 
     /* Merge end-entity flags */
-    ver_chain.items[0].flags |= ee_flags;
+    x509_crt_verify_chain_add_ee_flags( &ver_chain, ee_flags );
 
     /* Build final flags, calling callback on the way if any */
 #if !defined(MBEDTLS_X509_REMOVE_VERIFY_CALLBACK)
-    ret = x509_crt_merge_flags_with_cb( flags, &ver_chain, f_vrfy, p_vrfy );
+    ret = x509_crt_verify_chain_get_flags( &ver_chain, flags, f_vrfy, p_vrfy );
 #else
-    ret = x509_crt_merge_flags_with_cb( flags, &ver_chain, NULL, NULL );
+    ret = x509_crt_verify_chain_get_flags( &ver_chain, flags, NULL, NULL );
 #endif /* MBEDTLS_X509_REMOVE_VERIFY_CALLBACK */
 
 exit: