From c26bd760204b27149a4bb08bf2604cc34d3a85fd Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sun, 23 Oct 2022 19:48:18 -0400 Subject: [PATCH 01/18] x509 crt verify SAN iPAddress Signed-off-by: Glenn Strauss --- include/mbedtls/x509_crt.h | 2 +- library/x509_crt.c | 126 ++++++++++++++++++++++++++++++------- 2 files changed, 103 insertions(+), 25 deletions(-) diff --git a/include/mbedtls/x509_crt.h b/include/mbedtls/x509_crt.h index 6c86a6629e..a7951833ab 100644 --- a/include/mbedtls/x509_crt.h +++ b/include/mbedtls/x509_crt.h @@ -638,7 +638,7 @@ int mbedtls_x509_crt_verify_info(char *buf, size_t size, const char *prefix, * \param cn The expected Common Name. This will be checked to be * present in the certificate's subjectAltNames extension or, * if this extension is absent, as a CN component in its - * Subject name. Currently only DNS names are supported. This + * Subject name. DNS names and IP addresses are supported. This * may be \c NULL if the CN need not be verified. * \param flags The address at which to store the result of the verification. * If the verification couldn't be completed, the flag value is diff --git a/library/x509_crt.c b/library/x509_crt.c index cf62532f28..8775bcbb7e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -58,6 +58,10 @@ #if defined(MBEDTLS_HAVE_TIME) #if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32) +#define WIN32_LEAN_AND_MEAN +#ifndef _WIN32_WINNT +#define _WIN32_WINNT 0x0600 +#endif #include #else #include @@ -2524,6 +2528,61 @@ find_parent: } } +#ifdef _WIN32 +#ifdef _MSC_VER +#pragma comment(lib, "ws2_32.lib") +#include +#include +#elif (defined(__MINGW32__) || defined(__MINGW64__)) && _WIN32_WINNT >= 0x0600 +#include +#include +#endif +#elif defined(__sun) +/* Solaris requires -lsocket -lnsl for inet_pton() */ +#elif defined(__has_include) +#if __has_include() +#include +#endif +#if __has_include() +#include +#endif +#endif + +/* Use whether or not AF_INET6 is defined to indicate whether or not to use + * the platform inet_pton() or a local implementation (below). The local + * implementation may be used even in cases where the platform provides + * inet_pton(), e.g. when there are different includes required and/or the + * platform implementation requires dependencies on additional libraries. + * Specifically, Windows requires custom includes and additional link + * dependencies, and Solaris requires additional link dependencies. + * Also, as a coarse heuristic, use the local implementation if the compiler + * does not support __has_include(), or if the definition of AF_INET6 is not + * provided by headers included (or not) via __has_include() above. */ +#ifndef AF_INET6 + +#define x509_cn_inet_pton(cn, dst) (0) + +#else + +static int x509_inet_pton_ipv6(const char *src, void *dst) +{ + return inet_pton(AF_INET6, src, dst) == 1 ? 0 : -1; +} + +static int x509_inet_pton_ipv4(const char *src, void *dst) +{ + return inet_pton(AF_INET, src, dst) == 1 ? 0 : -1; +} + +#endif /* AF_INET6 */ + +static size_t x509_cn_inet_pton(const char *cn, void *dst) +{ + return strchr(cn, ':') == NULL + ? x509_inet_pton_ipv4(cn, dst) == 0 ? 4 : 0 + : x509_inet_pton_ipv6(cn, dst) == 0 ? 16 : 0; +} + /* * Check for CN match */ @@ -2544,24 +2603,51 @@ static int x509_crt_check_cn(const mbedtls_x509_buf *name, return -1; } +static int x509_crt_check_san_ip(const mbedtls_x509_sequence *san, + const char *cn, size_t cn_len) +{ + uint32_t ip[4]; + cn_len = x509_cn_inet_pton(cn, ip); + if (cn_len == 0) { + return -1; + } + + for (const mbedtls_x509_sequence *cur = san; cur != NULL; cur = cur->next) { + const unsigned char san_type = (unsigned char) cur->buf.tag & + MBEDTLS_ASN1_TAG_VALUE_MASK; + if (san_type == MBEDTLS_X509_SAN_IP_ADDRESS && + cur->buf.len == cn_len && memcmp(cur->buf.p, ip, cn_len) == 0) { + return 0; + } + } + + return -1; +} + /* * Check for SAN match, see RFC 5280 Section 4.2.1.6 */ -static int x509_crt_check_san(const mbedtls_x509_buf *name, +static int x509_crt_check_san(const mbedtls_x509_sequence *san, const char *cn, size_t cn_len) { - const unsigned char san_type = (unsigned char) name->tag & - MBEDTLS_ASN1_TAG_VALUE_MASK; - - /* dNSName */ - if (san_type == MBEDTLS_X509_SAN_DNS_NAME) { - return x509_crt_check_cn(name, cn, cn_len); + int san_ip = 0; + for (const mbedtls_x509_sequence *cur = san; cur != NULL; cur = cur->next) { + switch ((unsigned char) cur->buf.tag & MBEDTLS_ASN1_TAG_VALUE_MASK) { + case MBEDTLS_X509_SAN_DNS_NAME: /* dNSName */ + if (x509_crt_check_cn(&cur->buf, cn, cn_len) == 0) { + return 0; + } + break; + case MBEDTLS_X509_SAN_IP_ADDRESS: /* iPAddress */ + san_ip = 1; + break; + /* (We may handle other types here later.) */ + default: /* Unrecognized type */ + break; + } } - /* (We may handle other types here later.) */ - - /* Unrecognized type */ - return -1; + return san_ip ? x509_crt_check_san_ip(san, cn, cn_len) : -1; } /* @@ -2572,31 +2658,23 @@ static void x509_crt_verify_name(const mbedtls_x509_crt *crt, uint32_t *flags) { const mbedtls_x509_name *name; - const mbedtls_x509_sequence *cur; size_t cn_len = strlen(cn); if (crt->ext_types & MBEDTLS_X509_EXT_SUBJECT_ALT_NAME) { - for (cur = &crt->subject_alt_names; cur != NULL; cur = cur->next) { - if (x509_crt_check_san(&cur->buf, cn, cn_len) == 0) { - break; - } - } - - if (cur == NULL) { - *flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH; + if (x509_crt_check_san(&crt->subject_alt_names, cn, cn_len) == 0) { + return; } } else { for (name = &crt->subject; name != NULL; name = name->next) { if (MBEDTLS_OID_CMP(MBEDTLS_OID_AT_CN, &name->oid) == 0 && x509_crt_check_cn(&name->val, cn, cn_len) == 0) { - break; + return; } } - if (name == NULL) { - *flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH; - } } + + *flags |= MBEDTLS_X509_BADCERT_CN_MISMATCH; } /* From 416c2950780ff0f139b4960b99caa22e5b6e7b95 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 24 Oct 2022 23:00:02 -0400 Subject: [PATCH 02/18] x509 crt verify local implementation to parse IP x509 crt verify local implementation to parse IP if inet_pton() is not portably available Signed-off-by: Glenn Strauss --- library/x509_crt.c | 82 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 8775bcbb7e..d0b2a2aa7e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2560,7 +2560,87 @@ find_parent: * provided by headers included (or not) via __has_include() above. */ #ifndef AF_INET6 -#define x509_cn_inet_pton(cn, dst) (0) +/* definition located further below to possibly reduce compiler inlining */ +static int x509_inet_pton_ipv4(const char *src, void *dst); + +#define li_cton(c, n) \ + (((n) = (c) - '0') <= 9 || (((n) = ((c)&0xdf) - 'A') <= 5 ? ((n) += 10) : 0)) + +static int x509_inet_pton_ipv6(const char *src, void *dst) +{ + const unsigned char *v = (const unsigned char *) src; + int i = 0, j, dc = -1; + uint16_t addr[8]; + do { + /* note: allows excess leading 0's, e.g. 1:0002:3:... */ + uint16_t x = j = 0; + for (uint8_t n; j < 4 && li_cton(*v, n); x <<= 4, x |= n, ++v, ++j) { + ; + } + if (j != 0) { + addr[i++] = (x << 8) | (x >> 8); /* htons(x) */ + if (*v == '\0') { + break; + } else if (*v == '.' && (i != 0 || dc != -1) && (i < 7) && + /* walk back to prior ':', then parse as IPv4-mapped */ + (*--v == ':' || *--v == ':' || + *--v == ':' || *--v == ':') && + x509_inet_pton_ipv4((const char *) ++v, addr + --i) == 0) { + i += 2; + v = (const unsigned char *) ""; + break; + } else if (*v != ':') { + return -1; + } + } else { + if (dc != -1 || *v != ':' || ((dc = i) == 0 && *++v != ':')) { + return -1; + } + if (v[1] == '\0') { + ++v; + break; + } + } + ++v; + } while (i < 8); + if ((dc != -1 ? i > 6 : i != 8) || *v != '\0') { + return -1; + } + + if (dc != -1) { + i -= dc; + j = 8 - i - dc; + if (i) { + memmove(addr + dc + j, addr + dc, i * sizeof(*addr)); + } + memset(addr + dc, 0, j * sizeof(*addr)); + } + memcpy(dst, addr, sizeof(addr)); + return 0; +} + +static int x509_inet_pton_ipv4(const char *src, void *dst) +{ + /* note: allows leading 0's, e.g. 000.000.000.000 */ + const unsigned char *v = (const unsigned char *) src; + uint8_t *res = (uint8_t *) dst; + uint8_t d1, d2, d3, i = 0; + int ii; + const uint8_t tens[] = { 0, 10, 20, 30, 40, 50, 60, 70, 80, 90 }; + do { + if ((d1 = *(uint8_t *) v - '0') > 9) { + break; + } else if ((d2 = *(uint8_t *) ++v - '0') > 9) { + *res++ = d1; + } else if ((d3 = *(uint8_t *) ++v - '0') > 9) { + *res++ = tens[d1] + d2; + } else if ((ii = (d1 < 2 ? d1 == 1 ? 100 : 0 : d1 == 2 ? 200 : 999) + + tens[d2] + d3) < 256) { + *res++ = (uint8_t) ii, ++v; + } + } while (++i < 4 && *v++ == '.'); + return i == 4 && *v == '\0' ? 0 : -1; +} #else From 3208b0b39130481f746af43fda03fff659cddf6c Mon Sep 17 00:00:00 2001 From: Eugene K Date: Thu, 20 Aug 2020 11:26:01 -0400 Subject: [PATCH 03/18] add IP SAN tests changes per mbedTLS standards Signed-off-by: Eugene K --- ChangeLog.d/verify-ip-sans-properly.txt | 2 ++ tests/suites/test_suite_x509parse.data | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 ChangeLog.d/verify-ip-sans-properly.txt diff --git a/ChangeLog.d/verify-ip-sans-properly.txt b/ChangeLog.d/verify-ip-sans-properly.txt new file mode 100644 index 0000000000..00203a8ca4 --- /dev/null +++ b/ChangeLog.d/verify-ip-sans-properly.txt @@ -0,0 +1,2 @@ +Features + * X.509 hostname verification now supports IPAddress Subject Alternate Names. diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 685b8596d2..0966683f15 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1023,6 +1023,26 @@ X509 CRT verification: domain identical to IPv6 in SubjectAltName depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd.example.com":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" +X509 CRT verification: matching IPv4 in SubjectAltName +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100":0:0:"":"NULL" + +X509 CRT verification: mismatching IPv4 in SubjectAltName +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"7.8.9.10":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" + +X509 CRT verification: IPv4 with trailing data in SubjectAltName +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100?":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" + +X509 CRT verification: matching IPv6 in SubjectAltName +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:2E65\:7861\:6D70\:6C65\:2E63\:6F6D":0:0:"":"NULL" + +X509 CRT verification: mismatching IPv6 in SubjectAltName +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:\:6F6D":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" + X509 CRT verification with ca callback: failure depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_MD_CAN_SHA1:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR From 6f545acfaf129bd5fabad257c8d805fba8efab4d Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Tue, 25 Oct 2022 15:02:14 -0400 Subject: [PATCH 04/18] Add mbedtls_x509_crt_parse_cn_inet_pton() tests Extended from https://github.com/Mbed-TLS/mbedtls/pull/2906 contributed by Eugene K Signed-off-by: Glenn Strauss --- library/x509_crt.c | 6 +- library/x509_invasive.h | 53 +++++++++++++ tests/suites/test_suite_x509parse.data | 87 ++++++++++++++++++++++ tests/suites/test_suite_x509parse.function | 15 ++++ 4 files changed, 159 insertions(+), 2 deletions(-) create mode 100644 library/x509_invasive.h diff --git a/library/x509_crt.c b/library/x509_crt.c index d0b2a2aa7e..7def3e992c 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -49,6 +49,7 @@ #include "mbedtls/psa_util.h" #endif /* MBEDTLS_USE_PSA_CRYPTO */ #include "hash_info.h" +#include "x509_invasive.h" #include "mbedtls/platform.h" @@ -2656,7 +2657,8 @@ static int x509_inet_pton_ipv4(const char *src, void *dst) #endif /* AF_INET6 */ -static size_t x509_cn_inet_pton(const char *cn, void *dst) +MBEDTLS_STATIC_TESTABLE +size_t mbedtls_x509_crt_parse_cn_inet_pton(const char *cn, void *dst) { return strchr(cn, ':') == NULL ? x509_inet_pton_ipv4(cn, dst) == 0 ? 4 : 0 @@ -2687,7 +2689,7 @@ static int x509_crt_check_san_ip(const mbedtls_x509_sequence *san, const char *cn, size_t cn_len) { uint32_t ip[4]; - cn_len = x509_cn_inet_pton(cn, ip); + cn_len = mbedtls_x509_crt_parse_cn_inet_pton(cn, ip); if (cn_len == 0) { return -1; } diff --git a/library/x509_invasive.h b/library/x509_invasive.h new file mode 100644 index 0000000000..d8fd74be49 --- /dev/null +++ b/library/x509_invasive.h @@ -0,0 +1,53 @@ +/** + * \file x509_invasive.h + * + * \brief x509 module: interfaces for invasive testing only. + * + * The interfaces in this file are intended for testing purposes only. + * They SHOULD NOT be made available in library integrations except when + * building the library for testing. + */ +/* + * Copyright The Mbed TLS Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef MBEDTLS_X509_INVASIVE_H +#define MBEDTLS_X509_INVASIVE_H + +#include "common.h" + +#if defined(MBEDTLS_TEST_HOOKS) + +/** + * \brief This function parses a CN string as an IP address. + * + * \param cn The CN string to parse. CN string MUST be NUL-terminated. + * \param dst The target buffer to populate with the binary IP address. + * The buffer MUST be 16 bytes to save IPv6, and should be + * 4-byte aligned if the result will be used as struct in_addr. + * e.g. uint32_t dst[4] + * + * \note \cn is parsed as an IPv6 address if string contains ':', + * else \cn is parsed as an IPv4 address. + * + * \return Length of binary IP address; num bytes written to target. + * \return \c 0 on failure to parse CN string as an IP address. + */ +size_t mbedtls_x509_crt_parse_cn_inet_pton(const char *cn, void *dst); + +#endif /* MBEDTLS_TEST_HOOKS */ + +#endif /* MBEDTLS_X509_INVASIVE_H */ diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 0966683f15..c1055f1270 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1043,6 +1043,93 @@ X509 CRT verification: mismatching IPv6 in SubjectAltName depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:\:6F6D":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" +X509 CRT parse CN: IPv4 valid address +x509_crt_parse_cn_inet_pton:"10.10.10.10":"0A0A0A0A":4 + +X509 CRT parse CN: IPv4 excess 0s +x509_crt_parse_cn_inet_pton:"10.0000.10.10":"":0 + +X509 CRT parse CN: IPv4 short address +x509_crt_parse_cn_inet_pton:"10.10.10":"":0 + +X509 CRT parse CN: IPv4 invalid ? char +x509_crt_parse_cn_inet_pton:"10.10?10.10":"":0 + +X509 CRT parse CN: IPv4 invalid - char +x509_crt_parse_cn_inet_pton:"10.-10.10.10":"":0 + +X509 CRT parse CN: IPv4 invalid + char +x509_crt_parse_cn_inet_pton:"10.+10.10.10":"":0 + +X509 CRT parse CN: IPv4 begin dot +x509_crt_parse_cn_inet_pton:".10.10.10.10":"":0 + +X509 CRT parse CN: IPv4 end dot +x509_crt_parse_cn_inet_pton:"10.10.10.10.":"":0 + +X509 CRT parse CN: IPv4 consecutive dots +x509_crt_parse_cn_inet_pton:"10.10..10.10.":"":0 + +X509 CRT parse CN: IPv4 overlarge octet 256 +x509_crt_parse_cn_inet_pton:"10.256.10.10":"":0 + +X509 CRT parse CN: IPv4 overlarge octet 1000 +x509_crt_parse_cn_inet_pton:"10.1000.10.10":"":0 + +X509 CRT parse CN: IPv4 additional octet +x509_crt_parse_cn_inet_pton:"10.10.10.10.10":"":0 + +X509 CRT parse CN: IPv6 valid address +x509_crt_parse_cn_inet_pton:"1\:2\:3\:4\:5\:6\:7\:8":"00010002000300040005000600070008":16 + +X509 CRT parse CN: IPv6 valid address shorthand +x509_crt_parse_cn_inet_pton:"6263\:\:1":"62630000000000000000000000000001":16 + +X509 CRT parse CN: IPv6 valid address shorthand start +x509_crt_parse_cn_inet_pton:"\:\:1":"00000000000000000000000000000001":16 + +X509 CRT parse CN: IPv6 valid address extra 0s +x509_crt_parse_cn_inet_pton:"0001\:\:0001\:0001":"00010000000000000000000000010001":16 + +X509 CRT parse CN: IPv6 invalid address excess 0s +x509_crt_parse_cn_inet_pton:"1\:00000\:1\:0":"":0 + +X509 CRT parse CN: IPv6 invalid address - start single colon +x509_crt_parse_cn_inet_pton:"\:6263\:\:1":"":0 + +X509 CRT parse CN: IPv6 invalid address - end single colon +x509_crt_parse_cn_inet_pton:"6263\:\:1\:":"":0 + +X509 CRT parse CN: IPv6 short address +x509_crt_parse_cn_inet_pton:"1\:1\:1":"":0 + +X509 CRT parse CN: IPv6 wildcard address +x509_crt_parse_cn_inet_pton:"\:\:":"00000000000000000000000000000000":16 + +X509 CRT parse CN: IPv6 address too long +x509_crt_parse_cn_inet_pton:"1\:2\:3\:4\:5\:6\:7\:8\:9":"":0 + +X509 CRT parse CN: IPv6 long hextet +x509_crt_parse_cn_inet_pton:"12345\:\:1":"":0 + +X509 CRT parse CN: IPv6 invalid char +x509_crt_parse_cn_inet_pton:"\:\:\:1":"":0 + +X509 CRT parse CN: IPv6 invalid - char +x509_crt_parse_cn_inet_pton:"\:\:-1\:1":"":0 + +X509 CRT parse CN: IPv6 invalid + char +x509_crt_parse_cn_inet_pton:"\:\:+1\:1":"":0 + +X509 CRT parse CN: IPv6 valid address IPv4-mapped +x509_crt_parse_cn_inet_pton:"\:\:ffff\:1.2.3.4":"00000000000000000000ffff01020304":16 + +X509 CRT parse CN: IPv6 invalid address IPv4-mapped #1 +x509_crt_parse_cn_inet_pton:"\:\:ffff\:999.2.3.4":"":0 + +X509 CRT parse CN: IPv6 invalid address IPv4-mapped #2 +x509_crt_parse_cn_inet_pton:"\:\:1.2.3.4\:ffff":"":0 + X509 CRT verification with ca callback: failure depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_MD_CAN_SHA1:MBEDTLS_RSA_C:MBEDTLS_PKCS1_V15:MBEDTLS_X509_TRUSTED_CERTIFICATE_CALLBACK x509_verify_ca_cb_failure:"data_files/server1.crt":"data_files/test-ca.crt":"NULL":MBEDTLS_ERR_X509_FATAL_ERROR diff --git a/tests/suites/test_suite_x509parse.function b/tests/suites/test_suite_x509parse.function index 177bc97ad3..905d62f500 100644 --- a/tests/suites/test_suite_x509parse.function +++ b/tests/suites/test_suite_x509parse.function @@ -11,6 +11,8 @@ #include "mbedtls/pk.h" #include "string.h" +#include "x509_invasive.h" + #if MBEDTLS_X509_MAX_INTERMEDIATE_CA > 19 #error "The value of MBEDTLS_X509_MAX_INTERMEDIATE_C is larger \ than the current threshold 19. To test larger values, please \ @@ -436,6 +438,19 @@ void x509_accessor_ext_types(int ext_type, int has_ext_type) } /* END_CASE */ +/* BEGIN_CASE depends_on:MBEDTLS_X509_CRT_PARSE_C:MBEDTLS_TEST_HOOKS */ +void x509_crt_parse_cn_inet_pton(const char *cn, data_t *exp, int ref_ret) +{ + uint32_t addr[4]; + size_t addrlen = mbedtls_x509_crt_parse_cn_inet_pton(cn, addr); + TEST_EQUAL(addrlen, (size_t) ref_ret); + + if (addrlen) { + ASSERT_COMPARE(exp->x, exp->len, addr, addrlen); + } +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_FS_IO:MBEDTLS_X509_CRT_PARSE_C */ void x509_parse_san(char *crt_file, char *result_str, int parse_result) { From 700ffa074412b760290cf8136ac081b8a07b4553 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Mon, 27 Feb 2023 11:02:21 -0500 Subject: [PATCH 05/18] use MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA instead of MBEDTLS_SHA256_C in test data dependencies Signed-off-by: Glenn Strauss --- tests/suites/test_suite_x509parse.data | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index c1055f1270..be595f44b5 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1024,23 +1024,23 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_MD_CAN_SHA256 x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd.example.com":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100":0:0:"":"NULL" X509 CRT verification: mismatching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"7.8.9.10":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: IPv4 with trailing data in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100?":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:2E65\:7861\:6D70\:6C65\:2E63\:6F6D":0:0:"":"NULL" X509 CRT verification: mismatching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_SHA256_C:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:\:6F6D":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT parse CN: IPv4 valid address From 7bd00e07084989c491ad85cf74bf9b31094fa144 Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Tue, 28 Feb 2023 10:05:55 -0500 Subject: [PATCH 06/18] use MBEDTLS_PK_CAN_ECDSA_SOME instead of MBEDTLS_ECDSA_C in test data dependencies Signed-off-by: Glenn Strauss --- tests/suites/test_suite_x509parse.data | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index be595f44b5..a0e8dbab7b 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1024,23 +1024,23 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_MD_CAN_SHA256 x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd.example.com":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100":0:0:"":"NULL" X509 CRT verification: mismatching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"7.8.9.10":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: IPv4 with trailing data in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100?":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:2E65\:7861\:6D70\:6C65\:2E63\:6F6D":0:0:"":"NULL" X509 CRT verification: mismatching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_ECDSA_C:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:\:6F6D":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT parse CN: IPv4 valid address From b255e21e4848e70c0c87d485ddbe46fca5b94f9b Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Thu, 9 Mar 2023 16:00:54 -0500 Subject: [PATCH 07/18] Handle endianness in x509_inet_pton_ipv6() Signed-off-by: Glenn Strauss --- library/x509_crt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 7def3e992c..64016a552a 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2579,7 +2579,7 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) ; } if (j != 0) { - addr[i++] = (x << 8) | (x >> 8); /* htons(x) */ + addr[i++] = MBEDTLS_IS_BIG_ENDIAN ? x : (x << 8) | (x >> 8); if (*v == '\0') { break; } else if (*v == '.' && (i != 0 || dc != -1) && (i < 7) && From 13b8b780fea146c9f5cd200af564cdca72bc649c Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 12 Apr 2023 09:43:47 -0400 Subject: [PATCH 08/18] Improve x509_inet_pton_ipv4 readability Introduce descriptive variable names. Drop the table of tens. Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 64016a552a..791fad141f 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2623,24 +2623,23 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) static int x509_inet_pton_ipv4(const char *src, void *dst) { /* note: allows leading 0's, e.g. 000.000.000.000 */ - const unsigned char *v = (const unsigned char *) src; + const unsigned char *character = (const unsigned char *) src; uint8_t *res = (uint8_t *) dst; - uint8_t d1, d2, d3, i = 0; - int ii; - const uint8_t tens[] = { 0, 10, 20, 30, 40, 50, 60, 70, 80, 90 }; + uint8_t digit1, digit2, digit3, num_octets = 0; + uint16_t octet; + do { - if ((d1 = *(uint8_t *) v - '0') > 9) { + if ((digit1 = *(uint8_t *) character - '0') > 9) { break; - } else if ((d2 = *(uint8_t *) ++v - '0') > 9) { - *res++ = d1; - } else if ((d3 = *(uint8_t *) ++v - '0') > 9) { - *res++ = tens[d1] + d2; - } else if ((ii = (d1 < 2 ? d1 == 1 ? 100 : 0 : d1 == 2 ? 200 : 999) - + tens[d2] + d3) < 256) { - *res++ = (uint8_t) ii, ++v; + } else if ((digit2 = *(uint8_t *) ++character - '0') > 9) { + *res++ = digit1; + } else if ((digit3 = *(uint8_t *) ++character - '0') > 9) { + *res++ = digit1 * 10 + digit2; + } else if ((octet = digit1 * 100 + digit2 * 10 + digit3) < 256) { + *res++ = (uint8_t) octet, ++character; } - } while (++i < 4 && *v++ == '.'); - return i == 4 && *v == '\0' ? 0 : -1; + } while (++num_octets < 4 && *character++ == '.'); + return num_octets == 4 && *character == '\0' ? 0 : -1; } #else From e404612580a08688468d69ed0185da34544899d9 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 12 Apr 2023 09:44:44 -0400 Subject: [PATCH 09/18] Replace old macro in test_suite_x509parse MD_CAN_SHAXXX should be now used. Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_x509parse.data | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index a0e8dbab7b..499b58279c 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1024,23 +1024,23 @@ depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_VERIFY:MBEDTLS_MD_CAN_SHA256 x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"abcd.example.com":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100":0:0:"":"NULL" X509 CRT verification: mismatching IPv4 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"7.8.9.10":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: IPv4 with trailing data in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"97.98.99.100?":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT verification: matching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:2E65\:7861\:6D70\:6C65\:2E63\:6F6D":0:0:"":"NULL" X509 CRT verification: mismatching IPv6 in SubjectAltName -depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_HAS_ALG_SHA_256_VIA_MD_OR_PSA_BASED_ON_USE_PSA:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C +depends_on:MBEDTLS_PEM_PARSE_C:MBEDTLS_PK_CAN_ECDSA_SOME:MBEDTLS_MD_CAN_SHA256:MBEDTLS_ECP_DP_SECP256R1_ENABLED:MBEDTLS_RSA_C x509_verify:"data_files/server5-tricky-ip-san.crt":"data_files/server5-tricky-ip-san.crt":"data_files/crl_sha256.pem":"6162\:6364\:\:6F6D":MBEDTLS_ERR_X509_CERT_VERIFY_FAILED:MBEDTLS_X509_BADCERT_CN_MISMATCH:"":"NULL" X509 CRT parse CN: IPv4 valid address From fe050815c8dc1e53abef508aca4dad4f81d52f12 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 12 Apr 2023 09:45:07 -0400 Subject: [PATCH 10/18] Introduce an additional test for IPV4 parsing Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_x509parse.data | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index 499b58279c..b379959eb4 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1073,6 +1073,9 @@ x509_crt_parse_cn_inet_pton:"10.10..10.10.":"":0 X509 CRT parse CN: IPv4 overlarge octet 256 x509_crt_parse_cn_inet_pton:"10.256.10.10":"":0 +X509 CRT parse CN: IPv4 overlarge octet 999 +x509_crt_parse_cn_inet_pton:"10.10.10.999":"":0 + X509 CRT parse CN: IPv4 overlarge octet 1000 x509_crt_parse_cn_inet_pton:"10.1000.10.10":"":0 From 06969fc3a0656a075474b50bdbb05955db562339 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 12 Apr 2023 10:34:50 -0400 Subject: [PATCH 11/18] Introduce a test for a sw implementation of inet_pton Create a bypass define to simulate platforms without AF_INET6. Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 11 ++++++----- tests/scripts/all.sh | 11 +++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 791fad141f..58f7cc068e 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2558,10 +2558,11 @@ find_parent: * dependencies, and Solaris requires additional link dependencies. * Also, as a coarse heuristic, use the local implementation if the compiler * does not support __has_include(), or if the definition of AF_INET6 is not - * provided by headers included (or not) via __has_include() above. */ -#ifndef AF_INET6 - -/* definition located further below to possibly reduce compiler inlining */ + * provided by headers included (or not) via __has_include() above. + * MBEDTLS_TEST_SW_INET_PTON is a bypass define to force testing of this code //no-check-names + * despite having a platform that has inet_pton. */ +#if !defined(AF_INET6) || defined(MBEDTLS_TEST_SW_INET_PTON) //no-check-names +/* Definition located further below to possibly reduce compiler inlining */ static int x509_inet_pton_ipv4(const char *src, void *dst); #define li_cton(c, n) \ @@ -2654,7 +2655,7 @@ static int x509_inet_pton_ipv4(const char *src, void *dst) return inet_pton(AF_INET, src, dst) == 1 ? 0 : -1; } -#endif /* AF_INET6 */ +#endif /* !AF_INET6 || MBEDTLS_TEST_SW_INET_PTON */ //no-check-names MBEDTLS_STATIC_TESTABLE size_t mbedtls_x509_crt_parse_cn_inet_pton(const char *cn, void *dst) diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index 465f9bbdee..3a0eb78f44 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1219,6 +1219,17 @@ component_test_psa_external_rng_no_drbg_use_psa () { tests/ssl-opt.sh -f 'Default\|opaque' } +component_test_sw_inet_pton () { + msg "build: default plus MBEDTLS_TEST_SW_INET_PTON" + + # MBEDTLS_TEST_HOOKS required for x509_crt_parse_cn_inet_pton + scripts/config.py set MBEDTLS_TEST_HOOKS + make CFLAGS="-DMBEDTLS_TEST_SW_INET_PTON" + + msg "test: default plus MBEDTLS_TEST_SW_INET_PTON" + make test +} + component_test_crypto_full_md_light_only () { msg "build: crypto_full with only the light subset of MD" scripts/config.py crypto_full From 7f5a1a4525372ae7889f7154013806b31c80d02a Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 13 Apr 2023 08:02:27 -0400 Subject: [PATCH 12/18] Rename ipv6 parsing variables, introduce one new one This way the names are more descriptive. j was reused later on for calculation, num_zero_groups is used instead. Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 58 +++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 58f7cc068e..3c9d6133b4 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2570,52 +2570,58 @@ static int x509_inet_pton_ipv4(const char *src, void *dst); static int x509_inet_pton_ipv6(const char *src, void *dst) { - const unsigned char *v = (const unsigned char *) src; - int i = 0, j, dc = -1; + const unsigned char *character = (const unsigned char *) src; + int num_groups = 0, num_digits, zero_group_start = -1; uint16_t addr[8]; do { /* note: allows excess leading 0's, e.g. 1:0002:3:... */ - uint16_t x = j = 0; - for (uint8_t n; j < 4 && li_cton(*v, n); x <<= 4, x |= n, ++v, ++j) { + uint16_t group = num_digits = 0; + for (uint8_t digit; num_digits < 4 && li_cton(*character, digit); + group <<= 4, group |= digit, ++character, ++num_digits) { ; } - if (j != 0) { - addr[i++] = MBEDTLS_IS_BIG_ENDIAN ? x : (x << 8) | (x >> 8); - if (*v == '\0') { + if (num_digits != 0) { + addr[num_groups++] = MBEDTLS_IS_BIG_ENDIAN ? group : (group << 8) | (group >> 8); + if (*character == '\0') { break; - } else if (*v == '.' && (i != 0 || dc != -1) && (i < 7) && + } else if (*character == '.' && (num_groups != 0 || zero_group_start != -1) && + (num_groups < 7) && /* walk back to prior ':', then parse as IPv4-mapped */ - (*--v == ':' || *--v == ':' || - *--v == ':' || *--v == ':') && - x509_inet_pton_ipv4((const char *) ++v, addr + --i) == 0) { - i += 2; - v = (const unsigned char *) ""; + (*--character == ':' || *--character == ':' || + *--character == ':' || *--character == ':') && + x509_inet_pton_ipv4((const char *) ++character, addr + --num_groups) == 0) { + num_groups += 2; + character = (const unsigned char *) ""; break; - } else if (*v != ':') { + } else if (*character != ':') { return -1; } } else { - if (dc != -1 || *v != ':' || ((dc = i) == 0 && *++v != ':')) { + if (zero_group_start != -1 || *character != ':' || + ((zero_group_start = num_groups) == 0 && *++character != ':')) { return -1; } - if (v[1] == '\0') { - ++v; + if (character[1] == '\0') { + ++character; break; } } - ++v; - } while (i < 8); - if ((dc != -1 ? i > 6 : i != 8) || *v != '\0') { + ++character; + } while (num_groups < 8); + if ((zero_group_start != -1 ? num_groups > 6 : num_groups != 8) || *character != '\0') { return -1; } - if (dc != -1) { - i -= dc; - j = 8 - i - dc; - if (i) { - memmove(addr + dc + j, addr + dc, i * sizeof(*addr)); + if (zero_group_start != -1) { + int num_zero_groups = 0; + num_groups -= zero_group_start; + num_zero_groups = 8 - num_groups - zero_group_start; + if (num_groups) { + memmove(addr + zero_group_start + num_zero_groups, + addr + zero_group_start, + num_groups * sizeof(*addr)); } - memset(addr + dc, 0, j * sizeof(*addr)); + memset(addr + zero_group_start, 0, num_zero_groups * sizeof(*addr)); } memcpy(dst, addr, sizeof(addr)); return 0; From 0d57896f7ef24ebb8a92aa22d15ffcb2d1366f8a Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 13 Apr 2023 09:09:56 -0400 Subject: [PATCH 13/18] Refactor ipv6 parsing Introduce new variables to make it more readable. Clarify the calculations a bit. Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 3c9d6133b4..012c536914 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2571,7 +2571,7 @@ static int x509_inet_pton_ipv4(const char *src, void *dst); static int x509_inet_pton_ipv6(const char *src, void *dst) { const unsigned char *character = (const unsigned char *) src; - int num_groups = 0, num_digits, zero_group_start = -1; + int nonzero_groups = 0, num_digits, zero_group_start = -1; uint16_t addr[8]; do { /* note: allows excess leading 0's, e.g. 1:0002:3:... */ @@ -2581,16 +2581,19 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) ; } if (num_digits != 0) { - addr[num_groups++] = MBEDTLS_IS_BIG_ENDIAN ? group : (group << 8) | (group >> 8); + addr[nonzero_groups++] = MBEDTLS_IS_BIG_ENDIAN ? group : + (group << 8) | (group >> 8); if (*character == '\0') { break; - } else if (*character == '.' && (num_groups != 0 || zero_group_start != -1) && - (num_groups < 7) && + } else if (*character == '.' && (nonzero_groups != 0 || + zero_group_start != -1) && + (nonzero_groups < 7) && /* walk back to prior ':', then parse as IPv4-mapped */ (*--character == ':' || *--character == ':' || *--character == ':' || *--character == ':') && - x509_inet_pton_ipv4((const char *) ++character, addr + --num_groups) == 0) { - num_groups += 2; + x509_inet_pton_ipv4((const char *) ++character, + addr + --nonzero_groups) == 0) { + nonzero_groups += 2; character = (const unsigned char *) ""; break; } else if (*character != ':') { @@ -2598,7 +2601,8 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) } } else { if (zero_group_start != -1 || *character != ':' || - ((zero_group_start = num_groups) == 0 && *++character != ':')) { + ((zero_group_start = nonzero_groups) == 0 && + *++character != ':')) { return -1; } if (character[1] == '\0') { @@ -2607,21 +2611,23 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) } } ++character; - } while (num_groups < 8); - if ((zero_group_start != -1 ? num_groups > 6 : num_groups != 8) || *character != '\0') { + } while (nonzero_groups < 8); + if ((zero_group_start != -1 ? nonzero_groups > 6 : nonzero_groups != 8) || + *character != '\0') { return -1; } if (zero_group_start != -1) { - int num_zero_groups = 0; - num_groups -= zero_group_start; - num_zero_groups = 8 - num_groups - zero_group_start; - if (num_groups) { - memmove(addr + zero_group_start + num_zero_groups, + int zero_groups = 8 - nonzero_groups; + int groups_after_zero = nonzero_groups - zero_group_start; + + /* Move the non-zero part to after the zeroes */ + if (groups_after_zero) { + memmove(addr + zero_group_start + zero_groups, addr + zero_group_start, - num_groups * sizeof(*addr)); + groups_after_zero * sizeof(*addr)); } - memset(addr + zero_group_start, 0, num_zero_groups * sizeof(*addr)); + memset(addr + zero_group_start, 0, zero_groups * sizeof(*addr)); } memcpy(dst, addr, sizeof(addr)); return 0; From 6cbca6dd42d9c745460eef39df9f403dea9e25cf Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Thu, 13 Apr 2023 09:22:48 -0400 Subject: [PATCH 14/18] Rename a variable in ipv4 and ipv6 parsing Character was too elaborate. p is used in other x509 code to step through data. Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 48 +++++++++++++++++++++++----------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 012c536914..980858a586 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2570,50 +2570,50 @@ static int x509_inet_pton_ipv4(const char *src, void *dst); static int x509_inet_pton_ipv6(const char *src, void *dst) { - const unsigned char *character = (const unsigned char *) src; + const unsigned char *p = (const unsigned char *) src; int nonzero_groups = 0, num_digits, zero_group_start = -1; uint16_t addr[8]; do { /* note: allows excess leading 0's, e.g. 1:0002:3:... */ uint16_t group = num_digits = 0; - for (uint8_t digit; num_digits < 4 && li_cton(*character, digit); - group <<= 4, group |= digit, ++character, ++num_digits) { + for (uint8_t digit; num_digits < 4 && li_cton(*p, digit); + group <<= 4, group |= digit, ++p, ++num_digits) { ; } if (num_digits != 0) { addr[nonzero_groups++] = MBEDTLS_IS_BIG_ENDIAN ? group : (group << 8) | (group >> 8); - if (*character == '\0') { + if (*p == '\0') { break; - } else if (*character == '.' && (nonzero_groups != 0 || - zero_group_start != -1) && + } else if (*p == '.' && (nonzero_groups != 0 || + zero_group_start != -1) && (nonzero_groups < 7) && /* walk back to prior ':', then parse as IPv4-mapped */ - (*--character == ':' || *--character == ':' || - *--character == ':' || *--character == ':') && - x509_inet_pton_ipv4((const char *) ++character, + (*--p == ':' || *--p == ':' || + *--p == ':' || *--p == ':') && + x509_inet_pton_ipv4((const char *) ++p, addr + --nonzero_groups) == 0) { nonzero_groups += 2; - character = (const unsigned char *) ""; + p = (const unsigned char *) ""; break; - } else if (*character != ':') { + } else if (*p != ':') { return -1; } } else { - if (zero_group_start != -1 || *character != ':' || + if (zero_group_start != -1 || *p != ':' || ((zero_group_start = nonzero_groups) == 0 && - *++character != ':')) { + *++p != ':')) { return -1; } - if (character[1] == '\0') { - ++character; + if (p[1] == '\0') { + ++p; break; } } - ++character; + ++p; } while (nonzero_groups < 8); if ((zero_group_start != -1 ? nonzero_groups > 6 : nonzero_groups != 8) || - *character != '\0') { + *p != '\0') { return -1; } @@ -2636,23 +2636,23 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) static int x509_inet_pton_ipv4(const char *src, void *dst) { /* note: allows leading 0's, e.g. 000.000.000.000 */ - const unsigned char *character = (const unsigned char *) src; + const unsigned char *p = (const unsigned char *) src; uint8_t *res = (uint8_t *) dst; uint8_t digit1, digit2, digit3, num_octets = 0; uint16_t octet; do { - if ((digit1 = *(uint8_t *) character - '0') > 9) { + if ((digit1 = *(uint8_t *) p - '0') > 9) { break; - } else if ((digit2 = *(uint8_t *) ++character - '0') > 9) { + } else if ((digit2 = *(uint8_t *) ++p - '0') > 9) { *res++ = digit1; - } else if ((digit3 = *(uint8_t *) ++character - '0') > 9) { + } else if ((digit3 = *(uint8_t *) ++p - '0') > 9) { *res++ = digit1 * 10 + digit2; } else if ((octet = digit1 * 100 + digit2 * 10 + digit3) < 256) { - *res++ = (uint8_t) octet, ++character; + *res++ = (uint8_t) octet, ++p; } - } while (++num_octets < 4 && *character++ == '.'); - return num_octets == 4 && *character == '\0' ? 0 : -1; + } while (++num_octets < 4 && *p++ == '.'); + return num_octets == 4 && *p == '\0' ? 0 : -1; } #else From ea3e71fa37f5e9934cb9222809d8a425303f28b3 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 18 Apr 2023 04:39:12 -0400 Subject: [PATCH 15/18] Further refactor IPv4 parsing Make it more readable Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index 980858a586..d094ae0f87 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2638,20 +2638,28 @@ static int x509_inet_pton_ipv4(const char *src, void *dst) /* note: allows leading 0's, e.g. 000.000.000.000 */ const unsigned char *p = (const unsigned char *) src; uint8_t *res = (uint8_t *) dst; - uint8_t digit1, digit2, digit3, num_octets = 0; + uint8_t digit, num_digits = 0; + uint8_t num_octets = 0; uint16_t octet; do { - if ((digit1 = *(uint8_t *) p - '0') > 9) { + octet = num_digits = 0; + do { + digit = *p - '0'; + if (digit > 9) { + break; + } + octet = octet * 10 + digit; + num_digits++; + p++; + } while (num_digits < 3); + + if (octet >= 256 || num_digits > 3 || num_digits == 0) { break; - } else if ((digit2 = *(uint8_t *) ++p - '0') > 9) { - *res++ = digit1; - } else if ((digit3 = *(uint8_t *) ++p - '0') > 9) { - *res++ = digit1 * 10 + digit2; - } else if ((octet = digit1 * 100 + digit2 * 10 + digit3) < 256) { - *res++ = (uint8_t) octet, ++p; } - } while (++num_octets < 4 && *p++ == '.'); + *res++ = (uint8_t) octet; + num_octets++; + } while (num_octets < 4 && *p++ == '.'); return num_octets == 4 && *p == '\0' ? 0 : -1; } From 8bc2cc92b504d50546e619b11381d27c02096ede Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 18 Apr 2023 07:26:27 -0400 Subject: [PATCH 16/18] Refactor IPv6 parsing Make it more readable Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 53 ++++++++++++++++++++++++++++++++++------------ 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index d094ae0f87..fc62057082 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2576,23 +2576,42 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) do { /* note: allows excess leading 0's, e.g. 1:0002:3:... */ uint16_t group = num_digits = 0; - for (uint8_t digit; num_digits < 4 && li_cton(*p, digit); - group <<= 4, group |= digit, ++p, ++num_digits) { - ; + for (uint8_t digit; num_digits < 4; num_digits++) { + if (li_cton(*p, digit) == 0) { + break; + } + group = (group << 4) | digit; + p++; } if (num_digits != 0) { addr[nonzero_groups++] = MBEDTLS_IS_BIG_ENDIAN ? group : (group << 8) | (group >> 8); if (*p == '\0') { break; - } else if (*p == '.' && (nonzero_groups != 0 || - zero_group_start != -1) && - (nonzero_groups < 7) && - /* walk back to prior ':', then parse as IPv4-mapped */ - (*--p == ':' || *--p == ':' || - *--p == ':' || *--p == ':') && - x509_inet_pton_ipv4((const char *) ++p, - addr + --nonzero_groups) == 0) { + } else if (*p == '.') { + /* Don't accept IPv4 too early or late */ + if ((nonzero_groups == 0 && zero_group_start == -1) || + nonzero_groups >= 7) { + break; + } + + /* Walk back to prior ':', then parse as IPv4-mapped */ + int steps = 4; + do { + p--; + steps--; + } while (*p != ':' && steps > 0); + + if (*p != ':') { + break; + } + p++; + nonzero_groups--; + if (x509_inet_pton_ipv4((const char *) p, + addr + nonzero_groups) != 0) { + break; + } + nonzero_groups += 2; p = (const unsigned char *) ""; break; @@ -2600,11 +2619,17 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) return -1; } } else { - if (zero_group_start != -1 || *p != ':' || - ((zero_group_start = nonzero_groups) == 0 && - *++p != ':')) { + /* Don't accept a second zero group or an invalid delimiter */ + if (zero_group_start != -1 || *p != ':') { return -1; } + zero_group_start = nonzero_groups; + + /* Accept a zero group at start, but it has to be a double colon */ + if (zero_group_start == 0 && *++p != ':') { + return -1; + } + if (p[1] == '\0') { ++p; break; From af04f6307f861416c544b8c6164cf48a9b5e3aaf Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 18 Apr 2023 07:26:59 -0400 Subject: [PATCH 17/18] Add an IPv4 mapped IPv6 test Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_x509parse.data | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/suites/test_suite_x509parse.data b/tests/suites/test_suite_x509parse.data index b379959eb4..1d2247d6cb 100644 --- a/tests/suites/test_suite_x509parse.data +++ b/tests/suites/test_suite_x509parse.data @@ -1131,6 +1131,9 @@ X509 CRT parse CN: IPv6 invalid address IPv4-mapped #1 x509_crt_parse_cn_inet_pton:"\:\:ffff\:999.2.3.4":"":0 X509 CRT parse CN: IPv6 invalid address IPv4-mapped #2 +x509_crt_parse_cn_inet_pton:"\:\:ffff\:1111.2.3.4":"":0 + +X509 CRT parse CN: IPv6 invalid address IPv4-mapped #3 x509_crt_parse_cn_inet_pton:"\:\:1.2.3.4\:ffff":"":0 X509 CRT verification with ca callback: failure From 90117db5dc75508533ce1ed7bf08f5ea567145d6 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Tue, 18 Apr 2023 07:32:47 -0400 Subject: [PATCH 18/18] Split a complex condition into separate ones Make it more readable Signed-off-by: Andrzej Kurek --- library/x509_crt.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/library/x509_crt.c b/library/x509_crt.c index fc62057082..cddbcdb7bd 100644 --- a/library/x509_crt.c +++ b/library/x509_crt.c @@ -2637,12 +2637,15 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) } ++p; } while (nonzero_groups < 8); - if ((zero_group_start != -1 ? nonzero_groups > 6 : nonzero_groups != 8) || - *p != '\0') { + + if (*p != '\0') { return -1; } if (zero_group_start != -1) { + if (nonzero_groups > 6) { + return -1; + } int zero_groups = 8 - nonzero_groups; int groups_after_zero = nonzero_groups - zero_group_start; @@ -2653,6 +2656,10 @@ static int x509_inet_pton_ipv6(const char *src, void *dst) groups_after_zero * sizeof(*addr)); } memset(addr + zero_group_start, 0, zero_groups * sizeof(*addr)); + } else { + if (nonzero_groups != 8) { + return -1; + } } memcpy(dst, addr, sizeof(addr)); return 0;