Re: [PATCH v2] http: add support for different sslcert and sslkey types.
From: Jeff King <hidden>
Date: 2023-03-20 17:16:36
On Mon, Mar 20, 2023 at 03:48:49PM +0000, Stanislav Malishevskiy via GitGitGadget wrote:
From: Stanislav Malishevskiy <redacted> Basically git work with default curl ssl type - PEM. But for support eTokens like SafeNet tokens via pksc11 need setup 'ENG' as sslcert type and as sslkey type. So there added additional options for http to make that possible.
Seems like a reasonable thing to want, and the patch looks pretty clean. Two small points:
http.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
There are no tests here. I think it might be possible to add them, but our https test support is currently optional, and has bit-rotted a bit over the years. There's some discussion here: https://lore.kernel.org/git/Y9s7vyHKXP+TQPRm@pobox.com/ (local) So I don't think it makes sense to block this patch over the lack of tests, but it's something we might keep in mind to add if the test situation improves.
quoted hunk ↗ jump to hunk
@@ -1014,10 +1020,14 @@ static CURL *get_curl_handle(void) if (ssl_cert) curl_easy_setopt(result, CURLOPT_SSLCERT, ssl_cert); + if (ssl_cert_type) + curl_easy_setopt(result, CURLOPT_SSLCERTTYPE, ssl_cert_type);
We're just feeding curl whatever string the user gave us (which is good, since we don't know which ones are valid). But what happens with: GIT_SSL_CERT_TYPE=bogus git fetch ... Should we check for an error here, or will the actual request later complain properly? -Peff