Re: [PATCH v2 2/2] config: documentation for HTTPS proxy client cert.

["Jorge A López Silva" <jalopezsilva@gmail.com>]

> Thanks, this should be part of the previous patch, as it was that
> commit, not this one, that adds 4 options ;-)

Haha, yeah, you're right. I'll collapse the commits into a single one.

>  I think these files not merely "indicate" but they themselves
> "hold", "contain" and/or "store" the certificate and key.  Perhaps
> more like...
>         The pathname of a file that stores a client certificate to ...
> Also, it is customary to camelCase the configuration variable names.
> As I understand http.proxykey is roughly corresponds to existing
> http.sslKey (the former is for proxy, the latter is for the target
> host), I'd expect these two to be spelled http.proxySSLCert and
> http.proxySSLKey respectively (without omitting "SSL", as that is
> the underlying cURL option name if I am reading the code in 1/2
> correctly).

Good point. Better descriptions and names will be added.

> It is possible that the answer to these questions are the same---an
> on-disk password is a bad idea, so we deliberately omit a config
> that gives value to CURLOPT_KEYPASSWD and instead use the credential
> subsystem (see http.c::has_cert_password() and its caller).  If so,
> I think it would be prudent to follow the same pattern if possible?


Excellent point. Will adjust to re-use the same pattern.


On Thu, Feb 27, 2020 at 10:58 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Jorge Lopez Silva via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > From: Jorge Lopez Silva <jalopezsilva@gmail.com>
> >
> > The commit adds 4 options, client cert, key, key password and CA info.
> > The CA info can be used to specify a different CA path to validate the
> > HTTPS proxy cert.
> >
> > Signed-off-by: Jorge Lopez Silva <jalopezsilva@gmail.com>
> > ---
>
> Thanks, this should be part of the previous patch, as it was that
> commit, not this one, that adds 4 options ;-)
>
> > +http.proxycert::
> > +     File indicating a client certificate to use to authenticate with an HTTPS proxy.
> > +
> > +http.proxykey::
> > +     File indicating a private key to use to authenticate with an HTTPS proxy.
>
> I think these files not merely "indicate" but they themselves
> "hold", "contain" and/or "store" the certificate and key.  Perhaps
> more like...
>
>         The pathname of a file that stores a client certificate to ...
>
> Also, it is customary to camelCase the configuration variable names.
> As I understand http.proxykey is roughly corresponds to existing
> http.sslKey (the former is for proxy, the latter is for the target
> host), I'd expect these two to be spelled http.proxySSLCert and
> http.proxySSLKey respectively (without omitting "SSL", as that is
> the underlying cURL option name if I am reading the code in 1/2
> correctly).
>
> > +http.proxykeypass::
> > +     When communicating to the proxy using TLS (using an HTTPS proxy), use this
> > +     option along `http.proxykey` to indicate a password for the key.
>
> And this would be "http.proxyKeyPasswd" for the same two reasons.
>
> There are a couple of curious things, though:
>
>  * Is it a good idea to use a keyfile that is encrypted, but leave
>    the encryption password on disk in the configuration file to
>    begin with?
>
>  * This teaches our system about PROXY_KEYPASSWD that protects
>    PROXY_SSLKEY, but why isn't there a similar configuration
>    variable for CURLOPT_KEYPASSWD that protects CURLOPT_SSLKEY?
>
> It is possible that the answer to these questions are the same---an
> on-disk password is a bad idea, so we deliberately omit a config
> that gives value to CURLOPT_KEYPASSWD and instead use the credential
> subsystem (see http.c::has_cert_password() and its caller).  If so,
> I think it would be prudent to follow the same pattern if possible?
>
> > +http.proxycainfo::
> > +     File containing the certificates to verify the proxy with when using an HTTPS
> > +     proxy.
> > +
> >  http.emptyAuth::
> >       Attempt authentication without seeking a username or password.  This
> >       can be used to attempt GSS-Negotiate authentication without specifying