threads / patch / 60951

patchlibsecret: retrieve empty password

Subject: [PATCH] libsecret: retrieve empty password

## tl;dr

4 messages between Feb 18, 2024 and Feb 19, 2024. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

M Hickford via GitGitGadget· Feb 18, 2024, 22:51 UTC · lore

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

Since 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16) a test that stores empty username and password fails when t0303-credential-external.sh is run with GIT_TEST_CREDENTIAL_HELPER=libsecret.

Retrieve empty password carefully. This fixes test:

    ok 14 - helper (libsecret) can store empty username

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    libsecret: retrieve empty password

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1676%2Fhickford%2Flibsecret-empty-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1676/hickford/libsecret-empty-v1
Pull-Request: https://github.com/git/git/pull/1676

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

Show changes to contrib/credential/libsecret/git-credential-libsecret.c +3 −0

diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index 215a81d8bae..d9e9e4fd524 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -164,6 +164,9 @@ static int keyring_get(struct credential *c)
 			if (g_strv_length(parts) >= 1) {
 				g_free(c->password);
 				c->password = g_strdup(parts[0]);
+			} else {
+				g_free(c->password);
+				c->password = strdup("");
 			}
 			for (int i = 1; i < g_strv_length(parts); i++) {
 				if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {

base-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b

-- 
gitgitgadget

Patrick Steinhardt· Feb 19, 2024, 06:08 UTC · re: M Hickford via GitGitGadget · lore

Re: [PATCH] libsecret: retrieve empty password

On Sun, Feb 18, 2024 at 10:51:34PM +0000, M Hickford via GitGitGadget wrote:

Show 33 quoted lines

> From: M Hickford <mirth.hickford@gmail.com>
> 
> Since 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16)
> a test that stores empty username and password fails when
> t0303-credential-external.sh is run with
> GIT_TEST_CREDENTIAL_HELPER=libsecret.
> 
> Retrieve empty password carefully. This fixes test:
> 
>     ok 14 - helper (libsecret) can store empty username
> 
> Signed-off-by: M Hickford <mirth.hickford@gmail.com>
> ---
>     libsecret: retrieve empty password
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1676%2Fhickford%2Flibsecret-empty-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1676/hickford/libsecret-empty-v1
> Pull-Request: https://github.com/git/git/pull/1676
> 
>  contrib/credential/libsecret/git-credential-libsecret.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
> index 215a81d8bae..d9e9e4fd524 100644
> --- a/contrib/credential/libsecret/git-credential-libsecret.c
> +++ b/contrib/credential/libsecret/git-credential-libsecret.c
> @@ -164,6 +164,9 @@ static int keyring_get(struct credential *c)
>  			if (g_strv_length(parts) >= 1) {
>  				g_free(c->password);
>  				c->password = g_strdup(parts[0]);
> +			} else {
> +				g_free(c->password);
> +				c->password = strdup("");

Shouldn't we use `g_strdup()` here, like we do everywhere else in this credential helper?

Patrick

Show 8 quoted lines

>  			}
>  			for (int i = 1; i < g_strv_length(parts); i++) {
>  				if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {
> 
> base-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b
> -- 
> gitgitgadget
>

M Hickford· Feb 19, 2024, 20:00 UTC · re: Patrick Steinhardt · lore

Re: [PATCH] libsecret: retrieve empty password

Show 5 quoted lines

> > +                             g_free(c->password);
> > +                             c->password = strdup("");
>
> Shouldn't we use `g_strdup()` here, like we do everywhere else in this
> credential helper?

You're right. I'll correct in patch v2.

M Hickford via GitGitGadget· Feb 19, 2024, 20:40 UTC · re: M Hickford via GitGitGadget · lore

[PATCH v2] libsecret: retrieve empty password

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

Retrieve empty password carefully. This fixes test:

    ok 14 - helper (libsecret) can store empty username

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    libsecret: retrieve empty password
    
    cc: Patrick Steinhardt ps@pks.im

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1676%2Fhickford%2Flibsecret-empty-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1676/hickford/libsecret-empty-v2
Pull-Request: https://github.com/git/git/pull/1676

Range-diff vs v1:

 1:  877bbfb72ed ! 1:  2cdcba20622 libsecret: retrieve empty password
     @@ contrib/credential/libsecret/git-credential-libsecret.c: static int keyring_get(
       				c->password = g_strdup(parts[0]);
      +			} else {
      +				g_free(c->password);
     -+				c->password = strdup("");
     ++				c->password = g_strdup("");
       			}
       			for (int i = 1; i < g_strv_length(parts); i++) {
       				if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {

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

Show changes to contrib/credential/libsecret/git-credential-libsecret.c +3 −0

diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index 215a81d8bae..90034d0cf1e 100644
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -164,6 +164,9 @@ static int keyring_get(struct credential *c)
 			if (g_strv_length(parts) >= 1) {
 				g_free(c->password);
 				c->password = g_strdup(parts[0]);
+			} else {
+				g_free(c->password);
+				c->password = g_strdup("");
 			}
 			for (int i = 1; i < g_strv_length(parts); i++) {
 				if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {

base-commit: 3e0d3cd5c7def4808247caf168e17f2bbf47892b

-- 
gitgitgadget