fix: obfuscate logged appSettings values (#2110)

* Obfuscate license key when logging values read from app settings * Constant-ify the app settings keys used by the agent
newrelic · Dec 1, 2023 · 2d8da68 · 2d8da68
1 parent 9cdbe2f
commit 2d8da68
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 39 deletions.
diff --git a/src/Agent/NewRelic/Agent/Core/Config/ConfigurationLoader.cs b/src/Agent/NewRelic/Agent/Core/Config/ConfigurationLoader.cs
@@ -4,6 +4,7 @@
 using NewRelic.Agent.Core.Events;
 using NewRelic.Agent.Core.Utilities;
 using NewRelic.Core;
+using NewRelic.Agent.Core.Configuration;
 using NewRelic.Core.Logging;
 using NewRelic.SystemInterfaces;
 using System;
@@ -15,7 +16,6 @@
 using System.Xml.Serialization;
 #if NETSTANDARD2_0
 using System.Reflection;
-using NewRelic.Agent.Core.Configuration;
 #endif
 
 namespace NewRelic.Agent.Core.Config
@@ -139,7 +139,7 @@ public static string GetAgentConfigFileName()
             if (fileName != null)
                 return fileName;
 
-            throw new Exception(string.Format("Could not find {0} in NewRelic.ConfigFile path, application root, New Relic home directory, or working directory.", NewRelicConfigFileName));
+            throw new Exception(string.Format("Could not find {0} in {1} path, application root, New Relic home directory, or working directory.", NewRelicConfigFileName, Constants.AppSettingsConfigFile));
         }
 
         private static string TryGetAgentConfigFileFromAppConfig()
@@ -149,13 +149,13 @@ private static string TryGetAgentConfigFileFromAppConfig()
 
 			try
 			{
-				var fileName = AppSettingsConfigResolveWhenUsed.GetAppSetting("NewRelic.ConfigFile");
+				var fileName = AppSettingsConfigResolveWhenUsed.GetAppSetting(Constants.AppSettingsConfigFile);
 				if (!File.Exists(fileName))
 				{
 					return null;
 				}
 
-				Log.Info("Configuration file found in path pointed to by NewRelic.ConfigFile appSetting: {0}", fileName);
+				Log.Info("Configuration file found in path pointed to by {0} appSetting: {1}", Constants.AppSettingsConfigFile, fileName);
 				return fileName;
 			}
 			catch (Exception)
@@ -166,13 +166,13 @@ private static string TryGetAgentConfigFileFromAppConfig()
 #else
             try
             {
-                var fileName = GetConfigSetting("NewRelic.ConfigFile").Value;
+                var fileName = GetConfigSetting(Constants.AppSettingsConfigFile).Value;
                 if (!FileExists(fileName))
                 {
                     return null;
                 }
 
-                Log.Info("Configuration file found in path pointed to by NewRelic.ConfigFile appSetting of app/web config: {0}", fileName);
+                Log.Info("Configuration file found in path pointed to by {0} appSetting of app/web config: {1}", Constants.AppSettingsConfigFile, fileName);
                 return fileName;
             }
             catch (Exception)
@@ -674,7 +674,7 @@ public configuration Initialize(string xml, string provenance)
 
             try
             {
-                var enabledProvenance = ConfigurationLoader.GetConfigSetting("NewRelic.AgentEnabled");
+                var enabledProvenance = ConfigurationLoader.GetConfigSetting(Constants.AppSettingsAgentEnabled);
                 if (enabledProvenance != null && enabledProvenance.Value != null && bool.Parse(enabledProvenance.Value) == false)
                 {
                     agentEnabled = false;
@@ -683,7 +683,7 @@ public configuration Initialize(string xml, string provenance)
             }
             catch
             {
-                Log.Error("Failed to read NewRelic.AgentEnabled from local config.");
+                Log.Error($"Failed to read {Constants.AppSettingsAgentEnabled} from local config.");
             }
 
             return this;

diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/AppSettingsConfigResolveWhenUsed.cs b/src/Agent/NewRelic/Agent/Core/Configuration/AppSettingsConfigResolveWhenUsed.cs
@@ -5,6 +5,7 @@
 using System;
 using System.IO;
 using Microsoft.Extensions.Configuration;
+using NewRelic.Core;
 using NewRelic.Core.Logging;
 
 namespace NewRelic.Agent.Core.Configuration
@@ -79,6 +80,10 @@ public static string GetAppSetting(string key)
                 }
                 else
                 {
+                    if (key.Equals(Constants.AppSettingsLicenseKey))
+                    {
+                        value = Strings.ObfuscateLicenseKey(value);
+                    }
                     Log.Debug($"Reading value from appsettings.json and appsettings.*.json: '{key}={value}'");
                 }
             }

diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/Constants.cs b/src/Agent/NewRelic/Agent/Core/Configuration/Constants.cs
@@ -0,0 +1,15 @@
+// Copyright 2020 New Relic, Inc. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+namespace NewRelic.Agent.Core.Configuration
+{
+    public static class Constants
+    {
+        public const string AppSettingsLicenseKey = "NewRelic.LicenseKey";
+        public const string AppSettingsAgentEnabled = "NewRelic.AgentEnabled";
+        public const string AppSettingsAppName = "NewRelic.AppName";
+        public const string AppSettingsLabels = "NewRelic.Labels";
+        public const string AppSettingsConfigFile = "NewRelic.ConfigFile";
+
+    }
+}
diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs b/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs
@@ -198,7 +198,7 @@ public virtual bool AgentEnabled
                 {
                     lock (_lockObj)
                     {
-                        _agentEnabledAppSettingParsed ??= bool.TryParse(_configurationManagerStatic.GetAppSetting("NewRelic.AgentEnabled"),
+                        _agentEnabledAppSettingParsed ??= bool.TryParse(_configurationManagerStatic.GetAppSetting(Constants.AppSettingsAgentEnabled),
                             out _appSettingAgentEnabled);
                     }
                 }
@@ -216,7 +216,7 @@ public virtual string AgentLicenseKey
                 if (_agentLicenseKey != null)
                     return _agentLicenseKey;
 
-                _agentLicenseKey = _configurationManagerStatic.GetAppSetting("NewRelic.LicenseKey")
+                _agentLicenseKey = _configurationManagerStatic.GetAppSetting(Constants.AppSettingsLicenseKey)
                     ?? EnvironmentOverrides(_localConfiguration.service.licenseKey, "NEW_RELIC_LICENSE_KEY", "NEWRELIC_LICENSEKEY");
 
                 if (_agentLicenseKey != null)
@@ -243,7 +243,7 @@ private IEnumerable<string> GetApplicationNames()
                 return runtimeAppNames;
             }
 
-            var appName = _configurationManagerStatic.GetAppSetting("NewRelic.AppName");
+            var appName = _configurationManagerStatic.GetAppSetting(Constants.AppSettingsAppName);
             if (appName != null)
             {
                 Log.Info("Application name from web.config or app.config.");
@@ -1361,7 +1361,7 @@ public virtual string Labels
             {
                 if (!_labelsChecked)
                 {
-                    var labels = _configurationManagerStatic.GetAppSetting("NewRelic.Labels");
+                    var labels = _configurationManagerStatic.GetAppSetting(Constants.AppSettingsLabels);
                     if (labels != null)
                     {
                         Log.Info("Application labels from web.config, app.config, or appsettings.json.");

diff --git a/tests/Agent/UnitTests/CompositeTests/CompositeTestAgent.cs b/tests/Agent/UnitTests/CompositeTests/CompositeTestAgent.cs
@@ -141,7 +141,7 @@ public CompositeTestAgent(bool shouldAllowThreads, bool includeAsyncLocalStorage
                 .DoInstead<WaitCallback>(callback => { lock (_queuedCallbacksLockObject) { _queuedCallbacks.Add(callback); } });
 
             var configurationManagerStatic = Mock.Create<IConfigurationManagerStatic>();
-            Mock.Arrange(() => configurationManagerStatic.GetAppSetting("NewRelic.LicenseKey"))
+            Mock.Arrange(() => configurationManagerStatic.GetAppSetting(NewRelic.Agent.Core.Configuration.Constants.AppSettingsLicenseKey))
                 .Returns("Composite test license key");
 
             // Construct services

diff --git a/tests/Agent/UnitTests/Core.UnitTest/Config/ConfigurationLoaderTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Config/ConfigurationLoaderTests.cs
@@ -1,4 +1,4 @@
-// Copyright 2020 New Relic, Inc. All rights reserved.
+// Copyright 2020 New Relic, Inc. All rights reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 #if NETFRAMEWORK
@@ -11,6 +11,7 @@
 using System.Text;
 using System.Threading.Tasks;
 using System.Web;
+using NewRelic.Agent.Core.Configuration;
 using NUnit.Framework;
 
 namespace NewRelic.Agent.Core.Config
@@ -119,7 +120,7 @@ public void GetAgentConfigFileName_ReturnsConfigFileFromAppConfig()
             {
                 const string expectedFileName = "filenameFromAppConfig";
                 var testWebConfiguration = ConfigurationManager.OpenExeConfiguration(null);
-                testWebConfiguration.AppSettings.Settings.Add("NewRelic.ConfigFile", expectedFileName);
+                testWebConfiguration.AppSettings.Settings.Add(Constants.AppSettingsConfigFile, expectedFileName);
 
                 staticMocks.UseAppDomainAppIdFunc(() => "testAppId");
                 staticMocks.UseAppDomainAppVirtualPathFunc(() => "testVirtualPath");
@@ -138,7 +139,7 @@ public void TryGetAgentConfigFileFromAppConfig_ReturnsNullWhenFileDoesNotExist()
             {
                 const string expectedFileName = "filenameFromAppConfig";
                 var testWebConfiguration = ConfigurationManager.OpenExeConfiguration(null);
-                testWebConfiguration.AppSettings.Settings.Add("NewRelic.ConfigFile", expectedFileName);
+                testWebConfiguration.AppSettings.Settings.Add(Constants.AppSettingsConfigFile, expectedFileName);
 
                 staticMocks.UseAppDomainAppIdFunc(() => "testAppId");
                 staticMocks.UseAppDomainAppVirtualPathFunc(() => "testVirtualPath");
@@ -157,7 +158,7 @@ public void TryGetAgentConfigFileFromAppConfig_ReturnsNullOnException()
             {
                 const string expectedFileName = "filenameFromAppConfig";
                 var testWebConfiguration = ConfigurationManager.OpenExeConfiguration(null);
-                testWebConfiguration.AppSettings.Settings.Add("NewRelic.ConfigFile", expectedFileName);
+                testWebConfiguration.AppSettings.Settings.Add(Constants.AppSettingsConfigFile, expectedFileName);
 
                 staticMocks.UseAppDomainAppIdFunc(() => "testAppId");
                 staticMocks.UseAppDomainAppVirtualPathFunc(() => "testVirtualPath");