[PATCH 0/3] imap-send: future proofing and two correctness fixes

[PATCH 0/3] imap-send: future proofing and two correctness fixes

[Beat Bolli]

Hi!

Patch 1 future-proofs against a renamed ASN1_STRING function.

Patch 2 fixes an incorrect assumption about NUL-termination of
ASN1_STRINGs.

Patch 3 only checks the certificate subject common name if no DNS
subject alternative names are available, as defined by RFC 6125.


Beat Bolli (3):
  imap-send: prepare for OpenSSL 4.1
  imap-send: don't expect an ASN1_STRING to be NUL-terminated
  imap-send: only check the CN if no SAN DNS names are present

 imap-send.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

-- 
2.53.0

[PATCH 1/3] imap-send: prepare for OpenSSL 4.1

[Beat Bolli]

OpenSSL master (to be v4.1 after the release) renamed the function
ASN1_STRING_length() to ASN1_STRING_get_length(). Map the new name to
the old one if we're compiling with a pre-4.1 version.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/imap-send.c b/imap-send.c
index 0d16d02029..977d78005c 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -219,12 +219,17 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
 
 #else
 
+#if (OPENSSL_VERSION_NUMBER < 0x40100000L)
+// map to the pre-4.1 name
+#define ASN1_STRING_get_length(s) ASN1_STRING_length(s)
+#endif
+
 static int host_matches(const char *host, const ASN1_STRING *asn1_str)
 {
 	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
 
 	/* embedded NUL characters may open a security hole */
-	if (memchr(pattern, '\0', ASN1_STRING_length(asn1_str)))
+	if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
 	    return 0;
 
 	if (pattern[0] == '*' && pattern[1] == '.') {
-- 
2.53.0

Re: [PATCH 1/3] imap-send: prepare for OpenSSL 4.1

[Junio C Hamano]

Beat Bolli <dev+git@drbeat.li> writes:

> OpenSSL master (to be v4.1 after the release) renamed the function
> ASN1_STRING_length() to ASN1_STRING_get_length(). Map the new name to
> the old one if we're compiling with a pre-4.1 version.
>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>  imap-send.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/imap-send.c b/imap-send.c
> index 0d16d02029..977d78005c 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -219,12 +219,17 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
>  
>  #else
>  
> +#if (OPENSSL_VERSION_NUMBER < 0x40100000L)
> +// map to the pre-4.1 name

Style?

> +#define ASN1_STRING_get_length(s) ASN1_STRING_length(s)
> +#endif
> +
>  static int host_matches(const char *host, const ASN1_STRING *asn1_str)
>  {
>  	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
>  
>  	/* embedded NUL characters may open a security hole */
> -	if (memchr(pattern, '\0', ASN1_STRING_length(asn1_str)))
> +	if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
>  	    return 0;
>  
>  	if (pattern[0] == '*' && pattern[1] == '.') {

Re: [PATCH 1/3] imap-send: prepare for OpenSSL 4.1

[Patrick Steinhardt]

On Mon, Sep 07, 2026 at 11:12:08PM +0200, Beat Bolli wrote:
> OpenSSL master (to be v4.1 after the release) renamed the function
> ASN1_STRING_length() to ASN1_STRING_get_length(). Map the new name to
> the old one if we're compiling with a pre-4.1 version.

I can see [1] that the new functions indeed exist now. But it doesn't
say anything about the old functions, they still exist and don't seem to
be deprecated. So why do we even have to switch to the new function?

Patrick

[1]: https://docs.openssl.org/master/man3/ASN1_STRING_length/

Re: [PATCH 1/3] imap-send: prepare for OpenSSL 4.1

[Beat Bolli]

Hi Patrick

On 08.09.2026 10:26, Patrick Steinhardt wrote:
> On Mon, Sep 07, 2026 at 11:12:08PM +0200, Beat Bolli wrote:
>> OpenSSL master (to be v4.1 after the release) renamed the function
>> ASN1_STRING_length() to ASN1_STRING_get_length(). Map the new name to
>> the old one if we're compiling with a pre-4.1 version.
> 
> I can see [1] that the new functions indeed exist now. But it doesn't
> say anything about the old functions, they still exist and don't seem to
> be deprecated. So why do we even have to switch to the new function?
> 

The very page you give below contains this text:

---- 8< ----
The following functions have been deprecated since OpenSSL 4.1, and can 
be hidden entirely by defining OPENSSL_API_COMPAT with a suitable 
version value, see openssl_user_macros(7):

int ASN1_STRING_set(ASN1_STRING *str, const void *data, int len);
int ASN1_STRING_length(ASN1_STRING *x);
---- 8< ----

We also don't define any compatibility macros for OpenSSL, and the build 
failed because of the deprecation warning that was turned into an error 
because of DEVELOPER=1.

So I still think this patch is needed.

Beat

> [1]: https://docs.openssl.org/master/man3/ASN1_STRING_length/

Re: [PATCH 1/3] imap-send: prepare for OpenSSL 4.1

[Patrick Steinhardt]

On Mon, Sep 14, 2026 at 11:10:36AM +0200, Beat Bolli wrote:
> Hi Patrick
> 
> On 08.09.2026 10:26, Patrick Steinhardt wrote:
> > On Mon, Sep 07, 2026 at 11:12:08PM +0200, Beat Bolli wrote:
> > > OpenSSL master (to be v4.1 after the release) renamed the function
> > > ASN1_STRING_length() to ASN1_STRING_get_length(). Map the new name to
> > > the old one if we're compiling with a pre-4.1 version.
> > 
> > I can see [1] that the new functions indeed exist now. But it doesn't
> > say anything about the old functions, they still exist and don't seem to
> > be deprecated. So why do we even have to switch to the new function?
> > 
> 
> The very page you give below contains this text:
> 
> ---- 8< ----
> The following functions have been deprecated since OpenSSL 4.1, and can be
> hidden entirely by defining OPENSSL_API_COMPAT with a suitable version
> value, see openssl_user_macros(7):
> 
> int ASN1_STRING_set(ASN1_STRING *str, const void *data, int len);
> int ASN1_STRING_length(ASN1_STRING *x);
> ---- 8< ----
> 
> We also don't define any compatibility macros for OpenSSL, and the build
> failed because of the deprecation warning that was turned into an error
> because of DEVELOPER=1.

Ah, I missed the part about OPENSSL_API_COMPAT. I think it would make
sense to explicitly point that out in the commit message. Thanks!

Patrick

[PATCH 2/3] imap-send: don't expect an ASN1_STRING to be NUL-terminated

[Beat Bolli]

As highlighted by a recent OpenSSL commit[1], ASN1_STRINGs were never
documented to be terminated by a NUL byte, but our code treats the
pattern as such in the strcasecmp() call.

Make a NUL-terminated copy to avoid Undefined Behavior.

[1]: https://github.com/openssl/openssl/commit/4b581a4666c3e470a01a7323801b2ba8ccfa478c
     (Add a migration entry for ASN1_STRINGs, 2026-08-06)

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 977d78005c..9a807cdde8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -226,20 +226,25 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
 
 static int host_matches(const char *host, const ASN1_STRING *asn1_str)
 {
-	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
+	int ret = 0;
+	size_t len = ASN1_STRING_get_length(asn1_str);
+	char *pattern = xmemdupz(ASN1_STRING_get0_data(asn1_str), len);
 
 	/* embedded NUL characters may open a security hole */
-	if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
-	    return 0;
+	if (memchr(pattern, '\0', len))
+	    goto out;
 
 	if (pattern[0] == '*' && pattern[1] == '.') {
 		pattern += 2;
 		if (!(host = strchr(host, '.')))
-			return 0;
+			goto out;
 		host++;
 	}
 
-	return *host && *pattern && !strcasecmp(host, pattern);
+	ret = *host && *pattern && !strcasecmp(host, pattern);
+out:
+	free(pattern);
+	return ret;
 }
 
 static int verify_hostname(X509 *cert, const char *hostname)
-- 
2.53.0

Re: [PATCH 2/3] imap-send: don't expect an ASN1_STRING to be NUL-terminated

[Junio C Hamano]

Beat Bolli <dev+git@drbeat.li> writes:

> -	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
> +	int ret = 0;
> +	size_t len = ASN1_STRING_get_length(asn1_str);
> +	char *pattern = xmemdupz(ASN1_STRING_get0_data(asn1_str), len);
>  
>  	/* embedded NUL characters may open a security hole */
> -	if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
> -	    return 0;
> +	if (memchr(pattern, '\0', len))
> +	    goto out;
>  
>  	if (pattern[0] == '*' && pattern[1] == '.') {
>  		pattern += 2;
>  		if (!(host = strchr(host, '.')))
> -			return 0;
> +			goto out;
>  		host++;
>  	}
>  
> -	return *host && *pattern && !strcasecmp(host, pattern);
> +	ret = *host && *pattern && !strcasecmp(host, pattern);
> +out:
> +	free(pattern);

There is a code path that increments the "pattern" variable by 2.
Running free() on it would not have a pleasant outcome.

The pattern we often employ in our codebase is to have a separate
variable "char *pattern_to_free" and have it used only for a call
to free().


> +	return ret;
>  }
>  
>  static int verify_hostname(X509 *cert, const char *hostname)

Re: [PATCH 2/3] imap-send: don't expect an ASN1_STRING to be NUL-terminated

[Patrick Steinhardt]

On Mon, Sep 07, 2026 at 11:12:09PM +0200, Beat Bolli wrote:
> diff --git a/imap-send.c b/imap-send.c
> index 977d78005c..9a807cdde8 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -226,20 +226,25 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
>  
>  static int host_matches(const char *host, const ASN1_STRING *asn1_str)
>  {
> -	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
> +	int ret = 0;
> +	size_t len = ASN1_STRING_get_length(asn1_str);
> +	char *pattern = xmemdupz(ASN1_STRING_get0_data(asn1_str), len);
>  
>  	/* embedded NUL characters may open a security hole */
> -	if (memchr(pattern, '\0', ASN1_STRING_get_length(asn1_str)))
> -	    return 0;
> +	if (memchr(pattern, '\0', len))
> +	    goto out;
>  
>  	if (pattern[0] == '*' && pattern[1] == '.') {
>  		pattern += 2;
>  		if (!(host = strchr(host, '.')))
> -			return 0;
> +			goto out;
>  		host++;
>  	}
>  
> -	return *host && *pattern && !strcasecmp(host, pattern);
> +	ret = *host && *pattern && !strcasecmp(host, pattern);
> +out:
> +	free(pattern);
> +	return ret;
>  }

I don't quite see a reason why we even have to memdup the string. We
already use memchr, which is bounded by the length of the string. We do
have two other sites though:

  - We use strchr, but that can be adapted to use memchr.

  - Likewise, we use strcasecmp, but that can be adapted to use
    strncasecmp.

So with that, all calls that inspect the string would be bounded by the
length of the encoded string, and that means we don't have to copy the
string first, do we?

Patrick

[PATCH 3/3] imap-send: only check the CN if no SAN DNS names are present

[Beat Bolli]

Checking the certificate subject's common name may only be done if the
subjectAltNames extension contains no DNS entries. If no SAN DNS name
matches, there's no match.

Per RFC 6125 section 6.4.4[1]:

    As noted, a client MUST NOT seek a match for a reference identifier
    of CN-ID if the presented identifiers include a DNS-ID, SRV-ID,
    URI-ID, or any application-specific identifier types supported by the
    client.

This change was inspired by a similar commit in the HAProxy project[2].

[1]: https://datatracker.ietf.org/doc/html/rfc6125#section-6.4.4
[2]: https://github.com/haproxy/haproxy/commit/75129aaacb7a7b172f4e5334db71d6c1c50a3dbf

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 9a807cdde8..66d3dbfaa5 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -256,11 +256,11 @@ static int verify_hostname(X509 *cert, const char *hostname)
 #endif
 	const X509_NAME_ENTRY *cname_entry;
 	const ASN1_STRING *cname;
-	int i, found;
+	int i, found, has_san_dns;
 	STACK_OF(GENERAL_NAME) *subj_alt_names;
 
 	/* try the DNS subjectAltNames */
-	found = 0;
+	found = has_san_dns = 0;
 	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
 		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
 		for (i = 0; !found && i < num_subj_alt_names; i++) {
@@ -268,13 +268,18 @@ static int verify_hostname(X509 *cert, const char *hostname)
 			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
 			ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
 
-			if (ntype == GEN_DNS && host_matches(hostname, subj_alt_str))
-				found = 1;
+			if (ntype == GEN_DNS) {
+				has_san_dns = 1;
+				if (host_matches(hostname, subj_alt_str))
+					found = 1;
+			}
 		}
 		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
 	}
 	if (found)
 		return 0;
+	if (has_san_dns)
+		return error("none of the subjectAltNames matches hostname '%s'", hostname);
 
 	/* try the common name */
 	if (!(subj = X509_get_subject_name(cert)))
-- 
2.53.0

Re: [PATCH 3/3] imap-send: only check the CN if no SAN DNS names are present

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2810 bytes --]

On 2026-09-07 at 21:12:10, Beat Bolli wrote:
> Checking the certificate subject's common name may only be done if the
> subjectAltNames extension contains no DNS entries. If no SAN DNS name
> matches, there's no match.
> 
> Per RFC 6125 section 6.4.4[1]:
> 
>     As noted, a client MUST NOT seek a match for a reference identifier
>     of CN-ID if the presented identifiers include a DNS-ID, SRV-ID,
>     URI-ID, or any application-specific identifier types supported by the
>     client.
> 
> This change was inspired by a similar commit in the HAProxy project[2].

TLS is not supposed to use the CN at all these days and Go's
implementation completely ignores it.  subjectAltName is supposed to be
used in all cases.

> diff --git a/imap-send.c b/imap-send.c
> index 9a807cdde8..66d3dbfaa5 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -256,11 +256,11 @@ static int verify_hostname(X509 *cert, const char *hostname)
>  #endif
>  	const X509_NAME_ENTRY *cname_entry;
>  	const ASN1_STRING *cname;
> -	int i, found;
> +	int i, found, has_san_dns;
>  	STACK_OF(GENERAL_NAME) *subj_alt_names;
>  
>  	/* try the DNS subjectAltNames */
> -	found = 0;
> +	found = has_san_dns = 0;
>  	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
>  		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
>  		for (i = 0; !found && i < num_subj_alt_names; i++) {
> @@ -268,13 +268,18 @@ static int verify_hostname(X509 *cert, const char *hostname)
>  			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
>  			ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
>  
> -			if (ntype == GEN_DNS && host_matches(hostname, subj_alt_str))
> -				found = 1;
> +			if (ntype == GEN_DNS) {
> +				has_san_dns = 1;
> +				if (host_matches(hostname, subj_alt_str))
> +					found = 1;
> +			}

This handles certificates with DNS names but not IP addresses.  So, for
instance, this match wouldn't work for the certificates for 1.1.1.1
(assuming they had public IMAP service).

>  		}
>  		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
>  	}
>  	if (found)
>  		return 0;
> +	if (has_san_dns)
> +		return error("none of the subjectAltNames matches hostname '%s'", hostname);

