From b2ea36176f27b47dc71e3d528fbbdd1110972340 Mon Sep 17 00:00:00 2001 From: Gabe Stocco <98900+gfs@users.noreply.github.com> Date: Mon, 2 Aug 2021 10:21:16 -0700 Subject: [PATCH] Fix Loading Results in Report (#607) * Fix Loading Results in Report * Reformat sandbox rules which apply * Whitespace * Fix Unsigned Binaries rules * Log the string that failed to be hashed. * Reduce verbosity of some outputs * Fix nullability concerns * Bump dependencies * Update CollectObject.cs --- Benchmarks/LiteDbManager.cs | 2 +- Cli/AttackSurfaceAnalyzerClient.cs | 45 ++++++++++++++--------- Cli/Cli.csproj | 8 ++--- Cli/Components/States/Results.razor | 56 +++++++++++++++-------------- Cli/Pages/Report.razor | 8 ++++- Cli/Pages/Sandbox.razor | 15 ++++---- Lib/Collectors/BaseCompare.cs | 9 ++--- Lib/Lib.csproj | 6 ++-- Lib/Objects/CollectObject.cs | 17 ++++----- Lib/Objects/CompareResult.cs | 6 +--- Lib/Objects/SkipCompareAttribute.cs | 13 +++++++ Lib/Properties/Resources.resx | 4 +-- Lib/Utils/AsaHelpers.cs | 4 +-- Lib/Utils/CryptoHelpers.cs | 2 +- analyses.json | 14 +++----- 15 files changed, 117 insertions(+), 92 deletions(-) create mode 100644 Lib/Objects/SkipCompareAttribute.cs diff --git a/Benchmarks/LiteDbManager.cs b/Benchmarks/LiteDbManager.cs index 3dedfd958..51762acc2 100644 --- a/Benchmarks/LiteDbManager.cs +++ b/Benchmarks/LiteDbManager.cs @@ -361,7 +361,7 @@ public static PLATFORM RunIdToPlatform(string runid) var col = db?.GetCollection("Runs"); var results = col?.Find(x => x.RunId.Equals(runid)); - if (results.Any()) + if (results?.Any() ?? false) { return results.First().Platform; } diff --git a/Cli/AttackSurfaceAnalyzerClient.cs b/Cli/AttackSurfaceAnalyzerClient.cs index b73781399..35d80b008 100644 --- a/Cli/AttackSurfaceAnalyzerClient.cs +++ b/Cli/AttackSurfaceAnalyzerClient.cs @@ -192,7 +192,7 @@ private static ASA_ERROR RunGuidedModeCommand(GuidedModeCommandOptions opts) results.TryAdd(key, monitorResult[key]); }); - return ExportGuidedModeResults(results, opts); + return ExportGuidedModeResults(results, opts, analysisFile.GetHash()); } public static ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), List> AnalyzeMonitored(CompareCommandOptions opts) @@ -481,16 +481,16 @@ private static ASA_ERROR RunExportCollectCommand(ExportCollectCommandOptions opt }; var results = CompareRuns(options); - + var analysesHash = options.AnalysesFile.GetHash(); if (opts.SaveToDatabase) { - InsertCompareResults(results, opts.FirstRunId, opts.SecondRunId, options.AnalysesFile.GetHash()); + InsertCompareResults(results, opts.FirstRunId, opts.SecondRunId, analysesHash); } - return ExportCompareResults(results, opts, AsaHelpers.MakeValidFileName(opts.FirstRunId + "_vs_" + opts.SecondRunId)); + return ExportCompareResults(results, opts, AsaHelpers.MakeValidFileName(opts.FirstRunId + "_vs_" + opts.SecondRunId), analysesHash); } - private static ASA_ERROR ExportGuidedModeResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), List> resultsIn, GuidedModeCommandOptions opts) + private static ASA_ERROR ExportGuidedModeResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), List> resultsIn, GuidedModeCommandOptions opts, string analysesHash) { if (opts.RunId == null) { @@ -510,10 +510,11 @@ private static ASA_ERROR ExportGuidedModeResults(ConcurrentDictionary<(RESULT_TY { outputPath = Directory.GetCurrentDirectory(); } + var metadata = AsaHelpers.GenerateMetadata(); + metadata.Add("analyses-hash", analysesHash); if (opts.ExplodedOutput) { - results.Add("metadata", AsaHelpers.GenerateMetadata()); - + results.Add("metadata",metadata); string path = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(opts.RunId)); Directory.CreateDirectory(path); foreach (var key in results.Keys) @@ -534,7 +535,7 @@ private static ASA_ERROR ExportGuidedModeResults(ConcurrentDictionary<(RESULT_TY string path = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(opts.RunId + "_summary.json.txt")); var output = new Dictionary(); output["results"] = results; - output["metadata"] = AsaHelpers.GenerateMetadata(); + output["metadata"] = metadata; using (StreamWriter sw = new StreamWriter(path)) //lgtm[cs/path-injection] { using (JsonWriter writer = new JsonTextWriter(sw)) @@ -547,7 +548,7 @@ private static ASA_ERROR ExportGuidedModeResults(ConcurrentDictionary<(RESULT_TY return ASA_ERROR.NONE; } - private static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), List> resultsIn, ExportOptions opts, string baseFileName) + private static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, CHANGE_TYPE), List> resultsIn, ExportOptions opts, string baseFileName, string analysesHash) { var results = resultsIn.Select(x => new KeyValuePair($"{x.Key.Item1}_{x.Key.Item2}", x.Value)).ToDictionary(x => x.Key, x => x.Value); JsonSerializer serializer = JsonSerializer.Create(new JsonSerializerSettings() @@ -563,9 +564,11 @@ private static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, { outputPath = Directory.GetCurrentDirectory(); } + var metadata = AsaHelpers.GenerateMetadata(); + metadata.Add("analyses-hash", analysesHash); if (opts.ExplodedOutput) { - results.Add("metadata", AsaHelpers.GenerateMetadata()); + results.Add("metadata", metadata); string path = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(baseFileName)); Directory.CreateDirectory(path); @@ -587,7 +590,7 @@ private static ASA_ERROR ExportCompareResults(ConcurrentDictionary<(RESULT_TYPE, string path = Path.Combine(outputPath, AsaHelpers.MakeValidFileName(baseFileName + "_summary.json.txt")); var output = new Dictionary(); output["results"] = results; - output["metadata"] = AsaHelpers.GenerateMetadata(); + output["metadata"] = metadata; using (StreamWriter sw = new StreamWriter(path)) //lgtm[cs/path-injection] { using (JsonWriter writer = new JsonTextWriter(sw)) @@ -624,6 +627,14 @@ protected override JsonProperty CreateProperty(MemberInfo member, MemberSerializ } } + if (property.DeclaringType == typeof(CompareResult)) + { + if (property.PropertyName == "AnalysesHash") + { + property.ShouldSerialize = _ => { return false; }; + } + } + return property; } } @@ -658,12 +669,14 @@ private static ASA_ERROR RunExportMonitorCommand(ExportMonitorCommandOptions opt var monitorResult = AnalyzeMonitored(monitorCompareOpts); + var analysesHash = monitorCompareOpts.AnalysesFile.GetHash(); + if (opts.SaveToDatabase) { - InsertCompareResults(monitorResult, null, opts.RunId, monitorCompareOpts.AnalysesFile.GetHash()); + InsertCompareResults(monitorResult, null, opts.RunId, analysesHash); } - return ExportCompareResults(monitorResult, opts, AsaHelpers.MakeValidFileName(opts.RunId)); + return ExportCompareResults(monitorResult, opts, AsaHelpers.MakeValidFileName(opts.RunId), analysesHash); } public static void WriteMonitorJson(string RunId, int ResultType, string OutputPath) @@ -984,21 +997,21 @@ public static void AdminOrWarn() { if (!Elevation.IsAdministrator()) { - Log.Warning(Strings.Get("Err_RunAsAdmin")); + Log.Information(Strings.Get("Err_RunAsAdmin")); } } else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) { if (!Elevation.IsRunningAsRoot()) { - Log.Warning(Strings.Get("Err_RunAsRoot")); + Log.Information(Strings.Get("Err_RunAsRoot")); } } else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) { if (!Elevation.IsRunningAsRoot()) { - Log.Warning(Strings.Get("Err_RunAsRoot")); + Log.Information(Strings.Get("Err_RunAsRoot")); } } } diff --git a/Cli/Cli.csproj b/Cli/Cli.csproj index f0738cb3a..21a0e6e93 100644 --- a/Cli/Cli.csproj +++ b/Cli/Cli.csproj @@ -35,10 +35,10 @@ - - - - + + + + diff --git a/Cli/Components/States/Results.razor b/Cli/Components/States/Results.razor index ed4af8679..19196e967 100644 --- a/Cli/Components/States/Results.razor +++ b/Cli/Components/States/Results.razor @@ -22,28 +22,28 @@ @if (pageCount > 0) { -

@foundResultTypes[(RESULT_TYPE)Enum.Parse(typeof(RESULT_TYPE), SelectedResultType)] results found for type @SelectedResultType.

- +

@foundResultTypes[(RESULT_TYPE)Enum.Parse(typeof(RESULT_TYPE), SelectedResultType)] results found for type @SelectedResultType.

+ @for (int i = 0; i < analysisResults.Count; i++) { @@ -67,15 +67,19 @@ else } @code { + string _firstRunId = string.Empty; + string _secondRundId = string.Empty; + string _analysesHash = string.Empty; + string _monitorRunId = string.Empty; [Parameter] - public string FirstRunId { get; set; } = string.Empty; + public string FirstRunId { get { return _firstRunId; } set { _firstRunId = value; OnInitialized(); } } [Parameter] - public string SecondRunId { get; set; } = string.Empty; + public string SecondRunId { get { return _secondRundId; } set { _secondRundId = value; OnInitialized(); } } [Parameter] - public string AnalysesHash { get; set; } = string.Empty; + public string AnalysesHash { get { return _analysesHash; } set { _analysesHash = value; OnInitialized(); } } [Parameter] - public string MonitorRunId { get; set; } = string.Empty; + public string MonitorRunId { get { return _monitorRunId; } set { _monitorRunId = value; OnInitialized(); } } protected override void OnInitialized() { diff --git a/Cli/Pages/Report.razor b/Cli/Pages/Report.razor index abdcea143..070e45c98 100644 --- a/Cli/Pages/Report.razor +++ b/Cli/Pages/Report.razor @@ -10,7 +10,13 @@ diff --git a/Cli/Pages/Sandbox.razor b/Cli/Pages/Sandbox.razor index 3bf4f3f83..2cb502cfe 100644 --- a/Cli/Pages/Sandbox.razor +++ b/Cli/Pages/Sandbox.razor @@ -46,7 +46,6 @@ } -
@if (ParseErrors.Any()) @@ -90,14 +89,18 @@ @for (int i = 0; i < AppState.TestObjects.Count; i++) { - var results = analyzer.Analyze(AppState.Rules, AppState.TestObjects[i]); + var results = analyzer.Analyze(AppState.Rules, AppState.TestObjects[i]).ToList();
- @results.Count() rules applied. - @foreach (var result in results) +
@results.Count rules applied
+ + @for (int j = 0; j < results.Count; j++) { - @result.Name +
+ @results[j].Name: @results[j].Description +
} - + +
}
diff --git a/Lib/Collectors/BaseCompare.cs b/Lib/Collectors/BaseCompare.cs index fbfa5abce..3e33656b9 100644 --- a/Lib/Collectors/BaseCompare.cs +++ b/Lib/Collectors/BaseCompare.cs @@ -123,7 +123,6 @@ public void Compare(IEnumerable<(CollectObject, string)> differentObjects, IEnum { BaseRunId = firstRunId, CompareRunId = secondRunId, - BaseRowKey = colObj.RowKey, }; if (different.Item2.Equals(firstRunId)) @@ -152,9 +151,7 @@ public void Compare(IEnumerable<(CollectObject, string)> differentObjects, IEnum Base = first, Compare = second, BaseRunId = firstRunId, - CompareRunId = secondRunId, - BaseRowKey = modified.Item1.RowKey, - CompareRowKey = modified.Item2.RowKey, + CompareRunId = secondRunId }; var properties = first.GetType().GetProperties(); @@ -165,6 +162,10 @@ public void Compare(IEnumerable<(CollectObject, string)> differentObjects, IEnum { try { + if (Attribute.IsDefined(prop, typeof(SkipCompareAttribute))) + { + continue; + } List diffs; object? added = null; object? removed = null; diff --git a/Lib/Lib.csproj b/Lib/Lib.csproj index 7c9f1015f..24a28fa4d 100644 --- a/Lib/Lib.csproj +++ b/Lib/Lib.csproj @@ -35,9 +35,9 @@ - - - + + + diff --git a/Lib/Objects/CollectObject.cs b/Lib/Objects/CollectObject.cs index 1ee5eff27..34bd9864b 100644 --- a/Lib/Objects/CollectObject.cs +++ b/Lib/Objects/CollectObject.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. using Microsoft.CST.AttackSurfaceAnalyzer.Types; using Microsoft.CST.AttackSurfaceAnalyzer.Utils; +using Newtonsoft.Json; using System.Globalization; namespace Microsoft.CST.AttackSurfaceAnalyzer.Objects @@ -13,6 +14,8 @@ public abstract class CollectObject public abstract string Identity { get; } public RESULT_TYPE ResultType { get; set; } + [SkipCompare] + [JsonIgnore] public string RowKey { get @@ -20,7 +23,9 @@ public string RowKey return Serialized.GetHashCode().ToString(CultureInfo.InvariantCulture); } } - + + [SkipCompare] + [JsonIgnore] public string Serialized { get @@ -34,16 +39,6 @@ public string Serialized } } - public static bool ShouldSerializeRowKey() - { - return false; - } - - public static bool ShouldSerializeSerialized() - { - return false; - } - private string? _serialized = null; } } \ No newline at end of file diff --git a/Lib/Objects/CompareResult.cs b/Lib/Objects/CompareResult.cs index e35399a85..9a84b33fa 100644 --- a/Lib/Objects/CompareResult.cs +++ b/Lib/Objects/CompareResult.cs @@ -15,8 +15,6 @@ public CompareResult() public CollectObject? Base { get; set; } - public string? BaseRowKey { get; set; } - public string? BaseRunId { get; set; } public CHANGE_TYPE ChangeType @@ -50,8 +48,6 @@ public CHANGE_TYPE ChangeType public CollectObject? Compare { get; set; } - public string? CompareRowKey { get; set; } - public string? CompareRunId { get; set; } public List Diffs { get; set; } = new List(); @@ -95,7 +91,7 @@ public RESULT_TYPE ResultType } public List Rules { get; set; } = new List(); - public string AnalysesHash { get; set; } + public string AnalysesHash { get; set; } = string.Empty; public bool ShouldSerializeDiffs() { diff --git a/Lib/Objects/SkipCompareAttribute.cs b/Lib/Objects/SkipCompareAttribute.cs new file mode 100644 index 000000000..4bd1ff1d9 --- /dev/null +++ b/Lib/Objects/SkipCompareAttribute.cs @@ -0,0 +1,13 @@ +using System; + +namespace Microsoft.CST.AttackSurfaceAnalyzer.Objects +{ + [AttributeUsage(AttributeTargets.All) +] + public class SkipCompareAttribute : Attribute + { + public SkipCompareAttribute() + { + } + } +} diff --git a/Lib/Properties/Resources.resx b/Lib/Properties/Resources.resx index 9d43adbbd..c89fb51f4 100644 --- a/Lib/Properties/Resources.resx +++ b/Lib/Properties/Resources.resx @@ -439,10 +439,10 @@ Starting {0} {1}. - Attack Surface Analyzer should be run as Administrator. Results will not be complete. + Attack Surface Analyzer is not running as Administrator. Results may not be complete. - Attack Surface Analyzer should be run as root. Results will not be complete. + Attack Surface Analyzer is not running as root. Results may not be complete. Loaded Analyses from {0}. diff --git a/Lib/Utils/AsaHelpers.cs b/Lib/Utils/AsaHelpers.cs index 6b13f34b6..0fae71809 100644 --- a/Lib/Utils/AsaHelpers.cs +++ b/Lib/Utils/AsaHelpers.cs @@ -21,8 +21,8 @@ public static Dictionary GenerateMetadata() var dict = new Dictionary(); dict["compare-version"] = GetVersionString(); - dict["compare-os"] = GetOsName(); - dict["compare-osversion"] = GetOsVersion(); + dict["compare-os"] = GetOsName().Trim(); + dict["compare-osversion"] = GetOsVersion().Trim(); return dict; } diff --git a/Lib/Utils/CryptoHelpers.cs b/Lib/Utils/CryptoHelpers.cs index bd1a3ade8..2d7d10081 100644 --- a/Lib/Utils/CryptoHelpers.cs +++ b/Lib/Utils/CryptoHelpers.cs @@ -19,7 +19,7 @@ public static string CreateHash(string input) } catch (CryptographicException e) { - Log.Warning(e, Strings.Get("Err_CreateHash"), "string"); + Log.Warning(e, Strings.Get("Err_CreateHash"), input is null ? "null string" : $"'{input}'"); return string.Empty; } } diff --git a/analyses.json b/analyses.json index 25a036ff9..9dece1176 100644 --- a/analyses.json +++ b/analyses.json @@ -57,11 +57,8 @@ }, { "Label": "valid_windows_signature", - "Field": "SignatureStatus", - "Operation": "Equals", - "Data": [ - "Valid" - ] + "Field": "SignatureStatus.IsAuthenticodeValid", + "Operation": "IsTrue" }, { "Label": "null_mac_signature", @@ -88,11 +85,8 @@ }, { "Label": "valid_windows_signature", - "Field": "FileSystemObject.SignatureStatus", - "Operation": "Equals", - "Data": [ - "Valid" - ] + "Field": "FileSystemObject.SignatureStatus.IsAuthenticodeValid", + "Operation": "IsTrue" }, { "Label": "null_mac_signature",