From de7b93ba5d2066c0c1980ea163c95f3239dc7cf3 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Tue, 11 Jul 2023 15:40:09 -0700 Subject: [PATCH 1/2] wincredmgr: only match cred entries with correct namespace GCM by default creates entries in the Windows Credential Manager on Windows, and prefixes the 'target name' of the entry with "git:". This 'namespace' prefix is configurable, but is not often changed in practice outside of tests. Visual Studio, when adding GitHub accounts (either natively or by the older GitHub extension for VS), it creates three credential entries: 1. GitHub for Visual Studio - https://github.com 2. git:https://github.com 3. https://github.com Entry 1 is used by VS for it's own purposes. Entry 2 is created for the benefit for GCM, so that we are 'primed'. It is unknown what entry 3 is for at this time. There is an error in our existing logic for enumerating credentials that is also matching entry 3 as well as the expected entry 2. Modify and fix the matching logic to ensure that the namespace prefix matches, rather than just stripping it and matching (even if it doesn't exist!). --- .../Core/Interop/Windows/WindowsCredentialManager.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/shared/Core/Interop/Windows/WindowsCredentialManager.cs b/src/shared/Core/Interop/Windows/WindowsCredentialManager.cs index 5fa366e8a..63e9c9560 100644 --- a/src/shared/Core/Interop/Windows/WindowsCredentialManager.cs +++ b/src/shared/Core/Interop/Windows/WindowsCredentialManager.cs @@ -274,11 +274,19 @@ private WindowsCredential CreateCredentialFromStructure(Win32Credential credenti return false; } - // Trim the "LegacyGeneric" prefix Windows adds and any namespace we have been filtered with + // Trim the "LegacyGeneric" prefix Windows adds string targetName = credential.TargetName.TrimUntilIndexOf(TargetNameLegacyGenericPrefix); + + // Only match credentials with the namespace we have been configured with (if any) if (!string.IsNullOrWhiteSpace(_namespace)) { - targetName = targetName.TrimUntilIndexOf($"{_namespace}:"); + string nsPrefix = $"{_namespace}:"; + if (!targetName.StartsWith(nsPrefix, StringComparison.Ordinal)) + { + return false; + } + + targetName = targetName.Substring(nsPrefix.Length); } // If the target name matches the service name exactly then return 'match' From 88e1297d855362c74e50d71ddd8d1b2f05aaefc9 Mon Sep 17 00:00:00 2001 From: Matthew John Cheetham Date: Wed, 12 Jul 2023 07:46:56 -0700 Subject: [PATCH 2/2] wincred: add tests for namespace matching of creds --- .../Windows/WindowsCredentialManagerTests.cs | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/src/shared/Core.Tests/Interop/Windows/WindowsCredentialManagerTests.cs b/src/shared/Core.Tests/Interop/Windows/WindowsCredentialManagerTests.cs index 78295276e..b7b5cef62 100644 --- a/src/shared/Core.Tests/Interop/Windows/WindowsCredentialManagerTests.cs +++ b/src/shared/Core.Tests/Interop/Windows/WindowsCredentialManagerTests.cs @@ -270,6 +270,70 @@ public void WindowsCredentialManager_IsMatch( Assert.Equal(expected, actual); } + [PlatformFact(Platforms.Windows)] + public void WindowsCredentialManager_IsMatch_NoNamespace_NotMatched() + { + var win32Cred = new Win32Credential + { + UserName = "test", + TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com" + }; + + var credManager = new WindowsCredentialManager(TestNamespace); + + bool result = credManager.IsMatch("https://example.com", null, win32Cred); + + Assert.False(result); + } + + [PlatformFact(Platforms.Windows)] + public void WindowsCredentialManager_IsMatch_DifferentNamespace_NotMatched() + { + var win32Cred = new Win32Credential + { + UserName = "test", + TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:random-namespace:https://example.com" + }; + + var credManager = new WindowsCredentialManager(TestNamespace); + + bool result = credManager.IsMatch("https://example.com", null, win32Cred); + + Assert.False(result); + } + + [PlatformFact(Platforms.Windows)] + public void WindowsCredentialManager_IsMatch_CaseSensitiveNamespace_NotMatched() + { + var win32Cred = new Win32Credential + { + UserName = "test", + TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}:nAmEsPaCe:https://example.com" + }; + + var credManager = new WindowsCredentialManager("namespace"); + + bool result = credManager.IsMatch("https://example.com", null, win32Cred); + + Assert.False(result); + } + + [PlatformFact(Platforms.Windows)] + public void WindowsCredentialManager_IsMatch_NoNamespaceInQuery_IsMatched() + { + var win32Cred = new Win32Credential + { + UserName = "test", + TargetName = $"{WindowsCredentialManager.TargetNameLegacyGenericPrefix}https://example.com" + }; + + var credManager = new WindowsCredentialManager(); + + bool result = credManager.IsMatch("https://example.com", null, win32Cred); + + Assert.True(result); + } + [PlatformTheory(Platforms.Windows)] [InlineData("https://example.com", null, "https://example.com")] [InlineData("https://example.com", "bob", "https://bob@example.com")]