Skip to content

Commit 09ddfae

Browse files
committed
generic: ask if NTLM support should be enabled
In addressing CVE-2025-66413, Git for Windows has decided to disable NTLM support by default (very sensible!). When NTLM is detected, but not permitted by Git, we get the following input from Git: ``` ntlm=suppressed ``` When we see this, we show a warning to the user that NTLM has been disabled in Git and ask them what they'd like to do. We have a few options: 1) Do nothing - no NTLM enabled. 2) Re-enable NTLM just this once. If we echo back to Git `ntlm=allow` then it will permit NTLM again, for this specific request. 3) Re-enable NTLM permanently for this remote. If we set `http.<URL>.allowNTLMAuth` to `true` then Git will no longer prevent NTLM for that remote. Present these options, along with a warning about NTLM's insecure nature, with links for more information. Signed-off-by: Matthew John Cheetham <mjcheetham@outlook.com>
1 parent e83ba83 commit 09ddfae

File tree

7 files changed

+407
-2
lines changed

7 files changed

+407
-2
lines changed

src/shared/Core.Tests/GenericHostProviderTests.cs

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,136 @@ public async Task GenericHostProvider_CreateCredentialAsync_WiaNotSupported_Retu
190190
await TestCreateCredentialAsync_ReturnsBasicCredential(WindowsAuthenticationTypes.None);
191191
}
192192

193+
[WindowsFact]
194+
private static async Task GenericHostProvider_NtlmSuppressed_AllowOnce()
195+
{
196+
var input = new InputArguments(new Dictionary<string, string>
197+
{
198+
["protocol"] = "https",
199+
["host"] = "example.com",
200+
[Constants.CredentialProtocol.NtlmKey] = Constants.CredentialProtocol.NtlmSuppressed,
201+
});
202+
203+
var configKey =
204+
$"{Constants.GitConfiguration.Http.SectionName}.https://example.com.{Constants.GitConfiguration.Http.AllowNtlmAuth}";
205+
206+
var context = new TestCommandContext();
207+
context.Git.Configuration.Global.Clear();
208+
209+
var basicAuthMock = new Mock<IBasicAuthentication>();
210+
basicAuthMock.Setup(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()))
211+
.Verifiable();
212+
var wiaAuthMock = new Mock<IWindowsIntegratedAuthentication>();
213+
wiaAuthMock.Setup(x => x.GetAuthenticationTypesAsync(It.IsAny<Uri>()))
214+
.ReturnsAsync(WindowsAuthenticationTypes.Ntlm);
215+
wiaAuthMock.Setup(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()))
216+
.ReturnsAsync(NtlmSupport.Once);
217+
var oauthMock = new Mock<IOAuthAuthentication>();
218+
219+
var provider = new GenericHostProvider(context, basicAuthMock.Object, wiaAuthMock.Object, oauthMock.Object);
220+
221+
var result = await provider.GenerateCredentialAsync(input);
222+
ICredential credential = result.Credential;
223+
224+
Assert.NotNull(credential);
225+
Assert.Equal(string.Empty, credential.Account);
226+
Assert.Equal(string.Empty, credential.Password);
227+
Assert.True(result.AdditionalProperties.TryGetValue(Constants.CredentialProtocol.NtlmKey, out string ntlmValue));
228+
Assert.Equal(Constants.CredentialProtocol.NtlmAllow, ntlmValue);
229+
230+
wiaAuthMock.Verify(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()), Times.Once);
231+
basicAuthMock.Verify(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
232+
233+
Assert.False(context.Git.Configuration.Global.TryGetValue(configKey, out _));
234+
}
235+
236+
[WindowsFact]
237+
private static async Task GenericHostProvider_NtlmSuppressed_AllowAlways()
238+
{
239+
var input = new InputArguments(new Dictionary<string, string>
240+
{
241+
["protocol"] = "https",
242+
["host"] = "example.com",
243+
[Constants.CredentialProtocol.NtlmKey] = Constants.CredentialProtocol.NtlmSuppressed,
244+
});
245+
246+
var configKey =
247+
$"{Constants.GitConfiguration.Http.SectionName}.https://example.com.{Constants.GitConfiguration.Http.AllowNtlmAuth}";
248+
249+
var context = new TestCommandContext();
250+
context.Git.Configuration.Global.Clear();
251+
252+
var basicAuthMock = new Mock<IBasicAuthentication>();
253+
basicAuthMock.Setup(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()))
254+
.Verifiable();
255+
var wiaAuthMock = new Mock<IWindowsIntegratedAuthentication>();
256+
wiaAuthMock.Setup(x => x.GetAuthenticationTypesAsync(It.IsAny<Uri>()))
257+
.ReturnsAsync(WindowsAuthenticationTypes.Ntlm);
258+
wiaAuthMock.Setup(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()))
259+
.ReturnsAsync(NtlmSupport.Always);
260+
var oauthMock = new Mock<IOAuthAuthentication>();
261+
262+
var provider = new GenericHostProvider(context, basicAuthMock.Object, wiaAuthMock.Object, oauthMock.Object);
263+
264+
var result = await provider.GenerateCredentialAsync(input);
265+
ICredential credential = result.Credential;
266+
267+
Assert.NotNull(credential);
268+
Assert.Equal(string.Empty, credential.Account);
269+
Assert.Equal(string.Empty, credential.Password);
270+
Assert.True(result.AdditionalProperties.TryGetValue(Constants.CredentialProtocol.NtlmKey, out string ntlmValue));
271+
Assert.Equal(Constants.CredentialProtocol.NtlmAllow, ntlmValue);
272+
273+
wiaAuthMock.Verify(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()), Times.Once);
274+
basicAuthMock.Verify(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
275+
276+
Assert.True(context.Git.Configuration.Global.TryGetValue(configKey, out IList<string> configValues));
277+
string configValue = Assert.Single(configValues);
278+
Assert.True(configValue.IsTruthy());
279+
}
280+
281+
[WindowsFact]
282+
private static async Task GenericHostProvider_NtlmSuppressed_Disabled()
283+
{
284+
var input = new InputArguments(new Dictionary<string, string>
285+
{
286+
["protocol"] = "https",
287+
["host"] = "example.com",
288+
[Constants.CredentialProtocol.NtlmKey] = Constants.CredentialProtocol.NtlmSuppressed,
289+
});
290+
291+
var configKey =
292+
$"{Constants.GitConfiguration.Http.SectionName}.https://example.com.{Constants.GitConfiguration.Http.AllowNtlmAuth}";
293+
294+
var context = new TestCommandContext();
295+
context.Git.Configuration.Global.Clear();
296+
297+
var basicAuthMock = new Mock<IBasicAuthentication>();
298+
basicAuthMock.Setup(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()))
299+
.ReturnsAsync(new GitCredential("testUser", "testPassword"));
300+
var wiaAuthMock = new Mock<IWindowsIntegratedAuthentication>();
301+
wiaAuthMock.Setup(x => x.GetAuthenticationTypesAsync(It.IsAny<Uri>()))
302+
.ReturnsAsync(WindowsAuthenticationTypes.Ntlm);
303+
wiaAuthMock.Setup(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()))
304+
.ReturnsAsync(NtlmSupport.Disabled);
305+
var oauthMock = new Mock<IOAuthAuthentication>();
306+
307+
var provider = new GenericHostProvider(context, basicAuthMock.Object, wiaAuthMock.Object, oauthMock.Object);
308+
309+
var result = await provider.GenerateCredentialAsync(input);
310+
ICredential credential = result.Credential;
311+
312+
Assert.NotNull(credential);
313+
Assert.Equal("testUser", credential.Account);
314+
Assert.Equal("testPassword", credential.Password);
315+
Assert.False(result.AdditionalProperties.TryGetValue(Constants.CredentialProtocol.NtlmKey, out _));
316+
317+
wiaAuthMock.Verify(x => x.AskEnableNtlmAsync(It.IsAny<Uri>()), Times.Once);
318+
basicAuthMock.Verify(x => x.GetCredentialsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Once);
319+
320+
Assert.False(context.Git.Configuration.Global.TryGetValue(configKey, out _));
321+
}
322+
193323
[Fact]
194324
public async Task GenericHostProvider_GenerateCredentialAsync_OAuth_CompleteOAuthConfig_UsesOAuth()
195325
{

src/shared/Core/Authentication/WindowsIntegratedAuthentication.cs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,27 @@
11
using System;
22
using System.Net.Http;
33
using System.Net.Http.Headers;
4+
using System.Threading;
45
using System.Threading.Tasks;
6+
using GitCredentialManager.UI;
7+
using GitCredentialManager.UI.ViewModels;
8+
using GitCredentialManager.UI.Views;
59

610
namespace GitCredentialManager.Authentication
711
{
812
public interface IWindowsIntegratedAuthentication : IDisposable
913
{
14+
Task<NtlmSupport> AskEnableNtlmAsync(Uri uri);
1015
Task<WindowsAuthenticationTypes> GetAuthenticationTypesAsync(Uri uri);
1116
}
1217

18+
public enum NtlmSupport
19+
{
20+
Once,
21+
Always,
22+
Disabled,
23+
}
24+
1325
[Flags]
1426
public enum WindowsAuthenticationTypes
1527
{
@@ -31,6 +43,44 @@ public class WindowsIntegratedAuthentication : AuthenticationBase, IWindowsInteg
3143
public WindowsIntegratedAuthentication(ICommandContext context)
3244
: base(context) { }
3345

46+
public async Task<NtlmSupport> AskEnableNtlmAsync(Uri uri)
47+
{
48+
ThrowIfUserInteractionDisabled();
49+
50+
if (Context.SessionManager.IsDesktopSession && Context.Settings.IsGuiPromptsEnabled)
51+
{
52+
// Note: we do not support the UI helper for WIA so always show the in-proc GUI
53+
var vm = new EnableNtlmViewModel(Context.SessionManager)
54+
{
55+
Url = uri.ToString(),
56+
};
57+
await AvaloniaUi.ShowViewAsync<EnableNtlmView>(vm, GetParentWindowHandle(), CancellationToken.None);
58+
ThrowIfWindowCancelled(vm);
59+
60+
return vm.SelectedOption;
61+
}
62+
63+
ThrowIfTerminalPromptsDisabled();
64+
65+
var menu = new TerminalMenu(Context.Terminal, "Re-enable NTLM support in Git?");
66+
TerminalMenuItem onceItem = menu.Add("Yes - just this time");
67+
TerminalMenuItem alwaysItem = menu.Add($"Yes - always for {uri}");
68+
TerminalMenuItem noItem = menu.Add("No - do not enable NTLM");
69+
TerminalMenuItem choice = menu.Show(0);
70+
71+
if (choice == onceItem)
72+
{
73+
return NtlmSupport.Once;
74+
}
75+
76+
if (choice == alwaysItem)
77+
{
78+
return NtlmSupport.Always;
79+
}
80+
81+
return NtlmSupport.Disabled;
82+
}
83+
3484
public async Task<WindowsAuthenticationTypes> GetAuthenticationTypesAsync(Uri uri)
3585
{
3686
EnsureArgument.AbsoluteUri(uri, nameof(uri));

src/shared/Core/Constants.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ public static class Constants
3131
/// </summary>
3232
public static readonly Guid MsaTransferTenantId = new("f8cdef31-a31e-4b4a-93e4-5f571e91255a");
3333

34+
public static class CredentialProtocol
35+
{
36+
public const string NtlmKey = "ntlm";
37+
public const string NtlmSuppressed = "suppressed";
38+
public const string NtlmAllow = "allow";
39+
}
40+
3441
public static class CredentialStoreNames
3542
{
3643
public const string WindowsCredentialManager = "wincredman";
@@ -191,6 +198,7 @@ public static class Http
191198
public const string SslCaInfo = "sslCAInfo";
192199
public const string SslAutoClientCert = "sslAutoClientCert";
193200
public const string CookieFile = "cookieFile";
201+
public const string AllowNtlmAuth = "allowNTLMAuth";
194202
}
195203

196204
public static class Remote
@@ -232,6 +240,7 @@ public static class HelpUrls
232240
public const string GcmDefaultAccount = "https://aka.ms/gcm/defaultaccount";
233241
public const string GcmMultipleUsers = "https://aka.ms/gcm/multipleusers";
234242
public const string GcmUnsafeRemotes = "https://aka.ms/gcm/unsaferemotes";
243+
public const string GcmNtlm = "https://aka.ms/gcm/ntlm";
235244
}
236245

237246
private static Version _gcmVersion;

src/shared/Core/GenericHostProvider.cs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,11 +183,49 @@ await GetOAuthAccessToken(uri, input.UserName, oauthConfig, _context.Trace2)
183183
}
184184
else
185185
{
186-
_context.Trace.WriteLine("Host supports WIA - generating empty credential...");
186+
_context.Trace.WriteLine("Host supports WIA.");
187+
188+
var additionalProps = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
189+
190+
// Has Git suppressed its own built-in NTLM authentication support?
191+
if (input.TryGetArgument(Constants.CredentialProtocol.NtlmKey, out string ntlmArg) &&
192+
StringComparer.OrdinalIgnoreCase.Equals(Constants.CredentialProtocol.NtlmSuppressed, ntlmArg))
193+
{
194+
_context.Trace.WriteLine("NTLM support has been suppressed by Git - showing warning.");
195+
196+
// Show a warning that NTLM authentication will not work without Git's built-in support
197+
// and ask the user what they want to do about it.
198+
NtlmSupport ntlmSupport = await _winAuth.AskEnableNtlmAsync(uri);
199+
switch (ntlmSupport)
200+
{
201+
case NtlmSupport.Once:
202+
_context.Trace.WriteLine("Enabling NTLM support just once.");
203+
additionalProps[Constants.CredentialProtocol.NtlmKey] =
204+
Constants.CredentialProtocol.NtlmAllow;
205+
break;
206+
207+
case NtlmSupport.Always:
208+
_context.Trace.WriteLine($"Enabling NTLM support for {uri}.");
209+
additionalProps[Constants.CredentialProtocol.NtlmKey] =
210+
Constants.CredentialProtocol.NtlmAllow;
211+
EnableNtlmSupport(uri);
212+
break;
213+
214+
default:
215+
_context.Trace.WriteLine("User declined to enable NTLM support. Showing basic auth prompt.");
216+
return new GetCredentialResult(
217+
await _basicAuth.GetCredentialsAsync(uri.AbsoluteUri, null)
218+
);
219+
}
220+
}
187221

188222
// WIA is signaled to Git using an empty username/password
223+
_context.Trace.WriteLine("Returning empty username/password to trigger current user auth with WIA.");
189224
ICredential creds = new GitCredential(string.Empty, string.Empty);
190-
return new GetCredentialResult(creds);
225+
return new GetCredentialResult(creds)
226+
{
227+
AdditionalProperties = additionalProps
228+
};
191229
}
192230
}
193231
else
@@ -208,6 +246,21 @@ await _basicAuth.GetCredentialsAsync(uri.AbsoluteUri, input.UserName)
208246
);
209247
}
210248

