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

Harden SecKeyCopyExternalRepresentation against sensitive keys #99840

Merged
merged 3 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ internal static partial class AppleCrypto
private const int kSuccess = 1;
private const int kErrorSeeError = -2;
private const int kPlatformNotSupported = -5;
private const int kKeyIsSensitive = -6;
private const int kKeyIsNotExtractable = -7;

internal enum PAL_KeyAlgorithm : uint
{
Expand Down Expand Up @@ -125,12 +127,6 @@ internal static bool TrySecKeyCopyExternalRepresentation(
SafeSecKeyRefHandle key,
out byte[] externalRepresentation)
{
const int errSecPassphraseRequired = -25260;

// macOS Sonoma 14.4 started returning errSecInvalidKeyAttributeMask when a key could not be exported
// because it must be exported with a password.
const int errSecInvalidKeyAttributeMask = -67738;

int result = AppleCryptoNative_SecKeyCopyExternalRepresentation(
key,
out SafeCFDataHandle data,
Expand All @@ -144,12 +140,12 @@ internal static bool TrySecKeyCopyExternalRepresentation(
case kSuccess:
externalRepresentation = CoreFoundation.CFGetData(data);
return true;
case kKeyIsSensitive:
externalRepresentation = [];
return false;
case kKeyIsNotExtractable:
throw new CryptographicException(SR.Cryptography_KeyNotExtractable);
case kErrorSeeError:
if (Interop.CoreFoundation.GetErrorCode(errorHandle) is errSecPassphraseRequired or errSecInvalidKeyAttributeMask)
{
externalRepresentation = Array.Empty<byte>();
return false;
}
throw CreateExceptionForCFError(errorHandle);
default:
Debug.Fail($"SecKeyCopyExternalRepresentation returned {result}");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,9 @@
<data name="Cryptography_KeyBlobParsingError" xml:space="preserve">
<value>Key Blob not in expected format.</value>
</data>
<data name="Cryptography_KeyNotExtractable" xml:space="preserve">
<value>The key does not permit being exported.</value>
</data>
<data name="Cryptography_KeyTooSmall" xml:space="preserve">
<value>The key is too small for the requested operation.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,9 @@
<data name="Cryptography_Invalid_IA5String" xml:space="preserve">
<value>The string contains a character not in the 7 bit ASCII character set.</value>
</data>
<data name="Cryptography_KeyNotExtractable" xml:space="preserve">
<value>The key does not permit being exported.</value>
</data>
<data name="Cryptography_X509Store_WouldModifyUserTrust" xml:space="preserve">
<value>Removing the requested certificate would modify user trust settings, and has been denied.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,31 @@ int32_t AppleCryptoNative_SecKeyCopyExternalRepresentation(SecKeyRef pKey,
assert(pErrorOut != NULL);

*pErrorOut = NULL;
*ppDataOut = NULL;
int32_t ret = 0;
CFDictionaryRef attributes = SecKeyCopyAttributes(pKey);

*ppDataOut = SecKeyCopyExternalRepresentation(pKey, pErrorOut);
return *ppDataOut == NULL ? kErrorSeeError : 1;
if (attributes != NULL)
{
if (CFDictionaryGetValue(attributes, kSecAttrIsExtractable) == kCFBooleanFalse)
{
ret = kKeyIsNotExtractable;
}
else if (CFDictionaryGetValue(attributes, kSecAttrIsSensitive) == kCFBooleanTrue)
{
ret = kKeyIsSensitive;
}

CFRelease(attributes);
}

if (ret == 0)
{
*ppDataOut = SecKeyCopyExternalRepresentation(pKey, pErrorOut);
ret = *ppDataOut == NULL ? kErrorSeeError : 1;
}

return ret;
}

SecKeyRef AppleCryptoNative_SecKeyCopyPublicKey(SecKeyRef privateKey)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ static const int32_t kErrorSeeError = -2;
static const int32_t kErrorUnknownAlgorithm = -3;
static const int32_t kErrorUnknownState = -4;
static const int32_t kPlatformNotSupported = -5;
static const int32_t kKeyIsSensitive = -6;
static const int32_t kKeyIsNotExtractable = -7;

enum
{
Expand Down
Loading