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

[macOS/iOS] Implement RSA, ECDsa signing using macOS 10.12+ API #51914

Merged
merged 3 commits into from
Apr 27, 2021

Conversation

filipnavara
Copy link
Member

Also unify DSA signing under the same native API. It's not supported on iOS and throws error there.

@ghost
Copy link

ghost commented Apr 27, 2021

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

Issue Details

Also unify DSA signing under the same native API. It's not supported on iOS and throws error there.

Author: filipnavara
Assignees: -
Labels:

area-System.Security

Milestone: -

@filipnavara
Copy link
Member Author

@vcsjones FYI this takes parts of #38101.

In the next follow-up PR I plan to introduce the import/export of data/iOS keys but keep it enabled only for iOS. After the whole iOS bring-up is finished we can re-evaluate their usage on macOS. I've the necessary changes locally but I anticipate the iOS implementation of S.S.C.X509Certificates will need some larger changes and I want to shape that first.

Also unify DSA signing under the same native API. It's not supported on iOS and throws error there.

Co-authored-by: Kevin Jones <[email protected]>

if (!SecTransformSetAttribute(xform, kSecDigestTypeAttribute, cfHashName, pErrorOut))
}
// Requires macOS 10.13+ or iOS 11+
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The native support for RSA-PSS can be enabled on macOS if desired. However, minimum supported iOS version is still iOS 10 so we'd need a special code path there anyway and it didn't make sense to diverge the code.

Copy link
Member

@vcsjones vcsjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a skim and it looks more or less in line with what I envisioned.

@bartonjs
Copy link
Member

A few comments, but generally looks pretty slick.

@filipnavara
Copy link
Member Author

I think I addressed all the feedback. Local build still works, let's see what the CI thinks.

@bartonjs bartonjs merged commit d1fed28 into dotnet:main Apr 27, 2021
@filipnavara filipnavara deleted the ios-signverify branch April 27, 2021 21:33
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants