Re: [PATCH] http.c: shell command evaluation for extraheader

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 5, 2018, 13:47 UTC
Message-ID: <nycvar.QRO.7.76.6.1803051437000.20700@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>
In-Reply-To: <CngnTUCx1pVSyKUzINZV972DlD_JMH53F-L2PZtx0_RKMiwXYY4f2UzDPeJaC5fL-6oJBXAxQ5f0OC76I_fagEWxch5csI716N7bQDh3gR8=@urandom.co.uk>

Hi Colin,

On Sun, 4 Mar 2018, Colin Arnott wrote:

Show 31 quoted lines

> The http.extraHeader config parameter currently only supports storing
> constant values. There are two main use cases where this fails:
> 
>   0. Sensitive payloads: frequently this config parameter is used to pass
>      authentication credentials in place of or in addition to the
>      Authorization header, however since this value is required to be in
>      the clear this can create security issues.
> 
>   1. Mutating headers: some headers, especially new authentication
>      schemes, leverage short lived tokens that change over time.
> 
> There do exist solutions with current tools for these use cases, however
> none are optimal:
> 
>   0. Shell alias: by aliasing over git with a call to git that includes the
>      config directive and evaluates the header value inline, you can
>      fake the desired mutability:
>        `alias git='git -c http.extraHeader="$(gpg -d crypt.gpg)"'`
>      This presents two problems:
>      a. aliasing over commands can be confusing to new users, since git
>         config information is stored in shell configs
>      b. this solution scales only to your shell, not all shells
> 
>   1. Global hook: you could implement a hook that writes the config
>      entry before fetch / pull actions, so that it is up to date, but
>      this does nothing to secure it.
> 
>   2. git-credential-helper: the credential helper interface already
>      supports shelling out to arbitrary binaries or scripts, however
>      this interface can only be used to populate the Authorization
>      header.

As the credential-helper is already intended for sensitive data, and as it already allows to interact with a helper, I would strongly assume that it would make more sense to try to extend that feature (instead of the simple extraHeader one).

This would also help alleviate all the quoting/dequoting issues involved with shell scripting.

Besides, the http.extraHeader feature was designed to accommodate all kinds of extra headers, not only authentication ones (and indeed, the authentication was only intended for use in build agents, where both environment and logging can be controlled rather tightly).

I also see that in your implementation, only the extraHeader value is evaluated, without any access to the rest of the metadata (such as URL, and optionally specified user).

It would probably get a little more complicated than a shell script to write a credential-helper that will always be asked to generate an authentication, but I think even a moderate-level Perl script could be used for that, and it *would* know the URL and user for which the credentials are intended...

Ciao, Johannes