Skip to content

Commit

Permalink
log Status and StatusInformation, fix 2 tests (#3268)
Browse files Browse the repository at this point in the history
  • Loading branch information
@heng-liu
heng-liu committed Jun 16, 2020
1 parent 432f08d commit 0553223
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 91 deletions.
10 changes: 5 additions & 5 deletions src/NuGet.Core/NuGet.Packaging/Signing/Signatures/Signature.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public virtual SignatureVerificationSummary Verify(
var statusFlags = CertificateChainUtility.DefaultObservedStatusFlags;

IEnumerable<string> messages;
if (CertificateChainUtility.TryGetStatusMessage(chainStatuses, statusFlags, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatuses, statusFlags, out messages))
{
foreach (var message in messages)
{
Expand All @@ -234,15 +234,15 @@ public virtual SignatureVerificationSummary Verify(
// For all the special cases, chain status list only has unique elements for each chain status flag present
// therefore if we are checking for one specific chain status we can use the first of the returned list
// if we are combining checks for more than one, then we have to use the whole list.
if (CertificateChainUtility.TryGetStatusMessage(chainStatuses, X509ChainStatusFlags.Revoked, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatuses, X509ChainStatusFlags.Revoked, out messages))
{
issues.Add(SignatureLog.Error(NuGetLogCode.NU3012, string.Format(CultureInfo.CurrentCulture, Strings.VerifyChainBuildingIssue, FriendlyName, messages.First())));
flags |= SignatureVerificationStatusFlags.CertificateRevoked;

return new SignatureVerificationSummary(Type, SignatureVerificationStatus.Suspect, flags, timestamp, issues);
}

if (CertificateChainUtility.TryGetStatusMessage(chainStatuses, X509ChainStatusFlags.UntrustedRoot, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatuses, X509ChainStatusFlags.UntrustedRoot, out messages))
{
if (settings.ReportUntrustedRoot)
{
Expand All @@ -256,8 +256,8 @@ public virtual SignatureVerificationSummary Verify(
}
}

var offlineRevocationErrors = CertificateChainUtility.TryGetStatusMessage(chainStatuses, X509ChainStatusFlags.OfflineRevocation, out var _);
var unknownRevocationErrors = CertificateChainUtility.TryGetStatusMessage(chainStatuses, X509ChainStatusFlags.RevocationStatusUnknown, out var unknownRevocationStatusMessages);
var offlineRevocationErrors = CertificateChainUtility.TryGetStatusAndMessage(chainStatuses, X509ChainStatusFlags.OfflineRevocation, out var _);
var unknownRevocationErrors = CertificateChainUtility.TryGetStatusAndMessage(chainStatuses, X509ChainStatusFlags.RevocationStatusUnknown, out var unknownRevocationStatusMessages);
if (offlineRevocationErrors || unknownRevocationErrors)
{
if (settings.ReportUnknownRevocation)
Expand Down
10 changes: 5 additions & 5 deletions src/NuGet.Core/NuGet.Packaging/Signing/Timestamp/Timestamp.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ internal SignatureVerificationStatusFlags Verify(

var timestampInvalidCertificateFlags = CertificateChainUtility.DefaultObservedStatusFlags;

if (CertificateChainUtility.TryGetStatusMessage(chainStatusList, timestampInvalidCertificateFlags, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, timestampInvalidCertificateFlags, out messages))
{
foreach (var message in messages)
{
Expand All @@ -193,24 +193,24 @@ internal SignatureVerificationStatusFlags Verify(
// therefore if we are checking for one specific chain status we can use the first of the returned list
// if we are combining checks for more than one, then we have to use the whole list.

if (CertificateChainUtility.TryGetStatusMessage(chainStatusList, X509ChainStatusFlags.UntrustedRoot, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, X509ChainStatusFlags.UntrustedRoot, out messages))
{
issues.Add(SignatureLog.Error(NuGetLogCode.NU3028, string.Format(CultureInfo.CurrentCulture, Strings.VerifyError_TimestampVerifyChainBuildingIssue, signature.FriendlyName, messages.First())));

flags |= SignatureVerificationStatusFlags.UntrustedRoot;
chainBuildingHasIssues = true;
}

if (CertificateChainUtility.TryGetStatusMessage(chainStatusList, X509ChainStatusFlags.Revoked, out messages))
if (CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, X509ChainStatusFlags.Revoked, out messages))
{
issues.Add(SignatureLog.Error(NuGetLogCode.NU3028, string.Format(CultureInfo.CurrentCulture, Strings.VerifyError_TimestampVerifyChainBuildingIssue, signature.FriendlyName, messages.First())));
flags |= SignatureVerificationStatusFlags.CertificateRevoked;

return flags;
}

var offlineRevocationErrors = CertificateChainUtility.TryGetStatusMessage(chainStatusList, X509ChainStatusFlags.OfflineRevocation, out var _);
var unknownRevocationErrors = CertificateChainUtility.TryGetStatusMessage(chainStatusList, X509ChainStatusFlags.RevocationStatusUnknown, out var unknownRevocationStatusMessages);
var offlineRevocationErrors = CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, X509ChainStatusFlags.OfflineRevocation, out var _);
var unknownRevocationErrors = CertificateChainUtility.TryGetStatusAndMessage(chainStatusList, X509ChainStatusFlags.RevocationStatusUnknown, out var unknownRevocationStatusMessages);
if (offlineRevocationErrors || unknownRevocationErrors)
{
if (treatIssueAsError)
Expand Down
18 changes: 9 additions & 9 deletions src/NuGet.Core/NuGet.Packaging/Signing/Utility/CertificateChainUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,11 @@ public static IX509CertificateChain GetCertificateChain(
if ((chainStatus.Status & errorStatusFlags) != 0)
{
fatalStatuses.Add(chainStatus);
logger.Log(LogMessage.CreateError(logCode, chainStatus.StatusInformation?.Trim()));
logger.Log(LogMessage.CreateError(logCode, $"{chainStatus.Status}: {chainStatus.StatusInformation?.Trim()}"));
}
else if ((chainStatus.Status & warningStatusFlags) != 0)
{
logger.Log(LogMessage.CreateWarning(logCode, chainStatus.StatusInformation?.Trim()));
logger.Log(LogMessage.CreateWarning(logCode, $"{chainStatus.Status}: {chainStatus.StatusInformation?.Trim()}"));
}
}

Expand Down Expand Up @@ -212,27 +212,27 @@ internal static bool ChainStatusListIncludesStatus(X509ChainStatus[] chainStatus
return chainStatus.Any();
}

internal static bool TryGetStatusMessage(X509ChainStatus[] chainStatuses, X509ChainStatusFlags status, out IEnumerable<string> messages)
internal static bool TryGetStatusAndMessage(X509ChainStatus[] chainStatuses, X509ChainStatusFlags status, out IEnumerable<string> statusAndMessages)
{
messages = null;
statusAndMessages = null;

if (ChainStatusListIncludesStatus(chainStatuses, status, out var chainStatus))
{
messages = GetMessagesFromChainStatuses(chainStatus);
statusAndMessages = GetStatusAndMessagesFromChainStatuses(chainStatus);

return true;
}

return false;
}

internal static IEnumerable<string> GetMessagesFromChainStatuses(IEnumerable<X509ChainStatus> chainStatuses)
internal static IEnumerable<string> GetStatusAndMessagesFromChainStatuses(IEnumerable<X509ChainStatus> chainStatuses)
{
return chainStatuses
.Select(x => x.StatusInformation?.Trim())
.Where(x => !string.IsNullOrEmpty(x))
.Where(x => !string.IsNullOrEmpty(x.StatusInformation?.Trim()))
.Select(x => $"{x.Status}: {x.StatusInformation?.Trim()}")
.Distinct(StringComparer.Ordinal)
.OrderBy(x => x, StringComparer.Ordinal);
}
}
}
}
4 changes: 2 additions & 2 deletions test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/InstallCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ public class InstallCommandTests
private static readonly string _NU3008 = "NU3008: {0}";
private static readonly string _NU3027Message = "The signature should be timestamped to enable long-term signature validity after the certificate has expired.";
private static readonly string _NU3027 = "NU3027: {0}";
private static readonly string _NU3012Message = "The author primary signature found a chain building issue: The certificate is revoked.";
private static readonly string _NU3012Message = "The author primary signature found a chain building issue: Revoked: The certificate is revoked.";
private static readonly string _NU3012 = "NU3012: {0}";
private static readonly string _NU3018Message = "The author primary signature found a chain building issue: A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider.";
private static readonly string _NU3018Message = "The author primary signature found a chain building issue: UntrustedRoot: A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider.";
private static readonly string _NU3018 = "NU3018: {0}";

private SignCommandTestFixture _testFixture;
Expand Down
8 changes: 4 additions & 4 deletions test/NuGet.Core.Tests/NuGet.Packaging.Test/SigningTests/CertificateChainUtilityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,17 +98,17 @@ public void GetCertificateChain_WithUntrustedRoot_Throws()
Assert.Equal(1, logger.Errors);
SigningTestUtility.AssertUntrustedRoot(logger.LogMessages, LogLevel.Error);

SigningTestUtility.AssertOfflineRevocation(logger.LogMessages, LogLevel.Warning);
SigningTestUtility.AssertRevocationStatusUnknown(logger.LogMessages, LogLevel.Warning);
if (RuntimeEnvironmentHelper.IsWindows)
{
Assert.Equal(2, logger.Warnings);
SigningTestUtility.AssertRevocationStatusUnknown(logger.LogMessages, LogLevel.Warning);
SigningTestUtility.AssertOfflineRevocation(logger.LogMessages, LogLevel.Warning);
}
else if (RuntimeEnvironmentHelper.IsLinux)
{
#if NETCORE5_0
Assert.Equal(2, logger.Warnings);
SigningTestUtility.AssertRevocationStatusUnknown(logger.LogMessages, LogLevel.Warning);
SigningTestUtility.AssertOfflineRevocation(logger.LogMessages, LogLevel.Warning);
#else
Assert.Equal(1, logger.Warnings);
#endif
Expand Down Expand Up @@ -149,7 +149,7 @@ public void GetCertificateChain_WithUntrustedSelfIssuedCertificate_ReturnsChain(
#if !NETCORE5_0
if (RuntimeEnvironmentHelper.IsLinux)
{
SigningTestUtility.AssertOfflineRevocation(logger.LogMessages, LogLevel.Warning);
SigningTestUtility.AssertRevocationStatusUnknown(logger.LogMessages, LogLevel.Warning);
}
#endif
}
Expand Down
2 changes: 1 addition & 1 deletion test/NuGet.Core.Tests/NuGet.Packaging.Test/SigningTests/SigningUtilityTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public void Verify_WithUntrustedSelfSignedCertificate_Succeeds()
#if !NETCORE5_0
if (RuntimeEnvironmentHelper.IsLinux)
{
SigningTestUtility.AssertOfflineRevocation(logger.LogMessages, LogLevel.Warning);
SigningTestUtility.AssertRevocationStatusUnknown(logger.LogMessages, LogLevel.Warning);
}
#endif
}
Expand Down
87 changes: 22 additions & 65 deletions test/TestUtilities/Test.Utility/Signing/SigningTestUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Linq;
using System.Security.Cryptography;
using System.Security.Cryptography.Pkcs;
using System.Security.Cryptography.X509Certificates;
Expand Down Expand Up @@ -394,7 +395,7 @@ public static X509Certificate2 GenerateCertificate(
var request = new CertificateRequest(subjectDN, algorithm, hashAlgorithm, RSASignaturePadding.Pkcs1);

request.CertificateExtensions.Add(
new X509BasicConstraintsExtension(certificateAuthority: true, hasPathLengthConstraint: false, pathLengthConstraint: 0, critical: true));
new X509BasicConstraintsExtension(certificateAuthority: true, hasPathLengthConstraint: false, pathLengthConstraint: 0, critical: true));
request.CertificateExtensions.Add(
new X509SubjectKeyIdentifierExtension(request.PublicKey, critical: false));
request.CertificateExtensions.Add(
Expand Down Expand Up @@ -664,25 +665,14 @@ public static void VerifyByteArrays(byte[] expected, byte[] actual)
//So if we use APIs above to verify the results of chain.build, we should use AssertOfflineRevocation
public static void AssertOfflineRevocation(IEnumerable<ILogMessage> issues, LogLevel logLevel)
{
string offlineRevocation;

if (RuntimeEnvironmentHelper.IsWindows)
{
offlineRevocation = "The revocation function was unable to check revocation because the revocation server was offline";
}
else if (RuntimeEnvironmentHelper.IsMacOSX)
{
offlineRevocation = "An incomplete certificate revocation check occurred.";
}
else
{
offlineRevocation = "unable to get certificate CRL";
}
string offlineRevocation = X509ChainStatusFlags.OfflineRevocation.ToString();

Assert.Contains(issues, issue =>
bool isOfflineRevocation = issues.Any(issue =>
issue.Code == NuGetLogCode.NU3018 &&
issue.Level == logLevel &&
issue.Message.Contains(offlineRevocation));
issue.Message.Split(new[] { ' ', ':' }).Where(WORDEXTFLAGS => WORDEXTFLAGS == offlineRevocation).Any());

Assert.True(isOfflineRevocation);
}

//We will change the original X509ChainStatus.StatusInformation of OfflineRevocation to VerifyCertTrustOfflineWhileRevocationModeOffline or VerifyCertTrustOfflineWhileRevocationModeOnline in Signature.cs and Timestamp.cs
Expand Down Expand Up @@ -710,48 +700,26 @@ public static void AssertRevocationStatusUnknown(IEnumerable<ILogMessage> issues

public static void AssertRevocationStatusUnknown(IEnumerable<ILogMessage> issues, LogLevel logLevel, NuGetLogCode code)
{
string revocationStatusUnknown;
string revocationStatusUnknown = X509ChainStatusFlags.RevocationStatusUnknown.ToString();

if (RuntimeEnvironmentHelper.IsWindows)
{
revocationStatusUnknown = "The revocation function was unable to check revocation for the certificate";
}
else if (RuntimeEnvironmentHelper.IsMacOSX)
{
revocationStatusUnknown = "An incomplete certificate revocation check occurred.";
}
else
{
revocationStatusUnknown = "unable to get certificate CRL";
}

Assert.Contains(issues, issue =>
bool isRevocationStatusUnknown = issues.Any(issue =>
issue.Code == code &&
issue.Level == logLevel &&
issue.Message.Contains(revocationStatusUnknown));
issue.Message.Split(new[] { ' ', ':' }).Where(WORDEXTFLAGS => WORDEXTFLAGS == revocationStatusUnknown).Any());

Assert.True(isRevocationStatusUnknown);
}

public static void AssertUntrustedRoot(IEnumerable<ILogMessage> issues, NuGetLogCode code, LogLevel logLevel)
{
string untrustedRoot;
string untrustedRoot = X509ChainStatusFlags.UntrustedRoot.ToString();

if (RuntimeEnvironmentHelper.IsWindows)
{
untrustedRoot = "A certificate chain processed, but terminated in a root certificate which is not trusted by the trust provider";
}
else if (RuntimeEnvironmentHelper.IsMacOSX)
{
untrustedRoot = "The certificate was not trusted.";
}
else
{
untrustedRoot = "self signed certificate";
}

Assert.Contains(issues, issue =>
bool isUntrustedRoot = issues.Any(issue =>
issue.Code == code &&
issue.Level == logLevel &&
issue.Message.Contains(untrustedRoot));
issue.Message.Split(new[] { ' ', ':' }).Where(WORDEXTFLAGS => WORDEXTFLAGS == untrustedRoot).Any()); ;

Assert.True(isUntrustedRoot);
}

public static void AssertUntrustedRoot(IEnumerable<ILogMessage> issues, LogLevel logLevel)
Expand All @@ -761,25 +729,14 @@ public static void AssertUntrustedRoot(IEnumerable<ILogMessage> issues, LogLevel

public static void AssertNotTimeValid(IEnumerable<ILogMessage> issues, LogLevel logLevel)
{
string notTimeValid;
string notTimeValid = X509ChainStatusFlags.NotTimeValid.ToString();

if (RuntimeEnvironmentHelper.IsWindows)
{
notTimeValid = "A required certificate is not within its validity period when verifying against the current system clock or the timestamp in the signed file";
}
else if (RuntimeEnvironmentHelper.IsMacOSX)
{
notTimeValid = "An expired certificate was detected.";
}
else
{
notTimeValid = "certificate has expired";
}

Assert.Contains(issues, issue =>
bool isNotTimeValid = issues.Any(issue =>
issue.Code == NuGetLogCode.NU3018 &&
issue.Level == logLevel &&
issue.Message.Contains(notTimeValid));
issue.Message.Split(new[] { ' ', ':' }).Where(WORDEXTFLAGS => WORDEXTFLAGS == notTimeValid).Any());

Assert.True(isNotTimeValid);
}

public static string AddSignatureLogPrefix(string log, PackageIdentity package, string source)
Expand Down

0 comments on commit 0553223

Please sign in to comment.