Skip to content

Commit

Permalink
Gfs/#329 (#333)
Browse files Browse the repository at this point in the history 
* Don't throw from WalkDirectory.

* Remove redundant RegistryKeyToRegistryObject code.
Simplify AddSubKeys flow.
Should fix an issue where failure to read one registry key might mean skipping processing a number of other keys.

* Remove overzealous filter per user request.

* Roslyn suggestions.

* Improve debug flush output.

* Improve registry collector debug statements.
  • Loading branch information
@gfs
gfs committed Dec 19, 2019
1 parent 8357625 commit e05f82e
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 106 deletions.
6 changes: 3 additions & 3 deletions Asa/Controllers/HomeController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public ActionResult WriteScanJson(int ResultType, string BaseId, string CompareI
return Json(true);
}

public ActionResult GetMonitorResults(string RunId, int ResultType, int Offset, int NumResults)
public ActionResult GetMonitorResults(string RunId, int Offset, int NumResults)
{

var results = new List<OutputFileMonitorResult>();
Expand Down Expand Up @@ -351,7 +351,7 @@ public ActionResult ChangeTelemetryState(bool DisableTelemetry)
return Json(true);
}

public ActionResult StartMonitoring(string RunId, string Directory, string Extension)
public ActionResult StartMonitoring(string RunId, string Directory)
{
if (RunId != null)
{
Expand Down Expand Up @@ -478,7 +478,7 @@ private static IEnumerable<DataRunModel> GetMonitorRunModels()
return runModels;
}

private IEnumerable<DataRunModel> GetRunModels()
private static IEnumerable<DataRunModel> GetRunModels()
{
List<string> Runs = AttackSurfaceAnalyzerClient.GetRuns("collect");

Expand Down
3 changes: 2 additions & 1 deletion Lib/Collectors/BaseCollector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,15 @@ public void Execute()
Log.Information(Strings.Get("Completed"), this.GetType().Name, answer);

var prevFlush = DatabaseManager.WriteQueue.Count;
var totFlush = prevFlush;

watch = System.Diagnostics.Stopwatch.StartNew();

while (DatabaseManager.HasElements())
{
Thread.Sleep(1000);
var sample = DatabaseManager.WriteQueue.Count;
Log.Debug("Flushing {0} results. ({1}/s)", DatabaseManager.WriteQueue.Count, prevFlush - sample);
Log.Debug("Flushing {0} results. ({1}/s {2:0.00}/s overall)", DatabaseManager.WriteQueue.Count, prevFlush - sample, ((double)(totFlush - sample)/watch.ElapsedMilliseconds) * 1000);
prevFlush = sample;
}

Expand Down
4 changes: 2 additions & 2 deletions Lib/Collectors/FileSystemUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public static string GetFilePermissions(FileSystemInfo fileInfo)
{
Log.Debug("Unable to get access control for {0}: {1}", fileInfo.FullName, e.Message);
}
catch (InvalidOperationException e)
catch (InvalidOperationException)
{
Log.Debug("Path probably doesn't exist: {0}", fileInfo.FullName);
}
Expand All @@ -72,7 +72,7 @@ public static string GetFilePermissions(FileSystemInfo fileInfo)
{
Log.Debug("Unable to get access control for {0}: {1}", fileInfo.FullName, e.Message);
}
catch (InvalidOperationException e)
catch (InvalidOperationException)
{
Log.Debug("Path probably doesn't exist: {0}", fileInfo.FullName);
}
Expand Down
53 changes: 1 addition & 52 deletions Lib/Collectors/RegistryCollector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,57 +87,6 @@ public static string GetName(RegistryAccessRule rule)
return SidMap[rule.IdentityReference.Value];
}

public static RegistryObject RegistryKeyToRegistryObject(RegistryKey registryKey)
{
RegistryObject regObj = null;
if (registryKey == null) { return regObj; }
try
{
regObj = new RegistryObject()
{
Key = registryKey.Name,
};

regObj.AddSubKeys(new List<string>(registryKey.GetSubKeyNames()));

foreach (RegistryAccessRule rule in registryKey.GetAccessControl().GetAccessRules(true, true, typeof(System.Security.Principal.SecurityIdentifier)))
{
string name = GetName(rule);

if (regObj.Permissions.ContainsKey(name))
{
regObj.Permissions[name].Add(rule.RegistryRights.ToString());
}
else
{
regObj.Permissions.Add(name, new List<string>() { rule.RegistryRights.ToString() });
}
}

foreach (string valueName in registryKey.GetValueNames())
{
try
{
regObj.Values.Add(valueName, (registryKey.GetValue(valueName) == null) ? "" : (registryKey.GetValue(valueName).ToString()));
}
catch (Exception ex)
{
Log.Debug(ex, "Found an exception processing registry values.");
}
}
}
catch (System.ArgumentException e)
{
Log.Debug(e, "Exception parsing {0}", registryKey.Name);
}
catch (Exception e)
{
Log.Debug(e, "Couldn't process reg key {0}", registryKey.Name);
}

return regObj;
}

public override void ExecuteInternal()
{
foreach (var hive in Hives)
Expand All @@ -156,7 +105,7 @@ public override void ExecuteInternal()
{
try
{
var regObj = RegistryKeyToRegistryObject(registryKey);
var regObj = RegistryWalker.RegistryKeyToRegistryObject(registryKey);
if (regObj != null)
{
Expand Down
2 changes: 1 addition & 1 deletion Lib/Objects/RegistryObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public RegistryObject()
Values = new Dictionary<string, string>();
}

public void AddSubKeys(List<string> subkeysIn)
public void AddSubKeys(string[] subkeysIn)
{
Subkeys.AddRange(subkeysIn);
}
Expand Down
100 changes: 54 additions & 46 deletions Lib/Utils/RegistryWalker.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public static IEnumerable<RegistryKey> WalkHive(RegistryHive Hive, string starti
{
if (startingKey != null)
{
x86_View = x86_View.OpenSubKey(startingKey);
x86_View = x86_View.OpenSubKey(startingKey, writable: false);
}
keys.Push(x86_View);
}
Expand All @@ -56,7 +56,7 @@ public static IEnumerable<RegistryKey> WalkHive(RegistryHive Hive, string starti
{
if (startingKey != null)
{
x64_View = x64_View.OpenSubKey(startingKey);
x64_View = x64_View.OpenSubKey(startingKey, writable: false);
}
keys.Push(x64_View);
}
Expand All @@ -75,33 +75,32 @@ public static IEnumerable<RegistryKey> WalkHive(RegistryHive Hive, string starti
}

// First push all the new subkeys onto our stack.
try
foreach (string key in currentKey.GetSubKeyNames())
{
foreach (string key in currentKey.GetSubKeyNames())
try
{

var next = currentKey.OpenSubKey(name: key, writable: false);
keys.Push(next);
}
// These are expected as we are running as administrator, not System.
catch (System.Security.SecurityException)
{
Log.Debug("Permission Denied Opening Subkey: {0}\\{1}", currentKey.Name, key);
}
// There seem to be some keys which are listed as existing by the APIs but don't actually exist.
// Unclear if these are just super transient keys or what the other cause might be.
// Since this isn't user actionable, also just supress these to the verbose stream.
catch (System.IO.IOException)
{
Log.Debug("IOError Reading: {0}\\{1}", currentKey.Name, key);
}
catch (Exception e)
{
Log.Information(e, "Unexpected error when parsing {0}\\{1}", currentKey.Name, key);
AsaTelemetry.TrackTrace(Microsoft.ApplicationInsights.DataContracts.SeverityLevel.Error, e);
}
}
// These are expected as we are running as administrator, not System.
catch (System.Security.SecurityException e)
{
Log.Verbose(e, "Permission Denied: {0}", currentKey.Name);
}
// There seem to be some keys which are listed as existing by the APIs but don't actually exist.
// Unclear if these are just super transient keys or what the other cause might be.
// Since this isn't user actionable, also just supress these to the verbose stream.
catch (System.IO.IOException e)
{
Log.Verbose(e, "Error Reading: {0}", currentKey.Name);
}
catch (Exception e)
{
Log.Information(e, "Unexpected error when parsing {0}:", currentKey.Name);
AsaTelemetry.TrackTrace(Microsoft.ApplicationInsights.DataContracts.SeverityLevel.Error, e);
}


yield return currentKey;
}

Expand All @@ -113,15 +112,26 @@ public static RegistryObject RegistryKeyToRegistryObject(RegistryKey registryKey
{
RegistryObject regObj = null;
if (registryKey == null) { return regObj; }
try
{

regObj = new RegistryObject()
{
Key = registryKey.Name,
};
try
{
regObj.AddSubKeys(registryKey.GetSubKeyNames());
}
catch (System.ArgumentException)
{
Log.Debug("Invalid Handle (ArgumentException) {0}", registryKey.Name);
}
catch (Exception e)
{
Log.Debug(e, "Couldn't process reg key {0}", registryKey.Name);
}

regObj.AddSubKeys(new List<string>(registryKey.GetSubKeyNames()));

try
{
foreach (RegistryAccessRule rule in registryKey.GetAccessControl().GetAccessRules(true, true, typeof(System.Security.Principal.SecurityIdentifier)))
{
string name = rule.IdentityReference.Value;
Expand All @@ -144,32 +154,30 @@ public static RegistryObject RegistryKeyToRegistryObject(RegistryKey registryKey
regObj.Permissions.Add(name, new List<string>() { rule.RegistryRights.ToString() });
}
}

foreach (string valueName in registryKey.GetValueNames())
{
try
{
if (registryKey.GetValue(valueName) == null)
{

}
regObj.Values.Add(valueName, (registryKey.GetValue(valueName) == null) ? "" : (registryKey.GetValue(valueName).ToString()));
}
catch (Exception ex)
{
Log.Debug(ex, "Found an exception processing registry values.");
}
}
}
catch (System.ArgumentException)
catch (ArgumentException)
{
Log.Debug("Invalid Handle (ArgumentException) {0}", registryKey.Name);
Log.Debug("Failed to get permissions (handle is invalid) for {0}", regObj.Key);
}
catch (Exception e)
{
Log.Debug(e, "Couldn't process reg key {0}", registryKey.Name);
Log.Debug(e, "Failed to get permissions for {0}", regObj.Key);
}


foreach (string valueName in registryKey.GetValueNames())
{
try
{
regObj.Values.Add(valueName, (registryKey.GetValue(valueName) == null) ? "" : (registryKey.GetValue(valueName).ToString()));
}
catch (Exception ex)
{
Log.Debug(ex, "Found an exception processing registry values of {0}.",registryKey.Name);
}
}


return regObj;
}
}
Expand Down
1 change: 0 additions & 1 deletion filters.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"^HKEY_USERS\\\\S-[0-9]*-[0-9]*-[0-9]*\\\\Software\\\\Microsoft\\\\Cryptography$",
"^HKEY_LOCAL_MACHINE\\\\SOFTWARE\\\\Microsoft\\\\Windows NT\\\\CurrentVersion\\\\Perflib",
"^HKEY_LOCAL_MACHINE\\\\SOFTWARE\\\\Microsoft\\\\Windows NT\\\\CurrentVersion\\\\AppCompatFlags\\\\CIT$",
"^HKEY_LOCAL_MACHINE\\\\SOFTWARE\\\\Microsoft\\\\Windows\\\\CurrentVersion",
"^HKEY_LOCAL_MACHINE\\\\SOFTWARE\\\\Microsoft\\\\WcmSvc\\\\wifinetworkmanager$",
"^HKEY_LOCAL_MACHINE\\\\SYSTEM\\\\ControlSet001\\\\Services\\\\ADOVMPPackages",
"^HKEY_LOCAL_MACHINE\\\\SYSTEM\\\\ControlSet001\\\\Services\\\\DeviceAssociationService\\\\State\\\\Store\\\\",
Expand Down

0 comments on commit e05f82e

Please sign in to comment.