Skip to content

Commit 969d424

Browse files
authored
Harden SecKeyCopyExternalRepresentation against sensitive keys
The previous approach relied on examining the error code returned from SecKeyCopyExternalRepresentation when a key needed to be exported with a password. Apple has changed the error code which resulted in breaking the detection of sensitive values. This change looks for the kSecAttrIsSensitive attribute on a key which according to the Apple documentation, "When set to kCFBooleanTrue, the item can only be exported in an encrypted format". This should be less brittle than checking the error result.
1 parent 5f7e495 commit 969d424

File tree

5 files changed

+39
-13
lines changed

5 files changed

+39
-13
lines changed

src/libraries/Common/src/Interop/OSX/System.Security.Cryptography.Native.Apple/Interop.SecKeyRef.cs

+7-11
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ internal static partial class AppleCrypto
1515
private const int kSuccess = 1;
1616
private const int kErrorSeeError = -2;
1717
private const int kPlatformNotSupported = -5;
18+
private const int kKeyIsSensitive = -6;
19+
private const int kKeyIsNotExtractable = -7;
1820

1921
internal enum PAL_KeyAlgorithm : uint
2022
{
@@ -125,12 +127,6 @@ internal static bool TrySecKeyCopyExternalRepresentation(
125127
SafeSecKeyRefHandle key,
126128
out byte[] externalRepresentation)
127129
{
128-
const int errSecPassphraseRequired = -25260;
129-
130-
// macOS Sonoma 14.4 started returning errSecInvalidKeyAttributeMask when a key could not be exported
131-
// because it must be exported with a password.
132-
const int errSecInvalidKeyAttributeMask = -67738;
133-
134130
int result = AppleCryptoNative_SecKeyCopyExternalRepresentation(
135131
key,
136132
out SafeCFDataHandle data,
@@ -144,12 +140,12 @@ internal static bool TrySecKeyCopyExternalRepresentation(
144140
case kSuccess:
145141
externalRepresentation = CoreFoundation.CFGetData(data);
146142
return true;
143+
case kKeyIsSensitive:
144+
externalRepresentation = [];
145+
return false;
146+
case kKeyIsNotExtractable:
147+
throw new CryptographicException(SR.Cryptography_KeyNotExtractable);
147148
case kErrorSeeError:
148-
if (Interop.CoreFoundation.GetErrorCode(errorHandle) is errSecPassphraseRequired or errSecInvalidKeyAttributeMask)
149-
{
150-
externalRepresentation = Array.Empty<byte>();
151-
return false;
152-
}
153149
throw CreateExceptionForCFError(errorHandle);
154150
default:
155151
Debug.Fail($"SecKeyCopyExternalRepresentation returned {result}");

src/libraries/System.Security.Cryptography/src/Resources/Strings.resx

+3
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,9 @@
465465
<data name="Cryptography_KeyBlobParsingError" xml:space="preserve">
466466
<value>Key Blob not in expected format.</value>
467467
</data>
468+
<data name="Cryptography_KeyNotExtractable" xml:space="preserve">
469+
<value>The key does not permit being exported.</value>
470+
</data>
468471
<data name="Cryptography_KeyTooSmall" xml:space="preserve">
469472
<value>The key is too small for the requested operation.</value>
470473
</data>

src/libraries/System.Security.Cryptography/tests/Resources/Strings.resx

+3
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,9 @@
7575
<data name="Cryptography_Invalid_IA5String" xml:space="preserve">
7676
<value>The string contains a character not in the 7 bit ASCII character set.</value>
7777
</data>
78+
<data name="Cryptography_KeyNotExtractable" xml:space="preserve">
79+
<value>The key does not permit being exported.</value>
80+
</data>
7881
<data name="Cryptography_X509Store_WouldModifyUserTrust" xml:space="preserve">
7982
<value>Removing the requested certificate would modify user trust settings, and has been denied.</value>
8083
</data>

src/native/libs/System.Security.Cryptography.Native.Apple/pal_seckey.c

+24-2
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,31 @@ int32_t AppleCryptoNative_SecKeyCopyExternalRepresentation(SecKeyRef pKey,
7272
assert(pErrorOut != NULL);
7373

7474
*pErrorOut = NULL;
75+
*ppDataOut = NULL;
76+
int32_t ret = 0;
77+
CFDictionaryRef attributes = SecKeyCopyAttributes(pKey);
7578

76-
*ppDataOut = SecKeyCopyExternalRepresentation(pKey, pErrorOut);
77-
return *ppDataOut == NULL ? kErrorSeeError : 1;
79+
if (attributes != NULL)
80+
{
81+
if (CFDictionaryGetValue(attributes, kSecAttrIsExtractable) == kCFBooleanFalse)
82+
{
83+
ret = kKeyIsNotExtractable;
84+
}
85+
else if (CFDictionaryGetValue(attributes, kSecAttrIsSensitive) == kCFBooleanTrue)
86+
{
87+
ret = kKeyIsSensitive;
88+
}
89+
90+
CFRelease(attributes);
91+
}
92+
93+
if (ret == 0)
94+
{
95+
*ppDataOut = SecKeyCopyExternalRepresentation(pKey, pErrorOut);
96+
ret = *ppDataOut == NULL ? kErrorSeeError : 1;
97+
}
98+
99+
return ret;
78100
}
79101

80102
SecKeyRef AppleCryptoNative_SecKeyCopyPublicKey(SecKeyRef privateKey)

src/native/libs/System.Security.Cryptography.Native.Apple/pal_seckey.h

+2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ static const int32_t kErrorSeeError = -2;
1616
static const int32_t kErrorUnknownAlgorithm = -3;
1717
static const int32_t kErrorUnknownState = -4;
1818
static const int32_t kPlatformNotSupported = -5;
19+
static const int32_t kKeyIsSensitive = -6;
20+
static const int32_t kKeyIsNotExtractable = -7;
1921

2022
enum
2123
{

0 commit comments

Comments
 (0)