Access ssl->hostname through abstractions
New abstractions to access ssl->hostname:
mbedtls_ssl_has_set_hostname_been_called() (only implemented approximatively
for now), mbedtls_ssl_get_hostname_pointer(), mbedtls_ssl_free_hostname().
Only access ssl->hostname directly in these functions and in
mbedtls_ssl_set_hostname().
Use these abstractions to access the hostname with the opportunity for
extra checks in mbedtls_ssl_verify_certificate().
No behavior change except for a new log message.
Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>
diff --git a/include/mbedtls/ssl_internal.h b/include/mbedtls/ssl_internal.h
index 3a40b4b..fdc1719 100644
--- a/include/mbedtls/ssl_internal.h
+++ b/include/mbedtls/ssl_internal.h
@@ -1214,6 +1214,18 @@
return 4;
}
+#if defined(MBEDTLS_SSL_SERVER_NAME_INDICATION)
+/** Get the host name from the SSL context.
+ *
+ * \param[in] ssl SSL context
+ *
+ * \return The \p hostname pointer from the SSL context.
+ * \c NULL if mbedtls_ssl_set_hostname() has never been called on
+ * \p ssl or if it was last called with \p NULL.
+ */
+const char *mbedtls_ssl_get_hostname_pointer(const mbedtls_ssl_context *ssl);
+#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION */
+
#if defined(MBEDTLS_SSL_PROTO_DTLS)
void mbedtls_ssl_send_flight_completed(mbedtls_ssl_context *ssl);
void mbedtls_ssl_recv_flight_completed(mbedtls_ssl_context *ssl);
diff --git a/library/ssl_cli.c b/library/ssl_cli.c
index 4fde783..2854e00 100644
--- a/library/ssl_cli.c
+++ b/library/ssl_cli.c
@@ -83,19 +83,20 @@
size_t *olen)
{
unsigned char *p = buf;
+ const char *hostname = mbedtls_ssl_get_hostname_pointer(ssl);
size_t hostname_len;
*olen = 0;
- if (ssl->hostname == NULL) {
+ if (hostname == NULL) {
return 0;
}
MBEDTLS_SSL_DEBUG_MSG(3,
("client hello, adding server name extension: %s",
- ssl->hostname));
+ hostname));
- hostname_len = strlen(ssl->hostname);
+ hostname_len = strlen(hostname);
MBEDTLS_SSL_CHK_BUF_PTR(p, end, hostname_len + 9);
@@ -139,7 +140,7 @@
MBEDTLS_PUT_UINT16_BE(hostname_len, p, 0);
p += 2;
- memcpy(p, ssl->hostname, hostname_len);
+ memcpy(p, hostname, hostname_len);
*olen = hostname_len + 9;
diff --git a/library/ssl_tls.c b/library/ssl_tls.c
index c3e005b..01acd1d 100644
--- a/library/ssl_tls.c
+++ b/library/ssl_tls.c
@@ -39,6 +39,47 @@
#endif
#if defined(MBEDTLS_X509_CRT_PARSE_C)
+
+#if defined(MBEDTLS_KEY_EXCHANGE_WITH_CERT_ENABLED)
+/** Whether mbedtls_ssl_set_hostname() has been called.
+ *
+ * \param[in] ssl SSL context
+ *
+ * \return \c 1 if mbedtls_ssl_set_hostname() has been called on \p ssl
+ * (including `mbedtls_ssl_set_hostname(ssl, NULL)`),
+ * otherwise \c 0.
+ */
+static int mbedtls_ssl_has_set_hostname_been_called(
+ const mbedtls_ssl_context *ssl)
+{
+ /* We can't tell the difference between the case where
+ * mbedtls_ssl_set_hostname() has not been called at all, and
+ * the case where it was last called with NULL. For the time
+ * being, we assume the latter, i.e. we behave as if there had
+ * been an implicit call to mbedtls_ssl_set_hostname(ssl, NULL). */
+ return ssl->hostname != NULL;
+}
+#endif
+
+/* Micro-optimization: don't export this function if it isn't needed outside
+ * of this source file. */
+#if !defined(MBEDTLS_SSL_SERVER_NAME_INDICATION)
+static
+#endif
+const char *mbedtls_ssl_get_hostname_pointer(const mbedtls_ssl_context *ssl)
+{
+ return ssl->hostname;
+}
+
+static void mbedtls_ssl_free_hostname(mbedtls_ssl_context *ssl)
+{
+ if (ssl->hostname != NULL) {
+ mbedtls_platform_zeroize(ssl->hostname, strlen(ssl->hostname));
+ mbedtls_free(ssl->hostname);
+ }
+ ssl->hostname = NULL;
+}
+
int mbedtls_ssl_set_hostname(mbedtls_ssl_context *ssl, const char *hostname)
{
/* Initialize to suppress unnecessary compiler warning */
@@ -56,11 +97,7 @@
/* Now it's clear that we will overwrite the old hostname,
* so we can free it safely */
-
- if (ssl->hostname != NULL) {
- mbedtls_platform_zeroize(ssl->hostname, strlen(ssl->hostname));
- mbedtls_free(ssl->hostname);
- }
+ mbedtls_ssl_free_hostname(ssl);
/* Passing NULL as hostname shall clear the old one */
@@ -2564,13 +2601,27 @@
return SSL_CERTIFICATE_EXPECTED;
}
+static int get_hostname_for_verification(mbedtls_ssl_context *ssl,
+ const char **hostname)
+{
+ if (!mbedtls_ssl_has_set_hostname_been_called(ssl)) {
+ MBEDTLS_SSL_DEBUG_MSG(1, ("Certificate verification without having set hostname"));
+ }
+
+ *hostname = mbedtls_ssl_get_hostname_pointer(ssl);
+ if (*hostname == NULL) {
+ MBEDTLS_SSL_DEBUG_MSG(2, ("Certificate verification without CN verification"));
+ }
+
+ return 0;
+}
+
MBEDTLS_CHECK_RETURN_CRITICAL
static int ssl_parse_certificate_verify(mbedtls_ssl_context *ssl,
int authmode,
mbedtls_x509_crt *chain,
void *rs_ctx)
{
- int ret = 0;
const mbedtls_ssl_ciphersuite_t *ciphersuite_info =
ssl->handshake->ciphersuite_info;
int have_ca_chain = 0;
@@ -2592,6 +2643,13 @@
p_vrfy = ssl->conf->p_vrfy;
}
+ const char *hostname = "";
+ int ret = get_hostname_for_verification(ssl, &hostname);
+ if (ret != 0) {
+ MBEDTLS_SSL_DEBUG_RET(1, "get_hostname_for_verification", ret);
+ return ret;
+ }
+
/*
* Main check: verify certificate
*/
@@ -2606,7 +2664,7 @@
ssl->conf->f_ca_cb,
ssl->conf->p_ca_cb,
ssl->conf->cert_profile,
- ssl->hostname,
+ hostname,
&ssl->session_negotiate->verify_result,
f_vrfy, p_vrfy);
} else
@@ -2634,7 +2692,7 @@
chain,
ca_chain, ca_crl,
ssl->conf->cert_profile,
- ssl->hostname,
+ hostname,
&ssl->session_negotiate->verify_result,
f_vrfy, p_vrfy, rs_ctx);
}
@@ -6816,10 +6874,7 @@
}
#if defined(MBEDTLS_X509_CRT_PARSE_C)
- if (ssl->hostname != NULL) {
- mbedtls_platform_zeroize(ssl->hostname, strlen(ssl->hostname));
- mbedtls_free(ssl->hostname);
- }
+ mbedtls_ssl_free_hostname(ssl);
#endif
#if defined(MBEDTLS_SSL_HW_RECORD_ACCEL)