Skip to content

Commit

Permalink
Use the managed implementation for keyUsage encoding and decoding eve…
Browse files Browse the repository at this point in the history 
…rywhere
  • Loading branch information
@vcsjones
vcsjones authored Oct 4, 2024
1 parent e67f230 commit 0b0637e
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 226 deletions.
6 changes: 0 additions & 6 deletions src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.ASN1.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,6 @@ internal static IntPtr GetObjectDefinitionByName(string friendlyName)
[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Asn1ObjectFree")]
internal static partial void Asn1ObjectFree(IntPtr o);

[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_DecodeAsn1BitString")]
internal static partial SafeAsn1BitStringHandle DecodeAsn1BitString(byte[] buf, int len);

[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Asn1BitStringFree")]
internal static partial void Asn1BitStringFree(IntPtr o);

[LibraryImport(Libraries.CryptoNative, EntryPoint = "CryptoNative_Asn1OctetStringNew")]
internal static partial SafeAsn1OctetStringHandle Asn1OctetStringNew();

Expand Down
20 changes: 0 additions & 20 deletions src/libraries/Common/src/Microsoft/Win32/SafeHandles/Asn1SafeHandles.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,26 +26,6 @@ public override bool IsInvalid
}
}

internal sealed class SafeAsn1BitStringHandle : SafeHandle
{
public SafeAsn1BitStringHandle() :
base(IntPtr.Zero, ownsHandle: true)
{
}

protected override bool ReleaseHandle()
{
Interop.Crypto.Asn1BitStringFree(handle);
SetHandle(IntPtr.Zero);
return true;
}

public override bool IsInvalid
{
get { return handle == IntPtr.Zero; }
}
}

internal sealed class SafeAsn1OctetStringHandle : SafeHandle
{
public SafeAsn1OctetStringHandle() :
Expand Down
2 changes: 0 additions & 2 deletions ...ystem.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/IX509Pal.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@ internal interface IX509Pal
string X500DistinguishedNameFormat(byte[] encodedDistinguishedName, bool multiLine);
X509ContentType GetCertContentType(ReadOnlySpan<byte> rawData);
X509ContentType GetCertContentType(string fileName);
byte[] EncodeX509KeyUsageExtension(X509KeyUsageFlags keyUsages);
void DecodeX509KeyUsageExtension(byte[] encoded, out X509KeyUsageFlags keyUsages);
bool SupportsLegacyBasicConstraintsExtension { get; }
byte[] EncodeX509BasicConstraints2Extension(bool certificateAuthority, bool hasPathLengthConstraint, int pathLengthConstraint);
void DecodeX509BasicConstraintsExtension(byte[] encoded, out bool certificateAuthority, out bool hasPathLengthConstraint, out int pathLengthConstraint);
Expand Down
63 changes: 0 additions & 63 deletions ...graphy/src/System/Security/Cryptography/X509Certificates/ManagedX509ExtensionProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,64 +10,6 @@ namespace System.Security.Cryptography.X509Certificates
{
internal class ManagedX509ExtensionProcessor
{
public virtual byte[] EncodeX509KeyUsageExtension(X509KeyUsageFlags keyUsages)
{
// The numeric values of X509KeyUsageFlags mean that if we interpret it as a little-endian
// ushort it will line up with the flags in the spec. We flip bit order of each byte to get
// the KeyUsageFlagsAsn order expected by AsnWriter.
KeyUsageFlagsAsn keyUsagesAsn =
(KeyUsageFlagsAsn)ReverseBitOrder((byte)keyUsages) |
(KeyUsageFlagsAsn)(ReverseBitOrder((byte)(((ushort)keyUsages >> 8))) << 8);

// The expected output of this method isn't the SEQUENCE value, but just the payload bytes.
AsnWriter writer = new AsnWriter(AsnEncodingRules.DER);
writer.WriteNamedBitList(keyUsagesAsn);
return writer.Encode();
}

public virtual void DecodeX509KeyUsageExtension(byte[] encoded, out X509KeyUsageFlags keyUsages)
{
KeyUsageFlagsAsn keyUsagesAsn;

try
{
AsnValueReader reader = new AsnValueReader(encoded, AsnEncodingRules.BER);
keyUsagesAsn = reader.ReadNamedBitListValue<KeyUsageFlagsAsn>();
reader.ThrowIfNotEmpty();
}
catch (AsnContentException e)
{
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e);
}

// DER encodings of BIT_STRING values number the bits as
// 01234567 89 (big endian), plus a number saying how many bits of the last byte were padding.
//
// So digitalSignature (0) doesn't mean 2^0 (0x01), it means the most significant bit
// is set in this byte stream.
//
// BIT_STRING values are compact. So a value of cRLSign (6) | keyEncipherment (2), which
// is 0b0010001 => 0b0010 0010 (1 bit padding) => 0x22 encoded is therefore
// 0x02 (length remaining) 0x01 (1 bit padding) 0x22.
//
// We will read that, and return 0x22. 0x22 lines up
// exactly with X509KeyUsageFlags.CrlSign (0x20) | X509KeyUsageFlags.KeyEncipherment (0x02)
//
// Once the decipherOnly (8) bit is added to the mix, the values become:
// 0b001000101 => 0b0010 0010 1000 0000 (7 bits padding)
// { 0x03 0x07 0x22 0x80 }
// And we read new byte[] { 0x22 0x80 }
//
// The value of X509KeyUsageFlags.DecipherOnly is 0x8000. 0x8000 in a little endian
// representation is { 0x00 0x80 }. This means that the DER storage mechanism has effectively
// ended up being little-endian for BIT_STRING values. Untwist the bytes, and now the bits all
// line up with the existing X509KeyUsageFlags.

keyUsages =
(X509KeyUsageFlags)ReverseBitOrder((byte)keyUsagesAsn) |
(X509KeyUsageFlags)(ReverseBitOrder((byte)(((ushort)keyUsagesAsn >> 8))) << 8);
}

public virtual byte[] EncodeX509BasicConstraints2Extension(
bool certificateAuthority,
bool hasPathLengthConstraint,
Expand Down Expand Up @@ -161,10 +103,5 @@ public virtual void DecodeX509EnhancedKeyUsageExtension(byte[] encoded, out OidC
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e);
}
}

private static byte ReverseBitOrder(byte b)
{
return (byte)(unchecked(b * 0x0202020202ul & 0x010884422010ul) % 1023);
}
}
}
53 changes: 0 additions & 53 deletions ...rity.Cryptography/src/System/Security/Cryptography/X509Certificates/OpenSslX509Encoder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -152,59 +152,6 @@ public X509ContentType GetCertContentType(string fileName)
throw new CryptographicException();
}

