Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot export RSAParameters from X509Certificate2 in .NET 9 RC2 #109059

Closed
ppekrol opened this issue Oct 20, 2024 · 8 comments · Fixed by #109119
Closed

Cannot export RSAParameters from X509Certificate2 in .NET 9 RC2 #109059

ppekrol opened this issue Oct 20, 2024 · 8 comments · Fixed by #109119
Assignees
Labels
area-System.Security in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Milestone

Comments

@ppekrol
Copy link

ppekrol commented Oct 20, 2024

Description

I've noticed a discrepancy in behavior between .NET 8 and 9 (RC2) when it comes to the ability to export the RSAParameters (with private) between .NET 8 and .NET 9.

From a quick investigation it looks like ExportPolicy is different between those two frameworks despite passing same X509KeyStorageFlags parameters to the X509Certificate2 ctor (tried X509CertificateLoader as well).

I'm running this on Windows.

Reproduction Steps

        var certBytes = File.ReadAllBytes("<path_to_pfx>");
        var cert = new X509Certificate2(certBytes, (string)null, X509KeyStorageFlags.Exportable | X509KeyStorageFlags.UserKeySet);
        var privateKey = (RSACng)cert.GetRSAPrivateKey();
        Console.WriteLine(privateKey.Key.ExportPolicy);

        var parameters = privateKey.ExportParameters(true);

I can paste the PFX if needed. Got it from the staging Let's Encrypt.

Expected behavior

On .NET 8 I'm getting following output from the console writeline

AllowExport, AllowPlaintextExport

Actual behavior

On .NET 9 RC2 I'm getting following output:

AllowExport
Unhandled exception. System.Security.Cryptography.CryptographicException: The requested operation is not supported.
   at System.Security.Cryptography.CngKey.Export(CngKeyBlobFormat format)
   at System.Security.Cryptography.RSACng.ExportParameters(Boolean includePrivateParameters)
   at Tryouts.Program.Main(String[] args) in D:\workspaces\ravendb_4\test\Tryouts\Program.cs:line 42
   at Tryouts.Program.<Main>(String[] args)

I'm suspecting that the exception is caused because of the missing 'AllowPlaintextExport' flag.

Regression?

Yes?

Known Workarounds

No response

Configuration

dotnet --info
.NET SDK:
 Version:           9.0.100-rc.2.24474.11
 Commit:            315e1305db
 Workload version:  9.0.100-manifests.4872d5d5
 MSBuild version:   17.12.0-preview-24473-03+fea15fbd1

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.22631
 OS Platform: Windows
 RID:         win-x64
 Base Path:   C:\Program Files\dotnet\sdk\9.0.100-rc.2.24474.11\

.NET workloads installed:
 [aspire]
   Installation Source: VS 17.11.35327.3, VS 17.12.35410.144
   Manifest Version:    8.2.0/8.0.100
   Manifest Path:       C:\Program Files\dotnet\sdk-manifests\8.0.100\microsoft.net.sdk.aspire\8.2.0\WorkloadManifest.json
   Install Type:              Msi

Configured to use loose manifests when installing new manifests.

Host:
  Version:      9.0.0-rc.2.24473.5
  Architecture: x64
  Commit:       990ebf52fc

.NET SDKs installed:
  8.0.206 [C:\Program Files\dotnet\sdk]
  8.0.403 [C:\Program Files\dotnet\sdk]
  9.0.100-rc.2.24474.11 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.35 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.7 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 9.0.0-rc.2.24474.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.35 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 9.0.0-rc.2.24473.5 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 6.0.35 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 7.0.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.6 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 8.0.10 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 9.0.0-rc.2.24474.4 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

Other architectures found:
  x86   [C:\Program Files (x86)\dotnet]
    registered at [HKLM\SOFTWARE\dotnet\Setup\InstalledVersions\x86\InstallLocation]

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Oct 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Oct 20, 2024
@vcsjones vcsjones added area-System.Security and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Oct 20, 2024
@vcsjones
Copy link
Member

vcsjones commented Oct 20, 2024

  • A PKCS12 / PFX that reproduces the problem would be greatly appreciated, as long as the private key is for testing purposes only.

  • What version of Windows, and what build? (Running winver will tell you the exact version)

    Nevermind, it was right in front of my face. "OS Version: 10.0.22631" which is Windows 11 23H2

@vcsjones vcsjones added the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 20, 2024
@ppekrol
Copy link
Author

ppekrol commented Oct 20, 2024

Cert.pfx.log

@vcsjones Thanks for quick response. Attached the file. Had to change the extension to .log because GH did not allow me to attach the PFX directly.

@dotnet-policy-service dotnet-policy-service bot removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Oct 20, 2024
@vcsjones
Copy link
Member

I can reproduce this.

Image

@vcsjones vcsjones added regression-from-last-release and removed untriaged New issue has not been triaged by the area owner labels Oct 21, 2024
@vcsjones
Copy link
Member

In general I am surprised this worked with .NET 8 - it is known that exporting plaintext from Windows does not work reliably. See #26031 and #77590 for some previous discussion on this.

