Signing: log additional context when root is untrusted on Linux and macOS

src/NuGet.Core/NuGet.Common/Errors/NuGetLogCode.cs

@@ -552,6 +552,11 @@ public enum NuGetLogCode
         /// </summary>
         NU3041 = 3041,
 
+        /// <summary>
+        /// An X.509 trust store does not contain a root certificate observed in a package signature.
+        /// </summary>
+        NU3042 = 3042,
+
         /// <summary>
         /// Undefined Package Error.
         /// </summary>

src/NuGet.Core/NuGet.Common/PublicAPI.Unshipped.txt

@@ -1 +1,2 @@
 #nullable enable
+NuGet.Common.NuGetLogCode.NU3042 = 3042 -> NuGet.Common.NuGetLogCode

src/NuGet.Core/NuGet.Packaging/Signing/ChainBuilding/DefaultX509ChainBuildPolicy.cs

@@ -12,7 +12,7 @@ internal sealed class DefaultX509ChainBuildPolicy : IX509ChainBuildPolicy
 
         private DefaultX509ChainBuildPolicy() { }
 
-        public bool Build(X509Chain chain, X509Certificate2 certificate)
+        public bool Build(IX509Chain chain, X509Certificate2 certificate)
         {
             if (chain is null)
             {

src/NuGet.Core/NuGet.Packaging/Signing/ChainBuilding/IX509ChainBuildPolicy.cs

@@ -11,6 +11,6 @@ namespace NuGet.Packaging.Signing
     /// </summary>
     internal interface IX509ChainBuildPolicy
     {
-        bool Build(X509Chain chain, X509Certificate2 certificate);
+        bool Build(IX509Chain chain, X509Certificate2 certificate);
     }
 }

src/NuGet.Core/NuGet.Packaging/Signing/ChainBuilding/RetriableX509ChainBuildPolicy.cs

@@ -36,7 +36,7 @@ internal RetriableX509ChainBuildPolicy(IX509ChainBuildPolicy innerPolicy, int re
             SleepInterval = sleepInterval;
         }
 
-        public bool Build(X509Chain chain, X509Certificate2 certificate)
+        public bool Build(IX509Chain chain, X509Certificate2 certificate)
         {
             if (chain is null)
             {

src/NuGet.Core/NuGet.Packaging/Signing/Signatures/Signature.cs

@@ -192,7 +192,7 @@ protected Signature(SignerInfo signerInfo, SignatureType type)
                 timestamp = timestamp ?? new Timestamp();
                 using (X509ChainHolder chainHolder = X509ChainHolder.CreateForCodeSigning())
                 {
-                    var chain = chainHolder.Chain;
+                    IX509Chain chain = chainHolder.Chain2;
 
                     // This flag should only be set for verification scenarios, not signing.
                     chain.ChainPolicy.VerificationFlags = X509VerificationFlags.IgnoreNotTimeValid;
@@ -209,7 +209,7 @@ protected Signature(SignerInfo signerInfo, SignatureType type)
                     }
 
                     var chainBuildingSucceeded = CertificateChainUtility.BuildCertificateChain(chain, certificate, out var chainStatuses);
-                    var x509ChainString = CertificateUtility.X509ChainToString(chain, fingerprintAlgorithm);
+                    string x509ChainString = CertificateUtility.X509ChainToString(chain.PrivateReference, fingerprintAlgorithm);
 
                     if (!string.IsNullOrWhiteSpace(x509ChainString))
                     {
@@ -249,6 +249,8 @@ protected Signature(SignerInfo signerInfo, SignatureType type)
                         {
                             if (settings.ReportUntrustedRoot)
                             {
+                                SignatureUtility.LogAdditionalContext(chain, issues);
+
                                 issues.Add(SignatureLog.Issue(!settings.AllowUntrusted, NuGetLogCode.NU3018, string.Format(CultureInfo.CurrentCulture, Strings.VerifyChainBuildingIssue_UntrustedRoot, FriendlyName)));
                             }
 

src/NuGet.Core/NuGet.Packaging/Signing/Timestamp/Timestamp.cs

@@ -144,7 +144,7 @@ public Timestamp(SignedCms timestampCms)
 
                 using (X509ChainHolder chainHolder = X509ChainHolder.CreateForTimestamping())
                 {
-                    var chain = chainHolder.Chain;
+                    IX509Chain chain = chainHolder.Chain2;
 
                     // This flag should only be set for verification scenarios, not signing.
                     chain.ChainPolicy.VerificationFlags = X509VerificationFlags.IgnoreNotTimeValid;
@@ -161,7 +161,7 @@ public Timestamp(SignedCms timestampCms)
                     }
 
                     var chainBuildSucceed = CertificateChainUtility.BuildCertificateChain(chain, timestamperCertificate, out var chainStatusList);
-                    var x509ChainString = CertificateUtility.X509ChainToString(chain, fingerprintAlgorithm);
+                    string x509ChainString = CertificateUtility.X509ChainToString(chain.PrivateReference, fingerprintAlgorithm);
 
                     if (!string.IsNullOrWhiteSpace(x509ChainString))
                     {
@@ -195,6 +195,8 @@ public Timestamp(SignedCms timestampCms)
 
                     if (CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, X509ChainStatusFlags.UntrustedRoot, out messages))
                     {
+                        SignatureUtility.LogAdditionalContext(chain, issues);
+
                         issues.Add(SignatureLog.Error(NuGetLogCode.NU3028, string.Format(CultureInfo.CurrentCulture, Strings.VerifyTimestampChainBuildingIssue_UntrustedRoot, signature.FriendlyName)));
 
                         flags |= SignatureVerificationStatusFlags.UntrustedRoot;

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/CertificateBundleX509ChainFactory.cs

@@ -4,9 +4,12 @@
 #if NET5_0_OR_GREATER
 
 using System;
+using System.Globalization;
 using System.IO;
+using System.Linq;
 using System.Security.Cryptography;
 using System.Security.Cryptography.X509Certificates;
+using NuGet.Common;
 
 namespace NuGet.Packaging.Signing
 {
@@ -21,7 +24,7 @@ protected CertificateBundleX509ChainFactory(X509Certificate2Collection certifica
             FilePath = filePath;
         }
 
-        public X509Chain Create()
+        public IX509Chain Create()
         {
             X509Chain x509Chain = new();
 
@@ -32,7 +35,7 @@ public X509Chain Create()
                 x509Chain.ChainPolicy.CustomTrustStore.AddRange(Certificates);
             }
 
-            return x509Chain;
+            return new X509ChainWrapper(x509Chain, GetAdditionalContext);
         }
 
         protected static bool TryImportFromPemFile(string filePath, out X509Certificate2Collection certificates)
@@ -57,6 +60,66 @@ protected static bool TryImportFromPemFile(string filePath, out X509Certificate2
 
             return false;
         }
+
+        private ILogMessage GetAdditionalContext(X509Chain chain)
+        {
+            if (chain is null)
+            {
+                throw new ArgumentNullException(nameof(chain));
+            }
+
+            ILogMessage logMessage = null;
+            int lastIndex = chain.ChainElements.Count - 1;
+
+            if (lastIndex < 0)
+            {
+                return logMessage;
+            }
+
+            X509ChainElement root = chain.ChainElements[lastIndex];
+
+            // If a certificate chain is untrusted simply the root certificate is untrusted, then create
+            // an additional message explaining how one might resolve this lack of trust.
+            if (root.ChainElementStatus.Any(status => status.Status.HasFlag(X509ChainStatusFlags.UntrustedRoot)) &&
+                !Certificates.Contains(root.Certificate))
+            {
+                string subject = root.Certificate.Subject;
+                string fingerprint = CertificateUtility.GetHashString(root.Certificate, Common.HashAlgorithmName.SHA256);
+                string pem = GetPemEncodedCertificate(root.Certificate);
+                string message;
+
+                if (string.IsNullOrEmpty(FilePath))
+                {
+                    message = string.Format(
+                        CultureInfo.CurrentCulture,
+                        Strings.UntrustedRoot_WithoutCertificateBundle,
+                        subject,
+                        fingerprint,
+                        pem);
+                }
+                else
+                {
+                    message = string.Format(
+                        CultureInfo.CurrentCulture,
+                        Strings.UntrustedRoot_WithCertificateBundle,
+                        FilePath,
+                        subject,
+                        fingerprint,
+                        pem);
+                }
+
+                logMessage = new LogMessage(LogLevel.Warning, message, NuGetLogCode.NU3042);
+            }
+
+            return logMessage;
+        }
+
+        private static string GetPemEncodedCertificate(X509Certificate2 certificate)
+        {
+            ReadOnlyMemory<char> pem = PemEncoding.Write("CERTIFICATE", certificate.RawData);
+
+            return new string(pem.Span);
+        }
     }
 }
 

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/DotNetDefaultTrustStoreX509ChainFactory.cs

@@ -7,9 +7,9 @@ namespace NuGet.Packaging.Signing
 {
     internal sealed class DotNetDefaultTrustStoreX509ChainFactory : IX509ChainFactory
     {
-        public X509Chain Create()
+        public IX509Chain Create()
         {
-            return new X509Chain();
+            return new X509ChainWrapper(new X509Chain());
         }
     }
 }

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/IX509Chain.cs

@@ -0,0 +1,27 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Security.Cryptography.X509Certificates;
+using NuGet.Common;
+
+namespace NuGet.Packaging.Signing
+{
+    internal interface IX509Chain : IDisposable
+    {
+        ILogMessage AdditionalContext { get; }
+
+        /// <summary>
+        /// This exists purely to avoid breaking existing public API's which require an <see cref="X509Chain" /> instance.
+        /// Internally, we should be cautious about using <see cref="PrivateReference" />.
+        /// Calling X509Chain.Build(...) directly (vs. IX509Chain.Build(...)) will break <see cref="AdditionalContext" />.
+        /// Calling any other X509Chain member is safe.
+        /// </summary>
+        X509Chain PrivateReference { get; }
+        X509ChainElementCollection ChainElements { get; }
+        X509ChainPolicy ChainPolicy { get; }
+        X509ChainStatus[] ChainStatus { get; }
+
+        bool Build(X509Certificate2 certificate);
+    }
+}

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/IX509ChainFactory.cs

@@ -1,12 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using System.Security.Cryptography.X509Certificates;
-
 namespace NuGet.Packaging.Signing
 {
     internal interface IX509ChainFactory
     {
-        X509Chain Create();
+        IX509Chain Create();
     }
 }

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/X509ChainWrapper.cs

@@ -0,0 +1,61 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Security.Cryptography.X509Certificates;
+using NuGet.Common;
+
+namespace NuGet.Packaging.Signing
+{
+    internal sealed class X509ChainWrapper : IX509Chain
+    {
+        private readonly X509Chain _chain;
+        private readonly Func<X509Chain, ILogMessage> _getAdditionalContext;
+
+        public ILogMessage AdditionalContext { get; private set; }
+        public X509ChainElementCollection ChainElements => _chain.ChainElements;
+        public X509ChainPolicy ChainPolicy => _chain.ChainPolicy;
+        public X509ChainStatus[] ChainStatus => _chain.ChainStatus;
+        public X509Chain PrivateReference => _chain;
+
+        internal X509ChainWrapper(X509Chain chain)
+            : this(chain, getAdditionalContext: null)
+        {
+        }
+
+        internal X509ChainWrapper(X509Chain chain, Func<X509Chain, ILogMessage> getAdditionalContext)
+        {
+            if (chain is null)
+            {
+                throw new ArgumentNullException(nameof(chain));
+            }
+
+            _chain = chain;
+            _getAdditionalContext = getAdditionalContext;
+        }
+
+        public bool Build(X509Certificate2 certificate)
+        {
+            if (certificate is null)
+            {
+                throw new ArgumentNullException(nameof(certificate));
+            }
+
+            bool result = _chain.Build(certificate);
+
+            if (!result && _getAdditionalContext is not null)
+            {
+                AdditionalContext = _getAdditionalContext(_chain);
+            }
+
+            return result;
+        }
+
+        public void Dispose()
+        {
+            _chain.Dispose();
+
+            GC.SuppressFinalize(this);
+        }
+    }
+}

src/NuGet.Core/NuGet.Packaging/Signing/Utility/CertificateChainUtility.cs

@@ -56,7 +56,7 @@ public static class CertificateChainUtility
             using (X509ChainHolder chainHolder = certificateType == CertificateType.Signature
                 ? X509ChainHolder.CreateForCodeSigning() : X509ChainHolder.CreateForTimestamping())
             {
-                var chain = chainHolder.Chain;
+                IX509Chain chain = chainHolder.Chain2;
 
                 SetCertBuildChainPolicy(
                     chain.ChainPolicy,
@@ -68,7 +68,7 @@ public static class CertificateChainUtility
 
                 if (BuildWithPolicy(chain, certificate))
                 {
-                    return GetCertificateChain(chain);
+                    return GetCertificateChain(chain.PrivateReference);
                 }
 
                 X509ChainStatusFlags errorStatusFlags;
@@ -79,6 +79,8 @@ public static class CertificateChainUtility
                 var fatalStatuses = new List<X509ChainStatus>();
                 var logCode = certificateType == CertificateType.Timestamp ? NuGetLogCode.NU3028 : NuGetLogCode.NU3018;
 
+                LogAdditionalContext(chain, logger);
+
                 foreach (var chainStatus in chain.ChainStatus)
                 {
                     if ((chainStatus.Status & errorStatusFlags) != 0)
@@ -102,7 +104,7 @@ public static class CertificateChainUtility
                     throw new SignatureException(logCode, Strings.CertificateChainValidationFailed);
                 }
 
-                return GetCertificateChain(chain);
+                return GetCertificateChain(chain.PrivateReference);
             }
         }
 
@@ -178,7 +180,7 @@ public static IX509CertificateChain GetCertificateChain(X509Chain x509Chain)
             }
         }
 
-        internal static bool BuildCertificateChain(X509Chain chain, X509Certificate2 certificate, out X509ChainStatus[] status)
+        internal static bool BuildCertificateChain(IX509Chain chain, X509Certificate2 certificate, out X509ChainStatus[] status)
         {
             if (certificate == null)
             {
@@ -193,7 +195,7 @@ internal static bool BuildCertificateChain(X509Chain chain, X509Certificate2 cer
             return buildSuccess && !CertificateUtility.IsCertificateValidityPeriodInTheFuture(certificate);
         }
 
-        internal static bool BuildWithPolicy(X509Chain chain, X509Certificate2 certificate)
+        internal static bool BuildWithPolicy(IX509Chain chain, X509Certificate2 certificate)
         {
             if (chain is null)
             {
@@ -252,5 +254,15 @@ internal static IEnumerable<string> GetStatusAndMessagesFromChainStatuses(IEnume
                 .Distinct(StringComparer.Ordinal)
                 .OrderBy(x => x, StringComparer.Ordinal);
         }
+
+        private static void LogAdditionalContext(IX509Chain chain, ILogger logger)
+        {
+            ILogMessage additionalContext = chain.AdditionalContext;
+
+            if (additionalContext is not null)
+            {
+                logger.Log(additionalContext);
+            }
+        }
     }
 }

src/NuGet.Core/NuGet.Packaging/Signing/Utility/CertificateUtility.cs

@@ -313,7 +313,7 @@ public static bool IsSelfIssued(X509Certificate2 certificate)
 
             using (X509ChainHolder chainHolder = X509ChainHolder.CreateForCodeSigning())
             {
-                X509Chain chain = chainHolder.Chain;
+                IX509Chain chain = chainHolder.Chain2;
 
                 chain.ChainPolicy.RevocationMode = X509RevocationMode.NoCheck;
                 chain.ChainPolicy.VerificationFlags = X509VerificationFlags.AllowUnknownCertificateAuthority |