{"thread":{"id":"59775","subject":"[PATCH] credential/wincred: store oauth_refresh_token","startedAt":"2023-05-20T07:20:54Z","lastAt":"2024-01-28T21:25:06Z","messageCount":7,"participants":["M Hickford via GitGitGadget","M Hickford","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"477612","messageId":"pull.1534.git.1684567247339.gitgitgadget@gmail.com","threadId":"59775","inReplyTo":null,"subject":"[PATCH] credential/wincred: store oauth_refresh_token","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-05-20T07:20:47Z","receivedAt":"2023-05-20T07:20:54Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\na5c7656 (credential: new attribute oauth_refresh_token) introduced\na new confidential credential attribute for helpers to store.\n\nWe encode the new attribute in the CredentialBlob, separated by\nnewline:\n\n    hunter2\n    oauth_refresh_token=xyzzy\n\nThis is extensible and backwards compatible. The credential protocol\nalready assumes that attribute values do not contain newlines.\n\nAlternatives considered: store oauth_refresh_token in a wincred\nattribute. This would be insecure because wincred assumes attribute\nvalues to be non-confidential.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store oauth_refresh_token\n    \n    I'm not confident in C and found string manipulation difficult, so\n    please review carefully.\n    \n    I tested on Linux, cross compiling with Zig make CC=\"zig cc -target\n    x86_64-windows-gnu\" and tested make GIT_TEST_CREDENTIAL_HELPER=wincred\n    t0303-credential-external.sh (with a shell script git-credential-wincred\n    that wraps wine64 git-credential-wincred.exe \"$@\")\n    \n    See also similar patch for credential-libsecret \"[PATCH v4]\n    credential/libsecret: store new attributes\"\n    https://lore.kernel.org/git/pull.1469.v4.git.git.1684306540947.gitgitgadget@gmail.com/T/#u\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1534\n\n .../wincred/git-credential-wincred.c          | 40 ++++++++++++++++---\n t/t0303-credential-external.sh                |  1 +\n 2 files changed, 35 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex 96f10613aee..dc34283b85e 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n }\n \n static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n-\t*password_expiry_utc;\n+\t*password_expiry_utc, *oauth_refresh_token;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -128,6 +128,11 @@ static void get_credential(void)\n \tDWORD num_creds;\n \tint i;\n \tCREDENTIAL_ATTRIBUTEW *attr;\n+\tWCHAR *secret;\n+\tWCHAR *line;\n+\tWCHAR *remaining_lines;\n+\tWCHAR *part;\n+\tWCHAR *remaining_parts;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -137,9 +142,17 @@ static void get_credential(void)\n \t\tif (match_cred(creds[i])) {\n \t\t\twrite_item(\"username\", creds[i]->UserName,\n \t\t\t\tcreds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n-\t\t\twrite_item(\"password\",\n-\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n-\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n+\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n+\t\t\twrite_item(\"password\", line, wcslen(line));\n+\t\t\twhile(line != NULL) {\n+\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n+\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n+\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, wcslen(remaining_parts));\n+\t\t\t\t}\n+\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n+\t\t\t}\n \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n \t\t\t\tattr = creds[i]->Attributes + j;\n \t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n@@ -148,6 +161,7 @@ static void get_credential(void)\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t}\n+\t\t\tfree(secret);\n \t\t\tbreak;\n \t\t}\n \n@@ -158,16 +172,26 @@ static void store_credential(void)\n {\n \tCREDENTIALW cred;\n \tCREDENTIAL_ATTRIBUTEW expiry_attr;\n+\tWCHAR *secret;\n+\tint wlen;\n \n \tif (!wusername || !password)\n \t\treturn;\n \n+\tif (oauth_refresh_token) {\n+\t\twlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t\tsecret = xmalloc(sizeof(WCHAR) * wlen);\n+\t\t_snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t} else {\n+\t\tsecret = _wcsdup(password);\n+\t}\n+\n \tcred.Flags = 0;\n \tcred.Type = CRED_TYPE_GENERIC;\n \tcred.TargetName = target;\n \tcred.Comment = L\"saved by git-credential-wincred\";\n-\tcred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n-\tcred.CredentialBlob = (LPVOID)password;\n+\tcred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n+\tcred.CredentialBlob = (LPVOID)_wcsdup(secret);\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n@@ -182,6 +206,8 @@ static void store_credential(void)\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n+\tfree(secret);\n+\n \tif (!CredWriteW(&cred, 0))\n \t\tdie(\"CredWrite failed\");\n }\n@@ -253,6 +279,8 @@ static void read_credential(void)\n \t\t\tpassword = utf8_to_utf16_dup(v);\n \t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n \t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"oauth_refresh_token\"))\n+\t\t\toauth_refresh_token = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\ndiff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\nindex f028fd14182..51886b8e259 100755\n--- a/t/t0303-credential-external.sh\n+++ b/t/t0303-credential-external.sh\n@@ -45,6 +45,7 @@ test -z \"$GIT_TEST_CREDENTIAL_HELPER_SETUP\" ||\n helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n \n helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n+helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n \n if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n \tsay \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n\nbase-commit: 9e49351c3060e1fa6e0d2de64505b7becf157f28\n-- \ngitgitgadget\n"},{"id":"478479","messageId":"CAGJzqs=K0_jYY-SOMjhLe_k3UzSgzEwy9mYVF=H=CA+ZUAv_uA@mail.gmail.com","threadId":"59775","inReplyTo":"pull.1534.git.1684567247339.gitgitgadget@gmail.com","subject":"Re: [PATCH] credential/wincred: store oauth_refresh_token","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-06-16T22:30:03Z","receivedAt":"2023-06-16T22:30:44Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Sat, 20 May 2023 at 08:20, M Hickford via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n>\n> From: M Hickford <mirth.hickford@gmail.com>\n>\n> a5c7656 (credential: new attribute oauth_refresh_token) introduced\n> a new confidential credential attribute for helpers to store.\n>\n> We encode the new attribute in the CredentialBlob, separated by\n> newline:\n>\n>     hunter2\n>     oauth_refresh_token=xyzzy\n>\n> This is extensible and backwards compatible. The credential protocol\n> already assumes that attribute values do not contain newlines.\n>\n> Alternatives considered: store oauth_refresh_token in a wincred\n> attribute. This would be insecure because wincred assumes attribute\n> values to be non-confidential.\n>\n> Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n> ---\n>     credential/wincred: store oauth_refresh_token\n>\n>     I'm not confident in C and found string manipulation difficult, so\n>     please review carefully.\n>\n>     I tested on Linux, cross compiling with Zig make CC=\"zig cc -target\n>     x86_64-windows-gnu\" and tested make GIT_TEST_CREDENTIAL_HELPER=wincred\n>     t0303-credential-external.sh (with a shell script git-credential-wincred\n>     that wraps wine64 git-credential-wincred.exe \"$@\")\n>\n>     See also similar patch for credential-libsecret \"[PATCH v4]\n>     credential/libsecret: store new attributes\"\n>     https://lore.kernel.org/git/pull.1469.v4.git.git.1684306540947.gitgitgadget@gmail.com/T/#u\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1534\n>\n>  .../wincred/git-credential-wincred.c          | 40 ++++++++++++++++---\n>  t/t0303-credential-external.sh                |  1 +\n>  2 files changed, 35 insertions(+), 6 deletions(-)\n>\n> diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\n> index 96f10613aee..dc34283b85e 100644\n> --- a/contrib/credential/wincred/git-credential-wincred.c\n> +++ b/contrib/credential/wincred/git-credential-wincred.c\n> @@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n>  }\n>\n>  static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n> -       *password_expiry_utc;\n> +       *password_expiry_utc, *oauth_refresh_token;\n>\n>  static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n>  {\n> @@ -128,6 +128,11 @@ static void get_credential(void)\n>         DWORD num_creds;\n>         int i;\n>         CREDENTIAL_ATTRIBUTEW *attr;\n> +       WCHAR *secret;\n> +       WCHAR *line;\n> +       WCHAR *remaining_lines;\n> +       WCHAR *part;\n> +       WCHAR *remaining_parts;\n>\n>         if (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n>                 return;\n> @@ -137,9 +142,17 @@ static void get_credential(void)\n>                 if (match_cred(creds[i])) {\n>                         write_item(\"username\", creds[i]->UserName,\n>                                 creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n> -                       write_item(\"password\",\n> -                               (LPCWSTR)creds[i]->CredentialBlob,\n> -                               creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +                       secret = xmalloc(creds[i]->CredentialBlobSize);\n> +                       wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +                       line = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n> +                       write_item(\"password\", line, wcslen(line));\n> +                       while(line != NULL) {\n> +                               part = wcstok_s(line, L\"=\", &remaining_parts);\n> +                               if (!wcscmp(part, L\"oauth_refresh_token\")) {\n> +                                       write_item(\"oauth_refresh_token\", remaining_parts, wcslen(remaining_parts));\n> +                               }\n> +                               line = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n> +                       }\n>                         for (int j = 0; j < creds[i]->AttributeCount; j++) {\n>                                 attr = creds[i]->Attributes + j;\n>                                 if (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n> @@ -148,6 +161,7 @@ static void get_credential(void)\n>                                         break;\n>                                 }\n>                         }\n> +                       free(secret);\n>                         break;\n>                 }\n>\n> @@ -158,16 +172,26 @@ static void store_credential(void)\n>  {\n>         CREDENTIALW cred;\n>         CREDENTIAL_ATTRIBUTEW expiry_attr;\n> +       WCHAR *secret;\n> +       int wlen;\n>\n>         if (!wusername || !password)\n>                 return;\n>\n> +       if (oauth_refresh_token) {\n> +               wlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> +               secret = xmalloc(sizeof(WCHAR) * wlen);\n> +               _snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> +       } else {\n> +               secret = _wcsdup(password);\n> +       }\n> +\n>         cred.Flags = 0;\n>         cred.Type = CRED_TYPE_GENERIC;\n>         cred.TargetName = target;\n>         cred.Comment = L\"saved by git-credential-wincred\";\n> -       cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n> -       cred.CredentialBlob = (LPVOID)password;\n> +       cred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n> +       cred.CredentialBlob = (LPVOID)_wcsdup(secret);\n>         cred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n>         cred.AttributeCount = 0;\n>         cred.Attributes = NULL;\n> @@ -182,6 +206,8 @@ static void store_credential(void)\n>         cred.TargetAlias = NULL;\n>         cred.UserName = wusername;\n>\n> +       free(secret);\n> +\n>         if (!CredWriteW(&cred, 0))\n>                 die(\"CredWrite failed\");\n>  }\n> @@ -253,6 +279,8 @@ static void read_credential(void)\n>                         password = utf8_to_utf16_dup(v);\n>                 else if (!strcmp(buf, \"password_expiry_utc\"))\n>                         password_expiry_utc = utf8_to_utf16_dup(v);\n> +               else if (!strcmp(buf, \"oauth_refresh_token\"))\n> +                       oauth_refresh_token = utf8_to_utf16_dup(v);\n>                 /*\n>                  * Ignore other lines; we don't know what they mean, but\n>                  * this future-proofs us when later versions of git do\n> diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\n> index f028fd14182..51886b8e259 100755\n> --- a/t/t0303-credential-external.sh\n> +++ b/t/t0303-credential-external.sh\n> @@ -45,6 +45,7 @@ test -z \"$GIT_TEST_CREDENTIAL_HELPER_SETUP\" ||\n>  helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n>\n>  helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n> +helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n>\n>  if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n>         say \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n>\n> base-commit: 9e49351c3060e1fa6e0d2de64505b7becf157f28\n> --\n> gitgitgadget\n\nHi. Is anyone familiar enough with Windows CRT to review this patch?\n"},{"id":"484078","messageId":"CAGJzqsnW7X44qeQzN_-2ckOaAjbAs3+mRh1XP+M0haYJ_om+EQ@mail.gmail.com","threadId":"59775","inReplyTo":"CAGJzqskekExeug45Zh-uQ1dpHpKP8HS3obWQyz8B_DzSZiuAwA@mail.gmail.com","subject":"Re: [PATCH] credential/wincred: store oauth_refresh_token","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-10-30T08:00:00Z","receivedAt":"2023-10-30T08:01:27Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"Hi. Is anyone with Windows development experience able to take a look\nat this patch to credential-wincred?\n\nhttps://lore.kernel.org/git/pull.1534.git.1684567247339.gitgitgadget@gmail.com/\nhttps://github.com/gitgitgadget/git/pull/1534\n\nOn Thu, 10 Aug 2023 at 20:58, M Hickford <mirth.hickford@gmail.com> wrote:\n>\n> Hi. A small patch to git-credential-wincred needs review on the Git\n> mailing list https://lore.kernel.org/git/pull.1534.git.1684567247339.gitgitgadget@gmail.com/\n> . Does anyone here have the expertise to review?\n>\n> On Sat, 20 May 2023 at 08:20, M Hickford via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> >\n> > From: M Hickford <mirth.hickford@gmail.com>\n> >\n> > a5c7656 (credential: new attribute oauth_refresh_token) introduced\n> > a new confidential credential attribute for helpers to store.\n> >\n> > We encode the new attribute in the CredentialBlob, separated by\n> > newline:\n> >\n> >     hunter2\n> >     oauth_refresh_token=xyzzy\n> >\n> > This is extensible and backwards compatible. The credential protocol\n> > already assumes that attribute values do not contain newlines.\n> >\n> > Alternatives considered: store oauth_refresh_token in a wincred\n> > attribute. This would be insecure because wincred assumes attribute\n> > values to be non-confidential.\n> >\n> > Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n> > ---\n> >     credential/wincred: store oauth_refresh_token\n> >\n> >     I'm not confident in C and found string manipulation difficult, so\n> >     please review carefully.\n> >\n> >     I tested on Linux, cross compiling with Zig make CC=\"zig cc -target\n> >     x86_64-windows-gnu\" and tested make GIT_TEST_CREDENTIAL_HELPER=wincred\n> >     t0303-credential-external.sh (with a shell script git-credential-wincred\n> >     that wraps wine64 git-credential-wincred.exe \"$@\")\n> >\n> >     See also similar patch for credential-libsecret \"[PATCH v4]\n> >     credential/libsecret: store new attributes\"\n> >     https://lore.kernel.org/git/pull.1469.v4.git.git.1684306540947.gitgitgadget@gmail.com/T/#u\n> >\n> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v1\n> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v1\n> > Pull-Request: https://github.com/gitgitgadget/git/pull/1534\n> >\n> >  .../wincred/git-credential-wincred.c          | 40 ++++++++++++++++---\n> >  t/t0303-credential-external.sh                |  1 +\n> >  2 files changed, 35 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\n> > index 96f10613aee..dc34283b85e 100644\n> > --- a/contrib/credential/wincred/git-credential-wincred.c\n> > +++ b/contrib/credential/wincred/git-credential-wincred.c\n> > @@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n> >  }\n> >\n> >  static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n> > -       *password_expiry_utc;\n> > +       *password_expiry_utc, *oauth_refresh_token;\n> >\n> >  static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n> >  {\n> > @@ -128,6 +128,11 @@ static void get_credential(void)\n> >         DWORD num_creds;\n> >         int i;\n> >         CREDENTIAL_ATTRIBUTEW *attr;\n> > +       WCHAR *secret;\n> > +       WCHAR *line;\n> > +       WCHAR *remaining_lines;\n> > +       WCHAR *part;\n> > +       WCHAR *remaining_parts;\n> >\n> >         if (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n> >                 return;\n> > @@ -137,9 +142,17 @@ static void get_credential(void)\n> >                 if (match_cred(creds[i])) {\n> >                         write_item(\"username\", creds[i]->UserName,\n> >                                 creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n> > -                       write_item(\"password\",\n> > -                               (LPCWSTR)creds[i]->CredentialBlob,\n> > -                               creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                       secret = xmalloc(creds[i]->CredentialBlobSize);\n> > +                       wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                       line = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n> > +                       write_item(\"password\", line, wcslen(line));\n> > +                       while(line != NULL) {\n> > +                               part = wcstok_s(line, L\"=\", &remaining_parts);\n> > +                               if (!wcscmp(part, L\"oauth_refresh_token\")) {\n> > +                                       write_item(\"oauth_refresh_token\", remaining_parts, wcslen(remaining_parts));\n> > +                               }\n> > +                               line = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n> > +                       }\n> >                         for (int j = 0; j < creds[i]->AttributeCount; j++) {\n> >                                 attr = creds[i]->Attributes + j;\n> >                                 if (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n> > @@ -148,6 +161,7 @@ static void get_credential(void)\n> >                                         break;\n> >                                 }\n> >                         }\n> > +                       free(secret);\n> >                         break;\n> >                 }\n> >\n> > @@ -158,16 +172,26 @@ static void store_credential(void)\n> >  {\n> >         CREDENTIALW cred;\n> >         CREDENTIAL_ATTRIBUTEW expiry_attr;\n> > +       WCHAR *secret;\n> > +       int wlen;\n> >\n> >         if (!wusername || !password)\n> >                 return;\n> >\n> > +       if (oauth_refresh_token) {\n> > +               wlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> > +               secret = xmalloc(sizeof(WCHAR) * wlen);\n> > +               _snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> > +       } else {\n> > +               secret = _wcsdup(password);\n> > +       }\n> > +\n> >         cred.Flags = 0;\n> >         cred.Type = CRED_TYPE_GENERIC;\n> >         cred.TargetName = target;\n> >         cred.Comment = L\"saved by git-credential-wincred\";\n> > -       cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n> > -       cred.CredentialBlob = (LPVOID)password;\n> > +       cred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n> > +       cred.CredentialBlob = (LPVOID)_wcsdup(secret);\n> >         cred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n> >         cred.AttributeCount = 0;\n> >         cred.Attributes = NULL;\n> > @@ -182,6 +206,8 @@ static void store_credential(void)\n> >         cred.TargetAlias = NULL;\n> >         cred.UserName = wusername;\n> >\n> > +       free(secret);\n> > +\n> >         if (!CredWriteW(&cred, 0))\n> >                 die(\"CredWrite failed\");\n> >  }\n> > @@ -253,6 +279,8 @@ static void read_credential(void)\n> >                         password = utf8_to_utf16_dup(v);\n> >                 else if (!strcmp(buf, \"password_expiry_utc\"))\n> >                         password_expiry_utc = utf8_to_utf16_dup(v);\n> > +               else if (!strcmp(buf, \"oauth_refresh_token\"))\n> > +                       oauth_refresh_token = utf8_to_utf16_dup(v);\n> >                 /*\n> >                  * Ignore other lines; we don't know what they mean, but\n> >                  * this future-proofs us when later versions of git do\n> > diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh\n> > index f028fd14182..51886b8e259 100755\n> > --- a/t/t0303-credential-external.sh\n> > +++ b/t/t0303-credential-external.sh\n> > @@ -45,6 +45,7 @@ test -z \"$GIT_TEST_CREDENTIAL_HELPER_SETUP\" ||\n> >  helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n> >\n> >  helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n> > +helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n> >\n> >  if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n> >         say \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n> >\n> > base-commit: 9e49351c3060e1fa6e0d2de64505b7becf157f28\n> > --\n> > gitgitgadget\n"},{"id":"484450","messageId":"pull.1534.v2.git.1699251395093.gitgitgadget@gmail.com","threadId":"59775","inReplyTo":"pull.1534.git.1684567247339.gitgitgadget@gmail.com","subject":"[PATCH v2] credential/wincred: store oauth_refresh_token","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-11-06T06:16:34Z","receivedAt":"2023-11-06T06:16:41Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\na5c7656 (credential: new attribute oauth_refresh_token) introduced\na new confidential credential attribute for helpers to store.\n\nWe encode the new attribute in the CredentialBlob, separated by\nnewline:\n\n    hunter2\n    oauth_refresh_token=xyzzy\n\nThis is extensible and backwards compatible. The credential protocol\nalready assumes that attribute values do not contain newlines.\n\nThis fixes test \"helper (wincred) gets oauth_refresh_token\" when\nt0303-credential-external.sh is run with\nGIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7\n(credential: new attribute oauth_refresh_token, 2023-04-21).\n\nAlternatives considered: store oauth_refresh_token in a wincred\nattribute. This would be insecure because wincred assumes attribute\nvalues to be non-confidential.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store oauth_refresh_token\n    \n    Patch v2 fixes a bug when password is empty.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/1534\n\nRange-diff vs v1:\n\n 1:  46ce22aa3a3 ! 1:  12f092f45b1 credential/wincred: store oauth_refresh_token\n     @@ Commit message\n          This is extensible and backwards compatible. The credential protocol\n          already assumes that attribute values do not contain newlines.\n      \n     +    This fixes test \"helper (wincred) gets oauth_refresh_token\" when\n     +    t0303-credential-external.sh is run with\n     +    GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7\n     +    (credential: new attribute oauth_refresh_token, 2023-04-21).\n     +\n          Alternatives considered: store oauth_refresh_token in a wincred\n          attribute. This would be insecure because wincred assumes attribute\n          values to be non-confidential.\n     @@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(\n       \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n       \t\treturn;\n      @@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(void)\n     - \t\tif (match_cred(creds[i])) {\n     + \t\tif (match_cred(creds[i], 0)) {\n       \t\t\twrite_item(\"username\", creds[i]->UserName,\n       \t\t\t\tcreds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n      -\t\t\twrite_item(\"password\",\n      -\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n      -\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n     -+\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n     -+\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n     -+\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n     -+\t\t\twrite_item(\"password\", line, wcslen(line));\n     -+\t\t\twhile(line != NULL) {\n     -+\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n     -+\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n     -+\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, wcslen(remaining_parts));\n     ++\t\t\tif (creds[i]->CredentialBlobSize > 0) {\n     ++\t\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n     ++\t\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n     ++\t\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n     ++\t\t\t\twrite_item(\"password\", line, line ? wcslen(line) : 0);\n     ++\t\t\t\twhile(line != NULL) {\n     ++\t\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n     ++\t\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n     ++\t\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);\n     ++\t\t\t\t\t}\n     ++\t\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n      +\t\t\t\t}\n     -+\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n     ++\t\t\t} else {\n     ++\t\t\t\twrite_item(\"password\",\n     ++\t\t\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n     ++\t\t\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n      +\t\t\t}\n       \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n       \t\t\t\tattr = creds[i]->Attributes + j;\n     @@ contrib/credential/wincred/git-credential-wincred.c: static void read_credential\n       \t\t/*\n       \t\t * Ignore other lines; we don't know what they mean, but\n       \t\t * this future-proofs us when later versions of git do\n     -\n     - ## t/t0303-credential-external.sh ##\n     -@@ t/t0303-credential-external.sh: test -z \"$GIT_TEST_CREDENTIAL_HELPER_SETUP\" ||\n     - helper_test_clean \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - \n     - helper_test \"$GIT_TEST_CREDENTIAL_HELPER\"\n     -+helper_test_oauth_refresh_token \"$GIT_TEST_CREDENTIAL_HELPER\"\n     - \n     - if test -z \"$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT\"; then\n     - \tsay \"# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)\"\n\n\n .../wincred/git-credential-wincred.c          | 46 ++++++++++++++++---\n 1 file changed, 40 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex 4cd56c42e24..5c6a7d65d4a 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n }\n \n static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n-\t*password_expiry_utc;\n+\t*password_expiry_utc, *oauth_refresh_token;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -140,6 +140,11 @@ static void get_credential(void)\n \tDWORD num_creds;\n \tint i;\n \tCREDENTIAL_ATTRIBUTEW *attr;\n+\tWCHAR *secret;\n+\tWCHAR *line;\n+\tWCHAR *remaining_lines;\n+\tWCHAR *part;\n+\tWCHAR *remaining_parts;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -149,9 +154,23 @@ static void get_credential(void)\n \t\tif (match_cred(creds[i], 0)) {\n \t\t\twrite_item(\"username\", creds[i]->UserName,\n \t\t\t\tcreds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n-\t\t\twrite_item(\"password\",\n-\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n-\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tif (creds[i]->CredentialBlobSize > 0) {\n+\t\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n+\t\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n+\t\t\t\twrite_item(\"password\", line, line ? wcslen(line) : 0);\n+\t\t\t\twhile(line != NULL) {\n+\t\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n+\t\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n+\t\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);\n+\t\t\t\t\t}\n+\t\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\twrite_item(\"password\",\n+\t\t\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n+\t\t\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\t}\n \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n \t\t\t\tattr = creds[i]->Attributes + j;\n \t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n@@ -160,6 +179,7 @@ static void get_credential(void)\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t}\n+\t\t\tfree(secret);\n \t\t\tbreak;\n \t\t}\n \n@@ -170,16 +190,26 @@ static void store_credential(void)\n {\n \tCREDENTIALW cred;\n \tCREDENTIAL_ATTRIBUTEW expiry_attr;\n+\tWCHAR *secret;\n+\tint wlen;\n \n \tif (!wusername || !password)\n \t\treturn;\n \n+\tif (oauth_refresh_token) {\n+\t\twlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t\tsecret = xmalloc(sizeof(WCHAR) * wlen);\n+\t\t_snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t} else {\n+\t\tsecret = _wcsdup(password);\n+\t}\n+\n \tcred.Flags = 0;\n \tcred.Type = CRED_TYPE_GENERIC;\n \tcred.TargetName = target;\n \tcred.Comment = L\"saved by git-credential-wincred\";\n-\tcred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n-\tcred.CredentialBlob = (LPVOID)password;\n+\tcred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n+\tcred.CredentialBlob = (LPVOID)_wcsdup(secret);\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n@@ -194,6 +224,8 @@ static void store_credential(void)\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n+\tfree(secret);\n+\n \tif (!CredWriteW(&cred, 0))\n \t\tdie(\"CredWrite failed\");\n }\n@@ -265,6 +297,8 @@ static void read_credential(void)\n \t\t\tpassword = utf8_to_utf16_dup(v);\n \t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n \t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"oauth_refresh_token\"))\n+\t\t\toauth_refresh_token = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\n\nbase-commit: bc5204569f7db44d22477485afd52ea410d83743\n-- \ngitgitgadget\n"},{"id":"484846","messageId":"xmqqbkbwzss5.fsf@gitster.g","threadId":"59775","inReplyTo":"pull.1534.v2.git.1699251395093.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] credential/wincred: store oauth_refresh_token","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-11-14T07:40:58Z","receivedAt":"2023-11-14T07:41:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"M Hickford via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: M Hickford <mirth.hickford@gmail.com>\n>\n> a5c7656 (credential: new attribute oauth_refresh_token) introduced\n> a new confidential credential attribute for helpers to store.\n>\n> We encode the new attribute in the CredentialBlob, separated by\n> newline:\n>\n>     hunter2\n>     oauth_refresh_token=xyzzy\n>\n> This is extensible and backwards compatible. The credential protocol\n> already assumes that attribute values do not contain newlines.\n>\n> This fixes test \"helper (wincred) gets oauth_refresh_token\" when\n> t0303-credential-external.sh is run with\n> GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7\n> (credential: new attribute oauth_refresh_token, 2023-04-21).\n>\n> Alternatives considered: store oauth_refresh_token in a wincred\n> attribute. This would be insecure because wincred assumes attribute\n> values to be non-confidential.\n\nEarlier, a5c76569 (credential: new attribute oauth_refresh_token,\n2023-04-21) built the \"git\" side support for the token, and taught\ncredential-cache to store the necessary information.  Then 0ce02e2f\n(credential/libsecret: store new attributes, 2023-06-16) was written\nfor libsecret to support the same interface.  \n\nAnd this one adds corresponding support for wincred.  Do I\nunderstand what is going on around this patch correctly?\n\nI do not do Windows, but some people on this list certainly do and\nwould be capable of giving the patch a necessary nudge ;-)\n\nThanks.\n\n>  .../wincred/git-credential-wincred.c          | 46 ++++++++++++++++---\n>  1 file changed, 40 insertions(+), 6 deletions(-)\n>\n> diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\n> index 4cd56c42e24..5c6a7d65d4a 100644\n> --- a/contrib/credential/wincred/git-credential-wincred.c\n> +++ b/contrib/credential/wincred/git-credential-wincred.c\n> @@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n>  }\n>  \n>  static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n> -\t*password_expiry_utc;\n> +\t*password_expiry_utc, *oauth_refresh_token;\n>  \n>  static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n>  {\n> @@ -140,6 +140,11 @@ static void get_credential(void)\n>  \tDWORD num_creds;\n>  \tint i;\n>  \tCREDENTIAL_ATTRIBUTEW *attr;\n> +\tWCHAR *secret;\n> +\tWCHAR *line;\n> +\tWCHAR *remaining_lines;\n> +\tWCHAR *part;\n> +\tWCHAR *remaining_parts;\n>  \n>  \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n>  \t\treturn;\n> @@ -149,9 +154,23 @@ static void get_credential(void)\n>  \t\tif (match_cred(creds[i], 0)) {\n>  \t\t\twrite_item(\"username\", creds[i]->UserName,\n>  \t\t\t\tcreds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n> -\t\t\twrite_item(\"password\",\n> -\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n> -\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +\t\t\tif (creds[i]->CredentialBlobSize > 0) {\n> +\t\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n> +\t\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +\t\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n> +\t\t\t\twrite_item(\"password\", line, line ? wcslen(line) : 0);\n> +\t\t\t\twhile(line != NULL) {\n> +\t\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n> +\t\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n> +\t\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);\n> +\t\t\t\t\t}\n> +\t\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n> +\t\t\t\t}\n> +\t\t\t} else {\n> +\t\t\t\twrite_item(\"password\",\n> +\t\t\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n> +\t\t\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +\t\t\t}\n>  \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n>  \t\t\t\tattr = creds[i]->Attributes + j;\n>  \t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n> @@ -160,6 +179,7 @@ static void get_credential(void)\n>  \t\t\t\t\tbreak;\n>  \t\t\t\t}\n>  \t\t\t}\n> +\t\t\tfree(secret);\n>  \t\t\tbreak;\n>  \t\t}\n>  \n> @@ -170,16 +190,26 @@ static void store_credential(void)\n>  {\n>  \tCREDENTIALW cred;\n>  \tCREDENTIAL_ATTRIBUTEW expiry_attr;\n> +\tWCHAR *secret;\n> +\tint wlen;\n>  \n>  \tif (!wusername || !password)\n>  \t\treturn;\n>  \n> +\tif (oauth_refresh_token) {\n> +\t\twlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> +\t\tsecret = xmalloc(sizeof(WCHAR) * wlen);\n> +\t\t_snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> +\t} else {\n> +\t\tsecret = _wcsdup(password);\n> +\t}\n> +\n>  \tcred.Flags = 0;\n>  \tcred.Type = CRED_TYPE_GENERIC;\n>  \tcred.TargetName = target;\n>  \tcred.Comment = L\"saved by git-credential-wincred\";\n> -\tcred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n> -\tcred.CredentialBlob = (LPVOID)password;\n> +\tcred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n> +\tcred.CredentialBlob = (LPVOID)_wcsdup(secret);\n>  \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n>  \tcred.AttributeCount = 0;\n>  \tcred.Attributes = NULL;\n> @@ -194,6 +224,8 @@ static void store_credential(void)\n>  \tcred.TargetAlias = NULL;\n>  \tcred.UserName = wusername;\n>  \n> +\tfree(secret);\n> +\n>  \tif (!CredWriteW(&cred, 0))\n>  \t\tdie(\"CredWrite failed\");\n>  }\n> @@ -265,6 +297,8 @@ static void read_credential(void)\n>  \t\t\tpassword = utf8_to_utf16_dup(v);\n>  \t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n>  \t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n> +\t\telse if (!strcmp(buf, \"oauth_refresh_token\"))\n> +\t\t\toauth_refresh_token = utf8_to_utf16_dup(v);\n>  \t\t/*\n>  \t\t * Ignore other lines; we don't know what they mean, but\n>  \t\t * this future-proofs us when later versions of git do\n>\n> base-commit: bc5204569f7db44d22477485afd52ea410d83743\n"},{"id":"485100","messageId":"CAGJzqskEM_ub9arQBAhem+cQ0R4jGqQUoN_DZ7w5FRS3hK=bpw@mail.gmail.com","threadId":"59775","inReplyTo":"xmqqbkbwzss5.fsf@gitster.g","subject":"Re: [PATCH v2] credential/wincred: store oauth_refresh_token","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-11-24T08:00:00Z","receivedAt":"2023-11-24T08:00:04Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Tue, 14 Nov 2023 at 07:41, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"M Hickford via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: M Hickford <mirth.hickford@gmail.com>\n> >\n> > a5c7656 (credential: new attribute oauth_refresh_token) introduced\n> > a new confidential credential attribute for helpers to store.\n> >\n> > We encode the new attribute in the CredentialBlob, separated by\n> > newline:\n> >\n> >     hunter2\n> >     oauth_refresh_token=xyzzy\n> >\n> > This is extensible and backwards compatible. The credential protocol\n> > already assumes that attribute values do not contain newlines.\n> >\n> > This fixes test \"helper (wincred) gets oauth_refresh_token\" when\n> > t0303-credential-external.sh is run with\n> > GIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7\n> > (credential: new attribute oauth_refresh_token, 2023-04-21).\n> >\n> > Alternatives considered: store oauth_refresh_token in a wincred\n> > attribute. This would be insecure because wincred assumes attribute\n> > values to be non-confidential.\n>\n> Earlier, a5c76569 (credential: new attribute oauth_refresh_token,\n> 2023-04-21) built the \"git\" side support for the token, and taught\n> credential-cache to store the necessary information.  Then 0ce02e2f\n> (credential/libsecret: store new attributes, 2023-06-16) was written\n> for libsecret to support the same interface.\n>\n> And this one adds corresponding support for wincred.  Do I\n> understand what is going on around this patch correctly?\n\nYes, that's right.\n\n\n>\n> I do not do Windows, but some people on this list certainly do and\n> would be capable of giving the patch a necessary nudge ;-)\n>\n> Thanks.\n>\n> >  .../wincred/git-credential-wincred.c          | 46 ++++++++++++++++---\n> >  1 file changed, 40 insertions(+), 6 deletions(-)\n> >\n> > diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\n> > index 4cd56c42e24..5c6a7d65d4a 100644\n> > --- a/contrib/credential/wincred/git-credential-wincred.c\n> > +++ b/contrib/credential/wincred/git-credential-wincred.c\n> > @@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n> >  }\n> >\n> >  static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n> > -     *password_expiry_utc;\n> > +     *password_expiry_utc, *oauth_refresh_token;\n> >\n> >  static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n> >  {\n> > @@ -140,6 +140,11 @@ static void get_credential(void)\n> >       DWORD num_creds;\n> >       int i;\n> >       CREDENTIAL_ATTRIBUTEW *attr;\n> > +     WCHAR *secret;\n> > +     WCHAR *line;\n> > +     WCHAR *remaining_lines;\n> > +     WCHAR *part;\n> > +     WCHAR *remaining_parts;\n> >\n> >       if (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n> >               return;\n> > @@ -149,9 +154,23 @@ static void get_credential(void)\n> >               if (match_cred(creds[i], 0)) {\n> >                       write_item(\"username\", creds[i]->UserName,\n> >                               creds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n> > -                     write_item(\"password\",\n> > -                             (LPCWSTR)creds[i]->CredentialBlob,\n> > -                             creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                     if (creds[i]->CredentialBlobSize > 0) {\n> > +                             secret = xmalloc(creds[i]->CredentialBlobSize);\n> > +                             wcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                             line = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n> > +                             write_item(\"password\", line, line ? wcslen(line) : 0);\n> > +                             while(line != NULL) {\n> > +                                     part = wcstok_s(line, L\"=\", &remaining_parts);\n> > +                                     if (!wcscmp(part, L\"oauth_refresh_token\")) {\n> > +                                             write_item(\"oauth_refresh_token\", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);\n> > +                                     }\n> > +                                     line = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n> > +                             }\n> > +                     } else {\n> > +                             write_item(\"password\",\n> > +                                             (LPCWSTR)creds[i]->CredentialBlob,\n> > +                                             creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                     }\n> >                       for (int j = 0; j < creds[i]->AttributeCount; j++) {\n> >                               attr = creds[i]->Attributes + j;\n> >                               if (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n> > @@ -160,6 +179,7 @@ static void get_credential(void)\n> >                                       break;\n> >                               }\n> >                       }\n> > +                     free(secret);\n> >                       break;\n> >               }\n> >\n> > @@ -170,16 +190,26 @@ static void store_credential(void)\n> >  {\n> >       CREDENTIALW cred;\n> >       CREDENTIAL_ATTRIBUTEW expiry_attr;\n> > +     WCHAR *secret;\n> > +     int wlen;\n> >\n> >       if (!wusername || !password)\n> >               return;\n> >\n> > +     if (oauth_refresh_token) {\n> > +             wlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> > +             secret = xmalloc(sizeof(WCHAR) * wlen);\n> > +             _snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n> > +     } else {\n> > +             secret = _wcsdup(password);\n> > +     }\n> > +\n> >       cred.Flags = 0;\n> >       cred.Type = CRED_TYPE_GENERIC;\n> >       cred.TargetName = target;\n> >       cred.Comment = L\"saved by git-credential-wincred\";\n> > -     cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n> > -     cred.CredentialBlob = (LPVOID)password;\n> > +     cred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n> > +     cred.CredentialBlob = (LPVOID)_wcsdup(secret);\n> >       cred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n> >       cred.AttributeCount = 0;\n> >       cred.Attributes = NULL;\n> > @@ -194,6 +224,8 @@ static void store_credential(void)\n> >       cred.TargetAlias = NULL;\n> >       cred.UserName = wusername;\n> >\n> > +     free(secret);\n> > +\n> >       if (!CredWriteW(&cred, 0))\n> >               die(\"CredWrite failed\");\n> >  }\n> > @@ -265,6 +297,8 @@ static void read_credential(void)\n> >                       password = utf8_to_utf16_dup(v);\n> >               else if (!strcmp(buf, \"password_expiry_utc\"))\n> >                       password_expiry_utc = utf8_to_utf16_dup(v);\n> > +             else if (!strcmp(buf, \"oauth_refresh_token\"))\n> > +                     oauth_refresh_token = utf8_to_utf16_dup(v);\n> >               /*\n> >                * Ignore other lines; we don't know what they mean, but\n> >                * this future-proofs us when later versions of git do\n> >\n> > base-commit: bc5204569f7db44d22477485afd52ea410d83743\n"},{"id":"487480","messageId":"pull.1534.v3.git.1706477103039.gitgitgadget@gmail.com","threadId":"59775","inReplyTo":"pull.1534.v2.git.1699251395093.gitgitgadget@gmail.com","subject":"[PATCH v3] credential/wincred: store oauth_refresh_token","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-01-28T21:25:02Z","receivedAt":"2024-01-28T21:25:06Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\na5c7656 (credential: new attribute oauth_refresh_token) introduced\na new confidential credential attribute and added support to\ncredential-cache. Later 0ce02e2f (credential/libsecret: store new\nattributes, 2023-06-16) added support in credential-libsecret.\n\nTo add support in credential-wincred, we encode the new attribute in the\nCredentialBlob, separated by newline:\n\n    hunter2\n    oauth_refresh_token=xyzzy\n\nThis is extensible and backwards compatible. The credential protocol\nalready assumes that attribute values do not contain newlines.\n\nThis fixes test \"helper (wincred) gets oauth_refresh_token\" when\nt0303-credential-external.sh is run with\nGIT_TEST_CREDENTIAL_HELPER=wincred. This test was added in a5c76569e7\n(credential: new attribute oauth_refresh_token, 2023-04-21).\n\nAlternatives considered: store oauth_refresh_token in a wincred\nattribute. This would be insecure because wincred assumes attribute\nvalues to be non-confidential.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store oauth_refresh_token\n    \n    Patch v3 fixes a maybe-uninitialized warning.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1534%2Fhickford%2Fwincred-refresh-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1534/hickford/wincred-refresh-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/1534\n\nRange-diff vs v2:\n\n 1:  12f092f45b1 ! 1:  a324c3aac41 credential/wincred: store oauth_refresh_token\n     @@ Commit message\n          credential/wincred: store oauth_refresh_token\n      \n          a5c7656 (credential: new attribute oauth_refresh_token) introduced\n     -    a new confidential credential attribute for helpers to store.\n     +    a new confidential credential attribute and added support to\n     +    credential-cache. Later 0ce02e2f (credential/libsecret: store new\n     +    attributes, 2023-06-16) added support in credential-libsecret.\n      \n     -    We encode the new attribute in the CredentialBlob, separated by\n     -    newline:\n     +    To add support in credential-wincred, we encode the new attribute in the\n     +    CredentialBlob, separated by newline:\n      \n              hunter2\n              oauth_refresh_token=xyzzy\n     @@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(\n      +\t\t\t\t\t}\n      +\t\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n      +\t\t\t\t}\n     ++\t\t\t\tfree(secret);\n      +\t\t\t} else {\n      +\t\t\t\twrite_item(\"password\",\n      +\t\t\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n     @@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(\n       \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n       \t\t\t\tattr = creds[i]->Attributes + j;\n       \t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n     -@@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(void)\n     - \t\t\t\t\tbreak;\n     - \t\t\t\t}\n     - \t\t\t}\n     -+\t\t\tfree(secret);\n     - \t\t\tbreak;\n     - \t\t}\n     - \n      @@ contrib/credential/wincred/git-credential-wincred.c: static void store_credential(void)\n       {\n       \tCREDENTIALW cred;\n\n\n .../wincred/git-credential-wincred.c          | 46 ++++++++++++++++---\n 1 file changed, 40 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex 4cd56c42e24..4be0d58cd89 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -35,7 +35,7 @@ static void *xmalloc(size_t size)\n }\n \n static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n-\t*password_expiry_utc;\n+\t*password_expiry_utc, *oauth_refresh_token;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -140,6 +140,11 @@ static void get_credential(void)\n \tDWORD num_creds;\n \tint i;\n \tCREDENTIAL_ATTRIBUTEW *attr;\n+\tWCHAR *secret;\n+\tWCHAR *line;\n+\tWCHAR *remaining_lines;\n+\tWCHAR *part;\n+\tWCHAR *remaining_parts;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -149,9 +154,24 @@ static void get_credential(void)\n \t\tif (match_cred(creds[i], 0)) {\n \t\t\twrite_item(\"username\", creds[i]->UserName,\n \t\t\t\tcreds[i]->UserName ? wcslen(creds[i]->UserName) : 0);\n-\t\t\twrite_item(\"password\",\n-\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n-\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tif (creds[i]->CredentialBlobSize > 0) {\n+\t\t\t\tsecret = xmalloc(creds[i]->CredentialBlobSize);\n+\t\t\t\twcsncpy_s(secret, creds[i]->CredentialBlobSize, (LPCWSTR)creds[i]->CredentialBlob, creds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\t\tline = wcstok_s(secret, L\"\\r\\n\", &remaining_lines);\n+\t\t\t\twrite_item(\"password\", line, line ? wcslen(line) : 0);\n+\t\t\t\twhile(line != NULL) {\n+\t\t\t\t\tpart = wcstok_s(line, L\"=\", &remaining_parts);\n+\t\t\t\t\tif (!wcscmp(part, L\"oauth_refresh_token\")) {\n+\t\t\t\t\t\twrite_item(\"oauth_refresh_token\", remaining_parts, remaining_parts ? wcslen(remaining_parts) : 0);\n+\t\t\t\t\t}\n+\t\t\t\t\tline = wcstok_s(NULL, L\"\\r\\n\", &remaining_lines);\n+\t\t\t\t}\n+\t\t\t\tfree(secret);\n+\t\t\t} else {\n+\t\t\t\twrite_item(\"password\",\n+\t\t\t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n+\t\t\t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\t}\n \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n \t\t\t\tattr = creds[i]->Attributes + j;\n \t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n@@ -170,16 +190,26 @@ static void store_credential(void)\n {\n \tCREDENTIALW cred;\n \tCREDENTIAL_ATTRIBUTEW expiry_attr;\n+\tWCHAR *secret;\n+\tint wlen;\n \n \tif (!wusername || !password)\n \t\treturn;\n \n+\tif (oauth_refresh_token) {\n+\t\twlen = _scwprintf(L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t\tsecret = xmalloc(sizeof(WCHAR) * wlen);\n+\t\t_snwprintf_s(secret, sizeof(WCHAR) * wlen, wlen, L\"%s\\r\\noauth_refresh_token=%s\", password, oauth_refresh_token);\n+\t} else {\n+\t\tsecret = _wcsdup(password);\n+\t}\n+\n \tcred.Flags = 0;\n \tcred.Type = CRED_TYPE_GENERIC;\n \tcred.TargetName = target;\n \tcred.Comment = L\"saved by git-credential-wincred\";\n-\tcred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);\n-\tcred.CredentialBlob = (LPVOID)password;\n+\tcred.CredentialBlobSize = wcslen(secret) * sizeof(WCHAR);\n+\tcred.CredentialBlob = (LPVOID)_wcsdup(secret);\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n@@ -194,6 +224,8 @@ static void store_credential(void)\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n+\tfree(secret);\n+\n \tif (!CredWriteW(&cred, 0))\n \t\tdie(\"CredWrite failed\");\n }\n@@ -265,6 +297,8 @@ static void read_credential(void)\n \t\t\tpassword = utf8_to_utf16_dup(v);\n \t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n \t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"oauth_refresh_token\"))\n+\t\t\toauth_refresh_token = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\n\nbase-commit: b50a608ba20348cb3dfc16a696816d51780e3f0f\n-- \ngitgitgadget\n"}]}