[v3] credential/libsecret: erase matching creds only

Commit Message

M Hickford July 26, 2023, 7:46 p.m. UTC

From: M Hickford <[e-mail]>

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

credential-libsecret erases a stored credential if it matches 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 (libsecret) does not erase a password distinct
from input" when t0303 is run with GIT_TEST_CREDENTIAL_HELPER set to
"libsecret". This test was added in aeb21ce22e (credential: avoid
erasing distinct password, 2023-06-13).

Signed-off-by: M Hickford <[e-mail]>
---
    credential/libsecret: erase more carefully
    
    Patch v3 adds instructions how to run the fixed test

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

Range-diff vs v2:

 1:  b5d5c3159ad ! 1:  6b3db416c61 credential/libsecret: erase matching creds only
     @@ Commit message
          Only erase credential if stored password matches request (or request
          omits password).
      
     -    This fixes test "helper ... does not erase a password distinct from
     -    input" introduced in aeb21ce22e (credential: avoid erasing distinct
     -    password, 2023-06-13)
     +    This fixes test "helper (libsecret) does not erase a password distinct
     +    from input" when t0303 is run with GIT_TEST_CREDENTIAL_HELPER set to
     +    "libsecret". This test was added in aeb21ce22e (credential: avoid
     +    erasing distinct password, 2023-06-13).
      
          Signed-off-by: M Hickford <[e-mail]>
      


 .../libsecret/git-credential-libsecret.c        | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)


base-commit: a80be152923a46f04a06bade7bcc72870e46ca09

Patch

diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index ef681f29d5b..9110714601e 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -52,6 +52,8 @@  struct credential_operation {
 
 #define CREDENTIAL_OP_END { NULL, NULL }
 
+static void credential_clear(struct credential *c);
+
 /* ----------------- Secret Service functions ----------------- */
 
 static char *make_label(struct credential *c)
@@ -185,6 +187,7 @@  static int keyring_erase(struct credential *c)
 {
 	GHashTable *attributes = NULL;
 	GError *error = NULL;
+	struct credential existing = CREDENTIAL_INIT;
 
 	/*
 	 * Sanity check that we actually have something to match
@@ -197,6 +200,20 @@  static int keyring_erase(struct credential *c)
 	if (!c->protocol && !c->host && !c->path && !c->username)
 		return EXIT_FAILURE;
 
+	if (c->password) {
+		existing.host = g_strdup(c->host);
+		existing.path = g_strdup(c->path);
+		existing.port = c->port;
+		existing.protocol = g_strdup(c->protocol);
+		existing.username = g_strdup(c->username);
+		keyring_get(&existing);
+		if (existing.password && strcmp(c->password, existing.password)) {
+			credential_clear(&existing);
+			return EXIT_SUCCESS;
+		}
+		credential_clear(&existing);
+	}
+
 	attributes = make_attr_list(c);
 	secret_password_clearv_sync(SECRET_SCHEMA_COMPAT_NETWORK,
 				    attributes,