Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

[release/3.1] Data loss - Don't delete private keys detected from SerializedCert imports #42881

Merged
merged 1 commit into from
Mar 25, 2020

Conversation

bartonjs
Copy link
Member

@bartonjs bartonjs added area-System.Security Servicing-consider Issue for next servicing release review labels Mar 18, 2020
@bartonjs bartonjs added this to the 3.1.x milestone Mar 18, 2020
@bartonjs bartonjs requested a review from stephentoub March 18, 2020 16:19
@bartonjs
Copy link
Member Author

bartonjs commented Mar 18, 2020

Description

Importing a certificate from a SerializedCert import can import the identifier for a private key, but doesn't actually import the key itself. This violates an assumption in the single-cert import path, and results in a massive surprise when the key is erased on Dispose/Finalize.

Customer Impact

If a customer has a certificate with a private key, exports it as SerializedCert, and then re-imports it later on the same system (or a system with a compatible persisted import) then the X509Certificate2 object will erase the persisted private key from disk when the object is freed. We believe usage of SerializedCert is very low, but the impact is potentially very high.

Regression

Regression from .NET Framework

Testing

Added a new test to cover the scenario for both single-cert import (affected) and collection import (not affected).

Risk

Low. There are a large number of unit tests for other, existing, behavior; and this change makes the single-cert import look more like it did in .NET Framework (and does in collection import) with regard to registering for key cleanup.

@danmoseley
Copy link
Member

original pr dotnet/runtime#33603
issue dotnet/runtime#33543

@danmoseley danmoseley changed the title [release/3.1] Don't delete private keys detected from SerializedCert imports [release/3.1] Data loss - Don't delete private keys detected from SerializedCert imports Mar 19, 2020
@leecow leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Mar 19, 2020
@leecow leecow modified the milestones: 3.1.x, 3.1.4 Mar 19, 2020
@wtgodbe
Copy link
Member

wtgodbe commented Mar 25, 2020

1 test failure in System.Security.Cryptography.X509Certificates.Tests - is it the same one being fixed in #42875?

@Anipik Anipik merged commit 041f40a into dotnet:release/3.1 Mar 25, 2020
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.

5 participants