Better handling of files with no signature.

vcsjones · vcsjones · commit 59e2fbd4a1d6 · 2016-04-13T16:40:16.000-04:00
If the file lacks an Authenticode signature each rule is guaranteed to fail.
diff --git a/AuthenticodeLint/CheckEngine.cs b/AuthenticodeLint/CheckEngine.cs
@@ -38,16 +38,24 @@ public RuleEngineResult RunAllRules(string file, Graph<SignerInfo> signatures, L
             {
                 RuleResult result;
                 var verboseWriter = verbose ? new VerboseSignatureLogger() : SignatureLoggerBase.Null;
-                if (suppressedRuleIDs.Contains(rule.RuleId))
+                if (signatures.Items.Count == 0)
                 {
-                    result = RuleResult.Skip;
+                    result = RuleResult.Fail;
+                    verboseWriter.LogMessage("File is not Authenticode signed.");
                 }
                 else
                 {
-                    result = rule.Validate(signatures, verboseWriter);
-                    if (result != RuleResult.Pass)
+                    if (suppressedRuleIDs.Contains(rule.RuleId))
                     {
-                        engineResult = RuleEngineResult.NotAllPass;
+                        result = RuleResult.Skip;
+                    }
+                    else
+                    {
+                        result = rule.Validate(signatures, verboseWriter);
+                        if (result != RuleResult.Pass)
+                        {
+                            engineResult = RuleEngineResult.NotAllPass;
+                        }
                     }
                 }
                 collectors.ForEach(c => c.CollectResult(rule, result, verboseWriter.Messages));
diff --git a/AuthenticodeLint/ConfigurationValidator.cs b/AuthenticodeLint/ConfigurationValidator.cs
@@ -38,7 +38,7 @@ public static bool ValidateAndPrint(CheckConfiguration configuration, TextWriter
             {
                 if (!File.Exists(path))
                 {
-                    printer.WriteLine($"The input path ${path} does not exist.");
+                    printer.WriteLine($"The input path {path} does not exist.");
                     success = false;
                 }
             }
diff --git a/AuthenticodeLint/Rules/NoWeakFileDigestAlgorithmsRule.cs b/AuthenticodeLint/Rules/NoWeakFileDigestAlgorithmsRule.cs
@@ -18,17 +18,17 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW
             {
                 if (signature.DigestAlgorithm.Value == KnownOids.MD2)
                 {
-                    verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD2)} digest algorithm.");
+                    verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD2)} digest algorithm.");
                     result = RuleResult.Fail;
                 }
                 else if (signature.DigestAlgorithm.Value == KnownOids.MD4)
                 {
-                    verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD4)} digest algorithm.");
+                    verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD4)} digest algorithm.");
                     result = RuleResult.Fail;
                 }
                 else if (signature.DigestAlgorithm.Value == KnownOids.MD5)
                 {
-                    verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD5)} digest algorithm.");
+                    verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD5)} digest algorithm.");
                     result = RuleResult.Fail;
                 }
             }
diff --git a/AuthenticodeLint/Rules/PublisherInformationRule.cs b/AuthenticodeLint/Rules/PublisherInformationRule.cs
@@ -29,24 +29,24 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW
                 if (info == null)
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature does not have any publisher information.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature does not have any publisher information.");
                 }
                 if (string.IsNullOrWhiteSpace(info.Description))
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature does not have an accompanying description.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying description.");
                 }
 
                 if (string.IsNullOrWhiteSpace(info.UrlLink))
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature does not have an accompanying URL.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying URL.");
                 }
                 Uri uri;
                 if (!Uri.TryCreate(info.UrlLink, UriKind.Absolute, out uri))
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature's accompanying URL is not a valid URI.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature's accompanying URL is not a valid URI.");
                 }
             }
             return result;
diff --git a/AuthenticodeLint/Rules/PublisherInformationUrlHttpsRule.cs b/AuthenticodeLint/Rules/PublisherInformationUrlHttpsRule.cs
@@ -29,17 +29,17 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW
                 if (info == null)
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature does not have any publisher information.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature does not have any publisher information.");
                 }
                 if (string.IsNullOrWhiteSpace(info.UrlLink))
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, "Signature does not have an accompanying URL.");
+                    verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying URL.");
                 }
                 if (!info.UrlLink.StartsWith(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))
                 {
                     result = RuleResult.Fail;
-                    verboseWriter.LogMessage(signature, $"Signature's publisher information URL \"{info.UrlLink}\" does not use the secure HTTPS scheme.");
+                    verboseWriter.LogSignatureMessage(signature, $"Signature's publisher information URL \"{info.UrlLink}\" does not use the secure HTTPS scheme.");
                 }
             }
             return result;
diff --git a/AuthenticodeLint/Rules/Sha1PrimarySignatureRule.cs b/AuthenticodeLint/Rules/Sha1PrimarySignatureRule.cs
@@ -23,7 +23,7 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW
             }
             if (primary.DigestAlgorithm.Value != KnownOids.SHA1)
             {
-                verboseWriter.LogMessage(primary, $"Expected {nameof(KnownOids.SHA1)} digest algorithm but is {primary.DigestAlgorithm.FriendlyName}.");
+                verboseWriter.LogSignatureMessage(primary, $"Expected {nameof(KnownOids.SHA1)} digest algorithm but is {primary.DigestAlgorithm.FriendlyName}.");
                 return RuleResult.Fail;
             }
             return RuleResult.Pass;
diff --git a/AuthenticodeLint/Rules/TimestampedRule.cs b/AuthenticodeLint/Rules/TimestampedRule.cs
@@ -47,12 +47,12 @@ public unsafe RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase v
                 }
                 if (!isSigned)
                 {
-                    verboseWriter.LogMessage(signature, $"Signature is not timestamped.");
+                    verboseWriter.LogSignatureMessage(signature, $"Signature is not timestamped.");
                     pass = false;
                 }
                 else if (!strongSign)
                 {
-                    verboseWriter.LogMessage(signature, $"Signature is not timestamped with the expected hash algorithm {signature.DigestAlgorithm.FriendlyName}.");
+                    verboseWriter.LogSignatureMessage(signature, $"Signature is not timestamped with the expected hash algorithm {signature.DigestAlgorithm.FriendlyName}.");
                     pass = false;
                 }
             }