You can work around this in .NET 9 like so, using the work arounds discussed in the linked issues:

using System;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.IO;
using System.Runtime.InteropServices;

Console.WriteLine(RuntimeInformation.FrameworkDescription);

var certBytes = File.ReadAllBytes("customer.pfx");
var cert = new X509Certificate2(certBytes, (string)null, X509KeyStorageFlags.Exportable | X509KeyStorageFlags.UserKeySet);

RSAParameters p;

using (RSA tmp = RSA.Create())
using (RSA key = cert.GetRSAPrivateKey())
{
    const string pwd = "TempPassword";
    PbeParameters pbeParameters = new PbeParameters(PbeEncryptionAlgorithm.Aes128Cbc, HashAlgorithmName.SHA256, 1);
    tmp.ImportEncryptedPkcs8PrivateKey(pwd, key.ExportEncryptedPkcs8PrivateKey(pwd, pbeParameters), out _);
    p = tmp.ExportParameters(true);
}

// p now contains the RSA parameters.

However, we should try look at why this worked in .NET 8.

@vcsjones
Copy link
Member

git bisect:

5d3bac120166670a91048637da0564eb1bf4c367 is the first bad commit
commit 5d3bac120166670a91048637da0564eb1bf4c367
Author: Jeremy Barton <jbarton@microsoft.com>
Date:   Tue Aug 27 14:37:47 2024 -0700

    New cert loader should load into CNG by default

    When no provider attribute is present on a key, Windows loads the key into the
    CAPI Base provider unless PKCS12_PREFER_CNG_KSP is set.  So, set that flag.

    On .NET Framework (or .NET Standard running on .NET Framework) we don't
    have the power to set that flag (without completely redefining how the PFX load
    loads), so inject a synthetic attribute to force keys into the CNG KSP when
    PreserveStorageProvider isn't set.

    Technically these two approaches differ when the incoming PFX has no name
    and PreserveStorageProvider is set (CoreFX: CNG, NetFX: CAPI Base), but that's
    unlikely, and consistent with .NET Framework imports.

 .../X509CertificateLoader.Pkcs12.cs                | 30 ++++++++++
 .../Cryptography/X509Certificates/TestData.cs      | 53 +++++++++++++++++
 .../X509CertificateLoaderPkcs12CollectionTests.cs  | 25 ++++++++
 .../X509CertificateLoaderPkcs12Tests.cs            | 67 +++++++++++++++++++++-
 .../tests/Microsoft.Bcl.Cryptography.Tests.csproj  |  8 +--
 .../X509Certificates/StorePal.Windows.Import.cs    | 20 -------
 .../X509CertificateLoader.Windows.cs               |  6 +-
 7 files changed, 183 insertions(+), 26 deletions(-)

So this was introduced by #107005

@bartonjs
Copy link
Member

bartonjs commented Oct 21, 2024

The commit message pointed out the problem here:

Technically these two approaches differ when the incoming PFX has no name
and PreserveStorageProvider is set (CoreFX: CNG, NetFX: CAPI Base), but that's
unlikely, and consistent with .NET Framework imports.

(Where I think I meant "no CSP attribute" instead of "no name").

This PFX does not specify a CSP for the private key, which is why the ctor and X509CertificateLoader both load it into CNG now. On .NET 8 and older (including .NET Framework) it was being loaded into the CAPI "Base Cryptographic Provider", which would mean that it gets really bad usability as cert.PrivateKey on .NET Framework, and even on .NET 8 would cause problems when using it as a TLS Server Cert when trying to do TLS 1.3.

But, really, this is just yet another instance of ".NET had ExportParameters long before there was a crypto subsystem that made a distinction between export-encrypted and export-plaintext... and we should consider hiding the difference in our layer instead of making someone write the workaround themselves."

The workaround being

// I didn't verify this compiles, it's the gist of the answer...
private static readonly PbeParameters s_pbeParams = new PbeParameters(PbeAlgorithm.TripleDesPkcs12, HashAlgorithmName.SHA1, 1);

...

using (RSA priv = cert.GetRSAPrivateKey())
using (RSA temp = RSA.Create())
{
    temp.ImportEncryptedPkcs8PrivateKey(priv.ExportEncryptedPkcs8PrivateKey("", s_pbeParams), "", out _);
    return temp.ExportParameters(true);
}

Since the workaround exists, and is trivial, and we already know the caller intent, and we're going to hit it a lot more often as people move to X509CertificateLoader... we should just tackle it in the platform.

@vcsjones vcsjones self-assigned this Oct 21, 2024
@vcsjones vcsjones added this to the 9.0.0 milestone Oct 21, 2024
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Oct 22, 2024
@vcsjones
Copy link
Member

Re-opening for backport.

@vcsjones
Copy link
Member

This will be fixed in .NET 9 GA by #109134.

Thank you very much for the report!

@github-actions github-actions bot locked and limited conversation to collaborators Nov 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security in-pr There is an active PR which will close this issue when it is merged regression-from-last-release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants