Git-Portable enables password storage by default

[ousia]

• I was not able to find an open or closed issue matching what I'm seeing

Sorry, but I downloaded PortableGit-2.21.0-64-bit.7z.exe and I decompressed on a computer to help another person.

I cloned a private repository of mine at GitLab and I saw that I had to commit a fix. I was surprised that Windows had stored my password.

I didn’t know why, but I discovered that the Windows Credential Manager has stored this information in the user account I was using (as generic credentials).

I found the way to instruct git so that it doesn’t let Windows to store remote account info (thanks to https://stackoverflow.com/a/37185202).

Excuse me, but for me it is almost impossible not to see that as a security issue. Or at least my question would be:

Do you think that it is a sensible policy that PortableGit allows user and password storage by default?

Please, consider that the portable version is perfect to be used in computers to which the user has only temporary and/or restricted access.

[dscho]

Please, consider that the portable version is perfect to be used in computers to which the user has only temporary and/or restricted access.

True. Will you fix it?

[ousia]

Sorry, I’m only an average computer user. I wish I could.

I use git for ConTeXt code, such as http://www.from-pandoc-to-context.tk.

[PhilipOakley]

hi @ousia, It should be fairly easy (I hope). The place to start is the Software Development Kit and it's download.

The SDK's build-extra repo contains all the bits that tweak the installation. There are instructions in the wiki (inc FAQs) to give some clues to finding the part that sets up the defaults.

Even if you don't get all the way, at least report what you found and where you got stuck so we can all help each other 'scratch the itch'.

To answer the question about whether it is a sensible policy, It does sound sensible. hopefully as we dig you will be able to point out any options that may check if the credentials 'match' the user in place.

Thank you for any help you can give.

[ousia]

@PhilipOakley,

many thanks for your reply. I’m afraid I only use Windows on very rare occasions, especially when I help other people.

But I found https://github.com/git-for-windows/build-extra/blob/master/portable/release.sh.

Removing lines 93 to 95 would do the job, I think.

This would mean to replace:

mkdir -p "$SCRIPT_PATH/root/mingw$BITNESS/etc" &&
cp /mingw$BITNESS/etc/gitconfig \
	"$SCRIPT_PATH/root/mingw$BITNESS/etc/gitconfig" &&
git config -f "$SCRIPT_PATH/root/mingw$BITNESS/etc/gitconfig" \
	credential.helper manager ||
die "Could not configure Git-Credential-Manager as default"
test 64 != $BITNESS ||
git config -f "$SCRIPT_PATH/root/mingw$BITNESS/etc/gitconfig" --unset pack.packSizeLimit

with

mkdir -p "$SCRIPT_PATH/root/mingw$BITNESS/etc" &&
cp /mingw$BITNESS/etc/gitconfig \
	"$SCRIPT_PATH/root/mingw$BITNESS/etc/gitconfig"
test 64 != $BITNESS ||
git config -f "$SCRIPT_PATH/root/mingw$BITNESS/etc/gitconfig" --unset pack.packSizeLimit

Is this fine? Would you want me to submit a PR to that repo?

[ousia]

To answer the question about whether it is a sensible policy, It does sound sensible. hopefully as we dig you will be able to point out any options that may check if the credentials 'match' the user in place.

I’m afraid I don’t get it, @PhilipOakley. Maybe my understanding on how PortableGit and Windows work is very limited.

But imagine that I have the following scenario: I clone the repository to a foreign computer in a foreign session. I use PortableGit to leave (almost) no traces.

I might remove the .git directory to delete the data for the repo. But Windows doesn’t give any hint about account data being stored on the computer.

So I’m leaving full access to my account on a foreign computer even without noticing it. I’d the only one to blame for whatever might happen next.

Sorry, but for a portable application, I think this default is insane.

But I might be missing something here. So I would be very pleased, if I were corrected about what I’m failing to consider here.

Many thanks again for your help.

[PhilipOakley]

Hi Pablo, @ousia,
The alternate XOR case I think I was considering was for the situation where the user is at their own PC but is trying a different version of Git-for-Windows, so use of the Cred Manager would be reasonable - in that case - but clearly not for your case. Hence the suggestion to consider if the question should be asked of the user.

It's very easy to accidentally take one use case as being the only use case and take it a little too far, hence the caution.

That said, thank you for the initial PR. I think dscho noted that the && chain should be added back (then simply force push it, while deleting the 'DCO robo text', assuming you are able to confirm that you can sign the small patch.

I didn't check the wider context to see if the patch was just for the portable version, which may be worth mentioning in the commit message, along with the rationale that started the issue. It will help future readers of the code.

Many thanks for your work on this.

[ousia]

@PhilipOakley, as you may have alread read git-for-windows/build-extra#241, the approach needs to be different. It really needs more than removing a previous patch.

I wanted to help with the issue I reported. Needless to say, I reported the issue only for the improvement of the project (I had solved my issue hours before). But coding is not something I can do now and I cannot afford learning.

Imagine that Norwegian would be a requirement to post issues in this project. Well, I might do it in Spanish, English or even German, but Norwegian would be too much.

I’m afraid that coding will be Norwegian for me.

[dscho]

I’m afraid that coding will be Norwegian for me.

:-)

As I commented in the PR you opened, I have a more complete solution in mind, and I respect that C would be Norwegian to you ;-)

