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

Commit

Permalink
Use a custom PFX reader/writer on Unix OSes
Browse files Browse the repository at this point in the history
This change moves PFX import and export primarily into managed code to work around inconsistencies across the operating systems.

Current issues:
* Linux
  * Reading
    * PKCS12_parse doesn't support multiple cert-with-keys.
    * PKCS12_parse doesn't support reading a PFX with no MAC.
    * OpenSSL 1.0 had a weird bug where an ECDSA cert inexplicably didn't match to its key.
  * Writing
    * PKCS12_create doesn't support multiple cert-with-keys.
    * PKCS12_create doesn't support writing empty collections.
* macOS
  * Reading
    * Either SecItemImport does not understand the NULL (vs Empty) password, or we called it wrong... it/we cannot load a PFX which is MACd with the NULL password.
    * SecItemImport can only support "normalized" PFXes, where "normalized" means "how Windows XP would have written it":
      * PFX
        * SafeContents0 (no encryption) (won't load keys from an encrypted SafeContents, IIRC)
          * ShroudedKey0 (won't load keys from KeyBag (unencrypted), only ShroudedKeyBag (encrypted))
          * ...
          * ShroudedKeyN
        * SafeContents1 (encrypted) (won't load certs from an unencrypted SafeContents, IIRC)
          * Cert0
          * ...
          * CertM
       * MAC
         * AlgId: HMAC-SHA-1 (IIRC this was a requirement, but it's also the only allowed algorithm on Win7 or Win8.1...)
  * Writing
    * SecItemExport fails to create a PFX with only public keys (or, at least, with non-keychain-based certificates).
    * SecItemExport fails to create a PFX where some elements are in different keychains than others (including "some elements are not in a keychain").


This change moves the necessary ASN types from the Pkcs12 library into Common so they're shared between Pkcs12Info/Pkcs12Builder and X509Certificates, then uses a managed loader and managed writer.

Quirks:
* SecItemImport(PKCS8) doesn't support marking keys as non-exportable, so non-exportable keyloads on macOS read a PFX, write a normalized PFX in memory, then call SecItemImport(PKCS12).
  * Because one of the failure modes of SecItemImport(PKCS12) is that it returns certs without private keys associated, it's not possible to call SecItemImport first and fall back to the managed loader.
* Windows and Linux both will happily return the wrong private key with a cert if the PFX says to do so, but on macOS the SecIdentityRef creation fails and the cert comes back with no private key.
  * This isn't a very realistic situation outside of our tests, so it's not something worth doing heroics for right now.  The easiest answer is to make HasPrivateKey be true but the GetPrivateKey methods throw... but that's still different than the other platforms, and would be very weird with SslStream.
  • Loading branch information
bartonjs authored Nov 8, 2019
1 parent 25154b5 commit 1338e4e
Show file tree
Hide file tree
Showing 72 changed files with 4,059 additions and 877 deletions.
25 changes: 8 additions & 17 deletions src/Common/src/Interop/OSX/Interop.CoreFoundation.CFData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,21 @@ internal static partial class CoreFoundation
[DllImport(Libraries.CoreFoundationLibrary)]
private static extern CFIndex CFDataGetLength(SafeCFDataHandle cfData);

internal static unsafe Span<byte> CFDataDangerousGetSpan(SafeCFDataHandle cfData)
{
long length = CFDataGetLength(cfData).ToInt64();
byte* dataBytes = CFDataGetBytePtr(cfData);
return new Span<byte>(dataBytes, checked((int)length));
}

internal static byte[] CFGetData(SafeCFDataHandle cfData)
{
bool addedRef = false;

try
{
cfData.DangerousAddRef(ref addedRef);
long length = CFDataGetLength(cfData).ToInt64();

if (length == 0)
{
return Array.Empty<byte>();
}

byte[] bytes = new byte[length];

unsafe
{
byte* dataBytes = CFDataGetBytePtr(cfData);
Marshal.Copy((IntPtr)dataBytes, bytes, 0, bytes.Length);
}

return bytes;

return CFDataDangerousGetSpan(cfData).ToArray();
}
finally
{
Expand Down
39 changes: 38 additions & 1 deletion src/Common/src/Interop/OSX/Interop.CoreFoundation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

using CFStringRef = System.IntPtr;
using CFArrayRef = System.IntPtr;

using CFIndex = System.IntPtr;

internal static partial class Interop
{
Expand Down Expand Up @@ -38,6 +38,24 @@ private enum CFStringBuiltInEncodings : uint
kCFStringEncodingUTF32LE = 0x1c000100
}

/// <summary>
/// Creates a CFStringRef from a specified range of memory with a specified encoding.
/// Follows the "Create Rule" where if you create it, you delete it.
/// </summary>
/// <param name="alloc">Should be IntPtr.Zero</param>
/// <param name="bytes">The pointer to the beginning of the encoded string.</param>
/// <param name="numBytes">The number of bytes in the encoding to read.</param>
/// <param name="encoding">The encoding type.</param>
/// <param name="isExternalRepresentation">Whether or not a BOM is present.</param>
/// <returns>A CFStringRef on success, otherwise a SafeCreateHandle(IntPtr.Zero).</returns>
[DllImport(Interop.Libraries.CoreFoundationLibrary)]
private static extern SafeCreateHandle CFStringCreateWithBytes(
IntPtr alloc,
IntPtr bytes,
CFIndex numBytes,
CFStringBuiltInEncodings encoding,
bool isExternalRepresentation);

/// <summary>
/// Creates a CFStringRef from a 8-bit String object. Follows the "Create Rule" where if you create it, you delete it.
/// </summary>
Expand Down Expand Up @@ -86,6 +104,25 @@ internal static SafeCreateHandle CFStringCreateWithCString(IntPtr utf8str)
return CFStringCreateWithCString(IntPtr.Zero, utf8str, CFStringBuiltInEncodings.kCFStringEncodingUTF8);
}

/// <summary>
/// Creates a CFStringRef from a span of chars.
/// Follows the "Create Rule" where if you create it, you delete it.
/// </summary>
/// <param name="source">The chars to make a CFString version of.</param>
/// <returns>A CFStringRef on success, otherwise a SafeCreateHandle(IntPtr.Zero).</returns>
internal static unsafe SafeCreateHandle CFStringCreateFromSpan(ReadOnlySpan<char> source)
{
fixed (char* sourcePtr = source)
{
return CFStringCreateWithBytes(
IntPtr.Zero,
(IntPtr)sourcePtr,
new CFIndex(source.Length * 2),
CFStringBuiltInEncodings.kCFStringEncodingUTF16,
isExternalRepresentation: false);
}
}

/// <summary>
/// Creates a pointer to an unmanaged CFArray containing the input values. Follows the "Create Rule" where if you create it, you delete it.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ private static extern int AppleCryptoNative_SecKeyExport(
out SafeCFDataHandle cfDataOut,
out int pOSStatus);

internal static byte[] SecKeyExport(
internal static SafeCFDataHandle SecKeyExportData(
SafeSecKeyRefHandle key,
bool exportPrivate,
string password)
ReadOnlySpan<char> password)
{
SafeCreateHandle exportPassword = exportPrivate
? CoreFoundation.CFStringCreateWithCString(password)
? CoreFoundation.CFStringCreateFromSpan(password)
: s_nullExportString;

int ret;
Expand All @@ -53,25 +53,31 @@ internal static byte[] SecKeyExport(
}
}

byte[] exportedData;

using (cfData)
if (ret == 1)
{
if (ret == 0)
{
throw CreateExceptionForOSStatus(osStatus);
}
return cfData;
}

if (ret != 1)
{
Debug.Fail($"AppleCryptoNative_SecKeyExport returned {ret}");
throw new CryptographicException();
}
cfData.Dispose();

exportedData = CoreFoundation.CFGetData(cfData);
if (ret == 0)
{
throw CreateExceptionForOSStatus(osStatus);
}

return exportedData;
Debug.Fail($"AppleCryptoNative_SecKeyExport returned {ret}");
throw new CryptographicException();
}

internal static byte[] SecKeyExport(
SafeSecKeyRefHandle key,
bool exportPrivate,
string password)
{
using (SafeCFDataHandle cfData = SecKeyExportData(key, exportPrivate, password))
{
return CoreFoundation.CFGetData(cfData);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -357,5 +357,18 @@ namespace System.Security.Cryptography.Apple
{
internal sealed class SafeSecKeyRefHandle : SafeKeychainItemHandle
{
protected override void Dispose(bool disposing)
{
if (disposing && SafeHandleCache<SafeSecKeyRefHandle>.IsCachedInvalidHandle(this))
{
return;
}

base.Dispose(disposing);
}

public static SafeSecKeyRefHandle InvalidHandle =>
SafeHandleCache<SafeSecKeyRefHandle>.GetInvalidHandle(
() => new SafeSecKeyRefHandle());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ private static extern int AppleCryptoNative_X509CopyWithPrivateKey(
out SafeSecIdentityHandle pIdentityHandleOut,
out int pOSStatus);

[DllImport(Libraries.AppleCryptoNative)]
private static extern int AppleCryptoNative_X509MoveToKeychain(
SafeSecCertificateHandle certHandle,
SafeKeychainHandle targetKeychain,
SafeSecKeyRefHandle privateKeyHandle,
out SafeSecIdentityHandle pIdentityHandleOut,
out int pOSStatus);

internal static byte[] X509GetRawData(SafeSecCertificateHandle cert)
{
int osStatus;
Expand Down Expand Up @@ -117,39 +125,24 @@ internal static SafeSecCertificateHandle X509ImportCertificate(
bool exportable,
out SafeSecIdentityHandle identityHandle)
{
SafeSecCertificateHandle certHandle;
int osStatus;
int ret;

SafeCreateHandle cfPassphrase = s_nullExportString;
SafeCreateHandle cfPassphrase = null;
bool releasePassword = false;

try
{
if (!importPassword.IsInvalid)
{
importPassword.DangerousAddRef(ref releasePassword);
IntPtr passwordHandle = importPassword.DangerousGetHandle();

if (passwordHandle != IntPtr.Zero)
{
cfPassphrase = CoreFoundation.CFStringCreateWithCString(passwordHandle);
}
cfPassphrase = CoreFoundation.CFStringCreateFromSpan(importPassword.DangerousGetSpan());
}

ret = AppleCryptoNative_X509ImportCertificate(
return X509ImportCertificate(
bytes,
bytes.Length,
contentType,
cfPassphrase,
keychain,
exportable ? 1 : 0,
out certHandle,
out identityHandle,
out osStatus);

SafeTemporaryKeychainHandle.TrackItem(certHandle);
SafeTemporaryKeychainHandle.TrackItem(identityHandle);
exportable,
out identityHandle);
}
finally
{
Expand All @@ -158,11 +151,36 @@ internal static SafeSecCertificateHandle X509ImportCertificate(
importPassword.DangerousRelease();
}

if (cfPassphrase != s_nullExportString)
{
cfPassphrase.Dispose();
}
cfPassphrase?.Dispose();
}
}

internal static SafeSecCertificateHandle X509ImportCertificate(
byte[] bytes,
X509ContentType contentType,
SafeCreateHandle importPassword,
SafeKeychainHandle keychain,
bool exportable,
out SafeSecIdentityHandle identityHandle)
{
SafeSecCertificateHandle certHandle;
int osStatus;

SafeCreateHandle cfPassphrase = importPassword ?? s_nullExportString;

int ret = AppleCryptoNative_X509ImportCertificate(
bytes,
bytes.Length,
contentType,
cfPassphrase,
keychain,
exportable ? 1 : 0,
out certHandle,
out identityHandle,
out osStatus);

SafeTemporaryKeychainHandle.TrackItem(certHandle);
SafeTemporaryKeychainHandle.TrackItem(identityHandle);

if (ret == 1)
{
Expand Down Expand Up @@ -385,6 +403,56 @@ internal static SafeSecIdentityHandle X509CopyWithPrivateKey(
throw new CryptographicException();
}

internal static SafeSecIdentityHandle X509MoveToKeychain(
SafeSecCertificateHandle cert,
SafeKeychainHandle targetKeychain,
SafeSecKeyRefHandle privateKey)
{
SafeSecIdentityHandle identityHandle;
int osStatus;

int result = AppleCryptoNative_X509MoveToKeychain(
cert,
targetKeychain,
privateKey ?? SafeSecKeyRefHandle.InvalidHandle,
out identityHandle,
out osStatus);

if (result == 0)
{
identityHandle.Dispose();
throw CreateExceptionForOSStatus(osStatus);
}

if (result != 1)
{
Debug.Fail($"AppleCryptoNative_X509MoveToKeychain returned {result}");
throw new CryptographicException();
}

if (privateKey?.IsInvalid == false)
{
// If a PFX has a mismatched association between a private key and the
// certificate public key then MoveToKeychain will write the NULL SecIdentityRef
// (after cleaning up the temporary key).
//
// When that happens, just treat the import as public-only.
if (!identityHandle.IsInvalid)
{
return identityHandle;
}
}

// If the cert in the PFX had no key, but it was imported with PersistKeySet (imports into
// the default keychain) and a matching private key was already there, then an
// identityHandle could be found. But that's not desirable, since neither Windows or Linux would
// do that matching.
//
// So dispose the handle, no matter what.
identityHandle.Dispose();
return null;
}

private static byte[] X509Export(X509ContentType contentType, SafeCreateHandle cfPassphrase, IntPtr[] certHandles)
{
Debug.Assert(contentType == X509ContentType.Pkcs7 || contentType == X509ContentType.Pkcs12);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal static partial class Crypto
internal static extern SafeX509CrlHandle DecodeX509Crl(byte[] buf, int len);

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_DecodeX509")]
internal static extern SafeX509Handle DecodeX509(byte[] buf, int len);
internal static extern SafeX509Handle DecodeX509(ref byte buf, int len);

[DllImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_GetX509DerSize")]
internal static extern int GetX509DerSize(SafeX509Handle x);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ internal bool HasNullEquivalentParameters()
return RepresentsNull(Parameters);
}

private static bool RepresentsNull(ReadOnlyMemory<byte>? parameters)
internal static bool RepresentsNull(ReadOnlyMemory<byte>? parameters)
{
if (parameters == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<asn:Sequence
xmlns:asn="http://schemas.dot.net/asnxml/201808/"
name="DigestInfoAsn"
namespace="System.Security.Cryptography.Pkcs.Asn1">
namespace="System.Security.Cryptography.Asn1">

<!--
https://tools.ietf.org/html/rfc2313#section-10.1.2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
using System.Security.Cryptography;
using System.Security.Cryptography.Asn1;

namespace System.Security.Cryptography.Pkcs.Asn1
namespace System.Security.Cryptography.Asn1
{
[StructLayout(LayoutKind.Sequential)]
internal partial struct DigestInfoAsn
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<asn:Sequence
xmlns:asn="http://schemas.dot.net/asnxml/201808/"
name="CertBagAsn"
namespace="System.Security.Cryptography.Pkcs.Asn1">
namespace="System.Security.Cryptography.Asn1.Pkcs12">

<!--
https://tools.ietf.org/html/rfc7292#section-4.2.3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
using System.Security.Cryptography;
using System.Security.Cryptography.Asn1;

namespace System.Security.Cryptography.Pkcs.Asn1
namespace System.Security.Cryptography.Asn1.Pkcs12
{
[StructLayout(LayoutKind.Sequential)]
internal partial struct CertBagAsn
Expand Down
Loading

0 comments on commit 1338e4e

Please sign in to comment.