Skip to content

Commit e6aeef3

Browse files
vcsjonesvcsjones
committed
Rule for untrusted certificates.
If the PKCS#7 signature contains a untrusted certificate, possibly as a means of changing data post signature, fail the rule.
1 parent 03ac6e3 commit e6aeef3

File tree

9 files changed

+185
-37
lines changed

9 files changed

+185
-37
lines changed

‎AuthenticodeLint/AuthenticodeLint.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,11 @@
6969
<Compile Include="PE\PortableExecutable.cs" />
7070
<Compile Include="PublisherInformation.cs" />
7171
<Compile Include="Rfc3161Signature.cs" />
72+
<Compile Include="Rules\CertificateChainRuleBase.cs" />
7273
<Compile Include="Rules\IAuthenticodeRule.cs" />
7374
<Compile Include="Program.cs" />
7475
<Compile Include="Properties\AssemblyInfo.cs" />
76+
<Compile Include="Rules\NoUnknownCertificatesRule.cs" />
7577
<Compile Include="Rules\NoWeakFileDigestAlgorithmsRule.cs" />
7678
<Compile Include="Rules\PublisherInformationUrlHttpsRule.cs" />
7779
<Compile Include="Rules\PublisherInformationRule.cs" />
@@ -81,6 +83,7 @@
8183
<Compile Include="Rules\SigningCertificateDigestAlgorithmRule.cs" />
8284
<Compile Include="Rules\TimestampedRule.cs" />
8385
<Compile Include="Rules\TrustedSignatureRule.cs" />
86+
<Compile Include="Rules\NoUnknownUnsignedAttibuteRule.cs" />
8487
<Compile Include="Rules\WinCertificatePaddingRule.cs" />
8588
<Compile Include="Signature.cs" />
8689
<Compile Include="SignatureExtractor.cs" />

‎AuthenticodeLint/CheckEngine.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ public IReadOnlyList<IAuthenticodeRule> GetRules()
2424
new PublisherInformationUrlHttpsRule(),
2525
new SigningCertificateDigestAlgorithmRule(),
2626
new TrustedSignatureRule(),
27-
new WinCertificatePaddingRule()
27+
new WinCertificatePaddingRule(),
28+
new NoUnknownUnsignedAttibuteRule(),
29+
new NoUnknownCertificatesRule()
2830
};
2931
}
3032

‎AuthenticodeLint/Interop/Crypt32.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,4 +360,14 @@ internal enum SpcLinkChoice : uint
360360
SPC_MONIKER_LINK_CHOICE = 2,
361361
SPC_FILE_LINK_CHOICE = 3
362362
}
363+
364+
[type: StructLayout(LayoutKind.Sequential)]
365+
internal struct CERT_CONTEXT
366+
{
367+
public EncodingType dwCertEncodingType;
368+
public IntPtr pbCertEncoded;
369+
public uint cbCertEncoded;
370+
public IntPtr pCertInfo;
371+
public IntPtr hCertStore;
372+
}
363373
}

‎AuthenticodeLint/KnownOids.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ internal static class KnownOids
1717
public const string MessageDigest = "1.2.840.113549.1.9.4";
1818
public const string OpusInfo = "1.3.6.1.4.1.311.2.1.12";
1919
public const string CodeSigning = "1.3.6.1.5.5.7.3.3";
20+
public const string NestedSignatureOid = "1.3.6.1.4.1.311.2.4.1";
21+
2022

2123

2224
public const string md5RSA = "1.2.840.113549.1.1.4";

‎AuthenticodeLint/Rules/CertificateChainRuleBase.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System.Security.Cryptography.X509Certificates;
2+
3+
namespace AuthenticodeLint.Rules
4+
{
5+
public abstract class CertificateChainRuleBase : IAuthenticodeRule
6+
{
7+
public abstract int RuleId { get; }
8+
public abstract string RuleName { get; }
9+
public abstract string ShortDescription { get; }
10+
11+
protected abstract bool ValidateChain(Signature signer, X509Chain chain, SignatureLogger verboseWriter);
12+
13+
public RuleResult Validate(Graph<Signature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration, string file)
14+
{
15+
var signatures = graph.VisitAll();
16+
var result = RuleResult.Pass;
17+
foreach (var signature in signatures)
18+
{
19+
var certificates = signature.AdditionalCertificates;
20+
using (var chain = new X509Chain())
21+
{
22+
chain.ChainPolicy.ExtraStore.AddRange(certificates);
23+
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
24+
//The purpose of this check is not to validate the chain, completely.
25+
//The chain is needed so we know which certificate is the root and intermediates so we know which to validate and which not to validate.
26+
//It is possible to have a valid Authenticode signature if the certificate is expired but was
27+
//timestamped while it was valid. In this case we still want to successfully build a chain to perform validation.
28+
chain.ChainPolicy.VerificationFlags = X509VerificationFlags.IgnoreNotTimeValid;
29+
bool success = chain.Build(signature.SignerInfo.Certificate);
30+
if (!success)
31+
{
32+
verboseWriter.LogSignatureMessage(signature.SignerInfo, $"Cannot build a chain successfully with signing certificate {signature.SignerInfo.Certificate.SerialNumber}.");
33+
result = RuleResult.Fail;
34+
continue;
35+
}
36+
if (!ValidateChain(signature, chain, verboseWriter))
37+
{
38+
result = RuleResult.Fail;
39+
}
40+
}
41+
}
42+
return result;
43+
}
44+
}
45+
}

‎AuthenticodeLint/Rules/NoUnknownCertificatesRule.cs‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
using AuthenticodeLint.Interop;
2+
using System;
3+
using System.Collections.Generic;
4+
using System.Linq;
5+
using System.Runtime.InteropServices;
6+
using System.Security.Cryptography;
7+
using System.Security.Cryptography.X509Certificates;
8+
9+
namespace AuthenticodeLint.Rules
10+
{
11+
public class NoUnknownCertificatesRule : IAuthenticodeRule
12+
{
13+
public int RuleId { get; } = 10010;
14+
15+
public string RuleName { get; } = "No Unknown Certificates";
16+
17+
public string ShortDescription { get; } = "Checks for unknown embedded certificates.";
18+
19+
public RuleResult Validate(Graph<Signature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration, string file)
20+
{
21+
var result = RuleResult.Pass;
22+
var signatures = graph.VisitAll();
23+
foreach(var signature in signatures)
24+
{
25+
var allEmbeddedCertificates = signature.AdditionalCertificates.Cast<X509Certificate2>().ToList();
26+
var certificatesRequiringEliminiation = new HashSet<X509Certificate2>(allEmbeddedCertificates, new CertificateThumbprintComparer());
27+
foreach(var certificate in allEmbeddedCertificates)
28+
{
29+
if (!certificatesRequiringEliminiation.Contains(certificate))
30+
{
31+
//This certificate was already eliminated because it was part of a previous chain.
32+
continue;
33+
}
34+
using (var chain = new X509Chain())
35+
{
36+
chain.ChainPolicy.ExtraStore.AddRange(signature.AdditionalCertificates);
37+
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
38+
chain.ChainPolicy.RevocationFlag = X509RevocationFlag.EntireChain;
39+
//All we care is that we can even find an authority.
40+
chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllFlags & ~X509VerificationFlags.AllowUnknownCertificateAuthority;
41+
if(chain.Build(certificate))
42+
{
43+
certificatesRequiringEliminiation.ExceptWith(chain.ChainElements.Cast<X509ChainElement>().Select(c => c.Certificate));
44+
}
45+
}
46+
}
47+
if (certificatesRequiringEliminiation.Count > 0)
48+
{
49+
foreach(var certificate in certificatesRequiringEliminiation)
50+
{
51+
verboseWriter.LogSignatureMessage(signature.SignerInfo, $"Signature contained untrusted certificate \"{certificate.Subject}\" ({certificate.Thumbprint}).");
52+
}
53+
result = RuleResult.Fail;
54+
}
55+
}
56+
return result;
57+
}
58+
59+
60+
61+
private class CertificateThumbprintComparer : IEqualityComparer<X509Certificate2>
62+
{
63+
public bool Equals(X509Certificate2 x, X509Certificate2 y)
64+
{
65+
return x.Thumbprint == y.Thumbprint;
66+
}
67+
68+
public int GetHashCode(X509Certificate2 obj)
69+
{
70+
return obj.Thumbprint.GetHashCode();
71+
}
72+
}
73+
}
74+
}

