dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.12k stars 4.7k forks source link

[API Proposal]: X509Certificate.ExportPkcs12 with PBE parameters #80314

Open vcsjones opened 1 year ago

vcsjones commented 1 year ago

Background and motivation

Right now Export(X509ContentType.Pkcs12, ...) assumes 3DES, SHA1, and 2000 rounds of PBKDF. These are not great in 2023 and there is no control over it today. Developers could use Pkcs12Builder themselves, but, that's a fairly low-level mechanism that requires understanding how PKCS12 works.

I propose we expose new APIs that allow stronger PKCS12 exports.

In the proposal, there are two overloads. An enum that provides simple options that match Windows behavior. The second is PbeParameters where people can have total control over the encryption, hash, and number ob PBES2 rounds.

Win32 by default does not give us the flexibility that is needed to implement PbeParameters, as it does not let us choose the number of PBES2 rounds. In that case, the export will be performed using an existing Win32 API, decoded, and re-encoded with the managed implementation using the specified parameters.

API Proposal

namespace System.Security.Cryptography.X509Certificates;

public partial class X509Certificate  {
    public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password);
    public byte[] ExportPkcs12(PbeParameters exportParameters, string? password);
}

public partial class X509Certificate2Collection  {
    public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password);
    public byte[] ExportPkcs12(PbeParameters exportParameters, string? password);
}

public enum Pkcs12ExportPbeParameters {
    // Initially will behave as `Pbes2TripleDesSha1`, but reserved to change behavior in the future.
    Default = 0,

    // TripleDes3KeyPkcs12, SHA1, 2000 PBKDF2 rounds. Matches Win32.
    Pbes2TripleDesSha1 = 1,

    // AES-256, SHA256, 2000 PBKDF2 rounds. Matches Win32.
    Pbes2Aes256Sha256 = 2,
}

API Usage

X509Certificate certificate = GetCert();
byte[] pkcs12 = certificate.ExportPkcs12(Pkcs12ExportPbeParameters.Pbes2Aes256Sha256, "potato");
ghost commented 1 year ago

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation Right now `Export(X509ContentType.Pkcs12, ...)` assumes 3DES, SHA1, and 2000 rounds of PBKDF. These are not great in 2023 and there is no control over it today. Developers _could_ use `Pkcs12Builder` themselves, but, that's a fairly low-level mechanism that requires understanding how PKCS12 works. Windows 10 1709 supports `PKCS12_EXPORT_PBES2_PARAMS` which doesn't allow total control over the PBE parameters, but it lets opting in to stronger AES+SHA256 PBE. I propose we expose this in a new method called `ExportPkcs12`. Since this is only present on recent versions of Windows, this would throw PNSE on down level platforms. Non-Windows platforms implement this in managed code, so we can swap out [`s_windowsPbe`](https://github.com/dotnet/runtime/blob/7fc84e4c47414ec99304f8a98a37af98c92c518b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/UnixExportProvider.cs#L20) as needed depending on the input. ### API Proposal ```csharp namespace System.Security.Cryptography.X509Certificates; public partial class X509Certificate { public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password); } public enum Pkcs12ExportPbeParameters { Default = 0, Pbes2Aes256Sha256 = 1, } ``` ### API Usage ```csharp X509Certificate certificate = GetCert(); byte[] pkcs12 = certificate.ExportPkcs12(Pkcs12ExportPbeParameters.Pbes2Aes256Sha256, "potato"); ``` ### Alternative Designs I originally had a more complicate design that attempted to be a "classenum" so that we have more flexibility in the future. A future version of this class might have a public constructor that accepted `PbeParameters`, for example. Or some kind of configuration that can't be expressed in an `enum`. After thinking about it though, I deemed this too complicated. If, in the future, we need this kind of capability, we can overload `ExportPkcs12`. I'll leave it here for posterity, maybe someone can convince me it's actually useful. ```C# namespace System.Security.Cryptography.X509Certificates; public partial class X509Certificate { public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password); } public sealed class Pkcs12ExportPbeParameters : IEquatable { public static Pkcs12ExportPbeParameters Default { get; } public static Pkcs12ExportPbeParameters Pbes2Aes256Sha256 { get; } public static bool operator ==(Pkcs12ExportPbeParameters? left, Pkcs12ExportPbeParameters? right); public static bool operator !=(Pkcs12ExportPbeParameters? left, Pkcs12ExportPbeParameters? right); // Override Equals, GetHashCode, etc. } ``` ### Risks It's a Window 10 1709+ API. This will throw PNSE on down level Windows if `Pbes2Aes256Sha256` is supplied.
Author: vcsjones
Assignees: -
Labels: `api-suggestion`, `area-System.Security`
Milestone: -
bartonjs commented 1 year ago

Amusingly, I was just answering an SO question yesterday where someone had an AES-256/SHA-256 PFX that they couldn't load (because it was being loaded by Mono, which doesn't support the newer revision with PBES2).

