Skip to content

Commit

Permalink
Impelement secrets detection for Az modules (#412)
Browse files Browse the repository at this point in the history
* Impelement secrets detection for Az modules

* Move some implementation from common to Az.Accounts and revise/rename ISanitizerSettings interface.

* Rename telemetry property name

* Make DefaultProviderResolver public due to reference needed in Az.Accounts

* Add IgnoredProperties in ISanitizerService to filter out special properties that may cause performance concern like lazy load properties.

* Return null for AzurePSSanitizer when AzureSession is not properly initialized

* Skip indexed properties in the object traverse

* Update sanitizer and move providers out to Az.Accounts
  • Loading branch information
vidai-msft authored Feb 28, 2024
1 parent 2cd3afb commit 0b6cb59
Show file tree
Hide file tree
Showing 7 changed files with 170 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ public static class ConfigKeysForCommon
//Use DisableErrorRecordsPersistence as opt-out for now, will replace it with EnableErrorRecordsPersistence as opt-in at next major release (November 2023)
public const string DisableErrorRecordsPersistence = "DisableErrorRecordsPersistence";
public const string EnableErrorRecordsPersistence = "EnableErrorRecordsPersistence";
public const string DisplaySecretsWarning = "DisplaySecretsWarning";
}
}
61 changes: 52 additions & 9 deletions src/Common/AzurePSCmdlet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using Microsoft.WindowsAzure.Commands.Common;
using Microsoft.WindowsAzure.Commands.Common.CustomAttributes;
using Microsoft.WindowsAzure.Commands.Common.Properties;
using Microsoft.WindowsAzure.Commands.Common.Sanitizer;
using Microsoft.WindowsAzure.Commands.Common.Utilities;
using System;
using System.Collections.Concurrent;
Expand Down Expand Up @@ -55,7 +56,7 @@ public abstract class AzurePSCmdlet : PSCmdlet, IDisposable
private object lockObject = new object();
private AzurePSDataCollectionProfile _cachedProfile = null;

protected IList<Regex> _matchers { get; private set; } = new List<Regex>();
protected IList<Regex> _matchers { get; private set; } = new List<Regex>();
private static readonly Regex _defaultMatcher = new Regex("(\\s*\"access_token\"\\s*:\\s*)\"[^\"]+\"");

protected AzurePSDataCollectionProfile _dataCollectionProfile
Expand Down Expand Up @@ -166,6 +167,17 @@ public RuntimeDefinedParameterDictionary AsJobDynamicParameters
}
}

private IOutputSanitizer OutputSanitizer
{
get
{
if (AzureSession.Instance != null && AzureSession.Instance.TryGetComponent<IOutputSanitizer>(nameof(IOutputSanitizer), out var outputSanitizer))
return outputSanitizer;

return null;
}
}

/// <summary>
/// Resolve user submitted paths correctly on all platforms
/// </summary>
Expand Down Expand Up @@ -331,7 +343,7 @@ protected override void BeginProcessing()
SessionState = base.SessionState;
var profile = _dataCollectionProfile;
//TODO: Inject from CI server
if(_metricHelper == null)
if (_metricHelper == null)
{
lock (lockObject)
{
Expand Down Expand Up @@ -390,14 +402,31 @@ private void WriteBreakingChangeOrPreviewMessage()
}
}

private void WriteSecretsWarningMessage()
{
if (_qosEvent?.SanitizerInfo != null)
{
var sanitizerInfo = _qosEvent.SanitizerInfo;
if (sanitizerInfo.ShowSecretsWarning)
{
if (sanitizerInfo.DetectedProperties?.Count > 0)
{
WriteWarning(string.Format(Resources.DisplaySecretsWarningMessage, MyInvocation.InvocationName, string.Join(", ", sanitizerInfo.DetectedProperties)));
}
WriteDebug($"Sanitizer took {sanitizerInfo.SanitizeDuration.TotalMilliseconds}ms to process the object of cmdlet {MyInvocation.InvocationName}.");
}
}
}

/// <summary>
/// Perform end of pipeline processing.
/// </summary>
protected override void EndProcessing()
{
WriteEndProcessingRecommendation();
WriteWarningMessageForVersionUpgrade();

WriteSecretsWarningMessage();

if (MetricHelper.IsCalledByUser()
&& SurveyHelper.GetInstance().ShouldPromptAzSurvey()
&& (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
Expand All @@ -414,10 +443,12 @@ protected override void EndProcessing()
// Send telemetry when cmdlet is directly called by user
LogQosEvent();
}
else {
else
{
// When cmdlet is called within another cmdlet, we will not add a new telemetry, but add the cmdlet name to InternalCalledCmdlets
MetricHelper.AppendInternalCalledCmdlet(this.MyInvocation?.MyCommand?.Name);
}

LogCmdletEndInvocationInfo();
TearDownDebuggingTraces();
TearDownHttpClientPipeline();
Expand Down Expand Up @@ -501,15 +532,26 @@ protected void WriteSurvey()
protected new void WriteObject(object sendToPipeline)
{
FlushDebugMessages();
SanitizeOutput(sendToPipeline);
base.WriteObject(sendToPipeline);
}

protected new void WriteObject(object sendToPipeline, bool enumerateCollection)
{
FlushDebugMessages();
SanitizeOutput(sendToPipeline);
base.WriteObject(sendToPipeline, enumerateCollection);
}

private void SanitizeOutput(object sendToPipeline)
{
if (OutputSanitizer != null && OutputSanitizer.RequireSecretsDetection)
{
OutputSanitizer.Sanitize(sendToPipeline, out var telemetry);
_qosEvent?.SanitizerInfo.Combine(telemetry);
}
}

protected new void WriteVerbose(string text)
{
FlushDebugMessages();
Expand Down Expand Up @@ -725,6 +767,8 @@ protected virtual void InitializeQosEvent()
s => string.Format(CultureInfo.InvariantCulture, "-{0} ***", s)));
}
}

_qosEvent.SanitizerInfo = new SanitizerTelemetry();
}

private void RecordDebugMessages()
Expand Down Expand Up @@ -777,7 +821,7 @@ private void RecordDebugMessages()
private bool ShouldRecordDebugMessages()
{
return (!AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
|| !configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisableErrorRecordsPersistence))
|| !configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisableErrorRecordsPersistence))
&& IsDataCollectionAllowed();
}

Expand Down Expand Up @@ -1009,7 +1053,6 @@ protected virtual void Dispose(bool disposing)
_adalListener.Dispose();
_adalListener = null;
}

}

public void Dispose()
Expand Down Expand Up @@ -1059,11 +1102,11 @@ protected string LoadModuleVersion(String Module, bool ListAvailable)
List<PSObject> outputs;
if (ListAvailable)
{
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module} -ListAvailable");
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module} -ListAvailable");
}
else
{
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module}");
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module}");
}
foreach (PSObject obj in outputs)
{
Expand Down Expand Up @@ -1105,7 +1148,7 @@ private string LoadPowerShellVersion()
foreach (PSObject obj in outputs)
{
string psVersion = obj.ToString();
return string.IsNullOrWhiteSpace(psVersion) ? DEFAULT_PSVERSION: psVersion;
return string.IsNullOrWhiteSpace(psVersion) ? DEFAULT_PSVERSION : psVersion;
}
}
catch (Exception e)
Expand Down
40 changes: 38 additions & 2 deletions src/Common/MetricHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
using System.Text;
using System.Threading.Tasks;
using System.ComponentModel;
using Microsoft.WindowsAzure.Commands.Common.Sanitizer;

namespace Microsoft.WindowsAzure.Commands.Common
{
Expand Down Expand Up @@ -429,7 +430,7 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
sb.Append($"{key.ToString().Substring(AzurePSErrorDataKeys.KeyPrefix.Length)}={qos.Exception.Data[key]}");
}
}
if(sb.Length > 0)
if (sb.Length > 0)
{
eventProperties["exception-data"] = sb.ToString();
}
Expand All @@ -450,6 +451,8 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
}
}

PopulateSanitizerPropertiesFromQos(qos, eventProperties);

if (qos.InputFromPipeline != null)
{
eventProperties.Add("InputFromPipeline", qos.InputFromPipeline.Value.ToString());
Expand All @@ -458,13 +461,44 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
{
eventProperties.Add("OutputToPipeline", qos.OutputToPipeline.Value.ToString());
}

foreach (var key in qos.CustomProperties.Keys)
{
eventProperties[key] = qos.CustomProperties[key];
}
}

private void PopulateSanitizerPropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string, string> eventProperties)
{
if (qos.SanitizerInfo != null)
{
eventProperties["secrets-warning"] = qos.SanitizerInfo.ShowSecretsWarning.ToString();
if (qos.SanitizerInfo.ShowSecretsWarning)
{
bool secretsDetected = qos.SanitizerInfo.DetectedProperties.Count > 0;
eventProperties["secrets-detected"] = secretsDetected.ToString();
if (secretsDetected)
{
eventProperties.Add("secrets-detected-properties", string.Join(";", qos.SanitizerInfo.DetectedProperties));
}
if (qos.SanitizerInfo.HasErrorInDetection && qos.SanitizerInfo.DetectionError != null)
{
eventProperties.Add("secrets-detection-exception-type", qos.SanitizerInfo.DetectionError.GetType().ToString());
eventProperties.Add("secrets-detection-exception-message", qos.SanitizerInfo.DetectionError.Message);
if (qos.SanitizerInfo.DetectionError.InnerException != null)
{
eventProperties.Add("secrets-detection-exception-inner-type", qos.SanitizerInfo.DetectionError.InnerException.GetType().ToString());
eventProperties.Add("secrets-detection-exception-inner-message", qos.SanitizerInfo.DetectionError.InnerException.Message);
}
StackTrace sanitizerTrace = new StackTrace(qos.SanitizerInfo.DetectionError);
string sanitizerExceptionStack = string.Join(";", sanitizerTrace.GetFrames().Take(3).Select(f => ConvertFrameToString(f)));
eventProperties.Add("secrets-detection-exception-stack", sanitizerExceptionStack);
}
eventProperties.Add("secrets-sanitize-duration", qos.SanitizerInfo.SanitizeDuration.ToString("c"));
}
}
}

private static string[] exceptionTrackAcceptModuleList = { "Az.Accounts", "Az.Compute", "Az.AKS", "Az.ContainerRegistry" };
private static string[] exceptionTrackAcceptCmdletList = { "Get-AzKeyVaultSecret", "Get-AzKeyVaultCert" };

Expand Down Expand Up @@ -624,6 +658,8 @@ public class AzurePSQoSEvent
public Dictionary<string, string> CustomProperties { get; private set; }
private static bool ShowTelemetry = string.Equals(bool.TrueString, Environment.GetEnvironmentVariable("AZUREPS_DEBUG_SHOW_TELEMETRY"), StringComparison.OrdinalIgnoreCase);

public SanitizerTelemetry SanitizerInfo { get; set; }

public AzurePSQoSEvent()
{
StartTime = DateTimeOffset.Now;
Expand Down
9 changes: 9 additions & 0 deletions src/Common/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Common/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -1760,4 +1760,7 @@ Note : Go to {0} for steps to suppress this breaking change warning, and other i
<data name="SurveyPreface" xml:space="preserve">
<value>[Survey] Help us improve Azure PowerShell by sharing your experience. This survey should take about 5 minutes. Run 'Open-AzSurveyLink' to open in browser. Learn more at {0}</value>
</data>
<data name="DisplaySecretsWarningMessage" xml:space="preserve">
<value>The output of cmdlet {0} may compromise security by showing the following secrets: {1}. Learn more at https://go.microsoft.com/fwlink/?linkid=2258844</value>
</data>
</root>
23 changes: 23 additions & 0 deletions src/Common/Sanitizer/IOutputSanitizer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
// ----------------------------------------------------------------------------------
//
// Copyright Microsoft Corporation
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------------------------------------------------------------

namespace Microsoft.WindowsAzure.Commands.Common.Sanitizer
{
public interface IOutputSanitizer
{
bool RequireSecretsDetection { get; }

void Sanitize(object sanitizingObject, out SanitizerTelemetry telemetryData);
}
}
44 changes: 44 additions & 0 deletions src/Common/Sanitizer/SanitizerTelemetry.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// ----------------------------------------------------------------------------------
//
// Copyright Microsoft Corporation
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
// http://www.apache.org/licenses/LICENSE-2.0
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// ----------------------------------------------------------------------------------

using System;
using System.Collections.Generic;

namespace Microsoft.WindowsAzure.Commands.Common.Sanitizer
{
public class SanitizerTelemetry
{
public bool ShowSecretsWarning { get; set; } = false;

public HashSet<string> DetectedProperties { get; set; } = new HashSet<string>();

public bool HasErrorInDetection { get; set; } = false;

public Exception DetectionError { get; set; }

public TimeSpan SanitizeDuration { get; set; }

public void Combine(SanitizerTelemetry telemetry)
{
if (telemetry != null)
{
ShowSecretsWarning = ShowSecretsWarning || telemetry.ShowSecretsWarning;
DetectedProperties.UnionWith(telemetry.DetectedProperties);
HasErrorInDetection = HasErrorInDetection || telemetry.HasErrorInDetection;
DetectionError = DetectionError ?? telemetry.DetectionError;
SanitizeDuration += telemetry.SanitizeDuration;
}
}
}
}

0 comments on commit 0b6cb59

Please sign in to comment.