From a40b9a306e9574f96a877d7d2b58a2a1ec517966 Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Sat, 24 May 2014 23:17:59 -0700 Subject: [PATCH] Fixed: API keys should be more reliably cleansed from the logs --- .../Instrumentation/CleanseLogMessage.cs | 20 +++++++++ .../Instrumentation/LogTargets.cs | 2 +- .../Instrumentation/NzbDroneFileTarget.cs | 13 ++++++ src/NzbDrone.Common/NzbDrone.Common.csproj | 2 + .../Download/Clients/Sabnzbd/SabnzbdProxy.cs | 2 +- .../Indexers/IndexerFetchService.cs | 2 +- .../Instrumentation/DatabaseTarget.cs | 3 +- .../Extensions/LoggerCleansedExtensions.cs | 45 ------------------- .../Instrumentation/SetLoggingLevel.cs | 4 +- src/NzbDrone.Core/NzbDrone.Core.csproj | 1 - 10 files changed, 42 insertions(+), 52 deletions(-) create mode 100644 src/NzbDrone.Common/Instrumentation/CleanseLogMessage.cs create mode 100644 src/NzbDrone.Common/Instrumentation/NzbDroneFileTarget.cs delete mode 100644 src/NzbDrone.Core/Instrumentation/Extensions/LoggerCleansedExtensions.cs diff --git a/src/NzbDrone.Common/Instrumentation/CleanseLogMessage.cs b/src/NzbDrone.Common/Instrumentation/CleanseLogMessage.cs new file mode 100644 index 000000000..8c5096976 --- /dev/null +++ b/src/NzbDrone.Common/Instrumentation/CleanseLogMessage.cs @@ -0,0 +1,20 @@ +using System.Text.RegularExpressions; + +namespace NzbDrone.Common.Instrumentation +{ + public class CleanseLogMessage + { + //TODO: remove password= + private static readonly Regex CleansingRegex = new Regex(@"(?<=apikey=)(\w+?)(?=\W|$|_)", RegexOptions.Compiled | RegexOptions.IgnoreCase); + + public static string Cleanse(string message) + { + if (message.IsNullOrWhiteSpace()) + { + return message; + } + + return CleansingRegex.Replace(message, ""); + } + } +} diff --git a/src/NzbDrone.Common/Instrumentation/LogTargets.cs b/src/NzbDrone.Common/Instrumentation/LogTargets.cs index 399eca7a7..9230a428d 100644 --- a/src/NzbDrone.Common/Instrumentation/LogTargets.cs +++ b/src/NzbDrone.Common/Instrumentation/LogTargets.cs @@ -78,7 +78,7 @@ namespace NzbDrone.Common.Instrumentation private static void RegisterAppFile(IAppFolderInfo appFolderInfo) { - var fileTarget = new FileTarget(); + var fileTarget = new NzbDroneFileTarget(); fileTarget.Name = "rollingFileLogger"; fileTarget.FileName = Path.Combine(appFolderInfo.GetLogFolder(), "nzbdrone.txt"); diff --git a/src/NzbDrone.Common/Instrumentation/NzbDroneFileTarget.cs b/src/NzbDrone.Common/Instrumentation/NzbDroneFileTarget.cs new file mode 100644 index 000000000..62e41b0e0 --- /dev/null +++ b/src/NzbDrone.Common/Instrumentation/NzbDroneFileTarget.cs @@ -0,0 +1,13 @@ +using NLog; +using NLog.Targets; + +namespace NzbDrone.Common.Instrumentation +{ + public class NzbDroneFileTarget : FileTarget + { + protected override string GetFormattedMessage(LogEventInfo logEvent) + { + return CleanseLogMessage.Cleanse(Layout.Render(logEvent)); + } + } +} diff --git a/src/NzbDrone.Common/NzbDrone.Common.csproj b/src/NzbDrone.Common/NzbDrone.Common.csproj index e0a90c375..f2b61218e 100644 --- a/src/NzbDrone.Common/NzbDrone.Common.csproj +++ b/src/NzbDrone.Common/NzbDrone.Common.csproj @@ -97,9 +97,11 @@ Component + + diff --git a/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdProxy.cs b/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdProxy.cs index 1623b6eab..4c699ab64 100644 --- a/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdProxy.cs +++ b/src/NzbDrone.Core/Download/Clients/Sabnzbd/SabnzbdProxy.cs @@ -135,7 +135,7 @@ namespace NzbDrone.Core.Download.Clients.Sabnzbd action, authentication); - _logger.CleansedDebug(url); + _logger.Debug(url); return new RestClient(url); } diff --git a/src/NzbDrone.Core/Indexers/IndexerFetchService.cs b/src/NzbDrone.Core/Indexers/IndexerFetchService.cs index 54f925199..678a5be1a 100644 --- a/src/NzbDrone.Core/Indexers/IndexerFetchService.cs +++ b/src/NzbDrone.Core/Indexers/IndexerFetchService.cs @@ -117,7 +117,7 @@ namespace NzbDrone.Core.Indexers { try { - _logger.CleansedDebug("Downloading Feed " + url); + _logger.Debug("Downloading Feed " + url); var xml = _httpProvider.DownloadString(url); if (!string.IsNullOrWhiteSpace(xml)) { diff --git a/src/NzbDrone.Core/Instrumentation/DatabaseTarget.cs b/src/NzbDrone.Core/Instrumentation/DatabaseTarget.cs index 9f43eadbb..f2e14f7ea 100644 --- a/src/NzbDrone.Core/Instrumentation/DatabaseTarget.cs +++ b/src/NzbDrone.Core/Instrumentation/DatabaseTarget.cs @@ -5,6 +5,7 @@ using NLog.Config; using NLog; using NLog.Layouts; using NLog.Targets; +using NzbDrone.Common.Instrumentation; using NzbDrone.Core.Lifecycle; using NzbDrone.Core.Messaging.Events; @@ -52,7 +53,7 @@ namespace NzbDrone.Core.Instrumentation { var log = new Log(); log.Time = logEvent.TimeStamp; - log.Message = logEvent.FormattedMessage; + log.Message = CleanseLogMessage.Cleanse(logEvent.FormattedMessage); log.Method = Layout.Render(logEvent); log.Logger = logEvent.LoggerName; diff --git a/src/NzbDrone.Core/Instrumentation/Extensions/LoggerCleansedExtensions.cs b/src/NzbDrone.Core/Instrumentation/Extensions/LoggerCleansedExtensions.cs deleted file mode 100644 index 3fb154c8c..000000000 --- a/src/NzbDrone.Core/Instrumentation/Extensions/LoggerCleansedExtensions.cs +++ /dev/null @@ -1,45 +0,0 @@ -using System; -using System.Text.RegularExpressions; -using NLog; - -namespace NzbDrone.Core.Instrumentation.Extensions -{ - public static class LoggerCleansedExtensions - { - private static readonly Regex CleansingRegex = new Regex(@"(?<=apikey=)(\w+?)(?=\W|$|_)", RegexOptions.Compiled | RegexOptions.IgnoreCase); - - public static void CleansedInfo(this Logger logger, string message, params object[] args) - { - var formattedMessage = String.Format(message, args); - LogCleansedMessage(logger, LogLevel.Info, formattedMessage); - } - - public static void CleansedDebug(this Logger logger, string message, params object[] args) - { - var formattedMessage = String.Format(message, args); - LogCleansedMessage(logger, LogLevel.Debug, formattedMessage); - } - - public static void CleansedTrace(this Logger logger, string message, params object[] args) - { - var formattedMessage = String.Format(message, args); - LogCleansedMessage(logger, LogLevel.Trace, formattedMessage); - } - - private static void LogCleansedMessage(Logger logger, LogLevel level, string message) - { - message = Cleanse(message); - - var logEvent = new LogEventInfo(level, logger.Name, message); - - logger.Log(logEvent); - } - - private static string Cleanse(string message) - { - //TODO: password= - - return CleansingRegex.Replace(message, ""); - } - } -} diff --git a/src/NzbDrone.Core/Instrumentation/SetLoggingLevel.cs b/src/NzbDrone.Core/Instrumentation/SetLoggingLevel.cs index a436c0fc1..e4ca64919 100644 --- a/src/NzbDrone.Core/Instrumentation/SetLoggingLevel.cs +++ b/src/NzbDrone.Core/Instrumentation/SetLoggingLevel.cs @@ -2,7 +2,7 @@ using System.Linq; using NLog; using NLog.Config; -using NLog.Targets; +using NzbDrone.Common.Instrumentation; using NzbDrone.Core.Configuration; using NzbDrone.Core.Configuration.Events; using NzbDrone.Core.Lifecycle; @@ -29,7 +29,7 @@ namespace NzbDrone.Core.Instrumentation var minimumLogLevel = LogLevel.FromString(_configFileProvider.LogLevel); var rules = LogManager.Configuration.LoggingRules; - var rollingFileLogger = rules.Single(s => s.Targets.Any(t => t is FileTarget)); + var rollingFileLogger = rules.Single(s => s.Targets.Any(t => t is NzbDroneFileTarget)); rollingFileLogger.EnableLoggingForLevel(LogLevel.Trace); SetMinimumLogLevel(rollingFileLogger, minimumLogLevel); diff --git a/src/NzbDrone.Core/NzbDrone.Core.csproj b/src/NzbDrone.Core/NzbDrone.Core.csproj index c1e0810b2..ab4e30745 100644 --- a/src/NzbDrone.Core/NzbDrone.Core.csproj +++ b/src/NzbDrone.Core/NzbDrone.Core.csproj @@ -324,7 +324,6 @@ -