git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/2] wincred: improve compatibility with windows versions

From
Karsten Blees <karsten.blees@gmail.com>
Date
Jan 10, 2013, 12:10 UTC
Message-ID
<50EEAFB1.2090604@gmail.com>
In-Reply-To
<CABPQNSb7MjTKgmeB9TcUV0+-FfjPZ1sgKPsfVDg6+uaw2f_azQ@mail.gmail.com>

On WinXP, the windows credential helper doesn't work at all (due to missing Cred[Un]PackAuthenticationBuffer APIs). On Win7, the credential format used by wincred is incompatible with native Windows tools (such as the control panel applet or 'cmdkey.exe /generic'). These Windows tools only set the TargetName, UserName and CredentialBlob members of the CREDENTIAL structure (where CredentialBlob is the UTF-16-encoded password).

Remove the unnecessary packing / unpacking of the password, along with the related API definitions, for compatibility with Windows XP.

Don't use CREDENTIAL_ATTRIBUTEs to identify credentials for compatibility with Windows credential manager tools. Parse the protocol, username, host and path fields from the credential's target name instead.

Credentials created with an old wincred version will have mangled or empty passwords after this change.

Signed-off-by: Karsten Blees <blees@dcon.de>
---
 .../credential/wincred/git-credential-wincred.c    | 199 ++++++++-------------
 1 file changed, 70 insertions(+), 129 deletions(-)
diff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c
index 94d7140..dac19ea 100644
--- a/contrib/credential/wincred/git-credential-wincred.c
+++ b/contrib/credential/wincred/git-credential-wincred.c
@@ -9,6 +9,8 @@
 
 /* common helpers */
 
+#define ARRAY_SIZE(x) (sizeof(x)/sizeof(x[0]))
+
 static void die(const char *err, ...)
 {
 	char msg[4096];
@@ -30,14 +32,6 @@ static void *xmalloc(size_t size)
 	return ret;
 }
 
-static char *xstrdup(const char *str)
-{
-	char *ret = strdup(str);
-	if (!ret)
-		die("Out of memory");
-	return ret;
-}
-
 /* MinGW doesn't have wincred.h, so we need to define stuff */
 
 typedef struct _CREDENTIAL_ATTRIBUTEW {
@@ -67,20 +61,14 @@ typedef struct _CREDENTIALW {
 #define CRED_MAX_ATTRIBUTES 64
 
 typedef BOOL (WINAPI *CredWriteWT)(PCREDENTIALW, DWORD);
-typedef BOOL (WINAPI *CredUnPackAuthenticationBufferWT)(DWORD, PVOID, DWORD,
-    LPWSTR, DWORD *, LPWSTR, DWORD *, LPWSTR, DWORD *);
 typedef BOOL (WINAPI *CredEnumerateWT)(LPCWSTR, DWORD, DWORD *,
     PCREDENTIALW **);
-typedef BOOL (WINAPI *CredPackAuthenticationBufferWT)(DWORD, LPWSTR, LPWSTR,
-    PBYTE, DWORD *);
 typedef VOID (WINAPI *CredFreeT)(PVOID);
 typedef BOOL (WINAPI *CredDeleteWT)(LPCWSTR, DWORD, DWORD);
 
-static HMODULE advapi, credui;
+static HMODULE advapi;
 static CredWriteWT CredWriteW;
-static CredUnPackAuthenticationBufferWT CredUnPackAuthenticationBufferW;
 static CredEnumerateWT CredEnumerateW;
-static CredPackAuthenticationBufferWT CredPackAuthenticationBufferW;
 static CredFreeT CredFree;
 static CredDeleteWT CredDeleteW;
 
@@ -88,74 +76,84 @@ static void load_cred_funcs(void)
 {
 	/* load DLLs */
 	advapi = LoadLibrary("advapi32.dll");
-	credui = LoadLibrary("credui.dll");
-	if (!advapi || !credui)
-		die("failed to load DLLs");
+	if (!advapi)
+		die("failed to load advapi32.dll");
 
 	/* get function pointers */
 	CredWriteW = (CredWriteWT)GetProcAddress(advapi, "CredWriteW");
-	CredUnPackAuthenticationBufferW = (CredUnPackAuthenticationBufferWT)
-	    GetProcAddress(credui, "CredUnPackAuthenticationBufferW");
 	CredEnumerateW = (CredEnumerateWT)GetProcAddress(advapi,
 	    "CredEnumerateW");
-	CredPackAuthenticationBufferW = (CredPackAuthenticationBufferWT)
-	    GetProcAddress(credui, "CredPackAuthenticationBufferW");
 	CredFree = (CredFreeT)GetProcAddress(advapi, "CredFree");
 	CredDeleteW = (CredDeleteWT)GetProcAddress(advapi, "CredDeleteW");
-	if (!CredWriteW || !CredUnPackAuthenticationBufferW ||
-	    !CredEnumerateW || !CredPackAuthenticationBufferW || !CredFree ||
-	    !CredDeleteW)
+	if (!CredWriteW || !CredEnumerateW || !CredFree || !CredDeleteW)
 		die("failed to load functions");
 }
 
-static char target_buf[1024];
-static char *protocol, *host, *path, *username;
-static WCHAR *wusername, *password, *target;
+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];
 
-static void write_item(const char *what, WCHAR *wbuf)
+static void write_item(const char *what, LPCWSTR wbuf, int wlen)
 {
 	char *buf;
-	int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, NULL, 0, NULL,
+	int len = WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, NULL, 0, NULL,
 	    FALSE);
 	buf = xmalloc(len);
 
-	if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, FALSE))
+	if (!WideCharToMultiByte(CP_UTF8, 0, wbuf, wlen, buf, len, NULL, FALSE))
 		die("WideCharToMultiByte failed!");
 
 	printf("%s=", what);