‎AuthenticodeLint/Rules/NoUnknownUnsignedAttibuteRule.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
using System;
2+
using System.Linq;
3+
4+
namespace AuthenticodeLint.Rules
5+
{
6+
public class NoUnknownUnsignedAttibuteRule : IAuthenticodeRule
7+
{
8+
public int RuleId { get; } = 10009;
9+
10+
public string RuleName { get; } = "No Unknown Unsigned Attributes";
11+
12+
public string ShortDescription { get; } = "Checks for the presence of unsigned attributes with unknown an OID.";
13+
14+
private static string[] _trustedUnsignedAttributes = new[]
15+
{
16+
KnownOids.AuthenticodeCounterSignature,
17+
KnownOids.Rfc3161CounterSignature,
18+
KnownOids.NestedSignatureOid
19+
};
20+
21+
public RuleResult Validate(Graph<Signature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration, string file)
22+
{
23+
var signatures = graph.VisitAll();
24+
var result = RuleResult.Pass;
25+
foreach(var signature in signatures)
26+
{
27+
var signer = signature.SignerInfo;
28+
foreach(var attribute in signer.UnsignedAttributes)
29+
{
30+
if (!_trustedUnsignedAttributes.Contains(attribute.Oid.Value))
31+
{
32+
result = RuleResult.Fail;
33+
var displayName = attribute.Oid.FriendlyName ?? "<no friendly name>";
34+
verboseWriter.LogSignatureMessage(signer, $"Signature contains unknown unsigned attribute {displayName} ({attribute.Oid.Value}).");
35+
}
36+
}
37+
}
38+
return result;
39+
}
40+
}
41+
}

‎AuthenticodeLint/Rules/SigningCertificateDigestAlgorithmRule.cs‎

Lines changed: 6 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -3,45 +3,17 @@
33

44
namespace AuthenticodeLint.Rules
55
{
6-
public class SigningCertificateDigestAlgorithmRule : IAuthenticodeRule
6+
public class SigningCertificateDigestAlgorithmRule : CertificateChainRuleBase
77
{
8-
public int RuleId { get; } = 10006;
8+
public override int RuleId { get; } = 10006;
99

10-
public string RuleName { get; } = "SHA2 Certificate Chain";
10+
public override string RuleName { get; } = "SHA2 Certificate Chain";
1111

12-
public string ShortDescription { get; } = "Checks the signing certificate's and chain's signature algorithm.";
12+
public override string ShortDescription { get; } = "Checks the signing certificate's and chain's signature algorithm.";
1313

14-
public RuleResult Validate(Graph<Signature> graph, SignatureLogger verboseWriter, CheckConfiguration configuration, string file)
14+
protected override bool ValidateChain(Signature signer, X509Chain chain, SignatureLogger verboseWriter)
1515
{
16-
var signatures = graph.VisitAll();
17-
var result = RuleResult.Pass;
18-
foreach(var signature in signatures)
19-
{
20-
var certificates = signature.AdditionalCertificates;
21-
using (var chain = new X509Chain())
22-
{
23-
chain.ChainPolicy.ExtraStore.AddRange(certificates);
24-
chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
25-
//The purpose of this check is not to validate the chain, completely.
26-
//The chain is needed so we know which certificate is the root and intermediates so we know which to validate and which not to validate.
27-
//It is possible to have a valid Authenticode signature if the certificate is expired but was
28-
//timestamped while it was valid. In this case we still want to successfully build a chain to perform validation.
29-
chain.ChainPolicy.VerificationFlags = X509VerificationFlags.IgnoreNotTimeValid;
30-
bool success = chain.Build(signature.SignerInfo.Certificate);
31-
if (!success)
32-
{
33-
verboseWriter.LogSignatureMessage(signature.SignerInfo, $"Cannot build a chain successfully with signing certificate {signature.SignerInfo.Certificate.SerialNumber}.");
34-
result = RuleResult.Fail;
35-
continue;
36-
}
37-
var chainResult = ValidateSha2Chain(signature.SignerInfo, chain, verboseWriter);
38-
if (!chainResult)
39-
{
40-
result = RuleResult.Fail;
41-
}
42-
}
43-
}
44-
return result;
16+
return ValidateSha2Chain(signer.SignerInfo, chain, verboseWriter);
4517
}
4618

4719
private static bool ValidateSha2Chain(SignerInfo signatureInfo, X509Chain chain, SignatureLogger verboseWriter)

‎AuthenticodeLint/SignatureExtractor.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ private unsafe Graph<Signature> GetSignatures(CryptMsgSafeHandle messageHandle)
6060

6161
public static Graph<Signature> RecursiveSigner(IList<byte[]> cmsData)
6262
{
63-
const string nestedSignatureOid = "1.3.6.1.4.1.311.2.4.1";
6463
var graphItems = new List<GraphItem<Signature>>();
6564
foreach (var data in cmsData)
6665
{
@@ -71,7 +70,7 @@ public static Graph<Signature> RecursiveSigner(IList<byte[]> cmsData)
7170
var childCms = new List<byte[]>();
7271
foreach (var attribute in signer.UnsignedAttributes)
7372
{
74-
if (attribute.Oid.Value == nestedSignatureOid)
73+
if (attribute.Oid.Value == KnownOids.NestedSignatureOid)
7574
{
7675
foreach (var value in attribute.Values)
7776
{

0 commit comments

Comments
 (0)