249+
private void EnableNtlmSupport(Uri uri)
250+
{
251+
string url = uri.AbsoluteUri.TrimEnd('/');
252+
IGitConfiguration config = _context.Git.GetConfiguration();
253+
string key = $"{Constants.GitConfiguration.Http.SectionName}.{url}.{Constants.GitConfiguration.Http.AllowNtlmAuth}";
254+
255+
try
256+
{
257+
config.Set(GitConfigurationLevel.Global, key, "true");
258+
}
259+
catch (Exception ex)
260+
{
261+
_context.Trace.WriteLine($"Failed to set Git configuration to enable NTLM support for {uri}");
262+
_context.Trace.WriteException(ex);
263+
}
211264
}
212265

213266
private async Task<ICredential> GetOAuthAccessToken(Uri remoteUri, string userName, GenericOAuthConfig config, ITrace2 trace2)
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
using System.Windows.Input;
2+
using GitCredentialManager.Authentication;
3+
4+
namespace GitCredentialManager.UI.ViewModels;
5+
6+
public class EnableNtlmViewModel : WindowViewModel
7+
{
8+
private readonly ISessionManager _sessionManager;
9+
10+
private string _url;
11+
private ICommand _onceCommand;
12+
private ICommand _alwaysCommand;
13+
private ICommand _noCommand;
14+
private ICommand _learnMoreCommand;
15+
16+
public EnableNtlmViewModel()
17+
{
18+
// For designer only
19+
}
20+
21+
public EnableNtlmViewModel(ISessionManager sessionManager) : this()
22+
{
23+
_sessionManager = sessionManager;
24+
25+
OnceCommand = new RelayCommand(() => SelectOption(NtlmSupport.Once));
26+
AlwaysCommand = new RelayCommand(() => SelectOption(NtlmSupport.Always));
27+
NoCommand = new RelayCommand(() => SelectOption(NtlmSupport.Disabled));
28+
LearnMoreCommand = new RelayCommand(() => sessionManager.OpenBrowser(Constants.HelpUrls.GcmNtlm));
29+
}
30+
31+
private void SelectOption(NtlmSupport option)
32+
{
33+
SelectedOption = option;
34+
Accept();
35+
}
36+
37+
public NtlmSupport SelectedOption { get; private set; }
38+
39+
public string Url
40+
{
41+
get => _url;
42+
set => SetAndRaisePropertyChanged(ref _url, value);
43+
}
44+
45+
public ICommand OnceCommand
46+
{
47+
get => _onceCommand;
48+
set => SetAndRaisePropertyChanged(ref _onceCommand, value);
49+
}
50+
51+
public ICommand AlwaysCommand
52+
{
53+
get => _alwaysCommand;
54+
set => SetAndRaisePropertyChanged(ref _alwaysCommand, value);
55+
}
56+
57+
public ICommand NoCommand
58+
{
59+
get => _noCommand;
60+
set => SetAndRaisePropertyChanged(ref _noCommand, value);
61+
}
62+
63+
public ICommand LearnMoreCommand
64+
{
65+
get => _learnMoreCommand;
66+
set => SetAndRaisePropertyChanged(ref _learnMoreCommand, value);
67+
}
68+
}

0 commit comments

Comments
 (0)