imap-send: explicitly verify the peer certificate

From: Johannes Schindelin <[email protected]>

It is a bug to obtain the peer certificate without verifying it.

Having said that, from my reading of
https://www.openssl.org/docs/man1.1.1/man3/SSL_set_verify.html, it would appear that Git is saved by the fact that it calls SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL) already early on.

In other words, that SSL_VERIFY_PEER combined with the NULL parameter (i.e. no overridden callback) would _already_ verify the peer certificate. The fact that we later call SSL_get_peer_certificate() is mistaken by CodeQL to mean that that peer certificate still needs to be verified, but that had already happened at that point.

Nevertheless, it is better to verify the peer certificate explicitly than to rely on some side effect that is really hard to reason about (and that took me more than one business day to analyze fully). It also makes it easier for static analyzers to validate the correctness of the code.

---
imap-send: explicitly verify the peer certificate

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1886%2Fdscho%2Fimap-send-verify-peer-certificate-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1886/dscho/imap-send-verify-peer-certificate-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1886

 imap-send.c | 2 ++
 1 file changed, 2 insertions(+)

index 6c8f84e836b..27dc033c7f8 100644
@@ -324,6 +324,8 @@ static int ssl_socket_connect(struct imap_socket *sock,
 		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;
 	}

base-commit: 683c54c999c301c2cd6f715c411407c413b1d84e

gitgitgadget