Skip to content

Commit

Permalink
Adding package id and source prefix to signing logs (#2360)
Browse files Browse the repository at this point in the history 
Fixes: NuGet/Home#6944
Regression: No

Details: This PR fixes signing logs by adding a prefix indicating the package id, version and source for the error or warning.

Tests Added: Yes
Validation done:  Manual validation
  • Loading branch information
@PatoBeltran
Ankit Mishra authored and PatoBeltran committed Oct 18, 2018
1 parent d9b78d3 commit 4ff26ec
Show file tree
Hide file tree
Showing 6 changed files with 60 additions and 23 deletions.
18 changes: 18 additions & 0 deletions src/NuGet.Core/NuGet.Packaging/PackageExtractor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using NuGet.Common;
using NuGet.Packaging.Core;
using NuGet.Packaging.Signing;
using NuGet.Shared;

namespace NuGet.Packaging
{
Expand Down Expand Up @@ -1091,6 +1092,11 @@ private static async Task VerifyPackageSignatureAsync(
{
await LogPackageSignatureVerificationAsync(source, package, packageExtractionContext.Logger, verifyResult);

// Update errors and warnings with package id and source
verifyResult.Results
.SelectMany(r => r.Issues)
.ForEach(e => AddPackageIdentityToSignatureLog(source, package, e));

if (verifyResult.Valid)
{
// log any warnings
Expand All @@ -1109,6 +1115,18 @@ private static async Task VerifyPackageSignatureAsync(
}
}

/// <summary>
/// Adds a package ID and package source as a prefix to log messages and adds package ID to the message.LibraryId.
/// </summary>
/// <param name="source">package source.</param>
/// <param name="package">package identity.</param>
/// <param name="message">ILogMessage to be modified.</param>
private static void AddPackageIdentityToSignatureLog(string source, PackageIdentity package, SignatureLog message)
{
message.LibraryId = package.Id;
message.Message = string.Format(CultureInfo.CurrentCulture, Strings.ExtractionLog_InformationPrefix, package.Id, package.Version, source, message.Message);
}

private static async Task LogPackageSignatureVerificationAsync(
string source,
PackageIdentity package,
Expand Down
9 changes: 9 additions & 0 deletions src/NuGet.Core/NuGet.Packaging/Strings.Designer.cs
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

31 changes: 17 additions & 14 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 @@ -23,10 +23,14 @@ namespace NuGet.CommandLine.FuncTest.Commands
[Collection(SignCommandTestCollection.Name)]
public class InstallCommandTests
{
private const string _NU3008 = "NU3008: The package integrity check failed.";
private const string _NU3012 = "NU3012: The author primary signature found a chain building issue: The certificate is revoked.";
private const string _NU3018 = "NU3018: 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 const string _NU3027 = "NU3027: The signature should be timestamped to enable long-term signature validity after the certificate has expired.";
private static readonly string _NU3008Message = "The package integrity check failed.";
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 _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 _NU3018 = "NU3018: {0}";

private SignCommandTestFixture _testFixture;
private TrustedTestCert<TestCertificate> _trustedTestCert;
Expand Down Expand Up @@ -68,11 +72,10 @@ public async Task Install_AuthorSignedPackage_SucceedsAsync()

// Assert
result.ExitCode.Should().Be(0);
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, nupkg.Identity, context.WorkingDirectory))}");
}
}


[CIOnlyFact]
public async Task Install_RepoSignedPackage_SucceedsAsync()
{
Expand Down Expand Up @@ -102,7 +105,7 @@ public async Task Install_RepoSignedPackage_SucceedsAsync()

// Assert
result.ExitCode.Should().Be(0);
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, nupkg.Identity, context.WorkingDirectory))}");
}
}

Expand Down Expand Up @@ -135,8 +138,8 @@ public async Task Install_UntrustedCertSignedPackage_WarnsAsync()

// Assert
result.ExitCode.Should().Be(0);
result.AllOutput.Should().Contain($"WARNING: {_NU3018}");
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3018, SigningTestUtility.AddSignatureLogPrefix(_NU3018Message, nupkg.Identity, context.WorkingDirectory))}");
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, nupkg.Identity, context.WorkingDirectory))}");
}
}

Expand Down Expand Up @@ -170,8 +173,8 @@ public async Task Install_TamperedPackage_FailsAsync()

// Assert
result.ExitCode.Should().Be(1);
result.Errors.Should().Contain(_NU3008);
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.Errors.Should().Contain(string.Format(_NU3008, SigningTestUtility.AddSignatureLogPrefix(_NU3008Message, nupkg.Identity, context.WorkingDirectory)));
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, nupkg.Identity, context.WorkingDirectory))}");
}
}

