Skip to content

Commit 0b6cb59

Browse files
vidai-msftvidai-msft
authored
Impelement secrets detection for Az modules (#412)
* 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
1 parent 2cd3afb commit 0b6cb59

File tree

7 files changed

+170
-11
lines changed

7 files changed

+170
-11
lines changed

‎src/Authentication.Abstractions/Models/ConfigKeysForCommon.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,6 @@ public static class ConfigKeysForCommon
3232
//Use DisableErrorRecordsPersistence as opt-out for now, will replace it with EnableErrorRecordsPersistence as opt-in at next major release (November 2023)
3333
public const string DisableErrorRecordsPersistence = "DisableErrorRecordsPersistence";
3434
public const string EnableErrorRecordsPersistence = "EnableErrorRecordsPersistence";
35+
public const string DisplaySecretsWarning = "DisplaySecretsWarning";
3536
}
3637
}

‎src/Common/AzurePSCmdlet.cs‎

Lines changed: 52 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
using Microsoft.WindowsAzure.Commands.Common;
2222
using Microsoft.WindowsAzure.Commands.Common.CustomAttributes;
2323
using Microsoft.WindowsAzure.Commands.Common.Properties;
24+
using Microsoft.WindowsAzure.Commands.Common.Sanitizer;
2425
using Microsoft.WindowsAzure.Commands.Common.Utilities;
2526
using System;
2627
using System.Collections.Concurrent;
@@ -55,7 +56,7 @@ public abstract class AzurePSCmdlet : PSCmdlet, IDisposable
5556
private object lockObject = new object();
5657
private AzurePSDataCollectionProfile _cachedProfile = null;
5758

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

6162
protected AzurePSDataCollectionProfile _dataCollectionProfile
@@ -166,6 +167,17 @@ public RuntimeDefinedParameterDictionary AsJobDynamicParameters
166167
}
167168
}
168169

170+
private IOutputSanitizer OutputSanitizer
171+
{
172+
get
173+
{
174+
if (AzureSession.Instance != null && AzureSession.Instance.TryGetComponent<IOutputSanitizer>(nameof(IOutputSanitizer), out var outputSanitizer))
175+
return outputSanitizer;
176+
177+
return null;
178+
}
179+
}
180+
169181
/// <summary>
170182
/// Resolve user submitted paths correctly on all platforms
171183
/// </summary>
@@ -331,7 +343,7 @@ protected override void BeginProcessing()
331343
SessionState = base.SessionState;
332344
var profile = _dataCollectionProfile;
333345
//TODO: Inject from CI server
334-
if(_metricHelper == null)
346+
if (_metricHelper == null)
335347
{
336348
lock (lockObject)
337349
{
@@ -390,14 +402,31 @@ private void WriteBreakingChangeOrPreviewMessage()
390402
}
391403
}
392404

405+
private void WriteSecretsWarningMessage()
406+
{
407+
if (_qosEvent?.SanitizerInfo != null)
408+
{
409+
var sanitizerInfo = _qosEvent.SanitizerInfo;
410+
if (sanitizerInfo.ShowSecretsWarning)
411+
{
412+
if (sanitizerInfo.DetectedProperties?.Count > 0)
413+
{
414+
WriteWarning(string.Format(Resources.DisplaySecretsWarningMessage, MyInvocation.InvocationName, string.Join(", ", sanitizerInfo.DetectedProperties)));
415+
}
416+
WriteDebug($"Sanitizer took {sanitizerInfo.SanitizeDuration.TotalMilliseconds}ms to process the object of cmdlet {MyInvocation.InvocationName}.");
417+
}
418+
}
419+
}
420+
393421
/// <summary>
394422
/// Perform end of pipeline processing.
395423
/// </summary>
396424
protected override void EndProcessing()
397425
{
398426
WriteEndProcessingRecommendation();
399427
WriteWarningMessageForVersionUpgrade();
400-
428+
WriteSecretsWarningMessage();
429+
401430
if (MetricHelper.IsCalledByUser()
402431
&& SurveyHelper.GetInstance().ShouldPromptAzSurvey()
403432
&& (AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
@@ -414,10 +443,12 @@ protected override void EndProcessing()
414443
// Send telemetry when cmdlet is directly called by user
415444
LogQosEvent();
416445
}
417-
else {
446+
else
447+
{
418448
// When cmdlet is called within another cmdlet, we will not add a new telemetry, but add the cmdlet name to InternalCalledCmdlets
419449
MetricHelper.AppendInternalCalledCmdlet(this.MyInvocation?.MyCommand?.Name);
420450
}
451+
421452
LogCmdletEndInvocationInfo();
422453
TearDownDebuggingTraces();
423454
TearDownHttpClientPipeline();
@@ -501,15 +532,26 @@ protected void WriteSurvey()
501532
protected new void WriteObject(object sendToPipeline)
502533
{
503534
FlushDebugMessages();
535+
SanitizeOutput(sendToPipeline);
504536
base.WriteObject(sendToPipeline);
505537
}
506538

507539
protected new void WriteObject(object sendToPipeline, bool enumerateCollection)
508540
{
509541
FlushDebugMessages();
542+
SanitizeOutput(sendToPipeline);
510543
base.WriteObject(sendToPipeline, enumerateCollection);
511544
}
512545

546+
private void SanitizeOutput(object sendToPipeline)
547+
{
548+
if (OutputSanitizer != null && OutputSanitizer.RequireSecretsDetection)
549+
{
550+
OutputSanitizer.Sanitize(sendToPipeline, out var telemetry);
551+
_qosEvent?.SanitizerInfo.Combine(telemetry);
552+
}
553+
}
554+
513555
protected new void WriteVerbose(string text)
514556
{
515557
FlushDebugMessages();
@@ -725,6 +767,8 @@ protected virtual void InitializeQosEvent()
725767
s => string.Format(CultureInfo.InvariantCulture, "-{0} ***", s)));
726768
}
727769
}
770+
771+
_qosEvent.SanitizerInfo = new SanitizerTelemetry();
728772
}
729773

