Re: [PATCH v2] http: add support for different sslcert and sslkey types.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 20, 2023 at 03:48:49PM +0000, Stanislav Malishevskiy via GitGitGadget wrote:

> From: Stanislav Malishevskiy <s.malishevskiy@xxxxxxxxxx>
> 
> 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@xxxxxxxxx/

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.

> @@ -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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux