From: Colin Arnott <colin@urandom.co.uk>
To: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: [PATCH] http.c: shell command evaluation for extraheader
Date: Sun, 04 Mar 2018 03:40:27 -0500 [thread overview]
Message-ID: <CngnTUCx1pVSyKUzINZV972DlD_JMH53F-L2PZtx0_RKMiwXYY4f2UzDPeJaC5fL-6oJBXAxQ5f0OC76I_fagEWxch5csI716N7bQDh3gR8=@urandom.co.uk> (raw)
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.
The optimal solution involves extending the current implementation of
http.extraHeader parsing to allow for arbitrary shell command execution.
There seem to be two paradigms for such features:
0. Overloading with '!' prefixes: seen in alias.* and credential.helper
1. New "Cmd" suffix parameters: seen in sendemail.toCmd sendemail.ccCmd
While the latter may be more clear without documentation, the addition
of a new config parameter seemed more complex for the codebase. As such,
new documentation is included.
Several edge cases came up during implementation and the following
design decisions were made:
0. Stdin and stderr for the child_process are exposed to the user:
this allows commands that print status information via stderr, and
accept input to function. The use case considered is text input for
decryption and error handling that is out of scope for git.
1. Failure to exec: if either the file does not exist, or any other
exec related failure occurs, no error is presented to the user,
and the header is not included
2. Non-zero return code: if the child_process returns a non-zero
value, no error is presented to the user, the return value is
consumed, and the header is not included in the request.
3. Headers starting with the '!' character require a shell command to
create: because no escaping syntax was implemented, the following
is required for such headers: "!printf '!magic: abra'"
Signed-off-by: Colin Arnott <colin@urandom.co.uk>
---
Documentation/config.txt | 7 +++++++
http.c | 20 ++++++++++++++++++++
t/t5551-http-fetch-smart.sh | 6 ++++--
3 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f57e9cf10..4b2171d60 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1918,6 +1918,13 @@ http.extraHeader::
more than one such entry exists, all of them are added as extra
headers. To allow overriding the settings inherited from the system
config, an empty value will reset the extra headers to the empty list.
+ If the value is prefixed with an exclamation point, it will
+ be treated as a shell command. For example, defining
+ "http.extraHeader = !gpg -d < secure_header.gpg", will pass the
+ decrypted header, if the command does not exec cleanly or has a
+ non-zero return value, no header will be added. Note that shell
+ commands will be executed from the top-level directory of a
+ repository, which may not necessarily be the current directory.
http.cookieFile::
The pathname of a file containing previously stored cookie lines,
diff --git a/http.c b/http.c
index 31755023a..11103df41 100644
--- a/http.c
+++ b/http.c
@@ -380,6 +380,26 @@ static int http_options(const char *var, const char *value, void *cb)
} else if (!*value) {
curl_slist_free_all(extra_http_headers);
extra_http_headers = NULL;
+ } else if (value[0] == '!') {
+ struct child_process cp = CHILD_PROCESS_INIT;
+ cp.git_cmd = 0;
+ cp.in = 0;
+ cp.out = -1;
+ cp.err = 0;
+ cp.use_shell = 1;
+ argv_array_push(&cp.args, value + 1);
+ if (!start_command(&cp)) {
+ struct strbuf output;
+ strbuf_init(&output, 0);
+ strbuf_read(&output, cp.out, 0);
+ close(cp.out);
+ cp.out = -1;
+ if (!finish_command(&cp)) {
+ extra_http_headers =
+ curl_slist_append(extra_http_headers, output.buf);
+ }
+ strbuf_release(&output);
+ }
} else {
extra_http_headers =
curl_slist_append(extra_http_headers, value);
diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index f5721b4a5..039afc76a 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -351,7 +351,9 @@ test_expect_success 'custom http headers' '
test_must_fail git -c http.extraheader="x-magic-two: cadabra" \
fetch "$HTTPD_URL/smart_headers/repo.git" &&
git -c http.extraheader="x-magic-one: abra" \
- -c http.extraheader="x-magic-two: cadabra" \
+ -c http.extraheader="!printf \"x-magic-two: cadabra\"" \
+ -c http.extraheader="!printf \"x-magic-three: alakazam; exit 2\"" \
+ -c http.extraheader="!shellcommanddoesnotexist" \
fetch "$HTTPD_URL/smart_headers/repo.git" &&
git update-index --add --cacheinfo 160000,$(git rev-parse HEAD),sub &&
git config -f .gitmodules submodule.sub.path sub &&
@@ -360,7 +362,7 @@ test_expect_success 'custom http headers' '
git submodule init sub &&
test_must_fail git submodule update sub &&
git -c http.extraheader="x-magic-one: abra" \
- -c http.extraheader="x-magic-two: cadabra" \
+ -c http.extraheader="!printf \"x-magic-two: cadabra\"" \
submodule update sub
'
--
2.16.2
next reply other threads:[~2018-03-04 8:40 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-04 8:40 Colin Arnott [this message]
2018-03-05 13:47 ` [PATCH] http.c: shell command evaluation for extraheader Johannes Schindelin
2018-03-06 6:09 ` Colin Arnott
2018-04-28 12:24 ` Johannes Schindelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CngnTUCx1pVSyKUzINZV972DlD_JMH53F-L2PZtx0_RKMiwXYY4f2UzDPeJaC5fL-6oJBXAxQ5f0OC76I_fagEWxch5csI716N7bQDh3gR8=@urandom.co.uk' \
--to=colin@urandom.co.uk \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).