Harden SecKeyCopyExternalRepresentation against sensitive keys

[vcsjones]

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 change checking for the error code.

[dotnet-policy-service]

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

[filipnavara]

I will check it tomorrow morning. There were some odd cases around the old CSSM keys and I don’t quite remember them all.

Thanks for looking into it!

[vcsjones]

Yeah, that's why my initial PR was just "handle the new error code" since I would have way more confidence with that in a backport.

The way I see it we have a few of options.

1. Trust that kSecAttrIsSensitive is present when we need it (what this PR does)
2. Close this PR and just handle new codes as they happen. This makes it feel too reactionary to me. On the other hand, its only happened once so far and doesn't really establish a pattern.
3. "Do both". In this PR I eliminated the error handling code path, but I suppose we could do both. Go down the SecItemExport path if either the attribute is present or we get an error code back
4. We try the SecItemExport path on any error.

[filipnavara]

I did some sanity check first to make sure that the assumptions are correct:

1. Modern SecKey implementations (non-CSSM) always have kSecAttrIsSensitive = false, kSecAttrIsExtractable = true, and SecKeyCopyExternalRepresentation essentially cannot fail.
2. Legacy SecKey implementations (CSSM; used in certs and keychains) mirror CSSM_KEYATTR_SENSITIVE, CSSM_KEYATTR_EXTRACTABLE flags.
3. CSSMERR_CSP_INVALID_KEYATTR_MASK/errSecInvalidKeyAttributeMask error is used to cover two situations:
   a) The key is not extractable (kSecAttrIsExtractable == false)
   b) The key is extractable but sensitive and thus password protection is required (kSecAttrIsExtractable == true && kSecAttrIsSensitive == true)

To summarize, it's safe to skip SecKeyCopyExternalRepresentation if kSecAttrIsSensitive is set to true because it's guaranteed to fail (and in the current Apple's implementation it can only happen for CSSM keys). That said, if we are checking the attributes already, maybe it would make more sense to move the check higher up and also produce more meaningful error message for kSecAttrIsExtractable == false.