Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[In-proc] Worker termination path sanitizing #10397

Merged
merged 3 commits into from
Aug 15, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions release_notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,4 @@
- Update PowerShell 7.2 worker to [4.0.4020](https://github.com/Azure/azure-functions-powershell-worker/releases/tag/v4.0.4020)
- Update PowerShell 7.4 worker to [4.0.4021](https://github.com/Azure/azure-functions-powershell-worker/releases/tag/v4.0.4021)
- Trim FunctionsNetHost artifacts (#10364)
- Worker termination path updated with sanitized logging (#10397)
24 changes: 23 additions & 1 deletion src/WebJobs.Script/Sanitizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.Text.RegularExpressions;
using Newtonsoft.Json.Linq;

namespace Microsoft.Azure.WebJobs.Logging
Expand All @@ -20,6 +21,21 @@ internal static class Sanitizer
internal static readonly string[] CredentialTokens = new string[] { "Token=", "DefaultEndpointsProtocol=http", "AccountKey=", "Data Source=", "Server=", "Password=", "pwd=", "&sig=", "&sig=", "?sig=", "SharedAccessKey=" };
private static readonly string[] CredentialNameFragments = new[] { "password", "pwd", "key", "secret", "token", "sas" };

// Pattern of format : "<protocol>://<username>:<password>@<address>:<port>"
private static readonly string Pattern = @"
\b([a-zA-Z]+) # Capture protocol
:\/\/ # '://'
([^:/\s]+) # Capture username
: # ':'
([^@/\s]+) # Capture password
@ # '@'
([^:/\s]+) # Capture address
: # ':'
([0-9]+)\b # Capture port number
";

private static readonly Regex Regex = new Regex(Pattern, RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace);

/// <summary>
/// Removes well-known credential strings from strings.
/// </summary>
Expand Down Expand Up @@ -73,6 +89,12 @@ internal static string Sanitize(string input)
}
}

// This check avoids unnecessary regex evaluation if the input does not contain any url
if (input.Contains(":"))
{
t = Regex.Replace(t, SecretReplacement);
}

return t;
}

Expand Down Expand Up @@ -153,6 +175,6 @@ static JToken Sanitize(JToken token)
/// Checks if a string even *possibly* contains one of our <see cref="CredentialTokens"/>.
/// Useful for short-circuiting more expensive checks and replacements if it's known we wouldn't do anything.
/// </summary>
internal static bool MayContainCredentials(string input) => input.Contains("=");
internal static bool MayContainCredentials(string input) => input.Contains("=") || input.Contains(":");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ private void OnProcessExited(object sender, EventArgs e)
else
{
string exceptionMessage = string.Join(",", _processStdErrDataQueue.Where(s => !string.IsNullOrEmpty(s)));
var processExitEx = new WorkerProcessExitException($"{Process.StartInfo.FileName} exited with code {Process.ExitCode} (0x{Process.ExitCode.ToString("X")})", new Exception(exceptionMessage));
string sanitizedExceptionMessage = Sanitizer.Sanitize(exceptionMessage);
var processExitEx = new WorkerProcessExitException($"{Process.StartInfo.FileName} exited with code {Process.ExitCode} (0x{Process.ExitCode.ToString("X")})", new Exception(sanitizedExceptionMessage));
processExitEx.ExitCode = Process.ExitCode;
processExitEx.Pid = Process.Id;
HandleWorkerProcessExitError(processExitEx);
Expand Down
4 changes: 4 additions & 0 deletions test/WebJobs.Script.Tests/SanitizerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ public class SanitizerTests
[InlineData("SharedAccessKey=foo", "[Hidden Credential]")]
[InlineData(@"Hey=AS1$@%#$%W-k2j"";SharedAccessKey=foo,Data Source=barzons,Server=bathouse'testing", @"Hey=AS1$@%#$%W-k2j"";[Hidden Credential]'testing")]
[InlineData("test?sig=", "test[Hidden Credential]")]
[InlineData("aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111", "[Hidden Credential]")]
[InlineData("test,aaa://aaa:aaaaaa1111aa@aaa.aaa.io:1111,test", "test,[Hidden Credential],test")]
[InlineData(@"some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 text", @"some text [Hidden Credential] some text [Hidden Credential] text")]
[InlineData(@"some text abc://abc:aaaaaa1111aa@aaa.abc.io:1111 some text AccountKey=heyyyyyyy text", @"some text [Hidden Credential] some text [Hidden Credential]")]
public void SanitizeString(string input, string expectedOutput)
{
var sanitized = Sanitizer.Sanitize(input);
Expand Down