I know OpenSSL has built-in hostname verification that can be used as of
OpenSSL 1.0.2[0].  Is there a reason we're still doing this by hand?

Relying on OpenSSL's verification would mean that (a) we would not have
to worry about getting verification wrong in a security-sensitive way
and (b) OpenSSL would handle the policy and standards compliance
functionality.

[0] https://wiki.openssl.org/index.php/Hostname_validation
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

[PATCH v2 1/2] imap-send: use the recommended TLS_method()

[Beat Bolli]

SSLv23_method() has been deprecated since OpenSSL 1.1.0, and
TLS_method() is the recommended function.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index bf1d2cf74d..9f21705d77 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -298,9 +298,9 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	SSL_library_init();
 	SSL_load_error_strings();
 
-	meth = SSLv23_method();
+	meth = TLS_method();
 	if (!meth) {
-		ssl_socket_perror("SSLv23_method");
+		ssl_socket_perror("TLS_method");
 		return -1;
 	}
 
-- 
2.53.0

[PATCH v3 1/3] imap-send: use the recommended TLS_method()

[Beat Bolli]

SSLv23_method() has been deprecated since OpenSSL 1.1.0, and
TLS_method() is the recommended function[1].

Let's use the current function, lest a reader of the code thinks we
don't support current TLS versions.

