From 5003c15443fd165e7b34aae475d40405b51a0ef6 Mon Sep 17 00:00:00 2001 From: Martin Toman Date: Thu, 5 Dec 2024 01:39:28 +0000 Subject: [PATCH] Initial draft --- src/Agent.Sdk/AgentWebProxy.cs | 19 ++++++++++++++++++- src/Agent.Sdk/Knob/AgentKnobs.cs | 6 ++++++ .../AgentCertificateManager.cs | 6 +++++- .../LinuxAgentCredentialStore.cs | 10 ++++++++-- .../MacOSAgentCredentialStore.cs | 17 +++++++++++++---- .../NoOpAgentCredentialStore.cs | 9 +++++++-- .../WindowsAgentCredentialStore.cs | 11 +++++++++-- .../IAgentCredentialStore.cs | 6 +++++- .../VstsAgentWebProxy.cs | 19 ++++++++++++++++--- 9 files changed, 87 insertions(+), 16 deletions(-) diff --git a/src/Agent.Sdk/AgentWebProxy.cs b/src/Agent.Sdk/AgentWebProxy.cs index 3b6ce8b831..1161c90738 100644 --- a/src/Agent.Sdk/AgentWebProxy.cs +++ b/src/Agent.Sdk/AgentWebProxy.cs @@ -29,6 +29,9 @@ public class AgentWebProxy : IWebProxy public ICredentials Credentials { get; set; } + // This was added to work around malloc-related crashes on MacOS + private static readonly NetworkCredential _neverDisposedCredentialObject = new(); + public AgentWebProxy() { } @@ -48,7 +51,21 @@ public void Update(string proxyAddress, string proxyUsername, string proxyPasswo } else { - Credentials = new NetworkCredential(proxyUsername, proxyPassword); + bool avoidConstructingNetCredential = + PlatformUtil.HostOS == PlatformUtil.OS.OSX + && Boolean.TryParse(Environment.GetEnvironmentVariable("AVOID_NET_CREDENTIAL_OBJECTS_ON_MAC"), out bool ffEnabled) + && ffEnabled; + + if (avoidConstructingNetCredential) + { + _neverDisposedCredentialObject.UserName = proxyUsername; + _neverDisposedCredentialObject.Password = proxyPassword; + Credentials = _neverDisposedCredentialObject; + } + else + { + Credentials = new NetworkCredential(proxyUsername, proxyPassword); + } } if (proxyBypassList != null) diff --git a/src/Agent.Sdk/Knob/AgentKnobs.cs b/src/Agent.Sdk/Knob/AgentKnobs.cs index 25ff9db01d..df5503c249 100644 --- a/src/Agent.Sdk/Knob/AgentKnobs.cs +++ b/src/Agent.Sdk/Knob/AgentKnobs.cs @@ -779,5 +779,11 @@ public class AgentKnobs "If true, agent will use sparse checkout in checkout task.", new RuntimeKnobSource("AGENT_USE_SPARSE_CHECKOUT_IN_CHECKOUT_TASK"), new BuiltInDefaultKnobSource("false")); + + public static readonly Knob AvoidNetCredentialObjectsOnMac = new Knob( + nameof(AvoidNetCredentialObjectsOnMac), + "Eliminates construction of NetwokCredential objects on macOS, which internally suffer from malloc-related crashes", + new EnvironmentKnobSource("AVOID_NET_CREDENTIAL_OBJECTS_ON_MAC"), + new BuiltInDefaultKnobSource("false")); } } diff --git a/src/Microsoft.VisualStudio.Services.Agent/AgentCertificateManager.cs b/src/Microsoft.VisualStudio.Services.Agent/AgentCertificateManager.cs index d2a111037b..7528e1d10c 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/AgentCertificateManager.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/AgentCertificateManager.cs @@ -10,6 +10,7 @@ using Microsoft.VisualStudio.Services.WebApi; using Agent.Sdk; using Agent.Sdk.Util; +using Agent.Sdk.Knob; namespace Microsoft.VisualStudio.Services.Agent { @@ -120,6 +121,7 @@ public void SaveCertificateSetting() Trace.Info($"Store client cert private key password with lookup key {lookupKey}"); var credStore = HostContext.GetService(); + var avoidNetCredentialFF = AgentKnobs.AvoidNetCredentialObjectsOnMac.GetValue(HostContext).AsBoolean(); credStore.Write($"VSTS_AGENT_CLIENT_CERT_PASSWORD_{lookupKey}", "VSTS", ClientCertificatePassword); setting.ClientCertPasswordLookupKey = lookupKey; @@ -198,7 +200,9 @@ public void LoadCertificateSettings() if (!string.IsNullOrEmpty(certSetting.ClientCertPasswordLookupKey)) { var cerdStore = HostContext.GetService(); - ClientCertificatePassword = cerdStore.Read($"VSTS_AGENT_CLIENT_CERT_PASSWORD_{certSetting.ClientCertPasswordLookupKey}").Password; + var avoidNetCredentialFF = AgentKnobs.AvoidNetCredentialObjectsOnMac.GetValue(HostContext).AsBoolean(); + var target = $"VSTS_AGENT_CLIENT_CERT_PASSWORD_{certSetting.ClientCertPasswordLookupKey}"; + ClientCertificatePassword = avoidNetCredentialFF ? cerdStore.Read2(target).Password : cerdStore.Read(target).Password; HostContext.SecretMasker.AddValue(ClientCertificatePassword, WellKnownSecretAliases.ClientCertificatePassword); } diff --git a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/LinuxAgentCredentialStore.cs b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/LinuxAgentCredentialStore.cs index 370c1eace3..7f59d6bfba 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/LinuxAgentCredentialStore.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/LinuxAgentCredentialStore.cs @@ -68,7 +68,7 @@ public override void Initialize(IHostContext hostContext) _symmetricKey = keyBuilder.ToArray(); } - public NetworkCredential Write(string target, string username, string password) + public void Write(string target, string username, string password) { Trace.Entering(); ArgUtil.NotNullOrEmpty(target, nameof(target)); @@ -79,7 +79,6 @@ public NetworkCredential Write(string target, string username, string password) Credential cred = new Credential(username, Encrypt(password)); _credStore[target] = cred; SyncCredentialStoreFile(); - return new NetworkCredential(username, password); } public NetworkCredential Read(string target) @@ -100,6 +99,13 @@ public NetworkCredential Read(string target) throw new KeyNotFoundException(target); } + public (string UserName, string Password) Read2(string target) + { + // NetworkCredential objects cause crashes only on macOS => we can invoke the original implemantation here + var ret = Read(target); + return (ret.UserName, ret.Password); + } + public void Delete(string target) { Trace.Entering(); diff --git a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/MacOSAgentCredentialStore.cs b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/MacOSAgentCredentialStore.cs index 349a77b290..4a931329ab 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/MacOSAgentCredentialStore.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/MacOSAgentCredentialStore.cs @@ -92,7 +92,7 @@ public override void Initialize(IHostContext hostContext) } } - public NetworkCredential Write(string target, string username, string password) + public void Write(string target, string username, string password) { Trace.Entering(); ArgUtil.NotNullOrEmpty(target, nameof(target)); @@ -161,8 +161,6 @@ public NetworkCredential Write(string target, string username, string password) throw new InvalidOperationException($"'security add-generic-password' failed with exit code {exitCode}."); } } - - return new NetworkCredential(username, password); } finally { @@ -171,6 +169,17 @@ public NetworkCredential Write(string target, string username, string password) } public NetworkCredential Read(string target) + { + var cred = ReadCredentialFromKeychain(target); + return new NetworkCredential(cred.UserName, cred.Password); + } + + public (string UserName, string Password) Read2(string target) + { + return ReadCredentialFromKeychain(target); + } + + public (string UserName, string Password) ReadCredentialFromKeychain(string target) { Trace.Entering(); ArgUtil.NotNullOrEmpty(target, nameof(target)); @@ -223,7 +232,7 @@ public NetworkCredential Read(string target) Trace.Info($"Successfully find-generic-password for {target} (VSTSAGENT)"); username = Encoding.UTF8.GetString(Convert.FromBase64String(secrets[0])); password = Encoding.UTF8.GetString(Convert.FromBase64String(secrets[1])); - return new NetworkCredential(username, password); + return (username, password); } else { diff --git a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/NoOpAgentCredentialStore.cs b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/NoOpAgentCredentialStore.cs index 095f2428d3..b7676f9cc2 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/NoOpAgentCredentialStore.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/NoOpAgentCredentialStore.cs @@ -14,7 +14,7 @@ public override void Initialize(IHostContext hostContext) base.Initialize(hostContext); } - public NetworkCredential Write(string target, string username, string password) + public void Write(string target, string username, string password) { Trace.Entering(); ArgUtil.NotNullOrEmpty(target, nameof(target)); @@ -22,7 +22,6 @@ public NetworkCredential Write(string target, string username, string password) ArgUtil.NotNullOrEmpty(password, nameof(password)); Trace.Info($"Attempt to store credential for '{target}' to cred store."); - return new NetworkCredential(username, password); } public NetworkCredential Read(string target) @@ -34,6 +33,12 @@ public NetworkCredential Read(string target) throw new KeyNotFoundException(target); } + public (string UserName, string Password) Read2(string target) + { + var ret = Read(target); + return (ret.UserName, ret.Password); + } + public void Delete(string target) { Trace.Entering(); diff --git a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/WindowsAgentCredentialStore.cs b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/WindowsAgentCredentialStore.cs index 297dc60b5b..9d5ea4ed3a 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/WindowsAgentCredentialStore.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/AgentCredentialStore/WindowsAgentCredentialStore.cs @@ -47,7 +47,7 @@ public override void Initialize(IHostContext hostContext) } } - public NetworkCredential Write(string target, string username, string password) + public void Write(string target, string username, string password) { Trace.Entering(); ArgUtil.NotNullOrEmpty(target, nameof(target)); @@ -67,7 +67,7 @@ public NetworkCredential Write(string target, string username, string password) SyncCredentialStoreFile(); // save to Windows Credential Store - return WriteInternal(target, username, password); + WriteInternal(target, username, password); } public NetworkCredential Read(string target) @@ -140,6 +140,13 @@ public NetworkCredential Read(string target) } } + public (string UserName, string Password) Read2(string target) + { + // NetworkCredential objects cause crashes only on macOS => we can invoke the original implemantation here + var ret = Read(target); + return (ret.UserName, ret.Password); + } + public void Delete(string target) { Trace.Entering(); diff --git a/src/Microsoft.VisualStudio.Services.Agent/IAgentCredentialStore.cs b/src/Microsoft.VisualStudio.Services.Agent/IAgentCredentialStore.cs index 91eb2a69d3..6b6a14cb09 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/IAgentCredentialStore.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/IAgentCredentialStore.cs @@ -14,11 +14,15 @@ namespace Microsoft.VisualStudio.Services.Agent )] public interface IAgentCredentialStore : IAgentService { - NetworkCredential Write(string target, string username, string password); + void Write(string target, string username, string password); // throw exception when target not found from cred store NetworkCredential Read(string target); + + // variant that does not return NetworkCredential class, which suffers from OS-level crashes on macOS + (string UserName, string Password) Read2(string target); + // throw exception when target not found from cred store void Delete(string target); } diff --git a/src/Microsoft.VisualStudio.Services.Agent/VstsAgentWebProxy.cs b/src/Microsoft.VisualStudio.Services.Agent/VstsAgentWebProxy.cs index a012f483c1..66b0fd9575 100644 --- a/src/Microsoft.VisualStudio.Services.Agent/VstsAgentWebProxy.cs +++ b/src/Microsoft.VisualStudio.Services.Agent/VstsAgentWebProxy.cs @@ -188,9 +188,22 @@ private void LoadProxySetting() if (!string.IsNullOrEmpty(lookupKey)) { var credStore = HostContext.GetService(); - var proxyCred = credStore.Read($"VSTS_AGENT_PROXY_{lookupKey}"); - ProxyUsername = proxyCred.UserName; - ProxyPassword = proxyCred.Password; + var avoidNetCredentialFF = AgentKnobs.AvoidNetCredentialObjectsOnMac.GetValue(HostContext).AsBoolean(); + var target = $"VSTS_AGENT_PROXY_{lookupKey}"; + + if (avoidNetCredentialFF) + { + var proxyCred = credStore.Read2(target); + ProxyUsername = proxyCred.UserName; + ProxyPassword = proxyCred.Password; + } + else + { + NetworkCredential proxyCred = credStore.Read(target); + ProxyUsername = proxyCred.UserName; + ProxyPassword = proxyCred.Password; + } + } }