From 67284cce00147d1e803b51169dfe19c88e86f3ca Mon Sep 17 00:00:00 2001 From: Hanno Becker Date: Thu, 21 Feb 2019 14:31:51 +0000 Subject: [PATCH] Add abort condition callback to `mbedtls_x509_name_cmp_raw()` There are three operations that need to be performed on an X.509 name: 1 Initial traversal to check well-formedness of the ASN.1 structure. 2 Comparison between two X.509 name sequences. 3 Checking whether an X.509 name matches a client's ServerName request. Each of these tasks involves traversing the nested ASN.1 structure, In the interest of saving code, we aim to provide a single function which can perform all of the above tasks. The existing comparison function is already suitable not only for task 2, but also for 1: One can simply pass two equal ASN.1 name buffers, in which case the function will succeed if and only if that buffer is a well-formed ASN.1 name. This commit further adds a callback to `mbedtls_x509_name_cmp_raw()` which is called after each successful step in the simultaneous name traversal and comparison; it may perform any operation on the current name and potentially signal that the comparison should be aborted. With that, task 3 can be implemented by passing equal names and a callback which aborts as soon as it finds the desired name component. --- include/mbedtls/x509.h | 8 ++++++-- library/x509.c | 24 +++++++++++++++++++++--- library/x509_crt.c | 12 ++++++++---- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/include/mbedtls/x509.h b/include/mbedtls/x509.h index ba7a17419..c7b8cc4c5 100644 --- a/include/mbedtls/x509.h +++ b/include/mbedtls/x509.h @@ -312,8 +312,12 @@ int mbedtls_x509_get_time( unsigned char **p, const unsigned char *end, mbedtls_x509_time *t ); int mbedtls_x509_get_serial( unsigned char **p, const unsigned char *end, mbedtls_x509_buf *serial ); -int mbedtls_x509_name_cmp_raw( const mbedtls_x509_buf_raw *a, - const mbedtls_x509_buf_raw *b ); +int mbedtls_x509_name_cmp_raw( mbedtls_x509_buf_raw const *a, + mbedtls_x509_buf_raw const *b, + int (*check)( void *ctx, + mbedtls_x509_buf *oid, + mbedtls_x509_buf *val ), + void *check_ctx ); int mbedtls_x509_memcasecmp( const void *s1, const void *s2, size_t len ); int mbedtls_x509_get_ext( unsigned char **p, const unsigned char *end, mbedtls_x509_buf *ext, int tag ); diff --git a/library/x509.c b/library/x509.c index f74d474a9..b49ecf3a7 100644 --- a/library/x509.c +++ b/library/x509.c @@ -548,9 +548,16 @@ static int x509_string_cmp( const mbedtls_x509_buf *a, * but never the other way. (In particular, we don't do Unicode normalisation * or space folding.) * + * Further, this function allows to pass a callback to be triggered for every + * pair of well-formed and equal entries in the two input name lists. + * * Returns: - * - 0 if both sequences are well-formed and present the same X.509 name. - * - 1 if a difference was detected. + * - 0 if both sequences are well-formed, present the same X.509 name, + * and the callback (if provided) hasn't returned a non-zero value + * on any of the name components. + * - 1 if a difference was detected in the name components. + * - A non-zero error code if the abort callback returns a non-zero value. + * In this case, the returned error code is the error code from the callback. * - A negative error code if a parsing error occurred in either * of the two buffers. * @@ -558,7 +565,11 @@ static int x509_string_cmp( const mbedtls_x509_buf *a, * ASN.1 encoded X.509 name by calling it with equal parameters. */ int mbedtls_x509_name_cmp_raw( mbedtls_x509_buf_raw const *a, - mbedtls_x509_buf_raw const *b ) + mbedtls_x509_buf_raw const *b, + int (*abort_check)( void *ctx, + mbedtls_x509_buf *oid, + mbedtls_x509_buf *val ), + void *abort_check_ctx ) { int ret; @@ -597,6 +608,13 @@ int mbedtls_x509_name_cmp_raw( mbedtls_x509_buf_raw const *a, if( ( set_a == p_a ) != ( set_b == p_b ) ) return( 1 ); + if( abort_check != NULL ) + { + ret = abort_check( abort_check_ctx, &oid_a, &val_a ); + if( ret != 0 ) + return( ret ); + } + if( p_a == end_a && p_b == end_b ) break; } diff --git a/library/x509_crt.c b/library/x509_crt.c index ad70a201b..85bccedcb 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -1747,7 +1747,8 @@ static int x509_crt_verifycrl( mbedtls_x509_crt *crt, mbedtls_x509_crt *ca, { if( crl_list->version == 0 || mbedtls_x509_name_cmp_raw( &crl_list->issuer_raw_no_hdr, - &ca->subject_raw_no_hdr ) != 0 ) + &ca->subject_raw_no_hdr, + NULL, NULL ) != 0 ) { crl_list = crl_list->next; continue; @@ -1869,7 +1870,8 @@ static int x509_crt_check_parent( const mbedtls_x509_crt *child, /* Parent must be the issuer */ if( mbedtls_x509_name_cmp_raw( &child->issuer_raw_no_hdr, - &parent->subject_raw_no_hdr ) != 0 ) + &parent->subject_raw_no_hdr, + NULL, NULL ) != 0 ) { return( -1 ); } @@ -2138,7 +2140,8 @@ static int x509_crt_check_ee_locally_trusted( /* must be self-issued */ if( mbedtls_x509_name_cmp_raw( &crt->issuer_raw_no_hdr, - &crt->subject_raw_no_hdr ) != 0 ) + &crt->subject_raw_no_hdr, + NULL, NULL ) != 0 ) { return( -1 ); } @@ -2306,7 +2309,8 @@ find_parent: * and should be excluded from max_pathlen checks. */ if( ver_chain->len != 1 && mbedtls_x509_name_cmp_raw( &child->issuer_raw_no_hdr, - &child->subject_raw_no_hdr ) == 0 ) + &child->subject_raw_no_hdr, + NULL, NULL ) == 0 ) { self_cnt++; }