Merge pull request #31 from vcsjones/net6

vcsjones · web-flow · commit ccfaec53ee5c · 2021-11-30T11:25:29.000-05:00
Upgrade to .NET 6 and nullable
diff --git a/AuthenticodeLint/AuthenticodeLint.csproj b/AuthenticodeLint/AuthenticodeLint.csproj
@@ -3,11 +3,11 @@
     <OutputType>WinExe</OutputType>
     <AssemblyName>authlint</AssemblyName>
     <PackageId>AuthenticodeLint</PackageId>
-    <TargetFramework>netcoreapp2.1</TargetFramework>
+    <TargetFramework>net6.0</TargetFramework>
     <VersionPrefix>0.12.0</VersionPrefix>
     <Authors>Kevin Jones</Authors>
     <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
-    <Copyright>Kevin Jones 2016-2018</Copyright>
+    <Copyright>Kevin Jones 2016-2021</Copyright>
     <PackAsTool>true</PackAsTool>
     <ToolCommandName>authlint</ToolCommandName>
     <Description>Authenticode Lint is a Windows command-line tool for linting an examining an Authenticode signed file.</Description>
@@ -17,6 +17,7 @@
     <RepositoryUrl>https://github.com/vcsjones/AuthenticodeLint</RepositoryUrl>
     <PublishRepositoryUrl>true</PublishRepositoryUrl>
     <EmbedUntrackedSources>true</EmbedUntrackedSources>
+    <Nullable>enable</Nullable>
   </PropertyGroup>
   <ItemGroup>
     <PackageReference Include="AuthenticodeExaminer" Version="0.3.0" />
diff --git a/AuthenticodeLint/BitStrengthCalculator.cs b/AuthenticodeLint/BitStrengthCalculator.cs
@@ -6,7 +6,7 @@ namespace AuthenticodeLint
 {
     public static class BitStrengthCalculator
     {
-        private static ConcurrentDictionary<string, int> _cachedEccCurveSizes = new ConcurrentDictionary<string, int>();
+        private static readonly ConcurrentDictionary<string, int> _cachedEccCurveSizes = new ConcurrentDictionary<string, int>();
 
         public static CertificateBitStrength CalculateStrength(X509Certificate2 certificate)
         {
@@ -16,19 +16,30 @@ public static CertificateBitStrength CalculateStrength(X509Certificate2 certific
             {
                 case KnownOids.X509Algorithms.Ecc:
                     keyAlgorithm = PublicKeyAlgorithm.ECDSA;
-                    var parameterOid = OidParser.ReadFromBytes(certificate.PublicKey.EncodedParameters.RawData);
-                    bitSize = _cachedEccCurveSizes.GetOrAdd(parameterOid.Value, oid =>
+                    string? parameterOid = OidParser.ReadFromBytes(certificate.PublicKey.EncodedParameters.RawData);
+
+                    if (parameterOid is null)
+                    {
+                        bitSize = null;
+                    }
+                    else
                     {
-                        var curve = ECCurve.CreateFromValue(oid);
-                        using (var ecdsa = ECDsa.Create(curve))
+                        bitSize = _cachedEccCurveSizes.GetOrAdd(parameterOid, static oid =>
                         {
-                            return ecdsa.KeySize;
-                        }
-                    });
+                            var curve = ECCurve.CreateFromValue(oid);
+                            using (var ecdsa = ECDsa.Create(curve))
+                            {
+                                return ecdsa.KeySize;
+                            }
+                        });
+                    }
                     break;
                 case KnownOids.X509Algorithms.RSA:
                     keyAlgorithm = PublicKeyAlgorithm.RSA;
-                    bitSize = certificate.PublicKey.Key.KeySize;
+                    using (RSA? rsa = certificate.GetRSAPublicKey())
+                    {
+                        bitSize = rsa?.KeySize;
+                    }
                     break;
                 default:
                     keyAlgorithm = PublicKeyAlgorithm.Other;
diff --git a/AuthenticodeLint/CertificatePaddingExtractor.cs b/AuthenticodeLint/CertificatePaddingExtractor.cs
@@ -7,7 +7,7 @@ namespace AuthenticodeLint
 {
     public static class CertificatePaddingExtractor
     {
-        public static byte[] ExtractPadding(string filePath)
+        public static byte[]? ExtractPadding(string filePath)
         {
             using (var file = new PortableExecutable(filePath))
             {
diff --git a/AuthenticodeLint/CheckEngine.cs b/AuthenticodeLint/CheckEngine.cs
@@ -19,7 +19,7 @@ public IReadOnlyList<IAuthenticodeRule> GetRules()
         {
             return (from type in typeof(IAuthenticodeRule).Assembly.GetExportedTypes()
                     where typeof(IAuthenticodeRule).IsAssignableFrom(type) && type.GetConstructor(Type.EmptyTypes) != null
-                    let instance = (IAuthenticodeRule)Activator.CreateInstance(type)
+                    let instance = (IAuthenticodeRule)Activator.CreateInstance(type)! // We know this should not be null.
                     orderby instance.RuleId
                     select instance
                     ).ToList();
@@ -53,17 +53,12 @@ public RuleEngineResult RunAllRules(string file, IReadOnlyList<ICmsSignature> si
                     }
                     else
                     {
-                        switch (rule)
+                        result = rule switch
                         {
-                            case IAuthenticodeFileRule fileRule:
-                                result = fileRule.Validate(file, verboseWriter, configuration);
-                                break;
-                            case IAuthenticodeSignatureRule sigRule:
-                                result = sigRule.Validate(signatures, verboseWriter, configuration);
-                                break;
-                            default:
-                                throw new NotSupportedException("Rule type is not supported.");
-                        }
+                            IAuthenticodeFileRule fileRule => fileRule.Validate(file, verboseWriter, configuration),
+                            IAuthenticodeSignatureRule sigRule => sigRule.Validate(signatures, verboseWriter, configuration),
+                            _ => throw new NotSupportedException("Rule type is not supported."),
+                        };
                     }
                 }
                 if (result == RuleResult.Fail)
diff --git a/AuthenticodeLint/CommandLineParser.cs b/AuthenticodeLint/CommandLineParser.cs
@@ -7,24 +7,25 @@ namespace AuthenticodeLint
 
     public readonly struct CommandLineParameter
     {
-        private readonly string _name, _value;
+        private readonly string _name;
+        private readonly string? _value;
 
-        public CommandLineParameter(string name, string value)
+        public CommandLineParameter(string name, string? value)
         {
             _name = name;
             _value = value;
         }
 
         public string Name => _name;
-        public string Value => _value;
+        public string? Value => _value;
     }
 
 
     public class CommandLineParser
     {
         public static IEnumerable<CommandLineParameter> CreateCommandLineParametersWithValues(IEnumerable<string> input)
         {
-            string parameterName = null;
+            string? parameterName = null;
             foreach (var token in input)
             {
                 if (string.IsNullOrWhiteSpace(token) || token.Length == 0)
diff --git a/AuthenticodeLint/ConfigurationValidator.cs b/AuthenticodeLint/ConfigurationValidator.cs
@@ -8,15 +8,15 @@ namespace AuthenticodeLint
     public class CheckConfiguration
     {
         public IReadOnlyList<string> InputPaths { get; }
-        public string ReportPath { get; }
+        public string? ReportPath { get; }
         public bool Quiet { get; }
         public HashSet<int> SuppressErrorIDs { get; }
         public bool Verbose { get; }
         public RevocationChecking RevocationMode {get;}
-        public string ExtractPath { get; }
+        public string? ExtractPath { get; }
         public RuleSet RuleSet { get; }
 
-        public CheckConfiguration(IReadOnlyList<string> inputPaths, string reportPath, bool quiet, HashSet<int> suppressErrorIDs, bool verbose, RevocationChecking revocationMode, string extract, RuleSet ruleSet)
+        public CheckConfiguration(IReadOnlyList<string> inputPaths, string? reportPath, bool quiet, HashSet<int> suppressErrorIDs, bool verbose, RevocationChecking revocationMode, string? extract, RuleSet ruleSet)
         {
             InputPaths = inputPaths;
             ReportPath = reportPath;
diff --git a/AuthenticodeLint/OidParser.cs b/AuthenticodeLint/OidParser.cs
@@ -1,67 +1,20 @@
-﻿using System;
-using System.Collections.Generic;
-using System.Linq;
-using System.Security.Cryptography;
+﻿using System.Formats.Asn1;
 
 namespace AuthenticodeLint
 {
     public static class OidParser
     {
-        private const byte MAGIC_OID_VALUE = 6;
-        private const int MAGIC_OID_OFFSET = 0;
-        private const int DATA_LENGTH_OFFSET = 1;
-        private const int FIRST_OCTET_OFFSET = 2;
-        private const int VLQ_DATA_OFFSET = 3;
-
-        public static Oid ReadFromBytes(byte[] data)
+        public static string? ReadFromBytes(byte[] data)
         {
-            if (data == null || data.Length < FIRST_OCTET_OFFSET)
-            {
-                return null;
-            }
-            var magicValue = data[MAGIC_OID_OFFSET];
-            if (magicValue != MAGIC_OID_VALUE)
-            {
-                return null;
-            }
-            var dataLength = data[DATA_LENGTH_OFFSET];
-            if (data.Length - FIRST_OCTET_OFFSET != dataLength)
-            {
-                return null;
-            }
-            var firstValue = data[FIRST_OCTET_OFFSET] / 40L;
-            var secondValue = data[FIRST_OCTET_OFFSET] % 40L;
-            var remainder = data.Skip(VLQ_DATA_OFFSET);
             try
             {
-                return new Oid(string.Join(".", new[] { firstValue, secondValue }.Concat(ReadVlqData(remainder))));
+                AsnReader reader = new AsnReader(data, AsnEncodingRules.DER);
+                return reader.ReadObjectIdentifier();
             }
-            catch (InvalidOperationException)
+            catch (AsnContentException)
             {
                 return null;
             }
         }
-
-        private static IEnumerable<long> ReadVlqData(IEnumerable<byte> data)
-        {
-            var value = 0L;
-            foreach (var item in data)
-            {
-                value <<= 7;
-                if ((item & 0x80) == 0x80)
-                {
-                    value |= (byte)(item & ~0x80);
-                }
-                else
-                {
-                    yield return value | item;
-                    value = 0;
-                }
-            }
-            if (value != 0)
-            {
-                throw new InvalidOperationException();
-            }
-        }
     }
 }
diff --git a/AuthenticodeLint/Program.cs b/AuthenticodeLint/Program.cs
@@ -17,7 +17,7 @@ static int Main(string[] args)
                 return ExitCodes.PlatformNotSupported;
             }
             var cli = Environment.CommandLine;
-            List<CommandLineParameter> parsedCommandLine;
+            List<CommandLineParameter>? parsedCommandLine;
             try
             {
                 var commandLine = CommandLineParser.LexCommandLine(cli).Skip(1);
@@ -39,8 +39,8 @@ static int Main(string[] args)
             var suppress = new HashSet<int>();
             bool quiet = false;
             bool verbose = false;
-            string report = null;
-            string extract = null;
+            string? report = null;
+            string? extract = null;
             var revocation = RevocationChecking.None;
             var ruleSet = RuleSet.Modern;
             foreach(var parameter in parsedCommandLine)
@@ -54,7 +54,7 @@ static int Main(string[] args)
                     }
                     var filePattern = Path.GetFileName(parameter.Value);
                     //The value contains a pattern.
-                    if (filePattern.Contains("*") || filePattern.Contains("?"))
+                    if (filePattern.Contains('*') || filePattern.Contains('?'))
                     {
                         var directory = Path.GetDirectoryName(parameter.Value);
                         if (Directory.Exists(directory))
diff --git a/AuthenticodeLint/Rules/10004-PublisherInformationRule.cs b/AuthenticodeLint/Rules/10004-PublisherInformationRule.cs
@@ -13,14 +13,14 @@ public class PublisherInformationPresentRule : IAuthenticodeSignatureRule
         public string ShortDescription => "Checks that the signature provided publisher information.";
 
         public RuleSet RuleSet => RuleSet.All;
-        
+
         public RuleResult Validate(IReadOnlyList<ICmsSignature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration)
         {
             var signatures = graph.VisitAll(SignatureKind.AnySignature, deep: true);
             var result = RuleResult.Pass;
             foreach (var signature in signatures)
             {
-                PublisherInformation info = null;
+                PublisherInformation? info = null;
                 foreach (var attribute in signature.SignedAttributes)
                 {
                     if (attribute.Oid.Value == KnownOids.OpusInfo)
diff --git a/AuthenticodeLint/Rules/10005-PublisherInformationUrlHttpsRule.cs b/AuthenticodeLint/Rules/10005-PublisherInformationUrlHttpsRule.cs
@@ -20,7 +20,7 @@ public RuleResult Validate(IReadOnlyList<ICmsSignature> graph, SignatureLogger v
             var result = RuleResult.Pass;
             foreach(var signature in signatures)
             {
-                PublisherInformation info = null;
+                PublisherInformation? info = null;
                 foreach(var attribute in signature.SignedAttributes)
                 {
                     if (attribute.Oid.Value == KnownOids.OpusInfo)
diff --git a/AuthenticodeLint/Rules/10006-SigningCertificateDigestAlgorithmRule.cs b/AuthenticodeLint/Rules/10006-SigningCertificateDigestAlgorithmRule.cs
@@ -12,24 +12,24 @@ public class SigningCertificateDigestAlgorithmRule : CertificateChainRuleBase
         public override string ShortDescription => "Checks the signing certificate's and chain's signature algorithm.";
 
         public override RuleSet RuleSet => RuleSet.All;
-        
+
         protected override bool ValidateChain(ICmsSignature signer, X509Chain chain, SignatureLogger verboseWriter)
         {
             return ValidateStrongChain(signer, chain, verboseWriter);
         }
 
         private static bool ValidateStrongChain(ICmsSignature signature, X509Chain chain, SignatureLogger verboseWriter)
         {
-            var signatureStrength = GetHashStrenghForComparison(signature.DigestAlgorithm.Value);
+            var signatureStrength = GetHashStrenghForComparison(signature.DigestAlgorithm.Value!);
             var strongShaChain = true;
             var leafCertificateSignatureAlgorithm = chain.ChainElements[0].Certificate.SignatureAlgorithm;
-            var leafCertificateSignatureAlgorithmStrength = GetHashStrenghForComparison(leafCertificateSignatureAlgorithm.Value);
+            var leafCertificateSignatureAlgorithmStrength = GetHashStrenghForComparison(leafCertificateSignatureAlgorithm.Value!);
             //We use count-1 because we don't want to validate the root certificate.
             for (var i = 0; i < chain.ChainElements.Count - 1; i++)
             {
                 var element = chain.ChainElements[i];
                 var signatureAlgorithm = element.Certificate.SignatureAlgorithm;
-                var certificateHashStrength = GetHashStrenghForComparison(signatureAlgorithm.Value);
+                var certificateHashStrength = GetHashStrenghForComparison(signatureAlgorithm.Value!);
                 if (certificateHashStrength < signatureStrength)
                 {
                     verboseWriter.LogSignatureMessage(signature, $"Certificate {element.Certificate.Thumbprint} in chain uses {element.Certificate.SignatureAlgorithm.FriendlyName} for its signature algorithm instead of at least {signature.DigestAlgorithm.FriendlyName}.");
@@ -48,33 +48,17 @@ private static bool ValidateStrongChain(ICmsSignature signature, X509Chain chain
         //angainst other values.
         private static int GetHashStrenghForComparison(string oid)
         {
-            switch (oid)
+            return oid switch
             {
-                case KnownOids.MD2:
-                    return 2;
-                case KnownOids.MD4:
-                    return 4;
-                case KnownOids.MD5:
-                    return 5;
-                case KnownOids.SHA1:
-                case KnownOids.sha1ECDSA:
-                case KnownOids.sha1RSA:
-                    return 10;
-                case KnownOids.SHA256:
-                case KnownOids.sha256ECDSA:
-                case KnownOids.sha256RSA:
-                    return 256;
-                case KnownOids.SHA384:
-                case KnownOids.sha384ECDSA:
-                case KnownOids.sha384RSA:
-                    return 384;
-                case KnownOids.SHA512:
-                case KnownOids.sha512ECDSA:
-                case KnownOids.sha512RSA:
-                    return 512;
-                default:
-                    return 0;
-            }
+                KnownOids.MD2 => 2,
+                KnownOids.MD4 => 4,
+                KnownOids.MD5 => 5,
+                KnownOids.SHA1 or KnownOids.sha1ECDSA or KnownOids.sha1RSA => 10,
+                KnownOids.SHA256 or KnownOids.sha256ECDSA or KnownOids.sha256RSA => 256,
+                KnownOids.SHA384 or KnownOids.sha384ECDSA or KnownOids.sha384RSA => 384,
+                KnownOids.SHA512 or KnownOids.sha512ECDSA or KnownOids.sha512RSA => 512,
+                _ => 0,
+            };
         }
     }
 }
diff --git a/AuthenticodeLint/Rules/10010-NoUnknownCertificatesRule.cs b/AuthenticodeLint/Rules/10010-NoUnknownCertificatesRule.cs
@@ -61,8 +61,17 @@ public RuleResult Validate(IReadOnlyList<ICmsSignature> graph, SignatureLogger v
 
         private class CertificateThumbprintComparer : IEqualityComparer<X509Certificate2>
         {
-            public bool Equals(X509Certificate2 x, X509Certificate2 y)
+            public bool Equals(X509Certificate2? x, X509Certificate2? y)
             {
+                if (x is null && y is null)
+                {
+                    return true;
+                }
+                if (x is null || y is null)
+                {
+                    return false;
+                }
+
                 return x.Thumbprint == y.Thumbprint;
             }
 
diff --git a/AuthenticodeLint/SignerInfoExtensions.cs b/AuthenticodeLint/SignerInfoExtensions.cs
@@ -6,7 +6,7 @@ namespace AuthenticodeLint
 {
     public static class SignerInfoExtensions
     {
-        public static byte[] SignatureDigest(this ICmsSignature signature)
+        public static byte[]? SignatureDigest(this ICmsSignature signature)
         {
             return signature.SignedAttributes
                 .Cast<CryptographicAttributeObject>()
diff --git a/AuthenticodeLint/StdOutRuleResultCollector.cs b/AuthenticodeLint/StdOutRuleResultCollector.cs
@@ -6,7 +6,7 @@ namespace AuthenticodeLint
 {
     public class StdOutRuleResultCollector : IRuleResultCollector
     {
-        private string _setName;
+        private string? _setName;
 
         public void BeginSet(string setName)
         {
diff --git a/AuthenticodeLint/XmlRuleResultCollector.cs b/AuthenticodeLint/XmlRuleResultCollector.cs
diff --git a/AuthenticodeLintTests/AuthenticodeLintTests.csproj b/AuthenticodeLintTests/AuthenticodeLintTests.csproj
diff --git a/Directory.Build.props b/Directory.Build.props

Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ namespace AuthenticodeLint`
`7`	`7`	`{`
`8`	`8`	`public static class CertificatePaddingExtractor`
`9`	`9`	`{`
`10`		`- public static byte[] ExtractPadding(string filePath)`
	`10`	`+ public static byte[]? ExtractPadding(string filePath)`
`11`	`11`	`{`
`12`	`12`	`using (var file = new PortableExecutable(filePath))`
`13`	`13`	`{`
Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ public IReadOnlyList<IAuthenticodeRule> GetRules()`
`19`	`19`	`{`
`20`	`20`	`return (from type in typeof(IAuthenticodeRule).Assembly.GetExportedTypes()`
`21`	`21`	`where typeof(IAuthenticodeRule).IsAssignableFrom(type) && type.GetConstructor(Type.EmptyTypes) != null`
`22`		`- let instance = (IAuthenticodeRule)Activator.CreateInstance(type)`
	`22`	`+ let instance = (IAuthenticodeRule)Activator.CreateInstance(type)! // We know this should not be null.`
`23`	`23`	`orderby instance.RuleId`
`24`	`24`	`select instance`
`25`	`25`	`).ToList();`
`@@ -53,17 +53,12 @@ public RuleEngineResult RunAllRules(string file, IReadOnlyList<ICmsSignature> si`
`53`	`53`	`}`
`54`	`54`	`else`
`55`	`55`	`{`
`56`		`- switch (rule)`
	`56`	`+ result = rule switch`
`57`	`57`	`{`
`58`		`- case IAuthenticodeFileRule fileRule:`
`59`		`- result = fileRule.Validate(file, verboseWriter, configuration);`
`60`		`- break;`
`61`		`- case IAuthenticodeSignatureRule sigRule:`
`62`		`- result = sigRule.Validate(signatures, verboseWriter, configuration);`
`63`		`- break;`
`64`		`- default:`
`65`		`- throw new NotSupportedException("Rule type is not supported.");`
`66`		`- }`
	`58`	`+ IAuthenticodeFileRule fileRule => fileRule.Validate(file, verboseWriter, configuration),`
	`59`	`+ IAuthenticodeSignatureRule sigRule => sigRule.Validate(signatures, verboseWriter, configuration),`
	`60`	`+ _ => throw new NotSupportedException("Rule type is not supported."),`
	`61`	`+ };`
`67`	`62`	`}`
`68`	`63`	`}`
`69`	`64`	`if (result == RuleResult.Fail)`
Original file line number	Diff line number	Diff line change
`@@ -7,24 +7,25 @@ namespace AuthenticodeLint`
`7`	`7`
`8`	`8`	`public readonly struct CommandLineParameter`
`9`	`9`	`{`
`10`		`- private readonly string _name, _value;`
	`10`	`+ private readonly string _name;`
	`11`	`+ private readonly string? _value;`
`11`	`12`
`12`		`- public CommandLineParameter(string name, string value)`
	`13`	`+ public CommandLineParameter(string name, string? value)`
`13`	`14`	`{`
`14`	`15`	`_name = name;`
`15`	`16`	`_value = value;`
`16`	`17`	`}`
`17`	`18`
`18`	`19`	`public string Name => _name;`
`19`		`- public string Value => _value;`
	`20`	`+ public string? Value => _value;`
`20`	`21`	`}`
`21`	`22`
`22`	`23`
`23`	`24`	`public class CommandLineParser`
`24`	`25`	`{`
`25`	`26`	`public static IEnumerable<CommandLineParameter> CreateCommandLineParametersWithValues(IEnumerable<string> input)`
`26`	`27`	`{`
`27`		`- string parameterName = null;`
	`28`	`+ string? parameterName = null;`
`28`	`29`	`foreach (var token in input)`
`29`	`30`	`{`
`30`	`31`	`if (string.IsNullOrWhiteSpace(token) \|\| token.Length == 0)`
Original file line number	Diff line number	Diff line change
`@@ -13,14 +13,14 @@ public class PublisherInformationPresentRule : IAuthenticodeSignatureRule`
`13`	`13`	`public string ShortDescription => "Checks that the signature provided publisher information.";`
`14`	`14`
`15`	`15`	`public RuleSet RuleSet => RuleSet.All;`
`16`		`-`
	`16`	`+`
`17`	`17`	`public RuleResult Validate(IReadOnlyList<ICmsSignature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration)`
`18`	`18`	`{`
`19`	`19`	`var signatures = graph.VisitAll(SignatureKind.AnySignature, deep: true);`
`20`	`20`	`var result = RuleResult.Pass;`
`21`	`21`	`foreach (var signature in signatures)`
`22`	`22`	`{`
`23`		`- PublisherInformation info = null;`
	`23`	`+ PublisherInformation? info = null;`
`24`	`24`	`foreach (var attribute in signature.SignedAttributes)`
`25`	`25`	`{`
`26`	`26`	`if (attribute.Oid.Value == KnownOids.OpusInfo)`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ public RuleResult Validate(IReadOnlyList<ICmsSignature> graph, SignatureLogger v`
`20`	`20`	`var result = RuleResult.Pass;`
`21`	`21`	`foreach(var signature in signatures)`
`22`	`22`	`{`
`23`		`- PublisherInformation info = null;`
	`23`	`+ PublisherInformation? info = null;`
`24`	`24`	`foreach(var attribute in signature.SignedAttributes)`
`25`	`25`	`{`
`26`	`26`	`if (attribute.Oid.Value == KnownOids.OpusInfo)`