730774
private void RecordDebugMessages()
@@ -777,7 +821,7 @@ private void RecordDebugMessages()
777821
private bool ShouldRecordDebugMessages()
778822
{
779823
return (!AzureSession.Instance.TryGetComponent<IConfigManager>(nameof(IConfigManager), out var configManager)
780-
|| !configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisableErrorRecordsPersistence))
824+
|| !configManager.GetConfigValue<bool>(ConfigKeysForCommon.DisableErrorRecordsPersistence))
781825
&& IsDataCollectionAllowed();
782826
}
783827

@@ -1009,7 +1053,6 @@ protected virtual void Dispose(bool disposing)
10091053
_adalListener.Dispose();
10101054
_adalListener = null;
10111055
}
1012-
10131056
}
10141057

10151058
public void Dispose()
@@ -1059,11 +1102,11 @@ protected string LoadModuleVersion(String Module, bool ListAvailable)
10591102
List<PSObject> outputs;
10601103
if (ListAvailable)
10611104
{
1062-
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module} -ListAvailable");
1105+
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module} -ListAvailable");
10631106
}
10641107
else
10651108
{
1066-
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module}");
1109+
outputs = this.ExecuteScript<PSObject>($"Get-Module -Name {Module}");
10671110
}
10681111
foreach (PSObject obj in outputs)
10691112
{
@@ -1105,7 +1148,7 @@ private string LoadPowerShellVersion()
11051148
foreach (PSObject obj in outputs)
11061149
{
11071150
string psVersion = obj.ToString();
1108-
return string.IsNullOrWhiteSpace(psVersion) ? DEFAULT_PSVERSION: psVersion;
1151+
return string.IsNullOrWhiteSpace(psVersion) ? DEFAULT_PSVERSION : psVersion;
11091152
}
11101153
}
11111154
catch (Exception e)

‎src/Common/MetricHelper.cs‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
using System.Text;
3535
using System.Threading.Tasks;
3636
using System.ComponentModel;
37+
using Microsoft.WindowsAzure.Commands.Common.Sanitizer;
3738

3839
namespace Microsoft.WindowsAzure.Commands.Common
3940
{
@@ -429,7 +430,7 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
429430
sb.Append($"{key.ToString().Substring(AzurePSErrorDataKeys.KeyPrefix.Length)}={qos.Exception.Data[key]}");
430431
}
431432
}
432-
if(sb.Length > 0)
433+
if (sb.Length > 0)
433434
{
434435
eventProperties["exception-data"] = sb.ToString();
435436
}
@@ -450,6 +451,8 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
450451
}
451452
}
452453

454+
PopulateSanitizerPropertiesFromQos(qos, eventProperties);
455+
453456
if (qos.InputFromPipeline != null)
454457
{
455458
eventProperties.Add("InputFromPipeline", qos.InputFromPipeline.Value.ToString());
@@ -458,13 +461,44 @@ private void PopulatePropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string,
458461
{
459462
eventProperties.Add("OutputToPipeline", qos.OutputToPipeline.Value.ToString());
460463
}
461-
464+
462465
foreach (var key in qos.CustomProperties.Keys)
463466
{
464467
eventProperties[key] = qos.CustomProperties[key];
465468
}
466469
}
467470

