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] Move standard cryptographic operations from OpenSSL to Apple's libraries #17597

Closed
ellismg opened this issue Jun 13, 2016 · 9 comments · Fixed by dotnet/corefx#17011
Closed
Labels
area-System.Security enhancement Product code improvement that does NOT require public API changes/additions os-mac-os-x macOS aka OSX
Milestone

Comments

@ellismg
Copy link
Contributor

ellismg commented Jun 13, 2016

Today when you install openssl via brew, it may print a message saying "Apple has deprecated use of OpenSSL in favor of its own TLS and crypto libraries".

We should add to the release notes that this is expected and lay out our plan on moving off OpenSLL and onto Apple stuff (maybe it happens for 1.1?, I think right know it's in the "we will do this in the future" bucket, but it would be good for us to start thinking about what the actual timeline is).

Edit: Since this issue is already known to people as the tracking item for the move, it has been repurposed from "make a roadmap" to "go, baby, go!".

@bartonjs
Copy link
Member

I merged the part about the warning in with the part of "we have a dependency" and "here's how to make an app-local copy" in https://github.com/dotnet/corefx/issues/9171#issuecomment-225690458.

I seriously doubt we'll be moving to CommonCrypto in 1.1; it's too big of a change in an already packed release. But I'll leave this open for 1.1 tracking that we should come up with either a plan, or a trigger condition.

@bartonjs
Copy link
Member

bartonjs commented Aug 3, 2016

New information leads to new conclusions. Some thinking that has gone on:

  • Since we already have an OpenSSL-based implementation working on macOS we want to keep that working
  • We need the native shim to load when OpenSSL isn't installed
  • Doing everything with dynamic bindings is dangerous, as it loses compiler protections

This means that the original shim idea of "implement these same functions, but for CommonCrypto instead" doesn't fly. And the current shim is pretty heavily OpenSSL focused, anyways.

  • Rename the current shim from System.Security.Cryptography.Native.dylib to System.Security.Cryptography.Native.OpenSsl.dylib
  • Make a new System.Security.Cryptography.Native.AppleCrypto.dylib
  • Convert System.Security.Cryptography.Algorithms to fully use AppleCrypto
    • RandomNumberGenerator
    • Hash algorithms, HMACs
    • Symmetric algorithms
    • RSA
    • ECDSA
  • Change OID<-->string lookups (and anything else in the Cryptography.Encoding library) to use AppleCrypto
  • Make a new library for Apple-based implementation types
    • RSA
    • ECDSA
  • Make X509 use the Apple type on Apple
  • Make SslStream and HttpClient support SecureTransport
  • Make PKCS (EnvelopedCms) work

I'm thinking "AppleCrypto" for the shim since it may involve any or all of CommonCrypto, SecureTransport, SecurityTransforms, and/or CDSA/CSSM.

@mellinoe
Copy link
Contributor

mellinoe commented Aug 3, 2016

Do we need to make a shim for this macOS-specific stuff? When I wrote the "NetworkInterfaceChanged" code for macOS, I just directly imported the functions, and we've done the same for some FileSystemWatcher stuff. Given the "AppleCrypto" name, is there any need to make a shim?

@bartonjs
Copy link
Member

bartonjs commented Aug 3, 2016

I had a chat with @ellismg and he convinced me that starting off with the shim is just better. Sometimes there are things that are just a pain to write in managed code, particularly when we're trying to make something work like in Windows (https://github.com/dotnet/corefx/blob/master/src/Native/Unix/System.Security.Cryptography.Native/openssl.c#L427 as an example).

So we'll get the benefit of not needing to maintain copies of any macros that exist, compile safety into calling the APIs, etc.

So the new shim is partially "we might need it in the future, so just start there". I'll let @ellismg speak to any other considerations he has.

@mellinoe
Copy link
Contributor

mellinoe commented Aug 3, 2016

Yeah, if there are problematic things like macros that you're forced to use by the Apple API then it probably makes sense to use a shim. Your example would probably be a nightmare to port. The existing stuff we are using from CoreFoundation and SystemConfiguration are a bit cumbersome, but the API isn't so complicated that it made the PInvoke approach untenable.

@ghost
Copy link

ghost commented Aug 4, 2016

Really need to either move away from openssl OR bundle openssl. Right now - only because of this issue - I can not create a dot net core application that is so easy to install that even my "grandmother" can install and use it on a Mac. Instead - now only patient software developers, that can manage to get brew and openssl working, can install a dot net core project. On my Mac I had considerable trouble with brew failing to update, openssl not having permissions to write to the right system folder and finally the old version of openssl rather then the new one being on the system path.- I almost gave up.

If dot net core want to be successful as a cross-platform alternative to Java/NodeJs it should not create serious installation barriers on Mac OS X

See also https://github.com/dotnet/corefx/issues/10117

@andyleejordan
Copy link
Member

I think this needs to be sped up. In addition the existing pain that was manually installing OpenSSL through home brew, recent updates to home brew's OpenSSL package Homebrew/brew#597 have broken this process entirely. At this point, to use .NET Core on OS X, you not only have to patch the rpath of the System.Security.Cryptography.Native.dylib library shipped with a .NET CLI install https://github.com/dotnet/cli/issues/3964, you then have to patch the one included after you dotnet restore .NET Core 1.0 packages.

User experience is currently completely broken.

@bartonjs bartonjs changed the title Document roadmap for moving away for OpenSSL on OSX [macOS] Move standard cryptographic operations from OpenSSL to Apple's libraries Aug 10, 2016
@karelz
Copy link
Member

karelz commented Mar 20, 2017

Thanks @bartonjs for finishing the work.
FYI: It was 8 months full-time feature for @bartonjs to finish it off. Great job!

@devandanger
Copy link

Tracking this issue through the vsts-agent install issue. Thanks for finishing this.

@bartonjs bartonjs removed their assignment Mar 28, 2017
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Security enhancement Product code improvement that does NOT require public API changes/additions os-mac-os-x macOS aka OSX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants