From 7f53583834f991eadcc371932e117317cb3c65c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com> Date: Mon, 4 May 2020 18:39:05 -0700 Subject: [PATCH 1/4] credential: update description for credential_from_url_gently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit c44088ecc4 (credential: treat URL without scheme as invalid, 2020-04-18) changes the implementation for this function to return -1 if protocol is missing. Update blurb to match implementation. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- credential.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/credential.h b/credential.h index d99ec42b2a..f51703d9ce 100644 --- a/credential.h +++ b/credential.h @@ -177,8 +177,8 @@ void credential_write(const struct credential *, FILE *); * Parse a url into a credential struct, replacing any existing contents. * * If the url can't be parsed (e.g., a missing "proto://" component), the - * resulting credential will be empty but we'll still return success from the - * "gently" form. + * resulting credential will be empty and the function will return an + * error (even in the "gently" form). * * If we encounter a component which cannot be represented as a credential * value (e.g., because it contains a newline), the "gently" form will return From bb987657690b447ae9162ef3a5841d26eb82d806 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com> Date: Mon, 4 May 2020 18:39:06 -0700 Subject: [PATCH 2/4] credential: correct order of parameters for credential_match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since the beginning in 118250728e (credential: apply helper config, 2011-12-10), the declaration for that function used a different order than the implementation. All callers use the same order than the implementation, so update the declaration in credential.h to match. Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- credential.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/credential.h b/credential.h index f51703d9ce..c0e17e3554 100644 --- a/credential.h +++ b/credential.h @@ -189,7 +189,7 @@ void credential_write(const struct credential *, FILE *); void credential_from_url(struct credential *, const char *url); int credential_from_url_gently(struct credential *, const char *url, int quiet); -int credential_match(const struct credential *have, - const struct credential *want); +int credential_match(const struct credential *want, + const struct credential *have); #endif /* CREDENTIAL_H */ From 4b8938be4c4ef2d87642988f3486bc15c366b617 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com> Date: Wed, 6 May 2020 14:47:25 -0700 Subject: [PATCH 3/4] credential: update gitcredentials documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarify the expected effect of all attributes and how the helpers are expected to handle them and the context where they operate. While at it, space the descriptions for clarity, and add a paragraph mentioning the early termination in the list processing of helpers, to complement the one about the special "quit" attribute. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/gitcredentials.txt | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt index 1814d2d23c..31cd7fde10 100644 --- a/Documentation/gitcredentials.txt +++ b/Documentation/gitcredentials.txt @@ -262,16 +262,26 @@ For a `get` operation, the helper should produce a list of attributes on stdout in the same format (see linkgit:git-credential[1] for common attributes). A helper is free to produce a subset, or even no values at all if it has nothing useful to provide. Any provided attributes will -overwrite those already known about by Git. If a helper outputs a -`quit` attribute with a value of `true` or `1`, no further helpers will -be consulted, nor will the user be prompted (if no credential has been -provided, the operation will then fail). +overwrite those already known about by Git's credential subsystem. + +While it is possible to override all attributes, well behaving helpers +should refrain from doing so for any attribute other than username and +password. + +If a helper outputs a `quit` attribute with a value of `true` or `1`, +no further helpers will be consulted, nor will the user be prompted +(if no credential has been provided, the operation will then fail). + +Similarly, no more helpers will be consulted once both username and +password had been provided. For a `store` or `erase` operation, the helper's output is ignored. -If it fails to perform the requested operation, it may complain to -stderr to inform the user. If it does not support the requested -operation (e.g., a read-only store), it should silently ignore the -request. + +If a helper fails to perform the requested operation or needs to notify +the user of a potential issue, it may write to stderr. + +If it does not support the requested operation (e.g., a read-only store), +it should silently ignore the request. If a helper receives any other operation, it should silently ignore the request. This leaves room for future operations to be added (older From 1aed817f99bf259ea18ec49829b8d46b914e0230 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlo=20Marcelo=20Arenas=20Bel=C3=B3n?= <carenas@gmail.com> Date: Wed, 6 May 2020 14:47:26 -0700 Subject: [PATCH 4/4] credential: document protocol updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document protocol changes after CVE-2020-11008, including the removal of references to the override of attributes which is no longer recommended after CVE-2020-5260 and that might be removed in the future. While at it do some improvements for clarity and consistency. Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/git-credential.txt | 34 ++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt index 6f0c7ca80f..8d990e92fd 100644 --- a/Documentation/git-credential.txt +++ b/Documentation/git-credential.txt @@ -103,17 +103,20 @@ INPUT/OUTPUT FORMAT `git credential` reads and/or writes (depending on the action used) credential information in its standard input/output. This information can correspond either to keys for which `git credential` will obtain -the login/password information (e.g. host, protocol, path), or to the -actual credential data to be obtained (login/password). +the login information (e.g. host, protocol, path), or to the actual +credential data to be obtained (username/password). The credential is split into a set of named attributes, with one -attribute per line. Each attribute is -specified by a key-value pair, separated by an `=` (equals) sign, -followed by a newline. The key may contain any bytes except `=`, -newline, or NUL. The value may contain any bytes except newline or NUL. +attribute per line. Each attribute is specified by a key-value pair, +separated by an `=` (equals) sign, followed by a newline. + +The key may contain any bytes except `=`, newline, or NUL. The value may +contain any bytes except newline or NUL. + In both cases, all bytes are treated as-is (i.e., there is no quoting, and one cannot transmit a value with newline or NUL in it). The list of attributes is terminated by a blank line or end-of-file. + Git understands the following attributes: `protocol`:: @@ -123,7 +126,8 @@ Git understands the following attributes: `host`:: - The remote hostname for a network credential. + The remote hostname for a network credential. This includes + the port number if one was specified (e.g., "example.com:8088"). `path`:: @@ -134,7 +138,7 @@ Git understands the following attributes: `username`:: The credential's username, if we already have one (e.g., from a - URL, from the user, or from a previously run helper). + URL, the configuration, the user, or from a previously run helper). `password`:: @@ -146,8 +150,12 @@ Git understands the following attributes: value is parsed as a URL and treated as if its constituent parts were read (e.g., `url=https://example.com` would behave as if `protocol=https` and `host=example.com` had been provided). This - can help callers avoid parsing URLs themselves. Note that any - components which are missing from the URL (e.g., there is no - username in the example above) will be set to empty; if you want - to provide a URL and override some attributes, provide the URL - attribute first, followed by any overrides. + can help callers avoid parsing URLs themselves. + + Note that specifying a protocol is mandatory and if the URL + doesn't specify a hostname (e.g., "cert:///path/to/file") the + credential will contain a hostname attribute whose value is an + empty string. + + Components which are missing from the URL (e.g., there is no + username in the example above) will be left unset.