[1]: https://docs.openssl.org/master/man3/SSL_CTX_new/#history

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index bf1d2cf74d..9f21705d77 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -298,9 +298,9 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	SSL_library_init();
 	SSL_load_error_strings();
 
-	meth = SSLv23_method();
+	meth = TLS_method();
 	if (!meth) {
-		ssl_socket_perror("SSLv23_method");
+		ssl_socket_perror("TLS_method");
 		return -1;
 	}
 
-- 
2.53.0

[PATCH v3 2/3] imap-send: let OpenSSL do the hostname verification

[Beat Bolli]

imap-send performs its own TLS hostname verification. This is not ideal
because it does not support all certificate features.

Let's use OpenSSL built-in verification, which is supported in its
current form since OpenSSL 1.1.0. This gets us all present and future
improvements done by OpenSSL for free. E.g., the old code did not handle
IP addresses in the subject alternative names, while the new code does.

If the verification fails, print the error message as returned by
OpenSSL.

[1]: https://wiki.openssl.org/index.php/Hostname_validation

Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
We cannot easily print the certificate's hostname in case of a mismatch,
but the cURL branch does not, either.

Unfortuately, the diff looks bad because two functions are replaced by
the new verify_peer() function that is completely unrelated.

 imap-send.c | 101 +++++++++++++++++-----------------------------------
 1 file changed, 33 insertions(+), 68 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 9f21705d77..ec4e28e423 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -222,64 +222,30 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
 
 #else
 
