{"thread":{"id":"63184","subject":"[PATCH] imap-send: explicitly verify the peer certificate","startedAt":"2025-03-24T12:28:05Z","lastAt":"2025-03-24T12:28:05Z","messageCount":1,"participants":["Johannes Schindelin via GitGitGadget"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"514918","messageId":"pull.1886.git.1742819282360.gitgitgadget@gmail.com","threadId":"63184","inReplyTo":null,"subject":"[PATCH] imap-send: explicitly verify the peer certificate","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2025-03-24T12:28:02Z","receivedAt":"2025-03-24T12:28:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIt is a bug to obtain the peer certificate without verifying it.\n\nHaving said that, from my reading of\nhttps://www.openssl.org/docs/man1.1.1/man3/SSL_set_verify.html, it would\nappear that Git is saved by the fact that it calls\n`SSL_CTX_set_verify(ctx, SSL_VERIFY_PEER, NULL)` already early on.\n\nIn other words, that `SSL_VERIFY_PEER` combined with the `NULL`\nparameter (i.e. no overridden callback) would _already_ verify the peer\ncertificate.  The fact that we later call `SSL_get_peer_certificate()`\nis mistaken by CodeQL to mean that that peer certificate still needs to\nbe verified, but that had already happened at that point.\n\nNevertheless, it is better to verify the peer certificate explicitly\nthan to rely on some side effect that is really hard to reason about\n(and that took me more than one business day to analyze fully). It also\nmakes it easier for static analyzers to validate the correctness of the\ncode.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n    imap-send: explicitly verify the peer certificate\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1886%2Fdscho%2Fimap-send-verify-peer-certificate-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1886/dscho/imap-send-verify-peer-certificate-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1886\n\n imap-send.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/imap-send.c b/imap-send.c\nindex 6c8f84e836b..27dc033c7f8 100644\n--- a/imap-send.c\n+++ b/imap-send.c\n@@ -324,6 +324,8 @@ static int ssl_socket_connect(struct imap_socket *sock,\n \t\tcert = SSL_get_peer_certificate(sock->ssl);\n \t\tif (!cert)\n \t\t\treturn error(\"unable to get peer certificate.\");\n+\t\tif (SSL_get_verify_result(sock->ssl) != X509_V_OK)\n+\t\t\treturn error(\"unable to verify peer certificate\");\n \t\tif (verify_hostname(cert, cfg->host) < 0)\n \t\t\treturn -1;\n \t}\n\nbase-commit: 683c54c999c301c2cd6f715c411407c413b1d84e\n-- \ngitgitgadget\n"}]}