Git Mailing List Archive mirror
 help / color / mirror / Atom feed
* [PATCH] credential/wincred: erase matching creds only
@ 2023-06-24  8:52 M Hickford via GitGitGadget
  2023-07-26 19:51 ` [PATCH v2] " M Hickford via GitGitGadget
  0 siblings, 1 reply; 2+ messages in thread
From: M Hickford via GitGitGadget @ 2023-06-24  8:52 UTC (permalink / raw
  To: git; +Cc: Jeff King, Johannes Sixt, M Hickford, M Hickford

From: M Hickford <mirth.hickford@gmail.com>

Fix test "helper ... does not erase a password distinct from input"
introduced in aeb21ce22e (credential: avoid erasing distinct password,
2023-06-13)

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    credential/wincred: erase more carefully

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1529%2Fhickford%2Ffix-wincred-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1529/hickford/fix-wincred-v1
Pull-Request: https://github.com/git/git/pull/1529

 .../wincred/git-credential-wincred.c          | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 96f10613aee..4cd56c42e24 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -109,7 +109,18 @@ static int match_part_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
 	return match_part_with_last(ptarget, want, delim, 1);
 }
 
-static int match_cred(const CREDENTIALW *cred)
+static int match_cred_password(const CREDENTIALW *cred) {
+	int ret;
+	WCHAR *cred_password = xmalloc(cred->CredentialBlobSize);
+	wcsncpy_s(cred_password, cred->CredentialBlobSize,
+		(LPCWSTR)cred->CredentialBlob,
+		cred->CredentialBlobSize / sizeof(WCHAR));
+	ret = !wcscmp(cred_password, password);
+	free(cred_password);
+	return ret;
+}
+
+static int match_cred(const CREDENTIALW *cred, int match_password)
 {
 	LPCWSTR target = cred->TargetName;
 	if (wusername && wcscmp(wusername, cred->UserName ? cred->UserName : L""))
@@ -119,7 +130,8 @@ static int match_cred(const CREDENTIALW *cred)
 		match_part(&target, protocol, L"://") &&
 		match_part_last(&target, wusername, L"@") &&
 		match_part(&target, host, L"/") &&
-		match_part(&target, path, L"");
+		match_part(&target, path, L"") &&
+		(!match_password || match_cred_password(cred));
 }
 
 static void get_credential(void)
@@ -134,7 +146,7 @@ static void get_credential(void)
 
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
-		if (match_cred(creds[i])) {
+		if (match_cred(creds[i], 0)) {
 			write_item("username", creds[i]->UserName,
 				creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);
 			write_item("password",
@@ -196,7 +208,7 @@ static void erase_credential(void)
 		return;
 
 	for (i = 0; i < num_creds; ++i) {
-		if (match_cred(creds[i]))
+		if (match_cred(creds[i], password != NULL))
 			CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);
 	}
 

base-commit: 6ff334181cfb6485d3ba50843038209a2a253907
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH v2] credential/wincred: erase matching creds only
  2023-06-24  8:52 [PATCH] credential/wincred: erase matching creds only M Hickford via GitGitGadget
@ 2023-07-26 19:51 ` M Hickford via GitGitGadget
  0 siblings, 0 replies; 2+ messages in thread
From: M Hickford via GitGitGadget @ 2023-07-26 19:51 UTC (permalink / raw
  To: git; +Cc: Jeff King, Johannes Sixt, Glen Choo, M Hickford, M Hickford

From: M Hickford <mirth.hickford@gmail.com>

The credential erase request typically includes protocol, host, username
and password.

credential-wincred erases stored credentials that match protocol,
host and username, regardless of password.

This is confusing in the case the stored password differs from that
in the request. This case can occur when multiple credential helpers are
configured.

Only erase credential if stored password matches request (or request
omits password).

This fixes test "helper (wincred) does not erase a password distinct
from input" when t0303 is run with GIT_TEST_CREDENTIAL_HELPER set to
"wincred". This test was added in aeb21ce22e (credential: avoid
erasing distinct password, 2023-06-13).

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    credential/wincred: erase more carefully
    
    Patch v2 expands the commit message

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1529%2Fhickford%2Ffix-wincred-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1529/hickford/fix-wincred-v2
Pull-Request: https://github.com/git/git/pull/1529

Range-diff vs v1:

 1:  d45c61d8f79 ! 1:  5e2aae38f72 credential/wincred: erase matching creds only
     @@ Metadata
       ## Commit message ##
          credential/wincred: erase matching creds only
      
     -    Fix test "helper ... does not erase a password distinct from input"
     -    introduced in aeb21ce22e (credential: avoid erasing distinct password,
     -    2023-06-13)
     +    The credential erase request typically includes protocol, host, username
     +    and password.
     +
     +    credential-wincred erases stored credentials that match protocol,
     +    host and username, regardless of password.
     +
     +    This is confusing in the case the stored password differs from that
     +    in the request. This case can occur when multiple credential helpers are
     +    configured.
     +
     +    Only erase credential if stored password matches request (or request
     +    omits password).
     +
     +    This fixes test "helper (wincred) does not erase a password distinct
     +    from input" when t0303 is run with GIT_TEST_CREDENTIAL_HELPER set to
     +    "wincred". This test was added in aeb21ce22e (credential: avoid
     +    erasing distinct password, 2023-06-13).
      
          Signed-off-by: M Hickford <mirth.hickford@gmail.com>
      


 .../wincred/git-credential-wincred.c          | 20 +++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 96f10613aee..4cd56c42e24 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -109,7 +109,18 @@ static int match_part_last(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
 	return match_part_with_last(ptarget, want, delim, 1);
 }
 
-static int match_cred(const CREDENTIALW *cred)
+static int match_cred_password(const CREDENTIALW *cred) {
+	int ret;
+	WCHAR *cred_password = xmalloc(cred->CredentialBlobSize);
+	wcsncpy_s(cred_password, cred->CredentialBlobSize,
+		(LPCWSTR)cred->CredentialBlob,
+		cred->CredentialBlobSize / sizeof(WCHAR));
+	ret = !wcscmp(cred_password, password);
+	free(cred_password);
+	return ret;
+}
+
+static int match_cred(const CREDENTIALW *cred, int match_password)
 {
 	LPCWSTR target = cred->TargetName;
 	if (wusername && wcscmp(wusername, cred->UserName ? cred->UserName : L""))
@@ -119,7 +130,8 @@ static int match_cred(const CREDENTIALW *cred)
 		match_part(&target, protocol, L"://") &&
 		match_part_last(&target, wusername, L"@") &&
 		match_part(&target, host, L"/") &&
-		match_part(&target, path, L"");
+		match_part(&target, path, L"") &&
+		(!match_password || match_cred_password(cred));
 }
 
 static void get_credential(void)
@@ -134,7 +146,7 @@ static void get_credential(void)
 
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
-		if (match_cred(creds[i])) {
+		if (match_cred(creds[i], 0)) {
 			write_item("username", creds[i]->UserName,
 				creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);
 			write_item("password",
@@ -196,7 +208,7 @@ static void erase_credential(void)
 		return;
 
 	for (i = 0; i < num_creds; ++i) {
-		if (match_cred(creds[i]))
+		if (match_cred(creds[i], password != NULL))
 			CredDeleteW(creds[i]->TargetName, creds[i]->Type, 0);
 	}
 

base-commit: a80be152923a46f04a06bade7bcc72870e46ca09
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-07-26 19:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-24  8:52 [PATCH] credential/wincred: erase matching creds only M Hickford via GitGitGadget
2023-07-26 19:51 ` [PATCH v2] " M Hickford via GitGitGadget

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).