-	fwrite(buf, 1, len - 1, stdout);
+	fwrite(buf, 1, len, stdout);
 	putchar('\n');
 	free(buf);
 }
 
-static int match_attr(const CREDENTIALW *cred, const WCHAR *keyword,
-    const char *want)
+/*
+ * Match an (optional) expected string and a delimiter in the target string,
+ * consuming the matched text by updating the target pointer.
+ */
+static int match_part(LPCWSTR *ptarget, LPCWSTR want, LPCWSTR delim)
 {
-	int i;
-	if (!want)
-		return 1;
-
-	for (i = 0; i < cred->AttributeCount; ++i)
-		if (!wcscmp(cred->Attributes[i].Keyword, keyword))
-			return !strcmp((const char *)cred->Attributes[i].Value,
-			    want);
-
-	return 0; /* not found */
+	LPCWSTR delim_pos, start = *ptarget;
+	int len;
+
+	/* find start of delimiter (or end-of-string if delim is empty) */
+	if (*delim)
+		delim_pos = wcsstr(start, delim);
+	else
+		delim_pos = start + wcslen(start);
+
+	/*
+	 * match text up to delimiter, or end of string (e.g. the '/' after
+	 * host is optional if not followed by a path)
+	 */
+	if (delim_pos)
+		len = delim_pos - start;
+	else
+		len = wcslen(start);
+
+	/* update ptarget if we either found a delimiter or need a match */
+	if (delim_pos || want)
+		*ptarget = delim_pos ? delim_pos + wcslen(delim) : start + len;
+
+	return !want || (!wcsncmp(want, start, len) && !want[len]);
 }
 
 static int match_cred(const CREDENTIALW *cred)
 {
-	return (!wusername || !wcscmp(wusername, cred->UserName)) &&
-	    match_attr(cred, L"git_protocol", protocol) &&
-	    match_attr(cred, L"git_host", host) &&
-	    match_attr(cred, L"git_path", path);
+	LPCWSTR target = cred->TargetName;
+	if (wusername && wcscmp(wusername, cred->UserName))
+		return 0;
+
+	return match_part(&target, L"git", L":") &&
+		match_part(&target, protocol, L"://") &&
+		match_part(&target, wusername, L"@") &&
+		match_part(&target, host, L"/") &&
+		match_part(&target, path, L"");
 }
 
 static void get_credential(void)
 {
-	WCHAR *user_buf, *pass_buf;
-	DWORD user_buf_size = 0, pass_buf_size = 0;
-	CREDENTIALW **creds, *cred = NULL;
+	CREDENTIALW **creds;
 	DWORD num_creds;
 	int i;
 
@@ -165,90 +163,36 @@ static void get_credential(void)
 	/* search for the first credential that matches username */
 	for (i = 0; i < num_creds; ++i)
 		if (match_cred(creds[i])) {
-			cred = creds[i];
+			write_item("username", creds[i]->UserName,
+				wcslen(creds[i]->UserName));
+			write_item("password",
+				(LPCWSTR)creds[i]->CredentialBlob,
+				creds[i]->CredentialBlobSize / sizeof(WCHAR));
 			break;
 		}
-	if (!cred)
-		return;
-
-	CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
-	    cred->CredentialBlobSize, NULL, &user_buf_size, NULL, NULL,
-	    NULL, &pass_buf_size);
-
-	user_buf = xmalloc(user_buf_size * sizeof(WCHAR));
-	pass_buf = xmalloc(pass_buf_size * sizeof(WCHAR));
-
-	if (!CredUnPackAuthenticationBufferW(0, cred->CredentialBlob,
-	    cred->CredentialBlobSize, user_buf, &user_buf_size, NULL, NULL,
-	    pass_buf, &pass_buf_size))
-		die("CredUnPackAuthenticationBuffer failed");
 
 	CredFree(creds);