diff --git a/AuthenticodeLint/VerboseSignatureTextWriter.cs b/AuthenticodeLint/VerboseSignatureTextWriter.cs
@@ -9,7 +9,9 @@ namespace AuthenticodeLint
 {
     public class VerboseSignatureLogger : SignatureLoggerBase
     {
-        public override void LogMessage(SignerInfo signature, string message)
+        public override void LogMessage(string message) => Messages.Add(message);
+
+        public override void LogSignatureMessage(SignerInfo signature, string message)
         {
             var digest = signature.SignatureDigest();
             var digestString = digest.Aggregate(new StringBuilder(), (acc, b) => acc.AppendFormat("{0:x2}", b)).ToString();
@@ -19,7 +21,11 @@ public override void LogMessage(SignerInfo signature, string message)
 
     public class NullSignatureLogger : SignatureLoggerBase
     {
-        public override void LogMessage(SignerInfo signature, string message)
+        public override void LogMessage(string message)
+        {
+        }
+
+        public override void LogSignatureMessage(SignerInfo signature, string message)
         {
         }
     }
@@ -30,6 +36,7 @@ public abstract class SignatureLoggerBase
 
         internal List<string> Messages { get; } = new List<string>();
 
-        public abstract void LogMessage(SignerInfo signature, string message);
+        public abstract void LogSignatureMessage(SignerInfo signature, string message);
+        public abstract void LogMessage(string message);
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -38,16 +38,24 @@ public RuleEngineResult RunAllRules(string file, Graph<SignerInfo> signatures, L`
`38`	`38`	`{`
`39`	`39`	`RuleResult result;`
`40`	`40`	`var verboseWriter = verbose ? new VerboseSignatureLogger() : SignatureLoggerBase.Null;`
`41`		`- if (suppressedRuleIDs.Contains(rule.RuleId))`
	`41`	`+ if (signatures.Items.Count == 0)`
`42`	`42`	`{`
`43`		`- result = RuleResult.Skip;`
	`43`	`+ result = RuleResult.Fail;`
	`44`	`+ verboseWriter.LogMessage("File is not Authenticode signed.");`
`44`	`45`	`}`
`45`	`46`	`else`
`46`	`47`	`{`
`47`		`- result = rule.Validate(signatures, verboseWriter);`
`48`		`- if (result != RuleResult.Pass)`
	`48`	`+ if (suppressedRuleIDs.Contains(rule.RuleId))`
`49`	`49`	`{`
`50`		`- engineResult = RuleEngineResult.NotAllPass;`
	`50`	`+ result = RuleResult.Skip;`
	`51`	`+ }`
	`52`	`+ else`
	`53`	`+ {`
	`54`	`+ result = rule.Validate(signatures, verboseWriter);`
	`55`	`+ if (result != RuleResult.Pass)`
	`56`	`+ {`
	`57`	`+ engineResult = RuleEngineResult.NotAllPass;`
	`58`	`+ }`
`51`	`59`	`}`
`52`	`60`	`}`
`53`	`61`	`collectors.ForEach(c => c.CollectResult(rule, result, verboseWriter.Messages));`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ public static bool ValidateAndPrint(CheckConfiguration configuration, TextWriter`
`38`	`38`	`{`
`39`	`39`	`if (!File.Exists(path))`
`40`	`40`	`{`
`41`		`- printer.WriteLine($"The input path ${path} does not exist.");`
	`41`	`+ printer.WriteLine($"The input path {path} does not exist.");`
`42`	`42`	`success = false;`
`43`	`43`	`}`
`44`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -18,17 +18,17 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW`
`18`	`18`	`{`
`19`	`19`	`if (signature.DigestAlgorithm.Value == KnownOids.MD2)`
`20`	`20`	`{`
`21`		`- verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD2)} digest algorithm.");`
	`21`	`+ verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD2)} digest algorithm.");`
`22`	`22`	`result = RuleResult.Fail;`
`23`	`23`	`}`
`24`	`24`	`else if (signature.DigestAlgorithm.Value == KnownOids.MD4)`
`25`	`25`	`{`
`26`		`- verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD4)} digest algorithm.");`
	`26`	`+ verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD4)} digest algorithm.");`
`27`	`27`	`result = RuleResult.Fail;`
`28`	`28`	`}`
`29`	`29`	`else if (signature.DigestAlgorithm.Value == KnownOids.MD5)`
`30`	`30`	`{`
`31`		`- verboseWriter.LogMessage(signature, $"Uses the {nameof(KnownOids.MD5)} digest algorithm.");`
	`31`	`+ verboseWriter.LogSignatureMessage(signature, $"Uses the {nameof(KnownOids.MD5)} digest algorithm.");`
`32`	`32`	`result = RuleResult.Fail;`
`33`	`33`	`}`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,24 +29,24 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW`
`29`	`29`	`if (info == null)`
`30`	`30`	`{`
`31`	`31`	`result = RuleResult.Fail;`
`32`		`- verboseWriter.LogMessage(signature, "Signature does not have any publisher information.");`
	`32`	`+ verboseWriter.LogSignatureMessage(signature, "Signature does not have any publisher information.");`
`33`	`33`	`}`
`34`	`34`	`if (string.IsNullOrWhiteSpace(info.Description))`
`35`	`35`	`{`
`36`	`36`	`result = RuleResult.Fail;`
`37`		`- verboseWriter.LogMessage(signature, "Signature does not have an accompanying description.");`
	`37`	`+ verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying description.");`
`38`	`38`	`}`
`39`	`39`
`40`	`40`	`if (string.IsNullOrWhiteSpace(info.UrlLink))`
`41`	`41`	`{`
`42`	`42`	`result = RuleResult.Fail;`
`43`		`- verboseWriter.LogMessage(signature, "Signature does not have an accompanying URL.");`
	`43`	`+ verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying URL.");`
`44`	`44`	`}`
`45`	`45`	`Uri uri;`
`46`	`46`	`if (!Uri.TryCreate(info.UrlLink, UriKind.Absolute, out uri))`
`47`	`47`	`{`
`48`	`48`	`result = RuleResult.Fail;`
`49`		`- verboseWriter.LogMessage(signature, "Signature's accompanying URL is not a valid URI.");`
	`49`	`+ verboseWriter.LogSignatureMessage(signature, "Signature's accompanying URL is not a valid URI.");`
`50`	`50`	`}`
`51`	`51`	`}`
`52`	`52`	`return result;`
Original file line number	Diff line number	Diff line change
`@@ -29,17 +29,17 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW`
`29`	`29`	`if (info == null)`
`30`	`30`	`{`
`31`	`31`	`result = RuleResult.Fail;`
`32`		`- verboseWriter.LogMessage(signature, "Signature does not have any publisher information.");`
	`32`	`+ verboseWriter.LogSignatureMessage(signature, "Signature does not have any publisher information.");`
`33`	`33`	`}`
`34`	`34`	`if (string.IsNullOrWhiteSpace(info.UrlLink))`
`35`	`35`	`{`
`36`	`36`	`result = RuleResult.Fail;`
`37`		`- verboseWriter.LogMessage(signature, "Signature does not have an accompanying URL.");`
	`37`	`+ verboseWriter.LogSignatureMessage(signature, "Signature does not have an accompanying URL.");`
`38`	`38`	`}`
`39`	`39`	`if (!info.UrlLink.StartsWith(Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase))`
`40`	`40`	`{`
`41`	`41`	`result = RuleResult.Fail;`
`42`		`- verboseWriter.LogMessage(signature, $"Signature's publisher information URL \"{info.UrlLink}\" does not use the secure HTTPS scheme.");`
	`42`	`+ verboseWriter.LogSignatureMessage(signature, $"Signature's publisher information URL \"{info.UrlLink}\" does not use the secure HTTPS scheme.");`
`43`	`43`	`}`
`44`	`44`	`}`
`45`	`45`	`return result;`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ public RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase verboseW`
`23`	`23`	`}`
`24`	`24`	`if (primary.DigestAlgorithm.Value != KnownOids.SHA1)`
`25`	`25`	`{`
`26`		`- verboseWriter.LogMessage(primary, $"Expected {nameof(KnownOids.SHA1)} digest algorithm but is {primary.DigestAlgorithm.FriendlyName}.");`
	`26`	`+ verboseWriter.LogSignatureMessage(primary, $"Expected {nameof(KnownOids.SHA1)} digest algorithm but is {primary.DigestAlgorithm.FriendlyName}.");`
`27`	`27`	`return RuleResult.Fail;`
`28`	`28`	`}`
`29`	`29`	`return RuleResult.Pass;`
Original file line number	Diff line number	Diff line change
`@@ -47,12 +47,12 @@ public unsafe RuleResult Validate(Graph<SignerInfo> graph, SignatureLoggerBase v`
`47`	`47`	`}`
`48`	`48`	`if (!isSigned)`
`49`	`49`	`{`
`50`		`- verboseWriter.LogMessage(signature, $"Signature is not timestamped.");`
	`50`	`+ verboseWriter.LogSignatureMessage(signature, $"Signature is not timestamped.");`
`51`	`51`	`pass = false;`
`52`	`52`	`}`
`53`	`53`	`else if (!strongSign)`
`54`	`54`	`{`
`55`		`- verboseWriter.LogMessage(signature, $"Signature is not timestamped with the expected hash algorithm {signature.DigestAlgorithm.FriendlyName}.");`
	`55`	`+ verboseWriter.LogSignatureMessage(signature, $"Signature is not timestamped with the expected hash algorithm {signature.DigestAlgorithm.FriendlyName}.");`
`56`	`56`	`pass = false;`
`57`	`57`	`}`
`58`	`58`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,9 @@ namespace AuthenticodeLint`
`9`	`9`	`{`
`10`	`10`	`public class VerboseSignatureLogger : SignatureLoggerBase`
`11`	`11`	`{`
`12`		`- public override void LogMessage(SignerInfo signature, string message)`
	`12`	`+ public override void LogMessage(string message) => Messages.Add(message);`
	`13`	`+`
	`14`	`+ public override void LogSignatureMessage(SignerInfo signature, string message)`
`13`	`15`	`{`
`14`	`16`	`var digest = signature.SignatureDigest();`
`15`	`17`	`var digestString = digest.Aggregate(new StringBuilder(), (acc, b) => acc.AppendFormat("{0:x2}", b)).ToString();`
`@@ -19,7 +21,11 @@ public override void LogMessage(SignerInfo signature, string message)`
`19`	`21`
`20`	`22`	`public class NullSignatureLogger : SignatureLoggerBase`
`21`	`23`	`{`
`22`		`- public override void LogMessage(SignerInfo signature, string message)`
	`24`	`+ public override void LogMessage(string message)`
	`25`	`+ {`
	`26`	`+ }`
	`27`	`+`
	`28`	`+ public override void LogSignatureMessage(SignerInfo signature, string message)`
`23`	`29`	`{`
`24`	`30`	`}`
`25`	`31`	`}`
`@@ -30,6 +36,7 @@ public abstract class SignatureLoggerBase`
`30`	`36`
`31`	`37`	`internal List<string> Messages { get; } = new List<string>();`
`32`	`38`
`33`		`- public abstract void LogMessage(SignerInfo signature, string message);`
	`39`	`+ public abstract void LogSignatureMessage(SignerInfo signature, string message);`
	`40`	`+ public abstract void LogMessage(string message);`
`34`	`41`	`}`
`35`	`42`	`}`