Skip to content

Commit 4f20b37

Browse files
vcsjonesvcsjones
committed
Fixed counter signature implementation.
1. They no longer leak memory. 2. They made invalid assumptions about when memory was freed. 3. They are recursive. While unlikely, a counter signature of a counter signature should work. 4. Counter signatures are now graphs from a signature.
1 parent 675f730 commit 4f20b37

File tree

6 files changed

+165
-99
lines changed

6 files changed

+165
-99
lines changed

‎AuthenticodeLint/AuthenticodeLint.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
<Compile Include="CheckEngine.cs" />
5858
<Compile Include="CommandLineParser.cs" />
5959
<Compile Include="ConfigurationValidator.cs" />
60+
<Compile Include="GraphBuilders.cs" />
6061
<Compile Include="Interop\CertStoreSafeHandle.cs" />
6162
<Compile Include="Interop\Crypt32.cs" />
6263
<Compile Include="Interop\CryptMsgSafeHandle.cs" />

‎AuthenticodeLint/GraphBuilders.cs

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
using System.Collections.Generic;
2+
using System.Security.Cryptography;
3+
using System.Security.Cryptography.Pkcs;
4+
5+
namespace AuthenticodeLint
6+
{
7+
public static class GraphBuilder
8+
{
9+
public static Graph<Signature> ExplodeGraph(byte[] entireMessage, string nestedOidType) => WalkGraph(new List<byte[]> { entireMessage }, nestedOidType);
10+
11+
public static Graph<ICounterSignature> WalkCounterSignatures(Signature signature) => WalkCounterSignatures(signature.SignerInfo.UnsignedAttributes);
12+
13+
private static Graph<ICounterSignature> WalkCounterSignatures(CryptographicAttributeObjectCollection attributes)
14+
{
15+
var graphItems = new List<GraphItem<ICounterSignature>>();
16+
foreach (var attribute in attributes)
17+
{
18+
foreach(var value in attribute.Values)
19+
{
20+
ICounterSignature signature;
21+
if (attribute.Oid.Value == KnownOids.AuthenticodeCounterSignature)
22+
{
23+
signature = new AuthenticodeSignature(value);
24+
}
25+
else if (attribute.Oid.Value == KnownOids.Rfc3161CounterSignature)
26+
{
27+
signature = new Rfc3161Signature(value);
28+
}
29+
else
30+
{
31+
continue;
32+
}
33+
var childAttributes = new CryptographicAttributeObjectCollection();
34+
foreach(var childAttribute in signature.UnsignedAttributes)
35+
{
36+
childAttributes.Add(childAttribute);
37+
}
38+
graphItems.Add(new GraphItem<ICounterSignature>(signature, WalkCounterSignatures(childAttributes)));
39+
}
40+
}
41+
return new Graph<ICounterSignature>(graphItems);
42+
}
43+
44+
private static Graph<Signature> WalkGraph(IList<byte[]> cmsData, string nestedOidType)
45+
{
46+
var graphItems = new List<GraphItem<Signature>>();
47+
foreach (var data in cmsData)
48+
{
49+
var cms = new SignedCms();
50+
cms.Decode(data);
51+
foreach (var signer in cms.SignerInfos)
52+
{
53+
var childCms = new List<byte[]>();
54+
foreach (var attribute in signer.UnsignedAttributes)
55+
{
56+
if (attribute.Oid.Value == nestedOidType)
57+
{
58+
foreach (var value in attribute.Values)
59+
{
60+
childCms.Add(value.RawData);
61+
}
62+
}
63+
}
64+
graphItems.Add(new GraphItem<Signature>(new Signature(signer, cms.Certificates), WalkGraph(childCms, nestedOidType)));
65+
}
66+
}
67+
return new Graph<Signature>(graphItems);
68+
}
69+
}
70+
}

‎AuthenticodeLint/Interop/Crypt32.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ internal struct CRYPT_ALGORITHM_IDENTIFIER
297297
public CRYPTOAPI_BLOB Parameters;
298298
}
299299

300-
[type: StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
300+
[type: StructLayout(LayoutKind.Sequential)]
301301
internal struct CMSG_SIGNER_INFO
302302
{
303303
public uint dwVersion;
@@ -370,4 +370,12 @@ internal struct CERT_CONTEXT
370370
public IntPtr pCertInfo;
371371
public IntPtr hCertStore;
372372
}
373+
374+
[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Ansi)]
375+
internal struct CRYPT_ATTRIBUTE
376+
{
377+
public string pszObjId;
378+
public uint cValue;
379+
public IntPtr rgValue;
380+
}
373381
}

‎AuthenticodeLint/Rfc3161Signature.cs

Lines changed: 80 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -6,58 +6,82 @@
66

77
namespace AuthenticodeLint
88
{
9-
public abstract class SignatureBase : IDisposable
9+
public interface ICounterSignature
1010
{
11-
internal CMSG_SIGNER_INFO _signerInfo;
12-
13-
14-
protected SignatureBase(AsnEncodedData data)
11+
Oid Oid { get; }
12+
Oid DigestAlgorithm { get; }
13+
Oid HashEncryptionAlgorithm { get; }
14+
CryptographicAttributeObjectCollection UnsignedAttributes { get; }
15+
CryptographicAttributeObjectCollection SignedAttributes { get; }
16+
}
17+
public abstract class CounterSignatureBase : ICounterSignature
18+
{
19+
20+
protected CounterSignatureBase(AsnEncodedData data)
1521
{
16-
22+
Oid = data.Oid;
1723
}
1824

19-
public Oid DigestAlgorithm => new Oid(_signerInfo.HashAlgorithm.pszObjId);
20-
public Oid HashEncryptionAlgorithm => new Oid(_signerInfo.HashEncryptionAlgorithm.pszObjId);
21-
22-
public byte[] SerialNumber
23-
{
24-
get
25-
{
26-
var buffer = new byte[_signerInfo.SerialNumber.cbData];
27-
Marshal.Copy(_signerInfo.SerialNumber.pbData, buffer, 0, buffer.Length);
28-
return buffer;
29-
}
30-
}
25+
public Oid Oid { get; }
3126

32-
public void Dispose()
33-
{
34-
Dispose(true);
35-
GC.SuppressFinalize(this);
36-
}
27+
public abstract Oid DigestAlgorithm { get; }
28+
public abstract Oid HashEncryptionAlgorithm { get; }
29+
public abstract CryptographicAttributeObjectCollection UnsignedAttributes { get; }
30+
public abstract CryptographicAttributeObjectCollection SignedAttributes { get; }
31+
public abstract byte[] SerialNumber { get; }
3732

38-
public virtual void Dispose(bool disposing)
33+
internal byte[] ReadBlob(CRYPTOAPI_BLOB blob)
3934
{
35+
var buffer = new byte[blob.cbData];
36+
Marshal.Copy(blob.pbData, buffer, 0, buffer.Length);
37+
return buffer;
4038
}
4139

42-
~SignatureBase()
40+
internal unsafe CryptographicAttributeObjectCollection ReadAttributes(CRYPT_ATTRIBUTES attributes)
4341
{
44-
Dispose(false);
42+
var collection = new CryptographicAttributeObjectCollection();
43+
var attributeSize = Marshal.SizeOf<CRYPT_ATTRIBUTE>();
44+
var blobSize = Marshal.SizeOf<CRYPTOAPI_BLOB>();
45+
for (var i = 0; i < attributes.cAttr; i++)
46+
{
47+
var structure = Marshal.PtrToStructure<CRYPT_ATTRIBUTE>(attributes.rgAttr + (i * attributeSize));
48+
var asnValues = new AsnEncodedDataCollection();
49+
for(var j = 0; j < structure.cValue; j++)
50+
{
51+
var blob = Marshal.PtrToStructure<CRYPTOAPI_BLOB>(structure.rgValue + j * blobSize);
52+
asnValues.Add(new AsnEncodedData(structure.pszObjId, ReadBlob(blob)));
53+
}
54+
collection.Add(new CryptographicAttributeObject(new Oid(structure.pszObjId), asnValues));
55+
}
56+
return collection;
4557
}
4658
}
4759

48-
public class AuthenticodeSignature : SignatureBase
60+
public class AuthenticodeSignature : CounterSignatureBase
4961
{
62+
63+
public override Oid DigestAlgorithm { get; }
64+
public override Oid HashEncryptionAlgorithm { get; }
65+
public override CryptographicAttributeObjectCollection UnsignedAttributes { get; }
66+
public override CryptographicAttributeObjectCollection SignedAttributes { get; }
67+
public override byte[] SerialNumber { get; }
68+
5069
public unsafe AuthenticodeSignature(AsnEncodedData data) : base(data)
5170
{
5271
fixed (byte* dataPtr = data.RawData)
5372
{
54-
LocalBufferSafeHandle ptr;
5573
uint size = 0;
56-
if (Crypt32.CryptDecodeObjectEx(EncodingType.PKCS_7_ASN_ENCODING | EncodingType.X509_ASN_ENCODING, (IntPtr)500, new IntPtr(dataPtr), (uint)data.RawData.Length, CryptDecodeFlags.CRYPT_DECODE_ALLOC_FLAG, IntPtr.Zero, out ptr, ref size))
74+
LocalBufferSafeHandle localBuffer;
75+
if (Crypt32.CryptDecodeObjectEx(EncodingType.PKCS_7_ASN_ENCODING | EncodingType.X509_ASN_ENCODING, (IntPtr)500, new IntPtr(dataPtr), (uint)data.RawData.Length, CryptDecodeFlags.CRYPT_DECODE_ALLOC_FLAG, IntPtr.Zero, out localBuffer, ref size))
5776
{
58-
using (ptr)
77+
using (localBuffer)
5978
{
60-
_signerInfo = Marshal.PtrToStructure<CMSG_SIGNER_INFO>(ptr.DangerousGetHandle());
79+
var signerInfo = Marshal.PtrToStructure<CMSG_SIGNER_INFO>(localBuffer.DangerousGetHandle());
80+
DigestAlgorithm = new Oid(signerInfo.HashAlgorithm.pszObjId);
81+
HashEncryptionAlgorithm = new Oid(signerInfo.HashEncryptionAlgorithm.pszObjId);
82+
SerialNumber = ReadBlob(signerInfo.SerialNumber);
83+
UnsignedAttributes = ReadAttributes(signerInfo.UnauthAttrs);
84+
SignedAttributes = ReadAttributes(signerInfo.AuthAttrs);
6185
}
6286
}
6387
else
@@ -68,10 +92,14 @@ public unsafe AuthenticodeSignature(AsnEncodedData data) : base(data)
6892
}
6993
}
7094

71-
public class Rfc3161Signature : SignatureBase
95+
public class Rfc3161Signature : CounterSignatureBase
7296
{
73-
private readonly X509Store _certificates;
74-
private readonly CryptMsgSafeHandle _messageHandle;
97+
public override Oid DigestAlgorithm { get; }
98+
public override Oid HashEncryptionAlgorithm { get; }
99+
public override CryptographicAttributeObjectCollection UnsignedAttributes { get; }
100+
public override CryptographicAttributeObjectCollection SignedAttributes { get; }
101+
public override byte[] SerialNumber { get; }
102+
public X509Store Certificates { get; }
75103

76104
public unsafe Rfc3161Signature(AsnEncodedData data) : base(data)
77105
{
@@ -111,39 +139,36 @@ public unsafe Rfc3161Signature(AsnEncodedData data) : base(data)
111139
}
112140
try
113141
{
114-
_certificates = new X509Store(store.DangerousGetHandle());
115-
_messageHandle = msgHandle;
142+
Certificates = new X509Store(store.DangerousGetHandle());
116143
var size = 0u;
117-
if (!Crypt32.CryptMsgGetParam(_messageHandle, CryptMsgParamType.CMSG_SIGNER_INFO_PARAM, 0, LocalBufferSafeHandle.Zero, ref size))
144+
if (!Crypt32.CryptMsgGetParam(msgHandle, CryptMsgParamType.CMSG_SIGNER_INFO_PARAM, 0, LocalBufferSafeHandle.Zero, ref size))
145+
{
146+
Certificates.Dispose();
147+
throw new InvalidOperationException("Unable to read signer information.");
148+
}
149+
var localBuffer = LocalBufferSafeHandle.Alloc(size);
150+
if (!Crypt32.CryptMsgGetParam(msgHandle, CryptMsgParamType.CMSG_SIGNER_INFO_PARAM, 0, localBuffer, ref size))
118151
{
119-
_messageHandle.Dispose();
152+
Certificates.Dispose();
153+
localBuffer.Dispose();
120154
throw new InvalidOperationException("Unable to read signer information.");
121155
}
122-
using (var localBuffer = LocalBufferSafeHandle.Alloc(size))
156+
using (localBuffer)
123157
{
124-
if (!Crypt32.CryptMsgGetParam(_messageHandle, CryptMsgParamType.CMSG_SIGNER_INFO_PARAM, 0, localBuffer, ref size))
125-
{
126-
_messageHandle.Dispose();
127-
throw new InvalidOperationException("Unable to read signer information.");
128-
}
129-
_signerInfo = Marshal.PtrToStructure<CMSG_SIGNER_INFO>(localBuffer.DangerousGetHandle());
158+
var signerInfo = Marshal.PtrToStructure<CMSG_SIGNER_INFO>(localBuffer.DangerousGetHandle());
159+
DigestAlgorithm = new Oid(signerInfo.HashAlgorithm.pszObjId);
160+
HashEncryptionAlgorithm = new Oid(signerInfo.HashEncryptionAlgorithm.pszObjId);
161+
SerialNumber = ReadBlob(signerInfo.SerialNumber);
162+
UnsignedAttributes = ReadAttributes(signerInfo.UnauthAttrs);
163+
SignedAttributes = ReadAttributes(signerInfo.AuthAttrs);
130164
}
131165
}
132166
finally
133167
{
168+
msgHandle.Dispose();
134169
store.Dispose();
135170
}
136171
}
137172
}
138-
139-
140-
public override void Dispose(bool disposing)
141-
{
142-
if (disposing)
143-
{
144-
_messageHandle.Dispose();
145-
_certificates.Dispose();
146-
}
147-
}
148173
}
149174
}

‎AuthenticodeLint/Rules/TimestampedRule.cs

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,15 @@ public unsafe RuleResult Validate(Graph<Signature> graph, SignatureLogger verbos
1616
var pass = true;
1717
foreach (var signature in signatures)
1818
{
19+
var counterSignaturesGraph = GraphBuilder.WalkCounterSignatures(signature);
1920
var signatureInfo = signature.SignerInfo;
21+
var counterSignatures = counterSignaturesGraph.VisitAll();
2022
var isSigned = false;
2123
var strongSign = false;
22-
foreach (var attribute in signatureInfo.UnsignedAttributes)
24+
foreach (var counterSignature in counterSignatures)
2325
{
24-
SignatureBase timeStampCounterSign = null;
25-
if (attribute.Oid.Value == KnownOids.Rfc3161CounterSignature)
26-
{
27-
timeStampCounterSign = new Rfc3161Signature(attribute.Values[0]);
28-
}
29-
else if (attribute.Oid.Value == KnownOids.AuthenticodeCounterSignature)
30-
{
31-
timeStampCounterSign = new AuthenticodeSignature(attribute.Values[0]);
32-
}
33-
if (timeStampCounterSign == null)
34-
{
35-
continue;
36-
}
3726
isSigned = true;
38-
if (timeStampCounterSign.DigestAlgorithm.Value == signatureInfo.DigestAlgorithm.Value)
27+
if (counterSignature.DigestAlgorithm.Value == signatureInfo.DigestAlgorithm.Value)
3928
{
4029
strongSign = true;
4130
break;

‎AuthenticodeLint/SignatureExtractor.cs

Lines changed: 1 addition & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -50,38 +50,11 @@ private unsafe Graph<Signature> GetSignatures(CryptMsgSafeHandle messageHandle)
5050
{
5151
if (Crypt32.CryptMsgGetParam(messageHandle, CryptMsgParamType.CMSG_ENCODED_MESSAGE, 0, buf, ref size))
5252
{
53-
return RecursiveSigner(new List<byte[]> { buffer });
53+
return GraphBuilder.ExplodeGraph(buffer, KnownOids.NestedSignatureOid);
5454
}
5555
}
5656
}
5757
return null;
5858
}
59-
60-
61-
private static Graph<Signature> RecursiveSigner(IList<byte[]> cmsData)
62-
{
63-
var graphItems = new List<GraphItem<Signature>>();
64-
foreach (var data in cmsData)
65-
{
66-
var cms = new SignedCms();
67-
cms.Decode(data);
68-
foreach (var signer in cms.SignerInfos)
69-
{
70-
var childCms = new List<byte[]>();
71-
foreach (var attribute in signer.UnsignedAttributes)
72-
{
73-
if (attribute.Oid.Value == KnownOids.NestedSignatureOid)
74-
{
75-
foreach (var value in attribute.Values)
76-
{
77-
childCms.Add(value.RawData);
78-
}
79-
}
80-
}
81-
graphItems.Add(new GraphItem<Signature>(new Signature(signer, cms.Certificates), RecursiveSigner(childCms)));
82-
}
83-
}
84-
return new Graph<Signature>(graphItems);
85-
}
8659
}
8760
}

0 commit comments

Comments
 (0)