-
-	/* zero-terminate (sizes include zero-termination) */
-	user_buf[user_buf_size - 1] = L'\0';
-	pass_buf[pass_buf_size - 1] = L'\0';
-
-	write_item("username", user_buf);
-	write_item("password", pass_buf);
-
-	free(user_buf);
-	free(pass_buf);
-}
-
-static void write_attr(CREDENTIAL_ATTRIBUTEW *attr, const WCHAR *keyword,
-    const char *value)
-{
-	attr->Keyword = (LPWSTR)keyword;
-	attr->Flags = 0;
-	attr->ValueSize = strlen(value) + 1; /* store zero-termination */
-	attr->Value = (LPBYTE)value;
 }
 
 static void store_credential(void)
 {
 	CREDENTIALW cred;
-	BYTE *auth_buf;
-	DWORD auth_buf_size = 0;
-	CREDENTIAL_ATTRIBUTEW attrs[CRED_MAX_ATTRIBUTES];
 
 	if (!wusername || !password)
 		return;
 
-	/* query buffer size */
-	CredPackAuthenticationBufferW(0, wusername, password,
-	    NULL, &auth_buf_size);
-
-	auth_buf = xmalloc(auth_buf_size);
-
-	if (!CredPackAuthenticationBufferW(0, wusername, password,
-	    auth_buf, &auth_buf_size))
-		die("CredPackAuthenticationBuffer failed");
-
 	cred.Flags = 0;
 	cred.Type = CRED_TYPE_GENERIC;
 	cred.TargetName = target;
 	cred.Comment = L"saved by git-credential-wincred";
-	cred.CredentialBlobSize = auth_buf_size;
-	cred.CredentialBlob = auth_buf;
+	cred.CredentialBlobSize = (wcslen(password)) * sizeof(WCHAR);
+	cred.CredentialBlob = (LPVOID)password;
 	cred.Persist = CRED_PERSIST_LOCAL_MACHINE;
-	cred.AttributeCount = 1;
-	cred.Attributes = attrs;
+	cred.AttributeCount = 0;
+	cred.Attributes = NULL;
 	cred.TargetAlias = NULL;
 	cred.UserName = wusername;
 
-	write_attr(attrs, L"git_protocol", protocol);
-
-	if (host) {
-		write_attr(attrs + cred.AttributeCount, L"git_host", host);
-		cred.AttributeCount++;
-	}
-
-	if (path) {
-		write_attr(attrs + cred.AttributeCount, L"git_path", path);
-		cred.AttributeCount++;
-	}
-
 	if (!CredWriteW(&cred, 0))
 		die("CredWrite failed");
 }
