Improve logging and validation for license keys. (#2982)

Author: jaffinito

src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs

@@ -210,21 +210,66 @@ public virtual string AgentLicenseKey
         {
             get
             {
-                if (_agentLicenseKey != null)
-                    return _agentLicenseKey;
+                _agentLicenseKey ??= TryGetLicenseKey();
+                if (string.IsNullOrWhiteSpace(_agentLicenseKey) && !ServerlessModeEnabled)
+                {
+                    TrySetAgentControlStatus(HealthCodes.LicenseKeyMissing);
+                }
 
-                _agentLicenseKey = _configurationManagerStatic.GetAppSetting(Constants.AppSettingsLicenseKey)
-                                   ?? EnvironmentOverrides(_localConfiguration.service.licenseKey, "NEW_RELIC_LICENSE_KEY", "NEWRELIC_LICENSEKEY");
+                return _agentLicenseKey;
+            }
+        }
 
-                _agentLicenseKey = _agentLicenseKey?.Trim();
+        private string TryGetLicenseKey()
+        {
+            // same order as old process - appsettings > env var(a/b) > newrelic.config
+            var candidateKeys = new Dictionary<string, string>
+            {
+                { "AppSettings", _configurationManagerStatic.GetAppSetting(Constants.AppSettingsLicenseKey) },
+                { "EnvironmentVariable", _environment.GetEnvironmentVariableFromList("NEW_RELIC_LICENSE_KEY", "NEWRELIC_LICENSEKEY") },
+                { "newrelic.config", _localConfiguration.service.licenseKey }
+            };
 
-                if (string.IsNullOrEmpty(_agentLicenseKey) && !ServerlessModeEnabled)
+            foreach (var candidateKey in candidateKeys)
+            {
+                // Did we find a key?
+                if (string.IsNullOrWhiteSpace(candidateKey.Value))
                 {
-                    TrySetAgentControlStatus(HealthCodes.LicenseKeyMissing);
+                    continue;
                 }
 
-                return _agentLicenseKey;
+                // We found something, but is it a valid key?
+
+                // If the key is the default value from newrelic.config, we return the default value
+                // AgentManager.AssertAgentEnabled() relies on this behavior and will throw an exception if the key is the default value
+                if (candidateKey.Value.ToLower().Contains("license"))
+                {
+                    // newrelic.config is the last place to look
+                    return candidateKey.Value;
+                }
+
+                // Keys are only 40 characters long, but we trim to be safe
+                var trimmedCandidateKey = candidateKey.Value.Trim();
+                if (trimmedCandidateKey.Length != 40)
+                {
+                    Log.Warn($"License key from {candidateKey.Key} is not 40 characters long. Checking for other license keys.");
+                    continue;
+                }
+
+                // Keys can only contain printable ASCII characters from 0x21 to 0x7E
+                if (!trimmedCandidateKey.All(c => c >= 0x21 && c <= 0x7E))
+                {
+                    Log.Warn($"License key from {candidateKey.Key} contains invalid characters. Checking for other license keys.");
+                    continue;
+                }
+
+                Log.Info($"License key from {candidateKey.Key} appears to be in the correct format.");
+                return trimmedCandidateKey;
             }
+
+            // return string.Empty instead of null to allow caching and prevent checking repeatedly
+            Log.Warn("No valid license key found.");
+            return string.Empty;
         }
 
         private IEnumerable<string> _applicationNames;

src/Agent/NewRelic/Agent/Core/DataTransport/ConnectionManager.cs

@@ -155,6 +155,10 @@ private void HandleConnectionException(Exception ex)
                         Log.Info("401 GONE response received from the collector.");
                         shouldRestart = false;
                     }
+                    else if (httpEx.StatusCode == HttpStatusCode.Unauthorized)
+                    {
+                        Log.Warn("Connection failed: Potential issue with license key based on HTTP status code 401 - Unauthorized.\n\tPlease verify the license key.");
+                    }
                     else
                         Log.Info("Connection failed: {0}", ex.Message);
                     break;

tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs

@@ -1449,12 +1449,32 @@ public string CustomHostEnvironmentOverridesLocal(string environment, string loc
             return _defaultConfig.CollectorHost;
         }
 
-        [TestCase(null, null, null, ExpectedResult = null)]
-        [TestCase(null, "foo", null, ExpectedResult = "foo")]
-        [TestCase("foo", null, null, ExpectedResult = "foo")]
-        [TestCase("foo", null, "foo", ExpectedResult = "foo")]
-        [TestCase("foo", "foo", null, ExpectedResult = "foo")]
-        [TestCase("foo", "foo", "bar", ExpectedResult = "foo")]
+        // all null returns empty string
+        [TestCase(null, null, null, ExpectedResult = "")]
+        // AppSetting overrides environment and local
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0", null, null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0", null, "bar1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0", "bar1234567890abcdefghijklmnopqrstuvwxyz0", null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0", "bar1234567890abcdefghijklmnopqrstuvwxyz0", "nar1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        // Environment overrides local
+        [TestCase(null, "foo1234567890abcdefghijklmnopqrstuvwxyz0", null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase(null, "foo1234567890abcdefghijklmnopqrstuvwxyz0", "bar1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        // local on its own
+        [TestCase(null, null, "foo1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase(null, null, "REPLACE_WITH_LICENSE_KEY", ExpectedResult = "REPLACE_WITH_LICENSE_KEY")]
+        // Length must be 40
+        [TestCase("       foo1234567890abcdefghijklmnopqrstuvwxyz0         ", null, null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0123456789", null, null, ExpectedResult = "")]
+        [TestCase("foo", null, null, ExpectedResult = "")]
+        // Allowed characters
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvyz\tzz", null, null, ExpectedResult = "")]
+        // Bad keys skipped for lower priority keys
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvwxyz0123456789", "foo1234567890abcdefghijklmnopqrstuvwxyz0", null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase(null, "foo1234567890abcdefghijklmnopqrstuvwxyz0123456789", "foo1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo", "foo1234567890abcdefghijklmnopqrstuvwxyz0", null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase(null, "foo", "foo1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase("foo1234567890abcdefghijklmnopqrstuvyz\tzz", "foo1234567890abcdefghijklmnopqrstuvwxyz0", null, ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
+        [TestCase(null, "foo1234567890abcdefghijklmnopqrstuvyz\tzz", "foo1234567890abcdefghijklmnopqrstuvwxyz0", ExpectedResult = "foo1234567890abcdefghijklmnopqrstuvwxyz0")]
         public string LicenseKeyEnvironmentOverridesLocal(string appSettingEnvironmentName, string newEnvironmentName, string local)
         {
             _localConfig.service.licenseKey = local;
@@ -4533,7 +4553,7 @@ public void InvalidLicenseKey_SetsLicenseKeyMissing_AgentControlStatus()
             // Assert
             Assert.Multiple(() =>
             {
-                Assert.That(licenseKey, Is.EqualTo(""));
+                Assert.That(licenseKey, Is.EqualTo(string.Empty));
                 Assert.That(healthCheck.IsHealthy, Is.False);
                 Assert.That(healthCheck.Status, Is.EqualTo("License key missing in configuration"));
                 Assert.That(healthCheck.LastError, Is.EqualTo("NR-APM-002"));