Fix SignedCms certificate collection modification with attribute certificates #79940
Conversation
…ificates. When adding or removing certificates from the certificateSet collection, we assumed that the collection would only contain X.509 certificates. This changes the implementation so that when looking for duplicates, we skip over choices that are not an X.509 certificate when looking for a duplicate. The tests peek in to the SignedData ASN.1 to ensure that the attribute certificates are preserved during a round trip when encoding and decoding a CMS.
|
Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones Issue DetailsWhen adding or removing certificates from the certificateSet collection, we assumed that the collection would only contain X.509 certificates. This changes the implementation so that when looking for duplicates, we skip over choices that are not an X.509 certificate when looking for a duplicate. The tests peek in to the SignedData ASN.1 to ensure that the attribute certificates are preserved during a round trip when encoding and decoding a CMS. Fixes #79935
|
|
I am guessing we will want this back ported for 7.0.x. Once this is green / merged, I'll open to appropriate back port stuff. |
|
Can you backport to 6.0.x too? The original fix was backported to .NET 6. #67462 |
Yeah, since 6.0 is still in support we should patch that one, too. (And since this is a NuGet package backports require edits to package authoring in addition to what the backport bot would do) |
Yeah. I'll take care of it. |
|
/backport to release/7.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3840679975 |
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/3840757479 |
|
@vcsjones backporting to release/6.0 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix SignedCms certificate collection modification with attribute certificates.
Using index info to reconstruct a base tree...
M src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs
M src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
M src/libraries/System.Security.Cryptography.Pkcs/tests/System.Security.Cryptography.Pkcs.Tests.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/tests/System.Security.Cryptography.Pkcs.Tests.csproj
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/src/System/Security/Cryptography/Pkcs/SignedCms.cs
Applying: Tests are not applicable for .NET Framework
Using index info to reconstruct a base tree...
M src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
M src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs
M src/libraries/System.Security.Cryptography.Pkcs/tests/System.Security.Cryptography.Pkcs.Tests.csproj
Falling back to patching base and 3-way merge...
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/tests/System.Security.Cryptography.Pkcs.Tests.csproj
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs
CONFLICT (content): Merge conflict in src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.netcoreapp.cs
Auto-merging src/libraries/System.Security.Cryptography.Pkcs/tests/SignedCms/SignedCmsTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Tests are not applicable for .NET Framework
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128Please backport manually! |
|
@vcsjones an error occurred while backporting to release/6.0, please check the run log for details! Error: git am failed, most likely due to a merge conflict. |
ghost
locked as resolved and limited conversation to collaborators
When adding or removing certificates from the certificateSet collection, we assumed that the collection would only contain X.509 certificates. This changes the implementation so that when looking for duplicates, we skip over choices that are not an X.509 certificate.
The tests peek in to the SignedData ASN.1 to ensure that the attribute certificates are preserved during a round trip when encoding and decoding a CMS.
Fixes #79935
/cc @dtivel @clairernovotny