471+
private void PopulateSanitizerPropertiesFromQos(AzurePSQoSEvent qos, IDictionary<string, string> eventProperties)
472+
{
473+
if (qos.SanitizerInfo != null)
474+
{
475+
eventProperties["secrets-warning"] = qos.SanitizerInfo.ShowSecretsWarning.ToString();
476+
if (qos.SanitizerInfo.ShowSecretsWarning)
477+
{
478+
bool secretsDetected = qos.SanitizerInfo.DetectedProperties.Count > 0;
479+
eventProperties["secrets-detected"] = secretsDetected.ToString();
480+
if (secretsDetected)
481+
{
482+
eventProperties.Add("secrets-detected-properties", string.Join(";", qos.SanitizerInfo.DetectedProperties));
483+
}
484+
if (qos.SanitizerInfo.HasErrorInDetection && qos.SanitizerInfo.DetectionError != null)
485+
{
486+
eventProperties.Add("secrets-detection-exception-type", qos.SanitizerInfo.DetectionError.GetType().ToString());
487+
eventProperties.Add("secrets-detection-exception-message", qos.SanitizerInfo.DetectionError.Message);
488+
if (qos.SanitizerInfo.DetectionError.InnerException != null)
489+
{
490+
eventProperties.Add("secrets-detection-exception-inner-type", qos.SanitizerInfo.DetectionError.InnerException.GetType().ToString());
491+
eventProperties.Add("secrets-detection-exception-inner-message", qos.SanitizerInfo.DetectionError.InnerException.Message);
492+
}
493+
StackTrace sanitizerTrace = new StackTrace(qos.SanitizerInfo.DetectionError);
494+
string sanitizerExceptionStack = string.Join(";", sanitizerTrace.GetFrames().Take(3).Select(f => ConvertFrameToString(f)));
495+
eventProperties.Add("secrets-detection-exception-stack", sanitizerExceptionStack);
496+
}
497+
eventProperties.Add("secrets-sanitize-duration", qos.SanitizerInfo.SanitizeDuration.ToString("c"));
498+
}
499+
}
500+
}
501+
468502
private static string[] exceptionTrackAcceptModuleList = { "Az.Accounts", "Az.Compute", "Az.AKS", "Az.ContainerRegistry" };
469503
private static string[] exceptionTrackAcceptCmdletList = { "Get-AzKeyVaultSecret", "Get-AzKeyVaultCert" };
470504

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

661+
public SanitizerTelemetry SanitizerInfo { get; set; }
662+
627663
public AzurePSQoSEvent()
628664
{
629665
StartTime = DateTimeOffset.Now;

‎src/Common/Properties/Resources.Designer.cs‎

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/Common/Properties/Resources.resx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,4 +1760,7 @@ Note : Go to {0} for steps to suppress this breaking change warning, and other i
17601760
<data name="SurveyPreface" xml:space="preserve">
17611761
<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>
17621762
</data>
1763+
<data name="DisplaySecretsWarningMessage" xml:space="preserve">
1764+
<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>
1765+
</data>
17631766
</root>

‎src/Common/Sanitizer/IOutputSanitizer.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// ----------------------------------------------------------------------------------
2+
//
3+
// Copyright Microsoft Corporation
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
// ----------------------------------------------------------------------------------
14+
15+
namespace Microsoft.WindowsAzure.Commands.Common.Sanitizer
16+
{
17+
public interface IOutputSanitizer
18+
{
19+
bool RequireSecretsDetection { get; }
20+
21+
void Sanitize(object sanitizingObject, out SanitizerTelemetry telemetryData);
22+
}
23+
}

‎src/Common/Sanitizer/SanitizerTelemetry.cs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// ----------------------------------------------------------------------------------
2+
//
3+
// Copyright Microsoft Corporation
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
// Unless required by applicable law or agreed to in writing, software
9+
// distributed under the License is distributed on an "AS IS" BASIS,
10+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
// See the License for the specific language governing permissions and
12+
// limitations under the License.
13+
// ----------------------------------------------------------------------------------
14+
15+
using System;
16+
using System.Collections.Generic;
17+
18+
namespace Microsoft.WindowsAzure.Commands.Common.Sanitizer
19+
{
20+
public class SanitizerTelemetry
21+
{
22+
public bool ShowSecretsWarning { get; set; } = false;
23+
24+
public HashSet<string> DetectedProperties { get; set; } = new HashSet<string>();
25+
26+
public bool HasErrorInDetection { get; set; } = false;
27+
28+
public Exception DetectionError { get; set; }
29+
30+
public TimeSpan SanitizeDuration { get; set; }
31+
32+
public void Combine(SanitizerTelemetry telemetry)
33+
{
34+
if (telemetry != null)
35+
{
36+
ShowSecretsWarning = ShowSecretsWarning || telemetry.ShowSecretsWarning;
37+
DetectedProperties.UnionWith(telemetry.DetectedProperties);
38+
HasErrorInDetection = HasErrorInDetection || telemetry.HasErrorInDetection;
39+
DetectionError = DetectionError ?? telemetry.DetectionError;
40+
SanitizeDuration += telemetry.SanitizeDuration;
41+
}
42+
}
43+
}
44+
}

0 commit comments

Comments
 (0)