FWIW I started to investigate what it would take to make a minimal chooser in C, based on the Win32 API...

[dscho]

I started to investigate what it would take to make a minimal chooser in C, based on the Win32 API...

I guess I should not distribute my comments over two tickets. I was referring to this comment:

[...] we could implement a meta credential helper and configure that by default: the first time Git asks for credentials, it would pop up and list all available credential helpers, letting the user choose which one to use (and offering to optionally configure that helper in the portable Git's system config to avoid calling the meta helper again).

[dscho]

Okay, mostly following http://zetcode.com/gui/winapi/, I came up with this code so far (compile with gcc -municode -DUNICODE -Wall -Werror -o git-credential-chooser.exe git-credential-chooser.c -lgdi32 -lcomctl32:

#include <windows.h>

static HINSTANCE instance;
static HWND remember_this_choice_checkbox;
static LPWSTR *helper_name, *helper_path;
static size_t helper_nr, selected_helper;

static int discover_helpers(void)
{
	/* TODO: walk `PATH` to populer these */
	helper_nr = 2;
	helper_name = malloc(helper_nr * sizeof(*helper_name));
	helper_path = malloc(helper_nr * sizeof(*helper_name));
	if (!helper_name || !helper_path) {
		MessageBoxW(NULL, L"Out of memory!", L"Error", MB_OK);
		return -1;
	}
	helper_name[0] = L"<no helper>";
	helper_path[0] = L"<none>";
	helper_name[1] = L"manager";
	helper_path[1] = L"C:\\Program Files\\Git\\mingw64\\libexec\\git-core\\git-credential-manager.exe";
	return 0;
}

static int width = 400, height = 300;
static int offset_x = 10, offset_y = 10;
static int line_height = 25, line_offset_y = 5;

static LRESULT CALLBACK window_proc(HWND hwnd, UINT message, WPARAM wParam,
				    LPARAM lParam)
{
	switch (message) {
	case WM_CREATE: {
		HWND hwnd2;
		RECT rect;
		int width;
		size_t i;

		GetClientRect(hwnd, &rect);
		width = rect.right - rect.left;
		CreateWindowW(L"Button", L"Choose credential helper",
			      WS_CHILD | WS_VISIBLE | BS_GROUPBOX,
			      offset_x, offset_y,
			      width - 2 * offset_x,
			      line_height * helper_nr + 2 * offset_x,
			      hwnd, (HMENU) 0, instance, NULL);
		for (i = 0; i < helper_nr; i++) {
			hwnd2 = CreateWindowW(L"Button", helper_name[i],
					      WS_CHILD | WS_VISIBLE |
					      BS_AUTORADIOBUTTON,
					      2 * offset_x,
					      3 * offset_y + line_height * i,
					      width - 4 * offset_x,
					      line_height + line_offset_y,
					      hwnd, (HMENU)i, instance, NULL);
			if (!wcscmp(helper_name[i], L"manager")) {
				SendMessage(hwnd2, BM_SETCHECK, BST_CHECKED, 1);
				SetFocus(hwnd2);
			}
		}

		remember_this_choice_checkbox =
			CreateWindowW(L"button", L"Remember this choice",
				      WS_VISIBLE | WS_CHILD | BS_CHECKBOX,
				      2 * offset_x,
				      3 * offset_y + line_height * helper_nr,
				      width - 2 * offset_x,
				      line_height + line_offset_y,
				      hwnd, (HMENU) helper_nr, NULL, NULL);
		break;
	}

	case WM_COMMAND:
		if (wParam == helper_nr) {
			if (IsDlgButtonChecked(hwnd, helper_nr))
				CheckDlgButton(hwnd, helper_nr, BST_UNCHECKED);
			else
				CheckDlgButton(hwnd, helper_nr, BST_CHECKED);
		} else if (HIWORD(wParam) == BN_CLICKED)
			selected_helper = LOWORD(wParam);
		break;

	case WM_DESTROY:
		PostQuitMessage(0);
		break;
	}

	return DefWindowProcW(hwnd, message, wParam, lParam);
}

int WINAPI wWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
		    PWSTR lpCmdLine, int nCmdShow)
{
	WNDCLASSW window_class = { 0 };
	MSG message;

	window_class.lpszClassName = L"CredentialHelperChooser";
	window_class.hInstance = hInstance;
	window_class.lpfnWndProc = window_proc;
	window_class.hbrBackground = GetSysColorBrush(COLOR_3DFACE);
	window_class.hCursor = LoadCursor(0, IDC_ARROW);

	discover_helpers();

	instance = hInstance;
	RegisterClassW(&window_class);
	CreateWindowW(window_class.lpszClassName, L"CredentialHelperChooser",
		      WS_OVERLAPPEDWINDOW | WS_VISIBLE,
		      CW_USEDEFAULT, CW_USEDEFAULT,
		      width, height, 0, 0, hInstance, 0);

	message.wParam = 1;
	while (GetMessage(&message, NULL, 0, 0)) {
		TranslateMessage(&message);
		DispatchMessage(&message);
	}

	return (int)message.wParam;
}