-static int host_matches(const char *host, const ASN1_STRING *asn1_str)
+static int verify_peer(const SSL *ssl, int did_connect)
 {
-	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
+	long verify;
 
-	/* embedded NUL characters may open a security hole */
-	if (memchr(pattern, '\0', ASN1_STRING_length(asn1_str)))
-	    return 0;
-
-	if (pattern[0] == '*' && pattern[1] == '.') {
-		pattern += 2;
-		if (!(host = strchr(host, '.')))
-			return 0;
-		host++;
-	}
-
-	return *host && *pattern && !strcasecmp(host, pattern);
-}
+	/*
+	 * If we connected, we require a certificate.
+	 * If we did not connect because of a certificate error, the server
+	 * certificate is not installed in the SSL*, but the verification
+	 * result will still be returned below.
+	 */
+	if (did_connect) {
+		X509 *cert = SSL_get_peer_certificate(ssl);
 
-static int verify_hostname(X509 *cert, const char *hostname)
-{
-#if (OPENSSL_VERSION_NUMBER >= 0x40000000L)
-	const X509_NAME *subj;
-#else
-	X509_NAME *subj;
-#endif
-	const X509_NAME_ENTRY *cname_entry;
-	const ASN1_STRING *cname;
-	int i, found;
-	STACK_OF(GENERAL_NAME) *subj_alt_names;
-
-	/* try the DNS subjectAltNames */
-	found = 0;
-	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
-		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
-		for (i = 0; !found && i < num_subj_alt_names; i++) {
-			int ntype;
-			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
-			ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
-
-			if (ntype == GEN_DNS && host_matches(hostname, subj_alt_str))
-				found = 1;
-		}
-		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
+		if (!cert)
+			return error("unable to get peer certificate.");
+		X509_free(cert);
 	}
-	if (found)
-		return 0;
 
-	/* try the common name */
-	if (!(subj = X509_get_subject_name(cert)))
-		return error("cannot get certificate subject");
-	if ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, -1)) < 0 ||
-	    (cname_entry = X509_NAME_get_entry(subj, i)) == NULL ||
-	    (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL)
-		return error("cannot get certificate common name");
-	if (host_matches(hostname, cname))
-		return 0;
-	return error("certificate owner '%s' does not match hostname '%s'",
-		     ASN1_STRING_get0_data(cname), hostname);
+	/* check the verification result, ignoring the connection success */
+	verify = SSL_get_verify_result(ssl);
+	if (verify != X509_V_OK)
+		return error("unable to verify peer certificate: %s",
+			     X509_verify_cert_error_string(verify));
+	return 0;
 }
 
 static int ssl_socket_connect(struct imap_socket *sock,
@@ -293,7 +259,6 @@ static int ssl_socket_connect(struct imap_socket *sock,
 #endif
 	SSL_CTX *ctx;
 	int ret;
-	X509 *cert;
 
 	SSL_library_init();
 	SSL_load_error_strings();
@@ -313,9 +278,6 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	if (use_tls_only)
 		SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
-	if (cfg->ssl_verify)
-		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
-
 	if (!SSL_CTX_set_default_verify_paths(ctx)) {
 		ssl_socket_perror("SSL_CTX_set_default_verify_paths");
 		return -1;
@@ -325,6 +287,16 @@ static int ssl_socket_connect(struct imap_socket *sock,
 		ssl_socket_perror("SSL_new");
 		return -1;
 	}
+	if (cfg->ssl_verify) {
+		/* follow RFC 9525 section 6.3: no partial wildcards */
+		SSL_set_hostflags(sock->ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+		if (!SSL_set1_host(sock->ssl, cfg->host)) {
+			ssl_socket_perror("SSL_set1_host");
+			return -1;
+		}
+		SSL_set_verify(sock->ssl, SSL_VERIFY_PEER, NULL);
+	}
+
 	if (!SSL_set_rfd(sock->ssl, sock->fd[0])) {
 		ssl_socket_perror("SSL_set_rfd");
 		return -1;
@@ -348,20 +320,13 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	ret = SSL_connect(sock->ssl);
 	if (ret <= 0) {
 		socket_perror("SSL_connect", sock, ret);
+		if (cfg->ssl_verify)
+			verify_peer(sock->ssl, 0);
 		return -1;
 	}
 
-	if (cfg->ssl_verify) {
-		/* make sure the hostname matches that of the certificate */
-		cert = SSL_get_peer_certificate(sock->ssl);
-		if (!cert)
-			return error("unable to get peer certificate.");
-		if (SSL_get_verify_result(sock->ssl) != X509_V_OK)
-			return error("unable to verify peer certificate");
-		if (verify_hostname(cert, cfg->host) < 0)
-			return -1;
-	}
-
+	if (cfg->ssl_verify)
+		return verify_peer(sock->ssl, 1);
 	return 0;
 }
 #endif
-- 
2.53.0

[PATCH v3 3/3] imap-send: prepare for OpenSSL 4.0

[Beat Bolli]

OpenSSL 4.0 deprecated the SSL_set1_host function if favor of the new
function SSL_set1_dnsname, which fortunately takes the same parameters.

Add compatibility #defines for the function and its string variant for
the error message.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index ec4e28e423..526bde2f55 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -248,6 +248,13 @@ static int verify_peer(const SSL *ssl, int did_connect)
 	return 0;
 }
 
+#if (OPENSSL_VERSION_NUMBER < 0x40000000L)
+#define SSL_set1_dnsname(ssl, host) SSL_set1_host((ssl), (host))
+#define SSL_SET_DNSNAME_FN "SSL_set1_host"
+#else
+#define SSL_SET_DNSNAME_FN "SSL_set1_dnsname"
+#endif
+
 static int ssl_socket_connect(struct imap_socket *sock,
 			      const struct imap_server_conf *cfg,
 			      int use_tls_only)
@@ -290,8 +297,8 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	if (cfg->ssl_verify) {
 		/* follow RFC 9525 section 6.3: no partial wildcards */
 		SSL_set_hostflags(sock->ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
-		if (!SSL_set1_host(sock->ssl, cfg->host)) {
-			ssl_socket_perror("SSL_set1_host");
+		if (!SSL_set1_dnsname(sock->ssl, cfg->host)) {
+			ssl_socket_perror(SSL_SET_DNSNAME_FN);
 			return -1;
 		}
 		SSL_set_verify(sock->ssl, SSL_VERIFY_PEER, NULL);
-- 
2.53.0

[PATCH v2 2/2] imap-send: let OpenSSL do the hostname verification

[Beat Bolli]

According to the OpenSSL Wiki[1], OpenSSL knows how to verify the peer's
hostname since version 1.0.2. A simpler interface is available since
1.1.0, which is what is implemented here.

As a side-effect, we get all present and future improvements done by
OpenSSL for free. E.g., the old code did not handle IP addresses in the
subject alternative names, while the new code does.

If the verification fails, print the error message as returned by
OpenSSL.

[1]: https://wiki.openssl.org/index.php/Hostname_validation

Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 imap-send.c | 86 +++++++++--------------------------------------------
 1 file changed, 14 insertions(+), 72 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 9f21705d77..cd9eae15d0 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -222,66 +222,6 @@ static int ssl_socket_connect(struct imap_socket *sock UNUSED,
 
 #else
 
-static int host_matches(const char *host, const ASN1_STRING *asn1_str)
-{
-	const char *pattern = (const char *)ASN1_STRING_get0_data(asn1_str);
-
-	/* embedded NUL characters may open a security hole */
-	if (memchr(pattern, '\0', ASN1_STRING_length(asn1_str)))
-	    return 0;
-
-	if (pattern[0] == '*' && pattern[1] == '.') {
-		pattern += 2;
-		if (!(host = strchr(host, '.')))
-			return 0;
-		host++;
-	}
-
-	return *host && *pattern && !strcasecmp(host, pattern);
-}
-
-static int verify_hostname(X509 *cert, const char *hostname)
-{
-#if (OPENSSL_VERSION_NUMBER >= 0x40000000L)
-	const X509_NAME *subj;
-#else
-	X509_NAME *subj;
-#endif
-	const X509_NAME_ENTRY *cname_entry;
-	const ASN1_STRING *cname;
-	int i, found;
-	STACK_OF(GENERAL_NAME) *subj_alt_names;
-
-	/* try the DNS subjectAltNames */
-	found = 0;
-	if ((subj_alt_names = X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL))) {
-		int num_subj_alt_names = sk_GENERAL_NAME_num(subj_alt_names);
-		for (i = 0; !found && i < num_subj_alt_names; i++) {
-			int ntype;
-			GENERAL_NAME *subj_alt_name = sk_GENERAL_NAME_value(subj_alt_names, i);
-			ASN1_STRING *subj_alt_str = GENERAL_NAME_get0_value(subj_alt_name, &ntype);
-
-			if (ntype == GEN_DNS && host_matches(hostname, subj_alt_str))
-				found = 1;
-		}
-		sk_GENERAL_NAME_pop_free(subj_alt_names, GENERAL_NAME_free);
-	}
-	if (found)
-		return 0;
-
-	/* try the common name */
-	if (!(subj = X509_get_subject_name(cert)))
-		return error("cannot get certificate subject");
-	if ((i = X509_NAME_get_index_by_NID(subj, NID_commonName, -1)) < 0 ||
-	    (cname_entry = X509_NAME_get_entry(subj, i)) == NULL ||
-	    (cname = X509_NAME_ENTRY_get_data(cname_entry)) == NULL)
-		return error("cannot get certificate common name");
-	if (host_matches(hostname, cname))
-		return 0;
-	return error("certificate owner '%s' does not match hostname '%s'",
-		     ASN1_STRING_get0_data(cname), hostname);
-}
-
 static int ssl_socket_connect(struct imap_socket *sock,
 			      const struct imap_server_conf *cfg,
 			      int use_tls_only)
@@ -293,7 +233,6 @@ static int ssl_socket_connect(struct imap_socket *sock,
 #endif
 	SSL_CTX *ctx;
 	int ret;
-	X509 *cert;
 
 	SSL_library_init();
 	SSL_load_error_strings();
@@ -313,9 +252,6 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	if (use_tls_only)
 		SSL_CTX_set_options(ctx, SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3);
 
-	if (cfg->ssl_verify)
-		SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL);
-
 	if (!SSL_CTX_set_default_verify_paths(ctx)) {
 		ssl_socket_perror("SSL_CTX_set_default_verify_paths");
 		return -1;
@@ -325,6 +261,15 @@ static int ssl_socket_connect(struct imap_socket *sock,
 		ssl_socket_perror("SSL_new");
 		return -1;
 	}
+	if (cfg->ssl_verify) {
+		SSL_set_hostflags(sock->ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);
+		if (!SSL_set1_host(sock->ssl, cfg->host)) {
+			ssl_socket_perror("SSL_set1_host");
+			return -1;
+		}
+		SSL_set_verify(sock->ssl, SSL_VERIFY_PEER, NULL);
+	}
+
 	if (!SSL_set_rfd(sock->ssl, sock->fd[0])) {
 		ssl_socket_perror("SSL_set_rfd");
 		return -1;
@@ -352,14 +297,11 @@ static int ssl_socket_connect(struct imap_socket *sock,
 	}
 
 	if (cfg->ssl_verify) {
-		/* make sure the hostname matches that of the certificate */
-		cert = SSL_get_peer_certificate(sock->ssl);
-		if (!cert)
-			return error("unable to get peer certificate.");
-		if (SSL_get_verify_result(sock->ssl) != X509_V_OK)
-			return error("unable to verify peer certificate");
-		if (verify_hostname(cert, cfg->host) < 0)
-			return -1;
+		/* make sure the certificate was verified */
+		long res = SSL_get_verify_result(sock->ssl);
+		if (res != X509_V_OK)
+			return error("unable to verify peer certificate: %s",
+				     X509_verify_cert_error_string(res));
 	}
 
 	return 0;
-- 
2.53.0

Re: [PATCH v2 2/2] imap-send: let OpenSSL do the hostname verification

[Aniket Gupta]

Hello Beat,

> +		long res = SSL_get_verify_result(sock->ssl);
> +		if (res != X509_V_OK)
> +			return error("unable to verify peer certificate: %s",

I don't think this can be reached. With SSL_VERIFY_PEER, SSL_connect()
already fails, so a hostname mismatch would only print "certificate
verify failed". It does not name the host like the old code did. Could
we print the verify result when SSL_connect() fails?

Thanks,
Aniket

Re: [PATCH v2 2/2] imap-send: let OpenSSL do the hostname verification

[Xiangfei Ding]

Hello Beat and Aniket,

> I don't think this can be reached. With SSL_VERIFY_PEER, SSL_connect()
> already fails, so a hostname mismatch would only print "certificate
> verify failed". It does not name the host like the old code did. Could
> we print the verify result when SSL_connect() fails?

I suppose that we will have to install a callback at SSL_set_verify,
from which we could leave the bad hostname in a "mailbox" so that
in case SSL_connect() fails we could pick it up and print it out.

Would you mind me posting a patch on top of yours?

Best,
Xiang

On Tue, Oct 13, 2026 at 12:00 AM Aniket Gupta <aniketgupta3001@gmail.com> wrote:
>
> Hello Beat,
>
> > +             long res = SSL_get_verify_result(sock->ssl);
> > +             if (res != X509_V_OK)
> > +                     return error("unable to verify peer certificate: %s",
>
> I don't think this can be reached. With SSL_VERIFY_PEER, SSL_connect()
> already fails, so a hostname mismatch would only print "certificate
> verify failed". It does not name the host like the old code did. Could
> we print the verify result when SSL_connect() fails?
>
> Thanks,
> Aniket

Re: [PATCH v2 2/2] imap-send: let OpenSSL do the hostname verification

[Patrick Steinhardt]

On Mon, Oct 12, 2026 at 10:30:57PM +0200, Beat Bolli wrote:
> According to the OpenSSL Wiki[1], OpenSSL knows how to verify the peer's
> hostname since version 1.0.2. A simpler interface is available since
> 1.1.0, which is what is implemented here.

What is implicit here is that Git performs its own hostname
verification. A short introduction explaining the status quo and why
the status quo is insufficient would help the reviewer.

What's also missing is a quick note on the minimum supported version of
OpenSSL.

> As a side-effect, we get all present and future improvements done by
> OpenSSL for free. E.g., the old code did not handle IP addresses in the
> subject alternative names, while the new code does.

That sounds like it's win-win: less code and more supported scenarios!

> If the verification fails, print the error message as returned by
> OpenSSL.
> 
> [1]: https://wiki.openssl.org/index.php/Hostname_validation
> 
> Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
> Signed-off-by: Beat Bolli <dev+git@drbeat.li>
> ---
>  imap-send.c | 86 +++++++++--------------------------------------------
>  1 file changed, 14 insertions(+), 72 deletions(-)

Don't we have any tests for this? If not, can we maybe add some to make
sure that we indeed still verify the hostname?

> diff --git a/imap-send.c b/imap-send.c
> index 9f21705d77..cd9eae15d0 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -325,6 +261,15 @@ static int ssl_socket_connect(struct imap_socket *sock,
>  		ssl_socket_perror("SSL_new");
>  		return -1;
>  	}
> +	if (cfg->ssl_verify) {
> +		SSL_set_hostflags(sock->ssl, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS);

This matches the documentation, but it also points out the
X509_CHECK_FLAG_NO_WILDCARDS as a second commonly used flag. Why do we
pick one over the other?

> @@ -352,14 +297,11 @@ static int ssl_socket_connect(struct imap_socket *sock,
>  	}
>  
>  	if (cfg->ssl_verify) {
> -		/* make sure the hostname matches that of the certificate */
> -		cert = SSL_get_peer_certificate(sock->ssl);
> -		if (!cert)
> -			return error("unable to get peer certificate.");
> -		if (SSL_get_verify_result(sock->ssl) != X509_V_OK)
> -			return error("unable to verify peer certificate");
> -		if (verify_hostname(cert, cfg->host) < 0)
> -			return -1;
> +		/* make sure the certificate was verified */
> +		long res = SSL_get_verify_result(sock->ssl);
> +		if (res != X509_V_OK)
> +			return error("unable to verify peer certificate: %s",
> +				     X509_verify_cert_error_string(res));

Wait, this feels fishy to me. Take the following from [1]:

  In other cases where no peer certificate or raw public key was
  presented, the returned result can still be X509_V_OK, for example
  when a server did not request a client certificate during the initial
  handshake. X509_V_OK alone does not establish how or whether the peer
  was authenticated. A suitably provisioned external PSK can
  authenticate a peer without a certificate or raw public key. When
  certificate or raw public key authentication is expected, applications
  must also check that the expected credential is present using
  SSL_get_peer_certificate(3) or SSL_get0_peer_rpk(3).

So this here basically allows for a MITM attack in case the intercepting
adversary simply strips the certificate altogether if I understand
correctly. And to fix that we must retain the call to
`SSL_get_peer_certificate()`.

That makes me want to have tests even more now.

[1]: https://docs.openssl.org/4.1/man3/SSL_get_verify_result/#bugs

Patrick