Web lists-archives.com

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




Hi Colin,

On Sun, 4 Mar 2018, Colin Arnott wrote:

> 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