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

Code Cleanup: Remove OSX 10.14 compatibility code #60138

Closed
bartonjs opened this issue Oct 7, 2021 · 7 comments
Closed

Code Cleanup: Remove OSX 10.14 compatibility code #60138

bartonjs opened this issue Oct 7, 2021 · 7 comments

Comments

@bartonjs
Copy link
Member

bartonjs commented Oct 7, 2021

10.14 looks to have hit implicit End of Life, since it has gone two months without a security update.

Assuming that .NET 7 changes the minimum macOS to 10.15, we can remove 10.14-specific compat code, like

private static bool HasWorkingPKCS1Padding { get; } = OperatingSystem.IsMacOSVersionAtLeast(10, 15);
private static void ImportPrivateKey(
RSAParameters rsaParameters,
out SafeSecKeyRefHandle privateKey,
out SafeSecKeyRefHandle publicKey)
{
// macOS 10.14 and older have broken PKCS#1 depadding for decryption
// of empty data. The bug doesn't affect the legacy CSSM keys so we
// use them instead.
if (HasWorkingPKCS1Padding)
{
privateKey = ImportKey(rsaParameters);
publicKey = Interop.AppleCrypto.CopyPublicKey(privateKey);
}
else
{
privateKey = ImportLegacyPrivateKey(rsaParameters);

There may be other code that can be removed.

@bartonjs bartonjs added this to the Future milestone Oct 7, 2021
@ghost
Copy link

ghost commented Oct 7, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

10.14 looks to have hit implicit End of Life, since it has gone two months without a security update.

Assuming that .NET 7 changes the minimum macOS to 10.15, we can remove 10.14-specific compat code, like

private static bool HasWorkingPKCS1Padding { get; } = OperatingSystem.IsMacOSVersionAtLeast(10, 15);
private static void ImportPrivateKey(
RSAParameters rsaParameters,
out SafeSecKeyRefHandle privateKey,
out SafeSecKeyRefHandle publicKey)
{
// macOS 10.14 and older have broken PKCS#1 depadding for decryption
// of empty data. The bug doesn't affect the legacy CSSM keys so we
// use them instead.
if (HasWorkingPKCS1Padding)
{
privateKey = ImportKey(rsaParameters);
publicKey = Interop.AppleCrypto.CopyPublicKey(privateKey);
}
else
{
privateKey = ImportLegacyPrivateKey(rsaParameters);

There may be other code that can be removed.

Author: bartonjs
Assignees: -
Labels:

area-System.Security

Milestone: Future

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Oct 7, 2021
@bartonjs
Copy link
Member Author

bartonjs commented Oct 7, 2021

case 72:
return 256;
case 104:
return 384;
case 141:
return 521;
is probably already dead code, since the subsequent 3 cases say they're for Mojave (10.14).

@vcsjones
Copy link
Member

vcsjones commented Oct 7, 2021

One thing to be mindful of is the shared code with iOS / iPad OS. We support much older iOS versions that prevented a lot of cleanup, the last time I looked.

@bartonjs
Copy link
Member Author

bartonjs commented Oct 7, 2021

True, true. But of the two things I've linked in so far, I think they're both "macOS-only"... i.e. Mojave's ECC size was changed to align with the size reported on iOS; and iOS only has the data keys, not the keychain keys, so there's no legacy import for that (and the file is in a "we're different between macOS and iOS here", anyways)

@jeffhandley jeffhandley removed the untriaged New issue has not been triaged by the area owner label Oct 10, 2021
@vcsjones
Copy link
Member

Everything else I can identify requires bumping the minimum iOS version to 11, while Maui still supports iOS 10.

@vcsjones
Copy link
Member

@bartonjs I don't think there is anything else we can do here, especially given how far back we support iOS. Should we close this, or are you aware of anything else that could use some cleanup?

@vcsjones
Copy link
Member

I think we can close this out. All identified places have been cleaned up, even the iOS 10 ones since we bumped out iOS minimum to iOS 11. We can open new issues if we identify more opportunities to remove legacy code.

@ghost ghost locked as resolved and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants