Skip to content

Commit dfcd284

Browse files
committed
imap-send: explicitly verify the peer certificate
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 <[email protected]>
1 parent 683c54c commit dfcd284

File tree

1 file changed

+2
-0
lines changed

1 file changed

+2
-0
lines changed

imap-send.c

+2
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,8 @@ static int ssl_socket_connect(struct imap_socket *sock,
324324
cert = SSL_get_peer_certificate(sock->ssl);
325325
if (!cert)
326326
return error("unable to get peer certificate.");
327+
if (SSL_get_verify_result(sock->ssl) != X509_V_OK)
328+
return error("unable to verify peer certificate");
327329
if (verify_hostname(cert, cfg->host) < 0)
328330
return -1;
329331
}

0 commit comments

Comments
 (0)