@@ -298,13 +242,12 @@ static void read_credential(void)
 		*v++ = '\0';
 
 		if (!strcmp(buf, "protocol"))
-			protocol = xstrdup(v);
+			protocol = utf8_to_utf16_dup(v);
 		else if (!strcmp(buf, "host"))
-			host = xstrdup(v);
+			host = utf8_to_utf16_dup(v);
 		else if (!strcmp(buf, "path"))
-			path = xstrdup(v);
+			path = utf8_to_utf16_dup(v);
 		else if (!strcmp(buf, "username")) {
-			username = xstrdup(v);
 			wusername = utf8_to_utf16_dup(v);
 		} else if (!strcmp(buf, "password"))
 			password = utf8_to_utf16_dup(v);
@@ -333,22 +276,20 @@ int main(int argc, char *argv[])
 		return 0;
 
 	/* prepare 'target', the unique key for the credential */
-	strncat(target_buf, "git:", sizeof(target_buf));
-	strncat(target_buf, protocol, sizeof(target_buf));
-	strncat(target_buf, "://", sizeof(target_buf));
-	if (username) {
-		strncat(target_buf, username, sizeof(target_buf));
-		strncat(target_buf, "@", sizeof(target_buf));
+	wcscpy(target, L"git:");
+	wcsncat(target, protocol, ARRAY_SIZE(target));
+	wcsncat(target, L"://", ARRAY_SIZE(target));
+	if (wusername) {
+		wcsncat(target, wusername, ARRAY_SIZE(target));
+		wcsncat(target, L"@", ARRAY_SIZE(target));
 	}
 	if (host)
-		strncat(target_buf, host, sizeof(target_buf));
+		wcsncat(target, host, ARRAY_SIZE(target));
 	if (path) {
-		strncat(target_buf, "/", sizeof(target_buf));
-		strncat(target_buf, path, sizeof(target_buf));
+		wcsncat(target, L"/", ARRAY_SIZE(target));
+		wcsncat(target, path, ARRAY_SIZE(target));
 	}
 
-	target = utf8_to_utf16_dup(target_buf);
-
 	if (!strcmp(argv[1], "get"))
 		get_credential();
 	else if (!strcmp(argv[1], "store"))

-- 
1.8.0.msysgit.0.4.g4e40dea

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
Previous: Karsten BleesNext: Erik Faye-Lund
Message 14 of 15 in “wincred: improve compatibility with windows versions”
  1. wincred: improve compatibility with windows versionsKarsten Blees, Jan 4, 2013
  2. Erik Faye-LundJan 4, 2013
  3. Karsten BleesJan 8, 2013
  4. Erik Faye-LundJan 8, 2013
  5. 0/2 improve-wincred-compatibilityKarsten Blees, Jan 10, 2013
  6. Erik Faye-LundJan 11, 2013
  7. Junio C HamanoFeb 25, 2013
  8. Karsten BleesFeb 25, 2013
  9. Junio C HamanoFeb 25, 2013
  10. Erik Faye-LundFeb 26, 2013
  11. Junio C HamanoFeb 26, 2013
  12. Johannes SchindelinFeb 26, 2013
  13. 1/2 wincred: accept CRLF on stdin to simplify console usageKarsten Blees, Jan 10, 2013
  14. 2/2 wincred: improve compatibility with windows versionsKarsten Blees, Jan 10, 2013
  15. Erik Faye-LundSep 10, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.