| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The credential protocol can't represent newlines in values, but URLs can
embed percent-encoded newlines in various components. A previous commit
taught the low-level writing routines to die() when encountering this,
but we can be a little friendlier to the user by detecting them earlier
and handling them gracefully.
This patch teaches credential_from_url() to notice such components,
issue a warning, and blank the credential (which will generally result
in prompting the user for a username and password). We blank the whole
credential in this case. Another option would be to blank only the
invalid component. However, we're probably better off not feeding a
partially-parsed URL result to a credential helper. We don't know how a
given helper would handle it, so we're better off to err on the side of
matching nothing rather than something unexpected.
The die() call in credential_write() is _probably_ impossible to reach
after this patch. Values should end up in credential structs only by URL
parsing (which is covered here), or by reading credential protocol input
(which by definition cannot read a newline into a value). But we should
definitely keep the low-level check, as it's our final and most accurate
line of defense against protocol injection attacks. Arguably it could
become a BUG(), but it probably doesn't matter much either way.
Note that the public interface of credential_from_url() grows a little
more than we need here. We'll use the extra flexibility in a future
patch to help fsck catch these cases.
|
|
The credential protocol that we use to speak to helpers can't represent
values with newlines in them. This was an intentional design choice to
keep the protocol simple, since none of the values we pass should
generally have newlines.
However, if we _do_ encounter a newline in a value, we blindly transmit
it in credential_write(). Such values may break the protocol syntax, or
worse, inject new valid lines into the protocol stream.
The most likely way for a newline to end up in a credential struct is by
decoding a URL with a percent-encoded newline. However, since the bug
occurs at the moment we write the value to the protocol, we'll catch it
there. That should leave no possibility of accidentally missing a code
path that can trigger the problem.
At this level of the code we have little choice but to die(). However,
since we'd not ever expect to see this case outside of a malicious URL,
that's an acceptable outcome.
Reported-by: Felix Wilhelm <fwilhelm@google.com>
|
|
Change various tests that use an idiom of the form:
>expect &&
test_cmp expect actual
To instead use:
test_must_be_empty actual
The test_must_be_empty() wrapper was introduced in ca8d148daf ("test:
test_must_be_empty helper", 2013-06-09). Many of these tests have been
added after that time. This was mostly found with, and manually pruned
from:
git grep '^\s+>.*expect.* &&$' t
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Sine the credential.helper key is a multi-valued config
list, there's no way to "unset" a helper once it's been set.
So if your system /etc/gitconfig sets one, you can never
avoid running it, but only add your own helpers on top.
Since an empty value for credential.helper is nonsensical
(it would just try to run "git-credential-"), we can assume
nobody is using it. Let's define it to reset the helper
list, letting you override lower-priority instances which
have come before.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When we are trying to fill a credential, we loop over the
set of defined credential-helpers, then fall back to running
askpass, and then finally prompt on the terminal. Helpers
which cannot find a credential are free to tell us nothing,
but they cannot currently ask us to stop prompting.
This patch lets them provide a "quit" attribute, which asks
us to stop the process entirely (avoiding running more
helpers, as well as the askpass/terminal prompt).
This has a few possible uses:
1. A helper which prompts the user itself (e.g., in a
dialog) can provide a "cancel" button to the user to
stop further prompts.
2. Some helpers may know that prompting cannot possibly
work. For example, if their role is to broker a ticket
from an external auth system and that auth system
cannot be contacted, there is no point in continuing
(we need a ticket to authenticate, and the user cannot
provide one by typing it in).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The Git CodingGuidelines prefer the $(...) construct for command
substitution instead of using the backquotes `...`.
The backquoted form is the traditional method for command
substitution, and is supported by POSIX. However, all but the
simplest uses become complicated quickly. In particular, embedded
command substitutions and/or the use of double quotes require
careful escaping with the backslash character.
The patch was generated by:
for _f in $(find . -name "*.sh")
do
sed -i 's@`\(.*\)`@$(\1)@g' ${_f}
done
and then carefully proof-read.
Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Instead of outputing only the username and password, print all the
attributes, even those that already appeared in the input.
This is closer to what the C API does, and allows one to take the exact
output of "git credential fill" as input to "git credential approve" or
"git credential reject".
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
* maint:
Update draft release notes to 1.7.9.3 for the last time
http.proxy: also mention https_proxy and all_proxy
t0300: work around bug in dash 0.5.6
t5512 (ls-remote): modernize style
tests: fix spurious error when run directly with Solaris /usr/xpg4/bin/sh
|
|
The construct 'while IFS== read' makes dash 0.5.6 execute
read without changing IFS, which results in test breakages
all over the place in t0300. Neither dash 0.5.5.1 and older
nor dash 0.5.7 and newer are affected: The problem was
introduded resp. fixed by the commits
55c46b7 ([BUILTIN] Honor tab as IFS whitespace when
splitting fields in readcmd, 2009-08-11)
1d806ac ([VAR] Do not poplocalvars prematurely on regular
utilities, 2010-05-27)
in http://git.kernel.org/?p=utils/dash/dash.git
Putting 'IFS==' before that line makes all versions of dash
work.
This looks like a dash bug, not a misinterpretation of the
standard. However, it's worth working around for two
reasons. One, this version of dash was released in Fedora
14-16, so the bug is found in the wild. And two, at least
one other shell, Solaris /bin/sh, choked on this by
persisting IFS after the read invocation. That is not a
shell we usually care about, and I think this use of IFS is
acceptable by POSIX (which allows other behavior near
"special builtins", but "read" is not one of those). But it
seems that this may be a subtle, not-well-tested case for
some shells. Given that the workaround is so simple, it's
worth just being defensive.
Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
t0300 creates some helper shell scripts, and marks them with
"!/bin/sh". Even though the scripts are fairly simple, they
can fail on broken shells (specifically, Solaris /bin/sh
will persist a temporary assignment to IFS in a "read"
command).
Rather than work around the problem for Solaris /bin/sh,
using write_script will make sure we point to a known-good
shell that the user has given us.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
When parsing a URL into a credential struct, we carefully
record each part of the URL, including the path on the
remote host, and use the result as part of the credential
context.
This had two practical implications:
1. Credential helpers which store a credential for later
access are likely to use the "path" portion as part of
the storage key. That means that a request to
https://example.com/foo.git
would not use the same credential that was stored in an
earlier request for:
https://example.com/bar.git
2. The prompt shown to the user includes all relevant
context, including the path.
In most cases, however, users will have a single password
per host. The behavior in (1) will be inconvenient, and the
prompt in (2) will be overly long.
This patch introduces a config option to toggle the
relevance of http paths. When turned on, we use the path as
before. When turned off, we drop the path component from the
context: helpers don't see it, and it does not appear in the
prompt.
This is nothing you couldn't do with a clever credential
helper at the start of your stack, like:
[credential "http://"]
helper = "!f() { grep -v ^path= ; }; f"
helper = your_real_helper
But doing this:
[credential]
useHttpPath = false
is way easier and more readable. Furthermore, since most
users will want the "off" behavior, that is the new default.
Users who want it "on" can set the variable (either for all
credentials, or just for a subset using
credential.*.useHttpPath).
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
Credential helpers can help users avoid having to type their
username and password over and over. However, some users may
not want a helper for their password, or they may be running
a helper which caches for a short time. In this case, it is
convenient to provide the non-secret username portion of
their credential via config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
The functionality for credential storage helpers is already
there; we just need to give the users a way to turn it on.
This patch provides a "credential.helper" configuration
variable which allows the user to provide one or more helper
strings.
Rather than simply matching credential.helper, we will also
compare URLs in subsection headings to the current context.
This means you can apply configuration to a subset of
credentials. For example:
[credential "https://example.com"]
helper = foo
would match a request for "https://example.com/foo.git", but
not one for "https://kernel.org/foo.git".
This is overkill for the "helper" variable, since users are
unlikely to want different helpers for different sites (and
since helpers run arbitrary code, they could do the matching
themselves anyway).
However, future patches will add new config variables where
this extra feature will be more useful.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
|
There are a few places in git that need to get a username
and password credential from the user; the most notable one
is HTTP authentication for smart-http pushing.
Right now the only choices for providing credentials are to
put them plaintext into your ~/.netrc, or to have git prompt
you (either on the terminal or via an askpass program). The
former is not very secure, and the latter is not very
convenient.
Unfortunately, there is no "always best" solution for
password management. The details will depend on the tradeoff
you want between security and convenience, as well as how
git can integrate with other security systems (e.g., many
operating systems provide a keychain or password wallet for
single sign-on).
This patch provides an abstract notion of credentials as a
data item, and provides three basic operations:
- fill (i.e., acquire from external storage or from the
user)
- approve (mark a credential as "working" for further
storage)
- reject (mark a credential as "not working", so it can
be removed from storage)
These operations can be backed by external helper processes
that interact with system- or user-specific secure storage.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
|
Junio C Hamano
Jeff King
Ævar Arnfjörð Bjarmason
Elia Pinto
Matthieu Moy
Michael J Gruber