Signing: update to August 2022 CTL #4791

dtivel · 2022-09-12T22:00:36Z

Bug

Fixes: NuGet/Home#12033

Regression? Last working version: No

Description

This PR updates NuGet signed package verification on Linux and macOS to use 2 separate fallback certificate bundles instead of 1.

Prior to this change, NuGet would use a single fallback certificate bundle which contained root certificates valid for both code signing and timestamping. Roots valid for only code signing or only timestamping were not in the certificate bundle because a consumer had no way of knowing which certificates were valid for which purpose(s).

With dotnet/sdk#27824, the .NET SDK will contain 2 separate fallback certificate bundles based on the Windows CTL:

all root certificates valid for code signing
all root certificates valid for timestamping

This will provide NuGet with more complete data on publicly trusted certificates with which to make trust decisions.

Note that the above PR needs to be merged first, and the NuGet CI needs the updated .NET SDK build for all NuGet tests to pass. A few new tests will fail until this is resolved.

Note that this PR does not change the enablement default. Signed package verification is still opt-in on both Linux and macOS.

PR Checklist

CC @richlander, @aortiz-msft

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

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

nkolev92

LGTM. Some suggestions about the public APIs.

The line # looks scary, but once you hide tests, it's pretty easy to follow.

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

ghost · 2022-09-27T01:01:24Z

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

ghost · 2022-10-04T01:01:25Z

This PR has been automatically marked as stale because it has no activity for 7 days. It will be closed if no further activity occurs within another 7 days of this comment. If it is closed, you may reopen it anytime when you're ready again, as long as you don't delete the branch.

dtivel · 2022-10-11T18:32:16Z

build/common.ps1

-        & powershell $DotNetInstall -Channel $cli.Channel -Quality signed -InstallDir $cli.Root -Version $cli.Version -Architecture $arch -NoPath
+        Trace-Log "$DotNetInstall -Channel $($cli.Channel) -InstallDir $($cli.Root) -Version $($cli.Version) -Architecture $arch -NoPath"
+
+        & powershell $DotNetInstall -Channel $cli.Channel -InstallDir $cli.Root -Version $cli.Version -Architecture $arch -NoPath


I removed the Quality argument here and in scripts/funcTests/runFuncTests.sh. Documentation says:

Don't use both version and quality parameters. When quality is specified, the script determines the proper version on its own.

[Quality is] Not applicable for current and LTS channels and will be ignored if one of those channels is used.

For some time, we have only effectively used Channel and Version.

dtivel · 2022-10-11T18:38:15Z

test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs

@@ -332,6 +332,7 @@ public void PackCommand_PackToolUsingAlias_DoesNotWarnAboutNoExactMatchInDepende
                    tfmProps["TargetFrameworkIdentifier"] = ".NETCoreApp";
                    tfmProps["TargetFrameworkVersion"] = "v3.1";
                    tfmProps["TargetFrameworkMoniker"] = ".NETCoreApp,Version=v3.1";
+                    tfmProps["RuntimeFrameworkVersion"] = "6.0";


NuGet/Home#12150

dtivel · 2022-10-11T18:41:42Z

test/NuGet.Core.FuncTests/Dotnet.Integration.Test/PackCommandTests.cs

@@ -1485,7 +1488,7 @@ public void PackCommand_PackProject_PacksFromNuspec_InstallPackageToOutputPath()
            }
        }

-        [PlatformTheory(Platform.Windows)]
+        [PlatformTheory(Platform.Windows, Skip = "https://github.com/dotnet/sdk/issues/28131")]


NuGet/Home#12151

dtivel · 2022-10-11T18:46:53Z

@nkolev92, @zivkan, @jeffkl, these are recent test-related changes necessary to target .NET 7 GA preview SDK builds.

Resolve NuGet/Home#12033.

Resolve NuGet/Home#12033. Co-authored-by: Damon Tivel <dtivel@microsoft.com>

Insert 6.4.0-rc.123 into rel/d17.4 on 11/07/2022 23:47:12 * tag '6.4.0.123': (60 commits) fix a logic error that caused AbandonedMutexException while executing migrations (release-6.4.x) (NuGet#4895) unblock source build failing due to fatal: transport 'file' not allowed error (NuGet#4867) (NuGet#4874) Signing: update to August 2022 CTL (NuGet#4791) (NuGet#4850) Merged PR 422933: Prefer BCL Directory create API over helper class (7.0.1xx-rc2) Fix empty combobox when package is not present in project file (NuGet#4844) (NuGet#4848) Fix component detection alert for microsoft.owin package (NuGet#4841) (NuGet#4845) Make release label RC, move to escrow mode Adds special case to include transitive origins in GetInstalledAndTransitivePackagesAsync API (NuGet#4824) Add longPathAware manifest to NuGet.Build.Tasks.Console (NuGet#4830) VsPackageInstallerServices should not post ProjectNotNominatedException faults (NuGet#4814) Skip test GetOrCreateAsync_WithUnhandledExceptionInPlugin_Throws (NuGet#4831) Improve OptProf pipeline job run names (NuGet#4825) Increase HttpClientHandler.MaxConnectionsPerServer to 64 to improve PM UI performance in Visual Studio (NuGet#4798) Suppress CA2213 warnings to unblock dev branch (NuGet#4823) Ensure IsVsOfflineFeed is calculated correctly on 64-bit machines (NuGet#4817) Add better handling of AggregateExceptions in static graph-based restore (NuGet#4809) Add Component Detection task into each pipeline (NuGet#4813) Localizes nuget.exe with default, embedded resource assembly lookup (NuGet#4773) Removes BrowseObjectBase class in NuGet Solution Explorer (NuGet#4807) Improve TryCreateContext (NuGet#4762) ...

dtivel requested a review from a team as a code owner September 12, 2022 22:00

dtivel force-pushed the dev-dtivel-truststore branch from 7970845 to 75cda75 Compare September 13, 2022 12:34

jeffkl reviewed Sep 13, 2022

View reviewed changes

src/NuGet.Core/NuGet.Packaging/Signing/Utility/X509ChainHolder.cs Outdated Show resolved Hide resolved

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/FallbackCertificateBundleX509ChainFactory.cs Show resolved Hide resolved

dtivel force-pushed the dev-dtivel-truststore branch from 75cda75 to e1d95e6 Compare September 13, 2022 18:40

nkolev92 reviewed Sep 13, 2022

View reviewed changes

src/NuGet.Core/NuGet.Packaging/Signing/TrustStore/X509StorePurpose.cs Outdated Show resolved Hide resolved

nkolev92 previously approved these changes Sep 15, 2022

View reviewed changes

jeffkl previously approved these changes Sep 15, 2022

View reviewed changes

dtivel dismissed stale reviews from jeffkl and nkolev92 via c99258c September 17, 2022 00:30

dtivel requested review from zivkan, nkolev92 and jeffkl September 18, 2022 14:31

nkolev92 previously approved these changes Sep 19, 2022

View reviewed changes

ghost added the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Sep 27, 2022

dtivel added 8 commits October 6, 2022 10:03

Update to August 2022 CTL

e6dbec1

Apply feedback.

49ccbcc

Fix test bugs

0427c50

Update to .NET 7 SDK for testing

522ccb2

Fix test

0efe903

Skip tests on macOS

0e9c6af

Implement test hack for preview SDK build

02834c1

Skip test while it fails on .NET 7

3965112

dtivel dismissed nkolev92’s stale review via 3965112 October 11, 2022 00:15

dtivel force-pushed the dev-dtivel-truststore branch from c99258c to 3965112 Compare October 11, 2022 00:15

ghost removed the Status:No recent activity PRs that have not had any recent activity and will be closed if the label is not removed label Oct 11, 2022

Update .NET 7 SDK preview version

5979fdc

dtivel commented Oct 11, 2022

View reviewed changes

dtivel requested a review from nkolev92 October 11, 2022 18:42

jeffkl approved these changes Oct 11, 2022

View reviewed changes

nkolev92 approved these changes Oct 11, 2022

View reviewed changes

zivkan approved these changes Oct 11, 2022

View reviewed changes

dtivel merged commit 493d6b3 into dev Oct 12, 2022

dtivel deleted the dev-dtivel-truststore branch October 12, 2022 12:18

kartheekp-ms pushed a commit that referenced this pull request Oct 12, 2022

Signing: update to August 2022 CTL (#4791)

04bb170

Resolve NuGet/Home#12033.

nkolev92 pushed a commit that referenced this pull request Oct 12, 2022

Signing: update to August 2022 CTL (#4791) (#4850)

125f673

Resolve NuGet/Home#12033. Co-authored-by: Damon Tivel <dtivel@microsoft.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Signing: update to August 2022 CTL #4791

Signing: update to August 2022 CTL #4791

dtivel commented Sep 12, 2022

nkolev92 left a comment

ghost commented Sep 27, 2022

ghost commented Oct 4, 2022

dtivel Oct 11, 2022

dtivel Oct 11, 2022

dtivel Oct 11, 2022

dtivel commented Oct 11, 2022

Signing: update to August 2022 CTL #4791

Signing: update to August 2022 CTL #4791

Conversation

dtivel commented Sep 12, 2022

Bug

Description

PR Checklist

nkolev92 left a comment

Choose a reason for hiding this comment

ghost commented Sep 27, 2022

ghost commented Oct 4, 2022

dtivel Oct 11, 2022

Choose a reason for hiding this comment

dtivel Oct 11, 2022

Choose a reason for hiding this comment

dtivel Oct 11, 2022

Choose a reason for hiding this comment

dtivel commented Oct 11, 2022