From 3409c84b8e26ca37bc91475344aee354d8bec4f7 Mon Sep 17 00:00:00 2001 From: Antonios Skourtis Date: Tue, 27 Jun 2023 13:18:59 +0300 Subject: [PATCH 1/2] ssl: fix critical extension handling regression Signed-off-by: Antonios Skourtis --- .../fix-critical-extension-handling.txt | 3 ++ include/mbedtls/ssl.h | 36 +++++++++++++++++++ library/ssl_tls.c | 33 ++++++++++++++--- 3 files changed, 68 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/fix-critical-extension-handling.txt diff --git a/ChangeLog.d/fix-critical-extension-handling.txt b/ChangeLog.d/fix-critical-extension-handling.txt new file mode 100644 index 0000000000..a0f8fcd2bb --- /dev/null +++ b/ChangeLog.d/fix-critical-extension-handling.txt @@ -0,0 +1,3 @@ +Bugfix + * Add critical extension handling support for TLS peer x509 certificates. + Fixes issue #7182. diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 4b73b41a1d..2bec41288e 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -1424,6 +1424,10 @@ struct mbedtls_ssl_config { /** Callback to customize X.509 certificate chain verification */ int(*MBEDTLS_PRIVATE(f_vrfy))(void *, mbedtls_x509_crt *, int, uint32_t *); void *MBEDTLS_PRIVATE(p_vrfy); /*!< context for X.509 verify calllback */ + + /** Callback to skip unsupported extentions */ + mbedtls_x509_crt_ext_cb_t MBEDTLS_PRIVATE(f_ext); + void *MBEDTLS_PRIVATE(p_ext); /*!< context for X.509 extention calllback */ #endif #if defined(MBEDTLS_SSL_HANDSHAKE_WITH_PSK_ENABLED) @@ -1632,6 +1636,10 @@ struct mbedtls_ssl_context { /** Callback to customize X.509 certificate chain verification */ int(*MBEDTLS_PRIVATE(f_vrfy))(void *, mbedtls_x509_crt *, int, uint32_t *); void *MBEDTLS_PRIVATE(p_vrfy); /*!< context for X.509 verify callback */ + + /** Callback to skip unsupported extentions */ + mbedtls_x509_crt_ext_cb_t MBEDTLS_PRIVATE(f_ext); + void *MBEDTLS_PRIVATE(p_ext); /*!< context for X.509 extention calllback */ #endif mbedtls_ssl_send_t *MBEDTLS_PRIVATE(f_send); /*!< Callback for network send */ @@ -2034,6 +2042,20 @@ void mbedtls_ssl_tls13_conf_max_early_data_size( void mbedtls_ssl_conf_verify(mbedtls_ssl_config *conf, int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *), void *p_vrfy); + +/** + * \brief Set the unsupported extention callback (Optional). + * + * If set, the provided extention callback is called whenever + * a certificate is parsed. For more information, please see + * the documentation of + * \c mbedtls_x509_crt_parse_der_with_ext_cb(). + * + * \param conf The SSL configuration to use. + * \param f_ext The extentions callback to use during CRT parsing. + * \param p_ext The opaque context to be passed to the callback. + */ +void mbedtls_ssl_conf_ext_cb(mbedtls_ssl_config *conf, mbedtls_x509_crt_ext_cb_t f_ext, void *p_ext); #endif /* MBEDTLS_X509_CRT_PARSE_C */ /** @@ -2353,6 +2375,20 @@ void mbedtls_ssl_set_mtu(mbedtls_ssl_context *ssl, uint16_t mtu); void mbedtls_ssl_set_verify(mbedtls_ssl_context *ssl, int (*f_vrfy)(void *, mbedtls_x509_crt *, int, uint32_t *), void *p_vrfy); + +/** + * \brief Set a connection-specific extention callback (optional). + * + * If set, the provided extention callback is called whenever + * a certificate is parsed. For more information, please see + * the documentation of + * \c mbedtls_x509_crt_parse_der_with_ext_cb(). + * + * \param ssl The SSL context to use. + * \param f_ext The extentions callback to use during CRT parsing. + * \param p_ext The opaque context to be passed to the callback. + */ +void mbedtls_ssl_set_ext_cb(mbedtls_ssl_context *ssl, mbedtls_x509_crt_ext_cb_t f_ext, void *p_ext); #endif /* MBEDTLS_X509_CRT_PARSE_C */ /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index f0067f4b2d..c1ea3d791c 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -1658,6 +1658,12 @@ void mbedtls_ssl_conf_verify(mbedtls_ssl_config *conf, conf->f_vrfy = f_vrfy; conf->p_vrfy = p_vrfy; } + +void mbedtls_ssl_conf_ext_cb(mbedtls_ssl_config *conf, mbedtls_x509_crt_ext_cb_t f_ext, void *p_ext) +{ + conf->f_ext = f_ext; + conf->p_ext = p_ext; +} #endif /* MBEDTLS_X509_CRT_PARSE_C */ void mbedtls_ssl_conf_rng(mbedtls_ssl_config *conf, @@ -1939,6 +1945,12 @@ void mbedtls_ssl_set_verify(mbedtls_ssl_context *ssl, ssl->f_vrfy = f_vrfy; ssl->p_vrfy = p_vrfy; } + +void mbedtls_ssl_set_ext_cb(mbedtls_ssl_context *ssl, mbedtls_x509_crt_ext_cb_t f_ext, void *p_ext) +{ + ssl->f_ext = f_ext; + ssl->p_ext = p_ext; +} #endif #if defined(MBEDTLS_KEY_EXCHANGE_ECJPAKE_ENABLED) @@ -7175,12 +7187,25 @@ static int ssl_parse_certificate_chain(mbedtls_ssl_context *ssl, /* Parse the next certificate in the chain. */ #if defined(MBEDTLS_SSL_KEEP_PEER_CERTIFICATE) - ret = mbedtls_x509_crt_parse_der(chain, ssl->in_msg + i, n); + enum { COPY_CRT = 1 }; #else - /* If we don't need to store the CRT chain permanently, parse - * it in-place from the input buffer instead of making a copy. */ - ret = mbedtls_x509_crt_parse_der_nocopy(chain, ssl->in_msg + i, n); + enum { COPY_CRT = 0 }; #endif /* MBEDTLS_SSL_KEEP_PEER_CERTIFICATE */ + + mbedtls_x509_crt_ext_cb_t f_ext; + void *p_ext; + + if (ssl->f_ext != NULL) { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use context-specific extension callback")); + f_ext = ssl->f_ext; + p_ext = ssl->p_ext; + } else { + MBEDTLS_SSL_DEBUG_MSG(3, ("Use configuration-specific extension callback")); + f_ext = ssl->conf->f_ext; + p_ext = ssl->conf->p_ext; + } + + ret = mbedtls_x509_crt_parse_der_with_ext_cb(chain, ssl->in_msg + i, n, COPY_CRT, f_ext, p_ext); switch (ret) { case 0: /*ok*/ case MBEDTLS_ERR_X509_UNKNOWN_SIG_ALG + MBEDTLS_ERR_OID_NOT_FOUND: From 53093ce4bd6ed5f938f31925265a22a14022896f Mon Sep 17 00:00:00 2001 From: Antonios Skourtis Date: Wed, 28 Jun 2023 16:33:55 +0300 Subject: [PATCH 2/2] Resolve coding style warnings Signed-off-by: Antonios Skourtis --- include/mbedtls/ssl.h | 3 ++- library/ssl_tls.c | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/ssl.h b/include/mbedtls/ssl.h index 2bec41288e..d396cdbdd1 100644 --- a/include/mbedtls/ssl.h +++ b/include/mbedtls/ssl.h @@ -2055,7 +2055,8 @@ void mbedtls_ssl_conf_verify(mbedtls_ssl_config *conf, * \param f_ext The extentions callback to use during CRT parsing. * \param p_ext The opaque context to be passed to the callback. */ -void mbedtls_ssl_conf_ext_cb(mbedtls_ssl_config *conf, mbedtls_x509_crt_ext_cb_t f_ext, void *p_ext); +void mbedtls_ssl_conf_ext_cb(mbedtls_ssl_config *conf, mbedtls_x509_crt_ext_cb_t f_ext, + void *p_ext); #endif /* MBEDTLS_X509_CRT_PARSE_C */ /** diff --git a/library/ssl_tls.c b/library/ssl_tls.c index c1ea3d791c..48f8bc0219 100644 --- a/library/ssl_tls.c +++ b/library/ssl_tls.c @@ -7205,7 +7205,12 @@ static int ssl_parse_certificate_chain(mbedtls_ssl_context *ssl, p_ext = ssl->conf->p_ext; } - ret = mbedtls_x509_crt_parse_der_with_ext_cb(chain, ssl->in_msg + i, n, COPY_CRT, f_ext, p_ext); + ret = mbedtls_x509_crt_parse_der_with_ext_cb(chain, + ssl->in_msg + i, + n, + COPY_CRT, + f_ext, + p_ext); switch (ret) { case 0: /*ok*/ case MBEDTLS_ERR_X509_UNKNOWN_SIG_ALG + MBEDTLS_ERR_OID_NOT_FOUND: