From 341b036de7363ba56512abb3e34c2adc97b69306 Mon Sep 17 00:00:00 2001 From: Greg Villicana <58237075+grvillic@users.noreply.github.com> Date: Thu, 6 Jun 2024 17:50:14 -0700 Subject: [PATCH] Sanitize Docker environment vars in logs (#1163) --- .../DockerService.cs | 20 +++ .../Utilities/StringUtilities.cs | 5 +- .../DockerServiceTests.cs | 139 ++++++++++++++++++ ...oft.ComponentDetection.Common.Tests.csproj | 1 + .../StringUtilitiesTests.cs | 5 +- 5 files changed, 166 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.ComponentDetection.Common/DockerService.cs b/src/Microsoft.ComponentDetection.Common/DockerService.cs index 66033f183..ee49394e4 100644 --- a/src/Microsoft.ComponentDetection.Common/DockerService.cs +++ b/src/Microsoft.ComponentDetection.Common/DockerService.cs @@ -110,6 +110,23 @@ public async Task TryPullImageAsync(string image, CancellationToken cancel } } + internal void SanitizeEnvironmentVariables(ImageInspectResponse inspectResponse) + { + var envVariables = inspectResponse?.Config?.Env; + if (envVariables == null || !envVariables.Any()) + { + return; + } + + var sanitizedVarList = new List(); + foreach (var variable in inspectResponse.Config.Env) + { + sanitizedVarList.Add(variable.RemoveSensitiveInformation()); + } + + inspectResponse.Config.Env = sanitizedVarList; + } + public async Task InspectImageAsync(string image, CancellationToken cancellationToken = default) { using var record = new DockerServiceInspectImageTelemetryRecord @@ -119,6 +136,9 @@ public async Task InspectImageAsync(string image, Cancellation try { var imageInspectResponse = await Client.Images.InspectImageAsync(image, cancellationToken); + + this.SanitizeEnvironmentVariables(imageInspectResponse); + record.ImageInspectResponse = JsonSerializer.Serialize(imageInspectResponse); var baseImageRef = string.Empty; diff --git a/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs b/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs index b63ac7a8e..47b4cd6f0 100644 --- a/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs +++ b/src/Microsoft.ComponentDetection.Common/Utilities/StringUtilities.cs @@ -5,7 +5,8 @@ namespace Microsoft.ComponentDetection.Common; public static class StringUtilities { - private static readonly Regex SensitiveInfoRegex = new Regex(@"(?<=https://)(.+)(?=@)", RegexOptions.Compiled, TimeSpan.FromSeconds(5)); + private static readonly Regex SensitiveInfoRegex = new Regex(@"(?<=https://)(.+)(?=@)", RegexOptions.Compiled | RegexOptions.IgnoreCase, TimeSpan.FromSeconds(5)); + public const string SensitivePlaceholder = "******"; /// /// Utility method to remove sensitive information from a string, currently focused on removing on the credentials placed within URL which can be part of CLI commands. @@ -21,7 +22,7 @@ public static string RemoveSensitiveInformation(this string inputString) try { - return SensitiveInfoRegex.Replace(inputString, "******"); + return SensitiveInfoRegex.Replace(inputString, SensitivePlaceholder); } catch (Exception) { diff --git a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs index 5911079f6..80ce92327 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs +++ b/test/Microsoft.ComponentDetection.Common.Tests/DockerServiceTests.cs @@ -3,6 +3,7 @@ namespace Microsoft.ComponentDetection.Common.Tests; using System; using System.Collections.Generic; using System.Threading.Tasks; +using Docker.DotNet.Models; using FluentAssertions; using Microsoft.ComponentDetection.TestsUtilities; using Microsoft.Extensions.Logging; @@ -80,4 +81,142 @@ public async Task DockerService_CanCreateAndRunImageAsync() stdout.Should().StartWith("\nHello from Docker!"); stderr.Should().BeEmpty(); } + + [TestMethod] + public void DockerService_SanitizeEnvironmentVariables() + { + var responseInput = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + "PIP_INDEX_URL=https://user:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@someregistry.localhost.com", + }, + }, + }; + + var expected = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + $"PIP_INDEX_URL=https://{StringUtilities.SensitivePlaceholder}@someregistry.localhost.com", + }, + }, + }; + + this.dockerService.SanitizeEnvironmentVariables(responseInput); + responseInput.Should().BeEquivalentTo(expected); + + responseInput = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + }, + }, + }; + + expected = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + }, + }, + }; + + this.dockerService.SanitizeEnvironmentVariables(responseInput); + responseInput.Should().BeEquivalentTo(expected); + + responseInput = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + "PIP_INDEX_URL=https://someregistry.localhost.com", + }, + }, + }; + + expected = new ImageInspectResponse + { + Config = new Config + { + Env = new List + { + "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", + "MARATHON_APP_RESOURCE_CPU=1", + "REGION=local", + "PIP_INDEX_URL=https://someregistry.localhost.com", + }, + }, + }; + + this.dockerService.SanitizeEnvironmentVariables(responseInput); + responseInput.Should().BeEquivalentTo(expected); + } + + [TestMethod] + public void DockerService_SanitizeEnvironmentVariables_DoesNotThrow() + { + var responseInput = new ImageInspectResponse + { + Config = new Config + { + Env = null, + }, + }; + + var action = () => this.dockerService.SanitizeEnvironmentVariables(responseInput); + action.Should().NotThrow(); + responseInput.Should().BeEquivalentTo(responseInput); + + responseInput = new ImageInspectResponse + { + Config = null, + }; + + action = () => this.dockerService.SanitizeEnvironmentVariables(responseInput); + action.Should().NotThrow(); + responseInput.Should().BeEquivalentTo(responseInput); + + responseInput = null; + + action = () => this.dockerService.SanitizeEnvironmentVariables(responseInput); + action.Should().NotThrow(); + responseInput.Should().BeNull(); + + responseInput = new ImageInspectResponse + { + Config = new Config + { + Env = new List(), + }, + }; + + action = () => this.dockerService.SanitizeEnvironmentVariables(responseInput); + action.Should().NotThrow(); + responseInput.Should().BeEquivalentTo(responseInput); + } } diff --git a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj index a71d06b13..3aed7eac5 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj +++ b/test/Microsoft.ComponentDetection.Common.Tests/Microsoft.ComponentDetection.Common.Tests.csproj @@ -6,6 +6,7 @@ + diff --git a/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs b/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs index 96c347bdb..62d8bd708 100644 --- a/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs +++ b/test/Microsoft.ComponentDetection.Common.Tests/StringUtilitiesTests.cs @@ -14,12 +14,13 @@ public class StringUtilitiesTests [DataRow(null, null)] [DataRow(" ", " ")] [DataRow(" https:// ", " https:// ")] - [DataRow("https://username:password@domain.me", "https://******@domain.me")] + [DataRow("https://username:password@domain.me", $"https://{StringUtilities.SensitivePlaceholder}@domain.me")] + [DataRow("HTTPS://username:password@domain.me", $"HTTPS://{StringUtilities.SensitivePlaceholder}@domain.me")] [DataRow("https://domain.me", "https://domain.me")] [DataRow("https://@domain.me", "https://@domain.me")] [DataRow( "install -r requirements.txt --dry-run --ignore-installed --quiet --report file.zvn --index-url https://user:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@someregistry.localhost.com", - "install -r requirements.txt --dry-run --ignore-installed --quiet --report file.zvn --index-url https://******@someregistry.localhost.com")] + $"install -r requirements.txt --dry-run --ignore-installed --quiet --report file.zvn --index-url https://{StringUtilities.SensitivePlaceholder}@someregistry.localhost.com")] public void RemoveSensitiveInformation_ReturnsAsExpected(string input, string expected) { var actual = StringUtilities.RemoveSensitiveInformation(input);