* [PATCH] imap-send: explicitly verify the peer certificate
@ 2025-03-24 12:28 Johannes Schindelin via GitGitGadget
0 siblings, 0 replies; only message in thread
From: Johannes Schindelin via GitGitGadget @ 2025-03-24 12:28 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin, Johannes Schindelin
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
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(+)
diff --git a/imap-send.c b/imap-send.c
index 6c8f84e836b..27dc033c7f8 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -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
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2025-03-24 12:28 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-24 12:28 [PATCH] imap-send: explicitly verify the peer certificate Johannes Schindelin via GitGitGadget
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).