public override void DecodeX509KeyUsageExtension(byte[] encoded, out X509KeyUsageFlags keyUsages)
{
using (SafeAsn1BitStringHandle bitString = Interop.Crypto.DecodeAsn1BitString(encoded, encoded.Length))
{
Interop.Crypto.CheckValidOpenSslHandle(bitString);

byte[] decoded = Interop.Crypto.GetAsn1StringBytes(bitString.DangerousGetHandle());

// Only 9 bits are defined.
if (decoded.Length > 2)
{
throw new CryptographicException();
}

// DER encodings of BIT_STRING values number the bits as
// 01234567 89 (big endian), plus a number saying how many bits of the last byte were padding.
//
// So digitalSignature (0) doesn't mean 2^0 (0x01), it means the most significant bit
// is set in this byte stream.
//
// BIT_STRING values are compact. So a value of cRLSign (6) | keyEncipherment (2), which
// is 0b0010001 => 0b0010 0010 (1 bit padding) => 0x22 encoded is therefore
// 0x02 (length remaining) 0x01 (1 bit padding) 0x22.
//
// OpenSSL's d2i_ASN1_BIT_STRING is going to take that, and return 0x22. 0x22 lines up
// exactly with X509KeyUsageFlags.CrlSign (0x20) | X509KeyUsageFlags.KeyEncipherment (0x02)
//
// Once the decipherOnly (8) bit is added to the mix, the values become:
// 0b001000101 => 0b0010 0010 1000 0000 (7 bits padding)
// { 0x03 0x07 0x22 0x80 }
// And OpenSSL returns new byte[] { 0x22 0x80 }
//
// The value of X509KeyUsageFlags.DecipherOnly is 0x8000. 0x8000 in a little endian
// representation is { 0x00 0x80 }. This means that the DER storage mechanism has effectively
// ended up being little-endian for BIT_STRING values. Untwist the bytes, and now the bits all
// line up with the existing X509KeyUsageFlags.

int value = 0;

if (decoded.Length > 0)
{
value = decoded[0];
}

if (decoded.Length > 1)
{
value |= decoded[1] << 8;
}

keyUsages = (X509KeyUsageFlags)value;
}
}

public override void DecodeX509BasicConstraints2Extension(
byte[] encoded,
out bool certificateAuthority,
Expand Down
70 changes: 68 additions & 2 deletions ...y.Cryptography/src/System/Security/Cryptography/X509Certificates/X509KeyUsageExtension.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Formats.Asn1;
using System.Security.Cryptography.X509Certificates.Asn1;

namespace System.Security.Cryptography.X509Certificates
{
public sealed class X509KeyUsageExtension : X509Extension
Expand All @@ -17,7 +20,7 @@ public X509KeyUsageExtension(AsnEncodedData encodedKeyUsage, bool critical)
}

public X509KeyUsageExtension(X509KeyUsageFlags keyUsages, bool critical)
: base(Oids.KeyUsageOid, X509Pal.Instance.EncodeX509KeyUsageExtension(keyUsages), critical, skipCopy: true)
: base(Oids.KeyUsageOid, EncodeX509KeyUsageExtension(keyUsages), critical, skipCopy: true)
{
}

Expand All @@ -27,7 +30,7 @@ public X509KeyUsageFlags KeyUsages
{
if (!_decoded)
{
X509Pal.Instance.DecodeX509KeyUsageExtension(RawData, out _keyUsages);
DecodeX509KeyUsageExtension(RawData, out _keyUsages);
_decoded = true;
}
return _keyUsages;
Expand All @@ -40,6 +43,69 @@ public override void CopyFrom(AsnEncodedData asnEncodedData)
_decoded = false;
}

private static byte[] EncodeX509KeyUsageExtension(X509KeyUsageFlags keyUsages)
{
// The numeric values of X509KeyUsageFlags mean that if we interpret it as a little-endian
// ushort it will line up with the flags in the spec. We flip bit order of each byte to get
// the KeyUsageFlagsAsn order expected by AsnWriter.
KeyUsageFlagsAsn keyUsagesAsn =
(KeyUsageFlagsAsn)ReverseBitOrder((byte)keyUsages) |
(KeyUsageFlagsAsn)(ReverseBitOrder((byte)(((ushort)keyUsages >> 8))) << 8);

// The expected output of this method isn't the SEQUENCE value, but just the payload bytes.
// We expect to encode at most 9 bits in the bit list, which can encode to at most 5 octets: two
// for the tag and definite length, two for the bit string, and one for the unused bit count.
AsnWriter writer = new AsnWriter(AsnEncodingRules.DER, initialCapacity: 5);
writer.WriteNamedBitList(keyUsagesAsn);
return writer.Encode();
}


private static void DecodeX509KeyUsageExtension(byte[] encoded, out X509KeyUsageFlags keyUsages)
{
KeyUsageFlagsAsn keyUsagesAsn;

try
{
AsnValueReader reader = new AsnValueReader(encoded, AsnEncodingRules.BER);
keyUsagesAsn = reader.ReadNamedBitListValue<KeyUsageFlagsAsn>();
reader.ThrowIfNotEmpty();
}
catch (AsnContentException e)
{
throw new CryptographicException(SR.Cryptography_Der_Invalid_Encoding, e);
}

// DER encodings of BIT_STRING values number the bits as
// 01234567 89 (big endian), plus a number saying how many bits of the last byte were padding.
//
// So digitalSignature (0) doesn't mean 2^0 (0x01), it means the most significant bit
// is set in this byte stream.
//
// BIT_STRING values are compact. So a value of cRLSign (6) | keyEncipherment (2), which
// is 0b0010001 => 0b0010 0010 (1 bit padding) => 0x22 encoded is therefore
// 0x02 (length remaining) 0x01 (1 bit padding) 0x22.
//
// We will read that, and return 0x22. 0x22 lines up
// exactly with X509KeyUsageFlags.CrlSign (0x20) | X509KeyUsageFlags.KeyEncipherment (0x02)
//
// Once the decipherOnly (8) bit is added to the mix, the values become:
// 0b001000101 => 0b0010 0010 1000 0000 (7 bits padding)
// { 0x03 0x07 0x22 0x80 }
// And we read new byte[] { 0x22 0x80 }
//
// The value of X509KeyUsageFlags.DecipherOnly is 0x8000. 0x8000 in a little endian
// representation is { 0x00 0x80 }. This means that the DER storage mechanism has effectively
// ended up being little-endian for BIT_STRING values. Untwist the bytes, and now the bits all
// line up with the existing X509KeyUsageFlags.

keyUsages =
(X509KeyUsageFlags)ReverseBitOrder((byte)keyUsagesAsn) |
(X509KeyUsageFlags)(ReverseBitOrder((byte)(((ushort)keyUsagesAsn >> 8))) << 8);
}

private static byte ReverseBitOrder(byte b) => (byte)(unchecked(b * 0x0202020202ul & 0x010884422010ul) % 1023);

private bool _decoded;
private X509KeyUsageFlags _keyUsages;
}
Expand Down
47 changes: 0 additions & 47 deletions ...phy/src/System/Security/Cryptography/X509Certificates/X509Pal.Windows.CustomExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,53 +13,6 @@ namespace System.Security.Cryptography.X509Certificates
/// </summary>
internal sealed partial class X509Pal : IX509Pal
{
public byte[] EncodeX509KeyUsageExtension(X509KeyUsageFlags keyUsages)
{
unsafe
{
ushort keyUsagesAsShort = (ushort)keyUsages;
Interop.Crypt32.CRYPT_BIT_BLOB blob = new Interop.Crypt32.CRYPT_BIT_BLOB()
{
cbData = 2,
pbData = new IntPtr((byte*)&keyUsagesAsShort),
cUnusedBits = 0,
};
return Interop.crypt32.EncodeObject(CryptDecodeObjectStructType.X509_KEY_USAGE, &blob);
}
}

public void DecodeX509KeyUsageExtension(byte[] encoded, out X509KeyUsageFlags keyUsages)
{
unsafe
{
uint keyUsagesAsUint = encoded.DecodeObject(
CryptDecodeObjectStructType.X509_KEY_USAGE,
static delegate (void* pvDecoded, int cbDecoded)
{
Debug.Assert(cbDecoded >= sizeof(Interop.Crypt32.CRYPT_BIT_BLOB));
Interop.Crypt32.CRYPT_BIT_BLOB* pBlob = (Interop.Crypt32.CRYPT_BIT_BLOB*)pvDecoded;
byte* pbData = (byte*)pBlob->pbData.ToPointer();

if (pbData != null)
{
Debug.Assert((uint)pBlob->cbData < 3, "Unexpected length for X509_KEY_USAGE data");

switch (pBlob->cbData)
{
case 1:
return *pbData;
case 2:
return *(ushort*)(pbData);
}
}

return 0u;
}
);
keyUsages = (X509KeyUsageFlags)keyUsagesAsUint;
}
}

public bool SupportsLegacyBasicConstraintsExtension
{
get { return true; }
Expand Down
2 changes: 0 additions & 2 deletions src/native/libs/System.Security.Cryptography.Native/entrypoints.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@

static const Entry s_cryptoNative[] =
{
DllImportEntry(CryptoNative_Asn1BitStringFree)
DllImportEntry(CryptoNative_Asn1ObjectFree)
DllImportEntry(CryptoNative_Asn1OctetStringFree)
DllImportEntry(CryptoNative_Asn1OctetStringNew)
Expand All @@ -53,7 +52,6 @@ static const Entry s_cryptoNative[] =
DllImportEntry(CryptoNative_CheckX509IpAddress)
DllImportEntry(CryptoNative_CreateMemoryBio)
DllImportEntry(CryptoNative_D2IPkcs7Bio)
DllImportEntry(CryptoNative_DecodeAsn1BitString)
DllImportEntry(CryptoNative_DecodeExtendedKeyUsage)
DllImportEntry(CryptoNative_DecodeOcspResponse)
DllImportEntry(CryptoNative_DecodePkcs7)
Expand Down
4 changes: 0 additions & 4 deletions src/native/libs/System.Security.Cryptography.Native/opensslshim.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,6 @@ extern bool g_libSslUses32BitTime;

#define FOR_ALL_OPENSSL_FUNCTIONS \
REQUIRED_FUNCTION(a2d_ASN1_OBJECT) \
REQUIRED_FUNCTION(ASN1_BIT_STRING_free) \
REQUIRED_FUNCTION(ASN1_d2i_bio) \
REQUIRED_FUNCTION(ASN1_i2d_bio) \
REQUIRED_FUNCTION(ASN1_GENERALIZEDTIME_free) \
Expand Down Expand Up @@ -299,7 +298,6 @@ extern bool g_libSslUses32BitTime;
REQUIRED_FUNCTION(CRYPTO_malloc) \
LEGACY_FUNCTION(CRYPTO_num_locks) \
LEGACY_FUNCTION(CRYPTO_set_locking_callback) \
REQUIRED_FUNCTION(d2i_ASN1_BIT_STRING) \
REQUIRED_FUNCTION(d2i_BASIC_CONSTRAINTS) \
REQUIRED_FUNCTION(d2i_EXTENDED_KEY_USAGE) \
REQUIRED_FUNCTION(d2i_OCSP_RESPONSE) \
Expand Down Expand Up @@ -796,7 +794,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr;
// Redefine all calls to OpenSSL functions as calls through pointers that are set
// to the functions from the libssl.so selected by the shim.
#define a2d_ASN1_OBJECT a2d_ASN1_OBJECT_ptr
#define ASN1_BIT_STRING_free ASN1_BIT_STRING_free_ptr
#define ASN1_GENERALIZEDTIME_free ASN1_GENERALIZEDTIME_free_ptr
#define ASN1_d2i_bio ASN1_d2i_bio_ptr
#define ASN1_i2d_bio ASN1_i2d_bio_ptr
Expand Down Expand Up @@ -851,7 +848,6 @@ extern TYPEOF(OPENSSL_gmtime)* OPENSSL_gmtime_ptr;
#define CRYPTO_malloc CRYPTO_malloc_ptr
#define CRYPTO_num_locks CRYPTO_num_locks_ptr
#define CRYPTO_set_locking_callback CRYPTO_set_locking_callback_ptr
#define d2i_ASN1_BIT_STRING d2i_ASN1_BIT_STRING_ptr
#define d2i_BASIC_CONSTRAINTS d2i_BASIC_CONSTRAINTS_ptr
#define d2i_EXTENDED_KEY_USAGE d2i_EXTENDED_KEY_USAGE_ptr
#define d2i_OCSP_RESPONSE d2i_OCSP_RESPONSE_ptr
Expand Down
Loading

0 comments on commit 0b0637e

Please sign in to comment.