Fix binary signature expiry check to validate signing time against certificate validity period

Lib/Collectors/WindowsFileSystemUtils.cs

@@ -10,6 +10,8 @@
 using System.Collections.Generic;
 using System.IO;
 using System.Runtime.InteropServices;
+using System.Security.Cryptography;
+using System.Security.Cryptography.Pkcs;
 
 namespace Microsoft.CST.AttackSurfaceAnalyzer.Collectors
 {
@@ -148,6 +150,7 @@ private static List<DLLCHARACTERISTICS> CharacteristicsTypeToListOfCharacteristi
                     var peHeader = new PeFile(stream);
                     var authenticodeInfo = new AuthenticodeInfo(peHeader);
                     var sig = new Signature(authenticodeInfo);
+                    sig.SigningTime = GetSigningTime(peHeader);
                     return sig;
                 }
             }
@@ -172,6 +175,7 @@ private static List<DLLCHARACTERISTICS> CharacteristicsTypeToListOfCharacteristi
                     var peHeader = new PeFile(mmf);
                     var ai = new AuthenticodeInfo(peHeader);
                     var sig = new Signature(ai);
+                    sig.SigningTime = GetSigningTime(peHeader);
                     return sig;
                 }
             }
@@ -197,6 +201,90 @@ private static List<DLLCHARACTERISTICS> CharacteristicsTypeToListOfCharacteristi
             return null;
         }
 
+        /// <summary>
+        /// Extracts the signing timestamp from a PE file's Authenticode signature.
+        /// The method first attempts to use the countersigner info in the PKCS#7 data,
+        /// then checks for RFC3161 timestamp tokens in the signer's UnsignedAttributes,
+        /// and finally falls back to the signer's own SignedAttributes if needed.
+        /// </summary>
+        internal static DateTime? GetSigningTime(PeFile peFile)
+        {
+            try
+            {
+                var certData = peFile.WinCertificate?.BCertificate.ToArray();
+                if (certData is null)
+                {
+                    return null;
+                }
+
+                var signedCms = new SignedCms();
+                signedCms.Decode(certData);
+
+                foreach (var signerInfo in signedCms.SignerInfos)
+                {
+                    // Check counter-signers for the Authenticode timestamp
+                    foreach (var counterSigner in signerInfo.CounterSignerInfos)
+                    {
+                        var time = GetPkcs9SigningTime(counterSigner.SignedAttributes);
+                        if (time.HasValue)
+                            return time;
+                    }
+
+                    // Check unsigned attributes for RFC 3161 timestamp tokens
+                    foreach (var attr in signerInfo.UnsignedAttributes)
+                    {
+                        // RFC 3161 timestamp token OID: 1.2.840.113549.1.9.16.2.14 (signatureTimeStampToken)
+                        if (string.Equals(attr.Oid?.Value, "1.2.840.113549.1.9.16.2.14", StringComparison.Ordinal))
+                        {
+                            foreach (var val in attr.Values)
+                            {
+                                try
+                                {
+                                    var tokenCms = new SignedCms();
+                                    tokenCms.Decode(val.RawData);
+                                    foreach (var tokenSigner in tokenCms.SignerInfos)
+                                    {
+                                        var time = GetPkcs9SigningTime(tokenSigner.SignedAttributes);
+                                        if (time.HasValue)
+                                            return time;
+                                    }
+                                }
+                                catch (CryptographicException)
+                                {
+                                    // Not a valid CMS structure, skip
+                                }
+                            }
+                        }
+                    }
+
+                    // Fallback: check the signer's own signed attributes
+                    var signerTime = GetPkcs9SigningTime(signerInfo.SignedAttributes);
+                    if (signerTime.HasValue)
+                        return signerTime;
+                }
+            }
+            catch (Exception e)
+            {
+                Log.Verbose(e, "Failed to extract signing time ({0}:{1})", e.GetType(), e.Message);
+            }
+            return null;
+        }
+
+        private static DateTime? GetPkcs9SigningTime(CryptographicAttributeObjectCollection attributes)
+        {
+            foreach (var attr in attributes)
+            {
+                foreach (var val in attr.Values)
+                {
+                    if (val is Pkcs9SigningTime signingTime)
+                    {
+                        return signingTime.SigningTime;
+                    }
+                }
+            }
+            return null;
+        }
+
         /// <summary>
         /// Try to determine if the file is locally present to avoid triggering a downloading files that are cloud stubs
         /// </summary>

Lib/Objects/Signature.cs

@@ -42,9 +42,9 @@ public bool IsTimeValid
         {
             get
             {
-                if (SigningCertificate != null)
+                if (SigningCertificate != null && SigningTime is DateTime signingTime)
                 {
-                    return DateTime.Now > SigningCertificate.NotBefore && DateTime.Now < SigningCertificate.NotAfter;
+                    return signingTime >= SigningCertificate.NotBefore && signingTime <= SigningCertificate.NotAfter;
                 }
                 return false;
             }
@@ -54,5 +54,6 @@ public bool IsTimeValid
         public string? SignedHash { get; set; }
         public string? SignerSerialNumber { get; set; }
         public SerializableCertificate? SigningCertificate { get; set; }
+        public DateTime? SigningTime { get; set; }
     }
 }

Tests/SignatureTests.cs

@@ -0,0 +1,130 @@
+// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License.
+using Microsoft.CST.AttackSurfaceAnalyzer.Objects;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using System;
+
+namespace Microsoft.CST.AttackSurfaceAnalyzer.Tests
+{
+    [TestClass, TestCategory("PipelineSafeTests")]
+    public class SignatureTests
+    {
+        [TestMethod]
+        public void IsTimeValid_SignedDuringCertValidity_ReturnsTrue()
+        {
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2025, 12, 31), // NotAfter
+                    new DateTime(2020, 1, 1),   // NotBefore
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = new DateTime(2023, 6, 15) // Signed within validity
+            };
+            Assert.IsTrue(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_SignedAfterCertExpiry_ReturnsFalse()
+        {
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2022, 12, 31), // NotAfter
+                    new DateTime(2020, 1, 1),   // NotBefore
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = new DateTime(2023, 6, 15) // Signed after expiry
+            };
+            Assert.IsFalse(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_SignedBeforeCertNotBefore_ReturnsFalse()
+        {
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2025, 12, 31), // NotAfter
+                    new DateTime(2020, 1, 1),   // NotBefore
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = new DateTime(2019, 6, 15) // Signed before NotBefore
+            };
+            Assert.IsFalse(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_NullSigningTime_ReturnsFalse()
+        {
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2025, 12, 31),
+                    new DateTime(2020, 1, 1),
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = null
+            };
+            Assert.IsFalse(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_NullSigningCertificate_ReturnsFalse()
+        {
+            var sig = new Signature()
+            {
+                SigningCertificate = null,
+                SigningTime = new DateTime(2023, 6, 15)
+            };
+            Assert.IsFalse(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_CertExpiredNow_ButSignedDuringValidity_ReturnsTrue()
+        {
+            // This is the key scenario: cert is currently expired, but was valid when signing occurred
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2020, 12, 31), // NotAfter - expired now
+                    new DateTime(2018, 1, 1),   // NotBefore
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = new DateTime(2019, 6, 15) // Signed while cert was valid
+            };
+            Assert.IsTrue(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_SignedExactlyAtNotBefore_ReturnsTrue()
+        {
+            var notBefore = new DateTime(2020, 1, 1);
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    new DateTime(2025, 12, 31),
+                    notBefore,
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = notBefore // Signed exactly at NotBefore boundary
+            };
+            Assert.IsTrue(sig.IsTimeValid);
+        }
+
+        [TestMethod]
+        public void IsTimeValid_SignedExactlyAtNotAfter_ReturnsTrue()
+        {
+            var notAfter = new DateTime(2025, 12, 31);
+            var sig = new Signature()
+            {
+                SigningCertificate = new SerializableCertificate(
+                    "thumbprint", "CN=Test", "key",
+                    notAfter,
+                    new DateTime(2020, 1, 1),
+                    "CN=Issuer", "serial", "hash", "pkcs7"),
+                SigningTime = notAfter // Signed exactly at NotAfter boundary
+            };
+            Assert.IsTrue(sig.IsTimeValid);
+        }
+    }
+}

analyses.json

@@ -1556,8 +1556,8 @@
       ]
     },
     {
-      "Name": "Binaries with expired signatures",
-      "Description": "These binaries have expired signatures.",
+      "Name": "Binaries with unverified signature validity",
+      "Description": "These binaries have signatures where the signing time could not be verified as being within the certificate's validity period. The signing time may be outside the validity period or could not be determined.",
       "Flag": "WARNING",
       "ResultType": "FILE",
       "ChangeTypes": [
@@ -1566,14 +1566,19 @@
       ],
       "Clauses": [
         {
-          "Field": "SignatureStatus.SigningCertificate.NotAfter",
-          "Operation": "IsExpired"
+          "Field": "SignatureStatus.IsAuthenticodeValid",
+          "Operation": "IsTrue"
+        },
+        {
+          "Field": "SignatureStatus.IsTimeValid",
+          "Operation": "IsTrue",
+          "Invert": true
         }
       ]
     },
     {
-      "Name": "Binaries with expired signatures",
-      "Description": "These binaries have expired signatures.",
+      "Name": "Binaries with unverified signature validity",
+      "Description": "These binaries have signatures where the signing time could not be verified as being within the certificate's validity period. The signing time may be outside the validity period or could not be determined.",
       "Flag": "WARNING",
       "ResultType": "FILEMONITOR",
       "ChangeTypes": [
@@ -1582,8 +1587,13 @@
       ],
       "Clauses": [
         {
-          "Field": "FileSystemObject.SignatureStatus.SigningCertificate.NotAfter",
-          "Operation": "IsExpired"
+          "Field": "FileSystemObject.SignatureStatus.IsAuthenticodeValid",
+          "Operation": "IsTrue"
+        },
+        {
+          "Field": "FileSystemObject.SignatureStatus.IsTimeValid",
+          "Operation": "IsTrue",
+          "Invert": true
         }
       ]
     },

nuget.config

@@ -1,7 +1,7 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <configuration>
   <packageSources>
     <clear />
     <add key="PublicRegistriesFeed" value="https://pkgs.dev.azure.com/microsoft-sdl/General/_packaging/PublicRegistriesFeed/nuget/v3/index.json" />
   </packageSources>
-</configuration>
+</configuration>