kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Taylor Blau	16b305cd2b	credential.c: store "wwwauth[]" values in `credential_read()` Teach git-credential to read "wwwauth[]" value(s) when parsing the output of a credential helper. These extra headers are not needed for Git's own HTTP support to use the feature internally, but the feature would not be available for a scripted caller (say, git-remote-mediawiki providing the header in the same way). As a bonus, this also makes it easier to use wwwauth[] in synthetic credential inputs in our test suite. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Elijah Newren	5e3f94dfe3	treewide: remove cache.h inclusion due to previous changes Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Elijah Newren	69a63fe663	treewide: be explicit about dependence on strbuf.h Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
M Hickford	a5c76569e7	credential: new attribute oauth_refresh_token Git authentication with OAuth access token is supported by every popular Git host including GitHub, GitLab and BitBucket [1][2][3]. Credential helpers Git Credential Manager (GCM) and git-credential-oauth generate OAuth credentials [4][5]. Following RFC 6749, the application prints a link for the user to authorize access in browser. A loopback redirect communicates the response including access token to the application. For security, RFC 6749 recommends that OAuth response also includes expiry date and refresh token [6]. After expiry, applications can use the refresh token to generate a new access token without user reauthorization in browser. GitLab and BitBucket set the expiry at two hours [2][3]. (GitHub doesn't populate expiry or refresh token.) However the Git credential protocol has no attribute to store the OAuth refresh token (unrecognised attributes are silently discarded). This means that the user has to regularly reauthorize the helper in browser. On a browserless system, this is particularly intrusive, requiring a second device. Introduce a new attribute oauth_refresh_token. This is especially useful when a storage helper and a read-only OAuth helper are configured together. Recall that `credential fill` calls each helper until it has a non-expired password. ``` [credential] helper = storage # eg. cache or osxkeychain helper = oauth ``` The OAuth helper can use the stored refresh token forwarded by `credential fill` to generate a fresh access token without opening the browser. See https://github.com/hickford/git-credential-oauth/pull/3/files for an implementation tested with this patch. Add support for the new attribute to credential-cache. Eventually, I hope to see support in other popular storage helpers. Alternatives considered: ask helpers to store all unrecognised attributes. This seems excessively complex for no obvious gain. Helpers would also need extra information to distinguish between confidential and non-confidential attributes. Workarounds: GCM abuses the helper get/store/erase contract to store the refresh token during credential get as the password for a fictitious host [7] (I wrote this hack). This workaround is only feasible for a monolithic helper with its own storage. [1] https://github.blog/2012-09-21-easier-builds-and-deployments-using-git-over-https-and-oauth/ [2] https://docs.gitlab.com/ee/api/oauth2.html#access-git-over-https-with-access-token [3] https://support.atlassian.com/bitbucket-cloud/docs/use-oauth-on-bitbucket-cloud/#Cloning-a-repository-with-an-access-token [4] https://github.com/GitCredentialManager/git-credential-manager [5] https://github.com/hickford/git-credential-oauth [6] https://datatracker.ietf.org/doc/html/rfc6749#section-5.1 [7] `66b94e489a/src/shared/GitLab/GitLabHostProvider.cs (L207)` Signed-off-by: M Hickford <mirth.hickford@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Elijah Newren	0b027f6ca7	abspath.h: move absolute path functions from cache.h This is another step towards letting us remove the include of cache.h in strbuf.c. It does mean that we also need to add includes of abspath.h in a number of C files. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Elijah Newren	f394e093df	treewide: be explicit about dependence on gettext.h Dozens of files made use of gettext functions, without explicitly including gettext.h. This made it more difficult to find which files could remove a dependence on cache.h. Make C files explicitly include gettext.h if they are using it. However, while compat/fsmonitor/fsm-ipc-darwin.c should also gain an include of gettext.h, it was left out to avoid conflicting with an in-flight topic. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Matthew John Cheetham	5f2117b24f	credential: add WWW-Authenticate header to cred requests Add the value of the WWW-Authenticate response header to credential requests. Credential helpers that understand and support HTTP authentication and authorization can use this standard header (RFC 2616 Section 14.47 [1]) to generate valid credentials. WWW-Authenticate headers can contain information pertaining to the authority, authentication mechanism, or extra parameters/scopes that are required. The current I/O format for credential helpers only allows for unique names for properties/attributes, so in order to transmit multiple header values (with a specific order) we introduce a new convention whereby a C-style array syntax is used in the property name to denote multiple ordered values for the same property. In this case we send multiple `wwwauth[]` properties where the order that the repeated attributes appear in the conversation reflects the order that the WWW-Authenticate headers appeared in the HTTP response. Add a set of tests to exercise the HTTP authentication header parsing and the interop with credential helpers. Credential helpers will receive WWW-Authenticate information in credential requests. [1] https://datatracker.ietf.org/doc/html/rfc2616#section-14.47 Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Matthew John Cheetham	6b8dda9a4f	http: read HTTP WWW-Authenticate response headers Read and store the HTTP WWW-Authenticate response headers made for a particular request. This will allow us to pass important authentication challenge information to credential helpers or others that would otherwise have been lost. libcurl only provides us with the ability to read all headers recieved for a particular request, including any intermediate redirect requests or proxies. The lines returned by libcurl include HTTP status lines delinating any intermediate requests such as "HTTP/1.1 200". We use these lines to reset the strvec of WWW-Authenticate header values as we encounter them in order to only capture the final response headers. The collection of all header values matching the WWW-Authenticate header is complicated by the fact that it is legal for header fields to be continued over multiple lines, but libcurl only gives us each physical line a time, not each logical header. This line folding feature is deprecated in RFC 7230 [1] but older servers may still emit them, so we need to handle them. In the future [2] we may be able to leverage functions to read headers from libcurl itself, but as of today we must do this ourselves. [1] https://www.rfc-editor.org/rfc/rfc7230#section-3.2 [2] https://daniel.haxx.se/blog/2022/03/22/a-headers-api-for-libcurl/ Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
M Hickford	d208bfdfef	credential: new attribute password_expiry_utc Some passwords have an expiry date known at generation. This may be years away for a personal access token or hours for an OAuth access token. When multiple credential helpers are configured, `credential fill` tries each helper in turn until it has a username and password, returning early. If Git authentication succeeds, `credential approve` stores the successful credential in all helpers. If authentication fails, `credential reject` erases matching credentials in all helpers. Helpers implement corresponding operations: get, store, erase. The credential protocol has no expiry attribute, so helpers cannot store expiry information. Even if a helper returned an improvised expiry attribute, git credential discards unrecognised attributes between operations and between helpers. This is a particular issue when a storage helper and a credential-generating helper are configured together: [credential] helper = storage # eg. cache or osxkeychain helper = generate # eg. oauth `credential approve` stores the generated credential in both helpers without expiry information. Later `credential fill` may return an expired credential from storage. There is no workaround, no matter how clever the second helper. The user sees authentication fail (a retry will succeed). Introduce a password expiry attribute. In `credential fill`, ignore expired passwords and continue to query subsequent helpers. In the example above, `credential fill` ignores the expired password and a fresh credential is generated. If authentication succeeds, `credential approve` replaces the expired password in storage. If authentication fails, the expired credential is erased by `credential reject`. It is unnecessary but harmless for storage helpers to self prune expired credentials. Add support for the new attribute to credential-cache. Eventually, I hope to see support in other popular storage helpers. Example usage in a credential-generating helper https://github.com/hickford/git-credential-oauth/pull/16 Signed-off-by: M Hickford <mirth.hickford@gmail.com> Reviewed-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	1 year ago
Ævar Arnfjörð Bjarmason	a41e8e7467	urlmatch.c: add and use a *_release() function Plug a memory leak in credential_apply_config() by adding and using a new urlmatch_config_release() function. This just does a string_list_clear() on the "vars" member. This finished up work on normalizing the init/free pattern in this API, started in `73ee449bbf` (urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT, 2021-10-01). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	2 years ago
Ævar Arnfjörð Bjarmason	73ee449bbf	urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT Change the initialization pattern of "struct urlmatch_config" to use an _INIT macro and designated initializers. Right now there's no other "struct" member of "struct urlmatch_config" which would require its own _INIT, but it's good practice not to assume that. Let's also change this to a designated initializer while we're at it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Mike Hommey	5146c2f148	credential: fix leak in credential_apply_config() Signed-off-by: Mike Hommey <mh@glandium.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Ævar Arnfjörð Bjarmason	5726a6b401	.c _init(): define in terms of corresponding _INIT macro Change the common patter in the codebase of duplicating the initialization logic between an _INIT macro and a corresponding _init() function to use the macro as the canonical source of truth. Now we no longer need to keep the function up-to-date with the macro version. This implements a suggestion by Jeff King who found that under -O2 [1] modern compilers will init new version in place without the extra copy[1]. The performance of a single _init() won't matter in most cases, but even if it does we're going to be producing efficient machine code to perform these operations. 1. https://lore.kernel.org/git/YNyrDxUO1PlGJvCn@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	3 years ago
Nikita Leonov	356c473295	credential: treat CR/LF as line endings in the credential protocol This fix makes using Git credentials more friendly to Windows users: it allows a credential helper to communicate using CR/LF line endings ("DOS line endings" commonly found on Windows) instead of LF-only line endings ("Unix line endings"). Note that this changes the behavior a bit: if a credential helper produces, say, a password with a trailing Carriage Return character, that will now be culled even when the rest of the lines end only in Line Feed characters, indicating that the Carriage Return was not meant to be part of the line ending. In practice, it seems _very_ unlikely that something like this happens. Passwords usually need to consist of non-control characters, URLs need to have special characters URL-encoded, and user names, well, are names. However, it _does_ help on Windows, where CR/LF line endings are common: as unrecognized commands are simply ignored by the credential machinery, even a command like `quit\r` (which is clearly intended to abort) would simply be ignored (silently) by Git. So let's change the credential machinery to accept both CR/LF and LF line endings. While we do this for the credential helper protocol, we do _not_ adjust `git credential-cache--daemon` (which won't work on Windows, anyway, because it requires Unix sockets) nor `git credential-store` (which writes the file `~/.git-credentials` which we consider an implementation detail that should be opaque to the user, read: we do expect users _not_ to edit this file manually). Signed-off-by: Nikita Leonov <nykyta.leonov@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Junio C Hamano	afbdba391e	run_command: teach API users to use embedded 'args' more The child_process structure has an embedded strvec for formulating the command line argument list these days, but code that predates the wide use of it prepared a separate char *argv[] array and manually set the child_process.argv pointer point at it. Teach these old-style code to lose the separate argv[] array. Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Johannes Schindelin	9a121b0d22	credential: handle `credential.<partial-URL>.<key>` again In the patches for CVE-2020-11008, the ability to specify credential settings in the config for partial URLs got lost. For example, it used to be possible to specify a credential helper for a specific protocol: [credential "https://"] helper = my-https-helper Likewise, it used to be possible to configure settings for a specific host, e.g.: [credential "dev.azure.com"] useHTTPPath = true Let's reinstate this behavior. While at it, increase the test coverage to document and verify the behavior with a couple other categories of partial URLs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	b44d0118ac	credential: fix matching URLs with multiple levels in path `46fd7b3900` ("credential: allow wildcard patterns when matching config", 2020-02-20) introduced support for matching credential helpers using urlmatch. In doing so, it introduced code to percent-encode the paths we get from the credential helper so that they could be effectively matched by the urlmatch code. Unfortunately, that code had a bug: it percent-encoded the slashes in the path, resulting in any URL path that contained multiple levels (i.e., a directory component) not matching. We are currently the only caller of the percent-encoding code and could simply change it not to encode slashes. However, we still want to encode slashes in the username component, so we need to have both behaviors available. So instead, let's add a flag to control encoding slashes, which is the behavior we want here, and use it when calling the code in this case. Add a test for credential helper URLs using multiple slashes in the path, which our test suite previously lacked, as well as one ensuring that we handle usernames with slashes gracefully. Since we're testing other percent-encoding handling, let's add one for non-ASCII UTF-8 characters as well. Reported-by: Ilya Tretyakov <it@it3xl.ru> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Johannes Schindelin	12294990c9	credential: handle `credential.<partial-URL>.<key>` again In the patches for CVE-2020-11008, the ability to specify credential settings in the config for partial URLs got lost. For example, it used to be possible to specify a credential helper for a specific protocol: [credential "https://"] helper = my-https-helper Likewise, it used to be possible to configure settings for a specific host, e.g.: [credential "dev.azure.com"] useHTTPPath = true Let's reinstate this behavior. While at it, increase the test coverage to document and verify the behavior with a couple other categories of partial URLs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Johannes Schindelin	f52b0cb418	credential: optionally allow partial URLs in credential_from_url_gently() Prior to the fixes for CVE-2020-11008, we were _very_ lenient in what we required from a URL in order to parse it into a `struct credential`. That led to serious vulnerabilities. There was one call site, though, that really needed that leniency: when parsing config settings a la `credential.dev.azure.com.useHTTPPath`. Settings like this might be desired when users want to use, say, a given user name on a given host, regardless of the protocol to be used. In preparation for fixing that bug, let's refactor the code to optionally allow for partial URLs. For the moment, this functionality is only exposed via the now-renamed function `credential_from_url_1()`, but it is not used. The intention is to make it easier to verify that this commit does not change the existing behavior unless explicitly allowing for partial URLs. Please note that this patch does more than just reinstating a way to imitate the behavior before those CVE-2020-11008 fixes: Before that, we would simply ignore URLs without a protocol. In other words, misleadingly, the following setting would be applied to _all_ URLs: [credential "example.com"] username = that-me The obvious intention is to match the host name only. With this patch, we allow precisely that: when parsing the URL with non-zero `allow_partial_url`, we do not simply return success if there was no protocol, but we simply leave the protocol unset and continue parsing the URL. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Johannes Schindelin	6828e5972b	credential: optionally allow partial URLs in credential_from_url_gently() Prior to the fixes for CVE-2020-11008, we were _very_ lenient in what we required from a URL in order to parse it into a `struct credential`. That led to serious vulnerabilities. There was one call site, though, that really needed that leniency: when parsing config settings a la `credential.dev.azure.com.useHTTPPath`. Settings like this might be desired when users want to use, say, a given user name on a given host, regardless of the protocol to be used. In preparation for fixing that bug, let's refactor the code to optionally allow for partial URLs. For the moment, this functionality is only exposed via the now-renamed function `credential_from_url_1()`, but it is not used. The intention is to make it easier to verify that this commit does not change the existing behavior unless explicitly allowing for partial URLs. Please note that this patch does more than just reinstating a way to imitate the behavior before those CVE-2020-11008 fixes: Before that, we would simply ignore URLs without a protocol. In other words, misleadingly, the following setting would be applied to _all_ URLs: [credential "example.com"] username = that-me The obvious intention is to match the host name only. With this patch, we allow precisely that: when parsing the URL with non-zero `allow_partial_url`, we do not simply return success if there was no protocol, but we simply leave the protocol unset and continue parsing the URL. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Jonathan Nieder	e7fab62b73	credential: treat URL with empty scheme as invalid Until "credential: refuse to operate when missing host or protocol", Git's credential handling code interpreted URLs with empty scheme to mean "give me credentials matching this host for any protocol". Luckily libcurl does not recognize such URLs (it tries to look for a protocol named "" and fails). Just in case that changes, let's reject them within Git as well. This way, credential_from_url is guaranteed to always produce a "struct credential" with protocol and host set. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>	4 years ago
Jonathan Nieder	c44088ecc4	credential: treat URL without scheme as invalid libcurl permits making requests without a URL scheme specified. In this case, it guesses the URL from the hostname, so I can run git ls-remote http::ftp.example.com/path/to/repo and it would make an FTP request. Any user intentionally using such a URL is likely to have made a typo. Unfortunately, credential_from_url is not able to determine the host and protocol in order to determine appropriate credentials to send, and until "credential: refuse to operate when missing host or protocol", this resulted in another host's credentials being leaked to the named host. Teach credential_from_url_gently to consider such a URL to be invalid so that fsck can detect and block gitmodules files with such URLs, allowing server operators to avoid serving them to downstream users running older versions of Git. This also means that when such URLs are passed on the command line, Git will print a clearer error so affected users can switch to the simpler URL that explicitly specifies the host and protocol they intend. One subtlety: .gitmodules files can contain relative URLs, representing a URL relative to the URL they were cloned from. The relative URL resolver used for .gitmodules can follow ".." components out of the path part and past the host part of a URL, meaning that such a relative URL can be used to traverse from a https://foo.example.com/innocent superproject to a https::attacker.example.com/exploit submodule. Fortunately a leading ':' in the first path component after a series of leading './' and '../' components is unlikely to show up in other contexts, so we can catch this by detecting that pattern. Reported-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> Reviewed-by: Jeff King <peff@peff.net>	4 years ago
Jeff King	fe29a9b7b0	credential: die() when parsing invalid urls When we try to initialize credential loading by URL and find that the URL is invalid, we set all fields to NULL in order to avoid acting on malicious input. Later when we request credentials, we diagonse the erroneous input: fatal: refusing to work with credential missing host field This is problematic in two ways: - The message doesn't tell the user why we are missing the host field, so they can't tell from this message alone how to recover. There can be intervening messages after the original warning of bad input, so the user may not have the context to put two and two together. - The error only occurs when we actually need to get a credential. If the URL permits anonymous access, the only encouragement the user gets to correct their bogus URL is a quiet warning. This is inconsistent with the check we perform in fsck, where any use of such a URL as a submodule is an error. When we see such a bogus URL, let's not try to be nice and continue without helpers. Instead, die() immediately. This is simpler and obviously safe. And there's very little chance of disrupting a normal workflow. It's _possible_ that somebody has a legitimate URL with a raw newline in it. It already wouldn't work with credential helpers, so this patch steps that up from an inconvenience to "we will refuse to work with it at all". If such a case does exist, we should figure out a way to work with it (especially if the newline is only in the path component, which we normally don't even pass to helpers). But until we see a real report, we're better off being defensive. Reported-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>	4 years ago
Jeff King	8ba8ed568e	credential: refuse to operate when missing host or protocol The credential helper protocol was designed to be very flexible: the fields it takes as input are treated as a pattern, and any missing fields are taken as wildcards. This allows unusual things like: echo protocol=https \| git credential reject to delete all stored https credentials (assuming the helpers themselves treat the input that way). But when helpers are invoked automatically by Git, this flexibility works against us. If for whatever reason we don't have a "host" field, then we'd match _any_ host. When you're filling a credential to send to a remote server, this is almost certainly not what you want. Prevent this at the layer that writes to the credential helper. Add a check to the credential API that the host and protocol are always passed in, and add an assertion to the credential_write function that speaks credential helper protocol to be doubly sure. There are a few ways this can be triggered in practice: - the "git credential" command passes along arbitrary credential parameters it reads from stdin. - until the previous patch, when the host field of a URL is empty, we would leave it unset (rather than setting it to the empty string) - a URL like "example.com/foo.git" is treated by curl as if "http://" was present, but our parser sees it as a non-URL and leaves all fields unset - the recent fix for URLs with embedded newlines blanks the URL but otherwise continues. Rather than having the desired effect of looking up no credential at all, many helpers will return _any_ credential Our earlier test for an embedded newline didn't catch this because it only checked that the credential was cleared, but didn't configure an actual helper. Configuring the "verbatim" helper in the test would show that it is invoked (it's obviously a silly helper which doesn't look at its input, but the point is that it shouldn't be run at all). Since we're switching this case to die(), we don't need to bother with a helper. We can see the new behavior just by checking that the operation fails. We'll add new tests covering partial input as well (these can be triggered through various means with url-parsing, but it's simpler to just check them directly, as we know we are covered even if the url parser changes behavior in the future). [jn: changed to die() instead of logging and showing a manual username/password prompt] Reported-by: Carlo Arenas <carenas@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>	4 years ago
Jeff King	24036686c4	credential: parse URL without host as empty host, not unset We may feed a URL like "cert:///path/to/cert.pem" into the credential machinery to get the key for a client-side certificate. That credential has no hostname field, which is about to be disallowed (to avoid confusion with protocols where a helper _would_ expect a hostname). This means as of the next patch, credential helpers won't work for unlocking certs. Let's fix that by doing two things: - when we parse a url with an empty host, set the host field to the empty string (asking only to match stored entries with an empty host) rather than NULL (asking to match _any_ host). - when we build a cert:// credential by hand, similarly assign an empty string It's the latter that is more likely to impact real users in practice, since it's what's used for http connections. But we don't have good infrastructure to test it. The url-parsing version will help anybody using git-credential in a script, and is easy to test. Signed-off-by: Jeff King <peff@peff.net> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>	4 years ago
Jeff King	4c5971e18a	credential: treat "?" and "#" in URLs as end of host It's unusual to see: https://example.com?query-parameters without an intervening slash, like: https://example.com/some-path?query-parameters or even: https://example.com/?query-parameters but it is a valid end to the hostname (actually "authority component") according to RFC 3986. Likewise for "#". And curl will parse the URL according to the standard, meaning it will contact example.com, but our credential code would ask about a bogus hostname with a "?" in it. Let's make sure we follow the standard, and more importantly ask about the same hosts that curl will be talking to. It would be nice if we could just ask curl to parse the URL for us. But it didn't grow a URL-parsing API until 7.62, so we'd be stuck with fallback code either way. Plus we'd need this code in the main Git binary, where we've tried to avoid having a link dependency on libcurl. But let's at least fix our parser. Moving to curl's parser would prevent other potential discrepancies, but this gives us immediate relief for the known problem, and would help our fallback code if we eventually use curl. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Jeff King	c716fe4bd9	credential: detect unrepresentable values when parsing urls 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.	4 years ago
Jeff King	9a6bbee800	credential: avoid writing values with newlines 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>	4 years ago
brian m. carlson	46fd7b3900	credential: allow wildcard patterns when matching config In some cases, a user will want to use a specific credential helper for a wildcard pattern, such as https://*.corp.example.com. We have code that handles this already with the urlmatch code, so let's use that instead of our custom code. Since the urlmatch code is a superset of our current matching in terms of capabilities, there shouldn't be any cases of things that matched previously that don't match now. However, in addition to wildcard matching, we now use partial path matching, which can cause slightly different behavior in the case that a helper applies to the prefix (considering path components) of the remote URL. While different, this is probably the behavior people were wanting anyway. Since we're using the urlmatch code, we need to encode the components we've gotten into a URL to match, so add a function to percent-encode data and format the URL with it. We now also no longer need to the custom code to match URLs, so let's remove it. Additionally, the urlmatch code always looks for the best match, whereas we want all matches for credential helpers to preserve existing behavior. Let's add an optional field, select_fn, that lets us control which items we want (in this case, all of them) and default it to the best-match code that already exists for other users. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	82eb249853	credential: use the last matching username in the config Everywhere else in the codebase, we use the rule that the last matching configuration option is the one that takes effect. This is helpful because it allows more specific configuration settings (e.g., per-repo configuration) to override less specific settings (e.g., per-user configuration). However, in the credential code, we didn't honor this setting, and instead picked the first setting we had, and stuck with it. This was likely to ensure we picked the value from the URL, which we want to honor over the configuration. It's possible to do both, though, so let's check if the value is the one we've gotten over our protocol connection, which if present will have come from the URL, and keep it if so. Otherwise, let's overwrite the value with the latest version we've got from the configuration, so we keep the last configuration value. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Erik E Brady	a0d51e8d0e	credential: ignore SIGPIPE when writing to credential helpers The credential subsystem can trigger SIGPIPE when writing to an external helper if that helper closes its stdin before reading the whole input. Normally this is rare, since helpers would need to read that input to make a decision about how to respond, but: 1. It's reasonable to configure a helper which only handles "get" while ignoring "store". Such a handler might not read stdin for "store", thereby rapidly closing stdin upon helper exit. 2. A broken or misbehaving helper might exit immediately. That's an error, but it's not reasonable for it to take down the parent Git process with SIGPIPE. Even with such a helper, seeing this problem should be rare. Getting SIGPIPE requires the helper racily exiting before we've written the fairly small credential output. Signed-off-by: Erik E Brady <brady@cisco.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	6 years ago
Ævar Arnfjörð Bjarmason	88ce3ef636	*.[ch] refactoring: make use of the FREE_AND_NULL() macro Replace occurrences of `free(ptr); ptr = NULL` which weren't caught by the coccinelle rule. These fall into two categories: - free/NULL assignments one after the other which coccinelle all put on one line, which is functionally equivalent code, but very ugly. - manually spotted occurrences where the NULL assignment isn't right after the free() call. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Ævar Arnfjörð Bjarmason	6a83d90207	coccinelle: make use of the "type" FREE_AND_NULL() rule Apply the result of the just-added coccinelle rule. This manually excludes a few occurrences, mostly things that resulted in many FREE_AND_NULL() on one line, that'll be manually fixed in a subsequent change. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Brandon Williams	b2141fc1d2	config: don't include config.h by default Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	7 years ago
Jeff King	24321375cd	credential: let empty credential specs reset helper list 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>	8 years ago
Junio C Hamano	8f309aeb82	strbuf: introduce strbuf_getline_{lf,nul}() The strbuf_getline() interface allows a byte other than LF or NUL as the line terminator, but this is only because I wrote these codepaths anticipating that there might be a value other than NUL and LF that could be useful when I introduced line_termination long time ago. No useful caller that uses other value has emerged. By now, it is clear that the interface is overly broad without a good reason. Many codepaths have hardcoded preference to read either LF terminated or NUL terminated records from their input, and then call strbuf_getline() with LF or NUL as the third parameter. This step introduces two thin wrappers around strbuf_getline(), namely, strbuf_getline_lf() and strbuf_getline_nul(), and mechanically rewrites these call sites to call either one of them. The changes contained in this patch are: * introduction of these two functions in strbuf.[ch] * mechanical conversion of all callers to strbuf_getline() with either '\n' or '\0' as the third parameter to instead call the respective thin wrapper. After this step, output from "git grep 'strbuf_getline('" would become a lot smaller. An interim goal of this series is to make this an empty set, so that we can have strbuf_getline_crlf() take over the shorter name strbuf_getline(). Signed-off-by: Junio C Hamano <gitster@pobox.com>	8 years ago
Jeff King	59b386526a	credential: let helpers tell us to quit 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>	10 years ago
René Scharfe	d318027932	run-command: introduce CHILD_PROCESS_INIT Most struct child_process variables are cleared using memset first after declaration. Provide a macro, CHILD_PROCESS_INIT, that can be used to initialize them statically instead. That's shorter, doesn't require a function call and is slightly more readable (especially given that we already have STRBUF_INIT, ARGV_ARRAY_INIT etc.). Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>	10 years ago
Jeff King	cf4fff579e	refactor skip_prefix to return a boolean The skip_prefix() function returns a pointer to the content past the prefix, or NULL if the prefix was not found. While this is nice and simple, in practice it makes it hard to use for two reasons: 1. When you want to conditionally skip or keep the string as-is, you have to introduce a temporary variable. For example: tmp = skip_prefix(buf, "foo"); if (tmp) buf = tmp; 2. It is verbose to check the outcome in a conditional, as you need extra parentheses to silence compiler warnings. For example: if ((cp = skip_prefix(buf, "foo")) /* do something with cp */ Both of these make it harder to use for long if-chains, and we tend to use starts_with() instead. However, the first line of "do something" is often to then skip forward in buf past the prefix, either using a magic constant or with an extra strlen(3) (which is generally computed at compile time, but means we are repeating ourselves). This patch refactors skip_prefix() to return a simple boolean, and to provide the pointer value as an out-parameter. If the prefix is not found, the out-parameter is untouched. This lets you write: if (skip_prefix(arg, "foo ", &arg)) do_foo(arg); else if (skip_prefix(arg, "bar ", &arg)) do_bar(arg); Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	10 years ago
Jeff King	9c183a7072	credential: convert "url" attribute into its parsed subparts The git-credential command requires that you feed it a broken-down credential, which means that the client needs to parse a URL itself. Since we have our own URL-parsing routines, we can easily allow the caller to just give us the URL as-is, saving them some code. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Matthieu Moy <Matthieu.Moy@imag.fr> Signed-off-by: Junio C Hamano <gitster@pobox.com>	12 years ago
Matthieu Moy	2d6dc182b8	git credential fill: output the whole 'struct credential' 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>	12 years ago
Jeff King	ce77aa4813	credential: use git_prompt instead of git_getpass We use git_getpass to retrieve the username and password from the terminal. However, git_getpass will not echo the username as the user types. We can fix this by using the more generic git_prompt, which underlies git_getpass but lets us specify an "echo" option. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	13 years ago
Jeff King	d3c58b83ae	move git_getpass to its own source file This is currently in connect.c, but really has nothing to do with the git protocol itself. Let's make a new source file all about prompting the user, which will make it cleaner to refactor. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	13 years ago
Jeff King	a78fbb4fb6	credential: make relevance of http path configurable 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>	13 years ago
Jeff King	d5742425eb	credential: add credential.*.username 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>	13 years ago
Jeff King	118250728e	credential: apply helper config 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>	13 years ago
Jeff King	d3e847c107	credential: add function for parsing url components All of the components of a credential struct can be found in a URL. For example, the URL: http://foo:bar@example.com/repo.git contains: protocol=http host=example.com path=repo.git username=foo password=bar We want to be able to turn URLs into broken-down credential structs so that we know two things: 1. Which parts of the username/password we still need 2. What the context of the request is (for prompting or as a key for storing credentials). This code is based on http_auth_init in http.c, but needed a few modifications in order to get all of the components that the credential object is interested in. Once the http code is switched over to the credential API, then http_auth_init can just go away. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	13 years ago
Jeff King	abca927dbe	introduce credentials API 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>	13 years ago

1 2

61 Commits (master)