Note: it does not do anything yet, it's just a start of the UI, and it even lacks keyboard handling. It's a start, is all it is.

I will not have time to work on this for at least a couple of days, so: if there is any volunteer, please do pick it up!

[dscho]

I just integrated all of this, and the next snapshot (or Git for Windows v2.22.0-rc0, which I expect to land this Monday) will have it!

It will come as the default credential helper in PortableGit, and will let the user choose which of the available credential helpers to use (with an option to persist the choice into the PortableGit's "system" config).

[dscho]

Once https://dev.azure.com/Git-for-Windows/git/_build/results?buildId=36148 is has succeeded (which it hopefully will 😄; it typically takes around 50 minutes to complete), there will be a new snapshot at https://wingit.blob.core.windows.net/files/index.html. @ousia could I trouble you to give it a try?

[ousia]

@dscho, of course I’m going to check it.

Many thanks for your implementation.

I hope I can have access to a Windows machine next Monday or Tuesday.

[ousia]

@dscho,

I’m afraid to write that it didn’t work as expected. Windows asked for credentials.

It was a company laptop and Windows 7 complained about unfinished installation (although git seemed to work fine when cloning a private repository).

Probably my testing conditions (some Windows configuration on that laptop) might have caused the issue.

Sorry, but I don’t have another option to test this on a Windows system.

[dscho]

It was a company laptop and Windows 7 complained about unfinished installation

Did you test the portable Git?

[ousia]

Did you test the portable Git?

I used this binary this binary: https://wingit.blob.core.windows.net/files/PortableGit-prerelease-2.21.0.windows.1.230.gd4e5e1ea92-64-bit.7z.exe.

[dscho]

Windows 7 complained about unfinished installation

Could you please provide more details?

[dscho]

Also, when you call the git-bash.exe from the installed Portable Git, what does git config --show-origin credential.helper say?

[dscho]

Windows asked for credentials.

Please, please, please more details? We're almost at v2.22.0 final and without you working with me to resolve this, we will ship a Git for Windows v2.22.0 with this issue. I won't stop the release for this minor bug, but I will hate every minute of an unfixed v2.22.0.

[ousia]

This is what I get:

$ git version
git version 2.21.0.windows.1.227.ge3538d8eb3
$ git config --show-origin credential.helper
file:C:/PortableGit/mingw64/etc/gitconfig    manager

The warning about unfinished installation doesn’t show up anymore.

Sorry for the delay, but having access to a Windows computer isn’t easy for me right now.

[dscho]

And this is what I get when I download the latest snapshot from https://wingit.blob.core.windows.net/files/PortableGit-prerelease-2.21.0.windows.1.698.g046e6ffe7c-64-bit.7z.exe:

$ git version
git version 2.21.0.windows.1.698.g046e6ffe7c
$ git config --show-origin credential.helper
file:C:/Users/me/Desktop/PortableGit-helper-selector/mingw64/etc/gitconfig helper-selector

(I installed into Desktop\PortableGit-helper-selector.) And when I do git ls-remote https://github.com/does/not-exist (the invalid URL will force Git to authenticate just in case that it is a private repository, I am greeted (as I expected) with this:

So I am afraid that I cannot reproduce the problem here. Maybe it is just a matter of using an even more recent snapshot. Dunno.

[ousia]

@dscho, I cannot promise anything, but tomorrow I will try to check it (if I get a Windows computer).

[ousia]

@dscho, everything works perfectly fine with the file you pointed to (https://wingit.blob.core.windows.net/files/PortableGit-prerelease-2.21.0.windows.1.698.g046e6ffe7c-64-bit.7z.exe).

Many thanks for your help and for the implementation.