Expand Down Expand Up @@ -215,9 +218,9 @@ public async Task Install_TamperedAndRevokedCertificateSignaturePackage_FailsAsy

// Assert
result.ExitCode.Should().Be(1);
result.Errors.Should().Contain(_NU3008);
result.Errors.Should().Contain(_NU3012);
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.Errors.Should().Contain(string.Format(_NU3008, SigningTestUtility.AddSignatureLogPrefix(_NU3008Message, nupkg.Identity, context.WorkingDirectory)));
result.Errors.Should().Contain(string.Format(_NU3012, SigningTestUtility.AddSignatureLogPrefix(_NU3012Message, nupkg.Identity, context.WorkingDirectory)));
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, nupkg.Identity, context.WorkingDirectory))}");
}
}

Expand Down
16 changes: 8 additions & 8 deletions ....Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandSignPackagesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ namespace NuGet.CommandLine.FuncTest.Commands
public class RestoreCommandSignPackagesTests
{
private static readonly string _NU3008Message = "The package integrity check failed.";
private static readonly string _NU3008 = $"NU3008: {_NU3008Message}";
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: {_NU3027Message}";
private static readonly string _NU3027 = "NU3027: {0}";

private SignCommandTestFixture _testFixture;
private TrustedTestCert<TestCertificate> _trustedTestCert;
Expand Down Expand Up @@ -89,8 +89,8 @@ public async Task Restore_TamperedPackageInPackagesConfig_FailsWithErrorAsync()

// Assert
result.ExitCode.Should().Be(1);
result.Errors.Should().Contain(_NU3008);
result.AllOutput.Should().Contain(_NU3027);
result.Errors.Should().Contain(string.Format(_NU3008, SigningTestUtility.AddSignatureLogPrefix(_NU3008Message, packageX.Identity, pathContext.PackageSource)));
result.AllOutput.Should().Contain(string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, packageX.Identity, pathContext.PackageSource)));
}
}

Expand Down Expand Up @@ -135,17 +135,17 @@ public async Task Restore_TamperedPackage_FailsAsync()

// Assert
result.ExitCode.Should().Be(1);
result.Errors.Should().Contain(_NU3008);
result.AllOutput.Should().Contain($"WARNING: {_NU3027}");
result.Errors.Should().Contain(string.Format(_NU3008, SigningTestUtility.AddSignatureLogPrefix(_NU3008Message, packageX.Identity, pathContext.PackageSource)));
result.AllOutput.Should().Contain($"WARNING: {string.Format(_NU3027, SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, packageX.Identity, pathContext.PackageSource))}");

errors.Count().Should().Be(1);
errors.First().Code.Should().Be(NuGetLogCode.NU3008);
errors.First().Message.Should().Be(_NU3008Message);
errors.First().Message.Should().Be(SigningTestUtility.AddSignatureLogPrefix(_NU3008Message, packageX.Identity, pathContext.PackageSource));
errors.First().LibraryId.Should().Be(packageX.ToString());

warnings.Count().Should().Be(1);
warnings.First().Code.Should().Be(NuGetLogCode.NU3027);
warnings.First().Message.Should().Be(_NU3027Message);
warnings.First().Message.Should().Be(SigningTestUtility.AddSignatureLogPrefix(_NU3027Message, packageX.Identity, pathContext.PackageSource));
warnings.First().LibraryId.Should().Be("X.9.0.0");
}
}
Expand Down
1 change: 0 additions & 1 deletion test/NuGet.Clients.FuncTests/NuGet.CommandLine.FuncTest/Commands/RestoreCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading.Tasks;
Expand Down
8 changes: 8 additions & 0 deletions test/TestUtilities/Test.Utility/Signing/SigningTestUtility.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading;
using System.Threading.Tasks;
using NuGet.Common;
using NuGet.Packaging.Core;
using NuGet.Packaging.Signing;
using NuGet.Shared;
using NuGet.Test.Utility;
Expand All @@ -31,6 +32,8 @@ namespace Test.Utility.Signing
{
public static class SigningTestUtility
{
private static readonly string _signatureLogPrefix = "Package '{0} {1}' from source '{2}':";

/// <summary>
/// Modification generator that can be passed to TestCertificate.Generate().
/// The generator will change the certificate EKU to ClientAuth.
Expand Down Expand Up @@ -669,5 +672,10 @@ public static void AssertUntrustedRoot(IEnumerable<ILogMessage> issues, LogLevel
issue.Level == logLevel &&
issue.Message == untrustedRoot);
}

public static string AddSignatureLogPrefix(string log, PackageIdentity package, string source)
{
return $"{string.Format(_signatureLogPrefix, package.Id, package.Version, source)} {log}";
}
}
}

0 comments on commit 4ff26ec

Please sign in to comment.