Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[release/2.1] Servicing SignedCms for Signed NuGet Packages#33463

Merged
bartonjs merged 3 commits intodotnet:release/2.1from
bartonjs:signedcms_compat_port
Nov 15, 2018
Merged

[release/2.1] Servicing SignedCms for Signed NuGet Packages#33463
bartonjs merged 3 commits intodotnet:release/2.1from
bartonjs:signedcms_compat_port

Conversation

@bartonjs
Copy link
Member

Backports of #32666 (registration for alternate RSA signature identifiers) and #32704 (indefinite-length encoded content) to release/2.1

Description

Windows CMS (and therefore .NET Framework) is more permissive on SignedCms content than the .NET Core implementation of SignedCms.

OIDs: The .NET Foundation has created Signed NuGet packages which validate on .NET Framework, but not .NET Core, as .NET Core does not recognize the signature algorithm identifier.

Encoding: The .NET Foundation has created Signed NuGet packages which cannot be countersigned by .NET Core.

Customer Impact

Customers will be unable to work with RSA-signed packages signed by the .NET Foundation using the X.509 signature algorithm identifier instead of the CMS algorithm identifier.

Packages signed by The .NET Foundation may decode correctly, but cannot be re-encoded.

Regression?

The OID entries are a behavioral regression from .NET Framework, but not a regression within .NET Core.

The indefinite-length encoding content issue was a regression from #30432 (which released in 2.1.5).

Packaging reviewed?

Packaging considered, reviewers tagged.

Risk

Low. Existing unit tests provide assurance that the formats written by .NET Core and .NET Framework still behave as expected, new unit tests guard against regressions from variants encountered from the signing service set up by The .NET Foundation based on Bouncy Castle.


Since both of these changes require the package harvesting manifest to be updated, the package index to be updated, the version of System.Security.Cryptography.Pkcs to be bumped, and System.Security.Cryptography.Pkcs to be registered for build/publish, they have been bundled into one change. They could be separated if desired.

This enables processing documents which use the X.509 hybrid identifier (e.g. `sha256WithRSAEncryption`) instead of `rsaEncryption`.
…dCms

SignedCms reads BER, because the spec says to, and writes DER, to provide
better interop.  If the incoming message used an indefinite length encoding
for SignedData.encapContentInfo.eContent's ANY value the call to Encode
will throw (as will any mutation operations which internally use Encode).

With this change, if writing DER fails then assemble the output in pieces to leave
the encapsulated content as-is while DER-normalizing the rest of the structure.
@bartonjs bartonjs added this to the 2.1.x milestone Nov 13, 2018
@bartonjs bartonjs self-assigned this Nov 13, 2018
@bartonjs bartonjs requested review from safern and wtgodbe November 13, 2018 17:56
@danmoseley danmoseley added the Servicing-consider Issue for next servicing release review label Nov 13, 2018
@danmoseley
Copy link
Member

@wtgodbe can you please verify the packaging changes.

Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Packaging stuff looks good.

@bartonjs
Copy link
Member Author

@dotnet-bot Test Windows x64 Debug Build please (network timeout)
Test OSX x64 Debug Build please (#28360)

@bartonjs
Copy link
Member Author

I seem to have won the lottery.

2018-11-14 07:53:05,281: INFO: proc(55): run_and_log_output: Output: Starting:    System.Collections.Concurrent.Tests
2018-11-14 07:53:11,069: INFO: proc(55): run_and_log_output: Output: Assertion Failed
2018-11-14 07:53:11,069: INFO: proc(55): run_and_log_output: Output: 
2018-11-14 07:53:11,070: INFO: proc(55): run_and_log_output: Output:    at System.Collections.Concurrent.ConcurrentBag`1.WorkStealingQueue.get_DangerousCount() in /Users/dotnet-bot/j/workspace/dotnet_corefx/release_2.1/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs:line 1055
2018-11-14 07:53:11,071: INFO: proc(55): run_and_log_output: Output:    at System.Collections.Concurrent.ConcurrentBag`1.get_DangerousCount() in /Users/dotnet-bot/j/workspace/dotnet_corefx/release_2.1/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs:line 521
2018-11-14 07:53:11,071: INFO: proc(55): run_and_log_output: Output:    at System.Collections.Concurrent.ConcurrentBag`1.ToArray() in /Users/dotnet-bot/j/workspace/dotnet_corefx/release_2.1/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs:line 390
2018-11-14 07:53:11,072: INFO: proc(55): run_and_log_output: Output:    at System.Collections.Concurrent.Tests.ConcurrentBagTests.<>c__DisplayClass18_0.<Clear_ConcurrentUsage_NoExceptions>b__1() in /Users/dotnet-bot/j/workspace/dotnet_corefx/release_2.1/osx-TGroup_netcoreapp+CGroup_Debug+AGroup_x64+TestOuter_false_prtest/src/System.Collections.Concurrent/tests/ConcurrentBagTests.netcoreapp.cs:line 85
2018-11-14 07:53:11,076: INFO: proc(55): run_and_log_output: Output:    at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
2018-11-14 07:53:11,076: INFO: proc(55): run_and_log_output: Output:    at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot)
2018-11-14 07:53:11,076: INFO: proc(55): run_and_log_output: Output:    at System.Threading.ThreadPoolWorkQueue.Dispatch()

https://mc.dot.net/#/user/bartonjs/pr~2Fjenkins~2Fdotnet~2Fcorefx~2Frelease~2F2.1~2F/test~2Ffunctional~2Fcli~2F/a27a04349e8d52a75261351271da8910c63ec177/workItem/System.Collections.Concurrent.Tests/wilogs

@stephentoub is taking a look to see if he can reason about it; but it seems fairly unrelated to this change (and hadn't failed on the previous run)

@vivmishra vivmishra added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Nov 15, 2018
@vivmishra vivmishra modified the milestones: 2.1.x, 2.1.7 Nov 15, 2018
@vivmishra
Copy link

Approved for 2.1.7 and 2.2.1

@bartonjs bartonjs merged commit 23422ce into dotnet:release/2.1 Nov 15, 2018
@bartonjs bartonjs removed their assignment Nov 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Security Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants