All of lore.kernel.org
 help / color / mirror / Atom feed
From: "M Hickford via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: M Hickford <mirth.hickford@gmail.com>,
	M Hickford <mirth.hickford@gmail.com>
Subject: [PATCH] credential/wincred: store oauth_refresh_token
Date: Sat, 20 May 2023 07:20:47 +0000	[thread overview]
Message-ID: <pull.1534.git.1684567247339.gitgitgadget@gmail.com> (raw)

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

a5c7656 (credential: new attribute oauth_refresh_token) introduced
a new confidential credential attribute for helpers to store.

We encode the new attribute in the CredentialBlob, separated by
newline:

    hunter2
    oauth_refresh_token=xyzzy

This is extensible and backwards compatible. The credential protocol
already assumes that attribute values do not contain newlines.

Alternatives considered: store oauth_refresh_token in a wincred
attribute. This would be insecure because wincred assumes attribute
values to be non-confidential.

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
---
    credential/wincred: store oauth_refresh_token
    
    I'm not confident in C and found string manipulation difficult, so
    please review carefully.
    
    I tested on Linux, cross compiling with Zig make CC="zig cc -target
    x86_64-windows-gnu" and tested make GIT_TEST_CREDENTIAL_HELPER=wincred
    t0303-credential-external.sh (with a shell script git-credential-wincred
    that wraps wine64 git-credential-wincred.exe "$@")
    
    See also similar patch for credential-libsecret "[PATCH v4]
    credential/libsecret: store new attributes"
    https://lore.kernel.org/git/pull.1469.v4.git.git.1684306540947.gitgitgadget@gmail.com/T/#u

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

 .../wincred/git-credential-wincred.c          | 40 ++++++++++++++++---
 t/t0303-credential-external.sh                |  1 +
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 96f10613aee..dc34283b85e 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -35,7 +35,7 @@ static void *xmalloc(size_t size)
 }
 
 static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],
-	*password_expiry_utc;
+	*password_expiry_utc, *oauth_refresh_token;
 
 static void write_item(const char *what, LPCWSTR wbuf, int wlen)
 {
@@ -128,6 +128,11 @@ static void get_credential(void)
 	DWORD num_creds;
 	int i;
 	CREDENTIAL_ATTRIBUTEW *attr;
+	WCHAR *secret;
+	WCHAR *line;
+	WCHAR *remaining_lines;
+	WCHAR *part;
+	WCHAR *remaining_parts;
 
 	if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
 		return;
@@ -137,9 +142,17 @@ static void get_credential(void)
 		if (match_cred(creds[i])) {
 			write_item("username", creds[i]->UserName,
 				creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);
-			write_item("password",
-				(LPCWSTR)creds[i]->CredentialBlob,
-				creds[i]->CredentialBlobSize / sizeof(WCHAR));
+			secret = xmalloc(creds[i]->CredentialBlobSize);
+			wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));
+			line = wcstok_s(secret, L"\r\n", &remaining_lines);
+			write_item("password", line, wcslen(line));
+			while(line != NULL) {
+				part = wcstok_s(line, L"=", &remaining_parts);
+				if (!wcscmp(part, L"oauth_refresh_token")) {
+					write_item("oauth_refresh_token", remaining_parts, wcslen(remaining_parts));
+				}
+				line = wcstok_s(NULL, L"\r\n", &remaining_lines);
+			}
 			for (int j = 0; j < creds[i]->AttributeCount; j++) {
 				attr = creds[i]->Attributes + j;
 				if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) {
@@ -148,6 +161,7 @@ static void get_credential(void)
 					break;
 				}
 			}
+			free(secret);
 			break;
 		}
 
@@ -158,16 +172,26 @@ static void store_credential(void)
 {
 	CREDENTIALW cred;
 	CREDENTIAL_ATTRIBUTEW expiry_attr;
+	WCHAR *secret;
+	int wlen;
 
 	if (!wusername || !password)
 		return;
 
+	if (oauth_refresh_token) {
+		wlen = _scwprintf(L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token);
+		secret = xmalloc(sizeof(WCHAR) * wlen);
+		_snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L"%s\r\noauth_refresh_token=%s", password, oauth_refresh_token);
+	} else {
+		secret = _wcsdup(password);
+	}
+
 	cred.Flags = 0;
 	cred.Type = CRED_TYPE_GENERIC;
 	cred.TargetName = target;
 	cred.Comment = L"saved by git-credential-wincred";
-	cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);
-	cred.CredentialBlob = (LPVOID)password;
+	cred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);
+	cred.CredentialBlob = (LPVOID)_wcsdup(secret);
 	cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
 	cred.AttributeCount = 0;
 	cred.Attributes = NULL;
@@ -182,6 +206,8 @@ static void store_credential(void)
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;
 
+	free(secret);
+
 	if (!CredWriteW(&cred, 0))
 		die("CredWrite failed");
 }
@@ -253,6 +279,8 @@ static void read_credential(void)
 			password = utf8_to_utf16_dup(v);
 		else if (!strcmp(buf, "password_expiry_utc"))
 			password_expiry_utc = utf8_to_utf16_dup(v);
+		else if (!strcmp(buf, "oauth_refresh_token"))
+			oauth_refresh_token = utf8_to_utf16_dup(v);
 		/*
 		 * Ignore other lines; we don't know what they mean, but
 		 * this future-proofs us when later versions of git do
diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh
index f028fd14182..51886b8e259 100755
--- a/t/t0303-credential-external.sh
+++ b/t/t0303-credential-external.sh
@@ -45,6 +45,7 @@ test -z "$GIT_TEST_CREDENTIAL_HELPER_SETUP" ||
 helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER"
 
 helper_test "$GIT_TEST_CREDENTIAL_HELPER"
+helper_test_oauth_refresh_token "$GIT_TEST_CREDENTIAL_HELPER"
 
 if test -z "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"; then
 	say "# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)"

base-commit: 9e49351c3060e1fa6e0d2de64505b7becf157f28
-- 
gitgitgadget

             reply	other threads:[~2023-05-20  7:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-20  7:20 M Hickford via GitGitGadget [this message]
2023-06-16 22:30 ` [PATCH] credential/wincred: store oauth_refresh_token M Hickford
     [not found] ` <CAGJzqskekExeug45Zh-uQ1dpHpKP8HS3obWQyz8B_DzSZiuAwA@mail.gmail.com>
2023-10-30  8:00   ` M Hickford
2023-11-06  6:16 ` [PATCH v2] " M Hickford via GitGitGadget
2023-11-14  7:40   ` Junio C Hamano
2023-11-24  8:00     ` M Hickford
2024-01-28 21:25   ` [PATCH v3] " M Hickford via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1534.git.1684567247339.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mirth.hickford@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.