diff --git a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs b/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs index b60023235..29270621f 100644 --- a/src/Agent/NewRelic/Agent/Core/Configuration/DefaultConfiguration.cs +++ b/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 + { + { "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 _applicationNames; diff --git a/src/Agent/NewRelic/Agent/Core/DataTransport/ConnectionManager.cs b/src/Agent/NewRelic/Agent/Core/DataTransport/ConnectionManager.cs index 8d4ed8b72..206282f31 100644 --- a/src/Agent/NewRelic/Agent/Core/DataTransport/ConnectionManager.cs +++ b/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; diff --git a/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs b/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs index 279eb5e94..9203f4450 100644 --- a/tests/Agent/UnitTests/Core.UnitTest/Configuration/DefaultConfigurationTests.cs +++ b/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"));