credential/wincred: store oauth_refresh_token
PATCH 1/1 credential/wincred: store oauth_refresh_token
From: M Hickford <[email protected]>
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.
---
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/[email protected]/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(-)
contrib/credential/wincred/git-credential-wincred.c +34 -6
@@ -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
t/t0303-credential-external.sh +1 -0
@@ -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
Signature · 1 line
gitgitgadget
Quoted 157 lines · M Hickford via GitGitGadget
From: M Hickford <[email protected]>
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=xyzzyThis is extensible and backwards compatible. The credential protocol already assumes that attribute values do not contain newlines.
Alternatives considered: store
oauth_refresh_tokenin a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.Signed-off-by M Hickford <[email protected]>---
credential/wincred: storeoauth_refresh_tokenI'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/[email protected]/T/#uPublished-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(-)contrib/credential/wincred/git-credential-wincred.c +34
-6@@ -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 t/t0303-credential-external.sh +1
-0@@ -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)"
Quoted 3 lines
base-commit:
9e49351c3060e1fa6e0d2de64505b7becf157f28--
gitgitgadget
Hi. Is anyone familiar enough with Windows CRT to review this patch?
A message that is not in the archive (sent off-list or not yet received): CAGJzqskekExeug45Zh-uQ1dpHpKP8HS3obWQyz8B_DzSZiuAwA@mail.gmail.com
Hi. Is anyone with Windows development experience able to take a look at this patch to credential-wincred?
https://lore.kernel.org/git/[email protected]/
https://github.com/gitgitgadget/git/pull/1534
Quoted 167 lines · M Hickford
Hi. A small patch to git-credential-wincred needs review on the Git
mailing list https://lore.kernel.org/git/[email protected]/
. Does anyone here have the expertise to review?Quoted 160 lines · M Hickford via GitGitGadget
From: M Hickford <[email protected]>
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=xyzzyThis is extensible and backwards compatible. The credential protocol already assumes that attribute values do not contain newlines.
Alternatives considered: store
oauth_refresh_tokenin a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.Signed-off-by M Hickford <[email protected]>---
credential/wincred: storeoauth_refresh_tokenI'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/[email protected]/T/#uPublished-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(-)contrib/credential/wincred/git-credential-wincred.c +34
-6@@ -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 t/t0303-credential-external.sh +1
-0@@ -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
PATCH v2 1/1 credential/wincred: store oauth_refresh_token
In reply to M Hickford via GitGitGadgetFrom: M Hickford <[email protected]>
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.
This fixes test "helper (wincred) gets oauth_refresh_token" when t0303-credential-external.sh is run with GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7 (credential: new attribute oauth_refresh_token, 2023-04-21).
Alternatives considered: store oauth_refresh_token in a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.
---
credential/wincred: store oauth_refresh_token
Patch v2 fixes a bug when password is empty.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1534
Range-diff vs v1:
1: 46ce22aa3a3 ! 1: 12f092f45b1 credential/wincred: store oauth_refresh_token
@@ Commit message
This is extensible and backwards compatible. The credential protocol
already assumes that attribute values do not contain newlines.
+ This fixes test "helper (wincred) gets oauth_refresh_token" when
+ t0303-credential-external.sh is run with
+ GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7
+ (credential: new attribute oauth_refresh_token, 2023-04-21).
+
Alternatives considered: store oauth_refresh_token in a wincred
attribute. This would be insecure because wincred assumes attribute
values to be non-confidential.
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(
if (!CredEnumerateW(L"git:*", 0, &num_creds, &creds))
return;
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(void)
- 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",
- (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));
++ if (creds[i]->CredentialBlobSize > 0) {
++ 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, line ? wcslen(line) : 0);
++ while(line != NULL) {
++ part = wcstok_s(line, L"=", &remaining_parts);
++ if (!wcscmp(part, L"oauth_refresh_token")) {
++ write_item("oauth_refresh_token", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);
++ }
++ line = wcstok_s(NULL, L"\r\n", &remaining_lines);
+ }
-+ line = wcstok_s(NULL, L"\r\n", &remaining_lines);
++ } else {
++ write_item("password",
++ (LPCWSTR)creds[i]->CredentialBlob,
++ creds[i]->CredentialBlobSize / sizeof(WCHAR));
+ }
for (int j = 0; j < creds[i]->AttributeCount; j++) {
attr = creds[i]->Attributes + j;
@@ contrib/credential/wincred/git-credential-wincred.c: static void read_credential
/*
* Ignore other lines; we don't know what they mean, but
* this future-proofs us when later versions of git do
-
- ## t/t0303-credential-external.sh ##
-@@ t/t0303-credential-external.sh: 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)".../wincred/git-credential-wincred.c | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-)
contrib/credential/wincred/git-credential-wincred.c +40 -6
@@ -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) {
@@ -140,6 +140,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 ;
@@ -149,9 +154,23 @@ static void get_credential(void)if (match_cred (creds[i],0 )) {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 )); +if (creds[i]->CredentialBlobSize >0 ) { + 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, line ?wcslen (line) :0 ); +while (line !=NULL ) { + part =wcstok_s (line,L"=" , &remaining_parts); +if (!wcscmp (part,L"oauth_refresh_token" )) { +write_item ("oauth_refresh_token" , remaining_parts, remaining_parts ?wcslen (remaining_parts) :0 ); + } + line =wcstok_s (NULL ,L"\r\n" , &remaining_lines); + } + }else { +write_item ("password" , + (LPCWSTR )creds[i]->CredentialBlob, + creds[i]->CredentialBlobSize /sizeof (WCHAR )); + }for (int j =0 ; j < creds[i]->AttributeCount; j++) { attr = creds[i]->Attributes + j;if (!wcscmp (attr->Keyword,L"git_password_expiry_utc" )) {
@@ -160,6 +179,7 @@ static void get_credential(void)break ; } } +free (secret);break ; }
@@ -170,16 +190,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 ;
@@ -194,6 +224,8 @@ static void store_credential(void) cred.TargetAlias =NULL ; cred.UserName = wusername; +free (secret); +if (!CredWriteW (&cred,0 ))die ("CredWrite failed" ); }
@@ -265,6 +297,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
base-commit: bc5204569f7db44d22477485afd52ea410d83743
Signature · 1 line
gitgitgadget
Quoted 14 lines · M Hickford via GitGitGadget
From: M Hickford <[email protected]>
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=xyzzyThis is extensible and backwards compatible. The credential protocol already assumes that attribute values do not contain newlines.
Quoted 8 lines
This fixes test "helper (wincred) gets
oauth_refresh_token" whent0303-credential-external.shis run with GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added ina5c76569e7(credential: new attributeoauth_refresh_token, 2023-04-21).Alternatives considered: store
oauth_refresh_tokenin a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.
Earlier, a5c76569 (credential: new attribute oauth_refresh_token, 2023-04-21) built the "git" side support for the token, and taught credential-cache to store the necessary information. Then 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16) was written for libsecret to support the same interface.
And this one adds corresponding support for wincred. Do I understand what is going on around this patch correctly?
I do not do Windows, but some people on this list certainly do and would be capable of giving the patch a necessary nudge ;-)
Thanks.
Quoted 112 lines
.../wincred/git-credential-wincred.c | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-)contrib/credential/wincred/git-credential-wincred.c +40
-6@@ -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) {@@ -140,6 +140,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 ;@@ -149,9 +154,23 @@ static void get_credential(void)if (match_cred (creds[i],0 )) {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 )); +if (creds[i]->CredentialBlobSize >0 ) { + 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, line ?wcslen (line) :0 ); +while (line !=NULL ) { + part =wcstok_s (line,L"=" , &remaining_parts); +if (!wcscmp (part,L"oauth_refresh_token" )) { +write_item ("oauth_refresh_token" , remaining_parts, remaining_parts ?wcslen (remaining_parts) :0 ); + } + line =wcstok_s (NULL ,L"\r\n" , &remaining_lines); + } + }else { +write_item ("password" , + (LPCWSTR )creds[i]->CredentialBlob, + creds[i]->CredentialBlobSize /sizeof (WCHAR )); + }for (int j =0 ; j < creds[i]->AttributeCount; j++) { attr = creds[i]->Attributes + j;if (!wcscmp (attr->Keyword,L"git_password_expiry_utc" )) {@@ -160,6 +179,7 @@ static void get_credential(void)break ; } } +free (secret);break ; }@@ -170,16 +190,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 ;@@ -194,6 +224,8 @@ static void store_credential(void) cred.TargetAlias =NULL ; cred.UserName = wusername; +free (secret); +if (!CredWriteW (&cred,0 ))die ("CredWrite failed" ); }@@ -265,6 +297,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 base-commit:
bc5204569f7db44d22477485afd52ea410d83743
Quoted 26 lines · Junio C Hamano
Quoted 14 lines · M Hickford via GitGitGadget
From: M Hickford <[email protected]>
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=xyzzyThis is extensible and backwards compatible. The credential protocol already assumes that attribute values do not contain newlines.
Quoted 8 lines
This fixes test "helper (wincred) gets
oauth_refresh_token" whent0303-credential-external.shis run with GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added ina5c76569e7(credential: new attributeoauth_refresh_token, 2023-04-21).Alternatives considered: store
oauth_refresh_tokenin a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.
Quoted 8 lines
Earlier,
a5c76569(credential: new attributeoauth_refresh_token, 2023-04-21) built the "git" side support for the token, and taught credential-cache to store the necessary information. Then0ce02e2f(credential/libsecret: store new attributes, 2023-06-16) was written for libsecret to support the same interface.And this one adds corresponding support for wincred. Do I understand what is going on around this patch correctly?
Yes, that's right.
Quoted 118 lines
I do not do Windows, but some people on this list certainly do and would be capable of giving the patch a necessary nudge ;-)
Thanks.
Quoted 112 lines
.../wincred/git-credential-wincred.c | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-)contrib/credential/wincred/git-credential-wincred.c +40
-6@@ -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) {@@ -140,6 +140,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 ;@@ -149,9 +154,23 @@ static void get_credential(void)if (match_cred (creds[i],0 )) {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 )); +if (creds[i]->CredentialBlobSize >0 ) { + 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, line ?wcslen (line) :0 ); +while (line !=NULL ) { + part =wcstok_s (line,L"=" , &remaining_parts); +if (!wcscmp (part,L"oauth_refresh_token" )) { +write_item ("oauth_refresh_token" , remaining_parts, remaining_parts ?wcslen (remaining_parts) :0 ); + } + line =wcstok_s (NULL ,L"\r\n" , &remaining_lines); + } + }else { +write_item ("password" , + (LPCWSTR )creds[i]->CredentialBlob, + creds[i]->CredentialBlobSize /sizeof (WCHAR )); + }for (int j =0 ; j < creds[i]->AttributeCount; j++) { attr = creds[i]->Attributes + j;if (!wcscmp (attr->Keyword,L"git_password_expiry_utc" )) {@@ -160,6 +179,7 @@ static void get_credential(void)break ; } } +free (secret);break ; }@@ -170,16 +190,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 ;@@ -194,6 +224,8 @@ static void store_credential(void) cred.TargetAlias =NULL ; cred.UserName = wusername; +free (secret); +if (!CredWriteW (&cred,0 ))die ("CredWrite failed" ); }@@ -265,6 +297,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 base-commit:
bc5204569f7db44d22477485afd52ea410d83743
PATCH v3 1/1 credential/wincred: store oauth_refresh_token
In reply to M Hickford via GitGitGadgetFrom: M Hickford <[email protected]>
a5c7656 (credential: new attribute oauth_refresh_token) introduced a new confidential credential attribute and added support to credential-cache. Later 0ce02e2f (credential/libsecret: store new attributes, 2023-06-16) added support in credential-libsecret.
To add support in credential-wincred, 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.
This fixes test "helper (wincred) gets oauth_refresh_token" when t0303-credential-external.sh is run with GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7 (credential: new attribute oauth_refresh_token, 2023-04-21).
Alternatives considered: store oauth_refresh_token in a wincred attribute. This would be insecure because wincred assumes attribute values to be non-confidential.
---
credential/wincred: store oauth_refresh_token
Patch v3 fixes a maybe-uninitialized warning.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/1534
Range-diff vs v2:
1: 12f092f45b1 ! 1: a324c3aac41 credential/wincred: store oauth_refresh_token
@@ Commit message
credential/wincred: store oauth_refresh_token
a5c7656 (credential: new attribute oauth_refresh_token) introduced
- a new confidential credential attribute for helpers to store.
+ a new confidential credential attribute and added support to
+ credential-cache. Later 0ce02e2f (credential/libsecret: store new
+ attributes, 2023-06-16) added support in credential-libsecret.
- We encode the new attribute in the CredentialBlob, separated by
- newline:
+ To add support in credential-wincred, we encode the new attribute in the
+ CredentialBlob, separated by newline:
hunter2
oauth_refresh_token=xyzzy
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(
+ }
+ line = wcstok_s(NULL, L"\r\n", &remaining_lines);
+ }
++ free(secret);
+ } else {
+ write_item("password",
+ (LPCWSTR)creds[i]->CredentialBlob,
@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(
for (int j = 0; j < creds[i]->AttributeCount; j++) {
attr = creds[i]->Attributes + j;
if (!wcscmp(attr->Keyword, L"git_password_expiry_utc")) {
-@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(void)
- break;
- }
- }
-+ free(secret);
- break;
- }
-
@@ contrib/credential/wincred/git-credential-wincred.c: static void store_credential(void)
{
CREDENTIALW cred;
.../wincred/git-credential-wincred.c | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-)
contrib/credential/wincred/git-credential-wincred.c +40 -6
@@ -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) {
@@ -140,6 +140,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 ;
@@ -149,9 +154,24 @@ static void get_credential(void)if (match_cred (creds[i],0 )) {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 )); +if (creds[i]->CredentialBlobSize >0 ) { + 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, line ?wcslen (line) :0 ); +while (line !=NULL ) { + part =wcstok_s (line,L"=" , &remaining_parts); +if (!wcscmp (part,L"oauth_refresh_token" )) { +write_item ("oauth_refresh_token" , remaining_parts, remaining_parts ?wcslen (remaining_parts) :0 ); + } + line =wcstok_s (NULL ,L"\r\n" , &remaining_lines); + } +free (secret); + }else { +write_item ("password" , + (LPCWSTR )creds[i]->CredentialBlob, + creds[i]->CredentialBlobSize /sizeof (WCHAR )); + }for (int j =0 ; j < creds[i]->AttributeCount; j++) { attr = creds[i]->Attributes + j;if (!wcscmp (attr->Keyword,L"git_password_expiry_utc" )) {
@@ -170,16 +190,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 ;
@@ -194,6 +224,8 @@ static void store_credential(void) cred.TargetAlias =NULL ; cred.UserName = wusername; +free (secret); +if (!CredWriteW (&cred,0 ))die ("CredWrite failed" ); }
@@ -265,6 +297,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
base-commit: b50a608ba20348cb3dfc16a696816d51780e3f0f
Signature · 1 line
gitgitgadget