The version with the enum is probably a good balance of allowing more modern algorithm choices without impeding a common operating too much... but I wonder if we should just go ahead and overload it with PbeParameters at the same time to allow the much richer control.

We should add TripleDes192Sha1 to the enum now, though, so that we can have "Default means we pick, but you have the option of specifying 'legacy' if we ever change the defaults".

It's a Window 10 1709+ API. This will throw PNSE on down level Windows if Pbes2Aes256Sha256 is supplied.

We could probably avoid the PNSE and just have both the OS-exporter and our custom-exporter available on Windows.

vcsjones commented 1 year ago

@bartonjs

We could probably avoid the PNSE and just have both the OS-exporter and our custom-exporter available on Windows.

I thought about that but.. I got a little concerned about all of the Win32-isms and Bag Attributes that get applied. It might be surprising to folks that using AES means "you don't get a keyAttributes on your PKCS12 bag anymore". But maybe that is acceptable for Windows 8 / Server 2012.

We should add TripleDes192Sha1 to the enum now, though, so that we can have "Default means we pick, but you have the option of specifying 'legacy' if we ever change the defaults".

That makes sense if we are okay with using a managed export on Windows. Win32 doesn't let us explicitly pick TripleDes192Sha1. It lets us pick "Whatever the OS decides" and "Aes256Sha256". Is there a world in which Windows changes their own default?

but I wonder if we should just go ahead and overload it with PbeParameters at the same time to allow the much richer control.

My concern is largely the same as the first one, where if on Windows you use the managed one, you might not get all of the bag attributes that Windows likes to shove in there.

If we're okay losing bag attributes on Windows... then... I guess we can do PbeParameters and 3DES on the enumeration.

bartonjs commented 1 year ago

Worst case: we could always let Windows export it, then we decompose and recompose it with the better algorithms...

vcsjones commented 1 year ago

we could always let Windows export it, then we decompose and recompose it with the better algorithms...

I am annoyed that did not occur to me. This sounds reasonable to me, and should make it (relatively) straight forward to make this work with any set of PbeParameters. In that sense then I'm not sure there is much value in using the new Win32 API at all. We can just use the old one, crack it open, and re-encode it.

I'll adjust the proposal.

It also occurs to me that we probably want symmetry with X509Certificate2Collection

bartonjs commented 1 year ago

Video

Looks good as proposed.

We discussed replacing the enum with prepopulated accelerators on PbeParameters, but the enum values don't specifically encode the iteration count (where we're following OS defaults).

namespace System.Security.Cryptography.X509Certificates;

public partial class X509Certificate  {
    public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password);
    public byte[] ExportPkcs12(PbeParameters exportParameters, string? password);
}

public partial class X509Certificate2Collection  {
    public byte[] ExportPkcs12(Pkcs12ExportPbeParameters exportParameters, string? password);
    public byte[] ExportPkcs12(PbeParameters exportParameters, string? password);
}

public enum Pkcs12ExportPbeParameters {
    // Initially will behave as `Pbes2TripleDesSha1`, but reserved to change behavior in the future.
    Default = 0,

    // TripleDes3KeyPkcs12, SHA1, 2000 PBKDF2 rounds. Matches Win32.
    Pbes2TripleDesSha1 = 1,

    // AES-256, SHA256, 2000 PBKDF2 rounds. Matches Win32.
    Pbes2Aes256Sha256 = 2,
}