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

System.Security.Cryptography.Native.Openssl should not load OpenSSL eagerly on macOS #46771

Closed
VSadov opened this issue Jan 8, 2021 · 7 comments
Assignees
Labels
area-Single-File untriaged New issue has not been triaged by the area owner

Comments

@VSadov
Copy link
Member

VSadov commented Jan 8, 2021

OpenSSL is optional on macOS - it's needed only for AES-GCM/CCM encryption.

However we have __attribute__((constructor)) static void InitializeOpenSSLShim() which aborts if OpenSSL is not found.
As a result, when System.Security.Cryptography.Native.Openssl is statically linked in a single-file scenario, every app will probe for OpenSSL and fail if not found.

We need to either delay-load on first use or invoke initialization explicitly from managed code.

@VSadov VSadov self-assigned this Jan 8, 2021
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added the untriaged New issue has not been triaged by the area owner label Jan 8, 2021
@ghost
Copy link

ghost commented Jan 8, 2021

Tagging subscribers to this area: @agocke, @vitek-karas
See info in area-owners.md if you want to be subscribed.

Issue Details

OpenSSL is optional on macOS - it's needed only for AES-GCM/CCM encryption.

However we have __attribute__((constructor)) static void InitializeAppleCryptoSslShim() which aborts if OpenSSL is not found.
As a result, when System.Security.Cryptography.Native.Openssl is statically linked in a single-file scenario, every app will probe for OpenSSL and fail if not found.

We need to either delay-load on first use or invoke initialization explicitly from managed code.

Author: VSadov
Assignees: VSadov
Labels:

area-Single-File

Milestone: -

@am11
Copy link
Member

am11 commented Jan 8, 2021

Related #46076.

delay-load on first use

like libgdiplus


would be great!

@EgorBo
Copy link
Member

EgorBo commented Jan 9, 2021

BTW, can we use https://developer.apple.com/documentation/cryptokit on mac? it should support AES-GCM/CCM so we can drop this OpenSSL requirement completely on macOS.
We only need to link System.Native.Security.Apple against Cryptokit framework and write a small wrapper in ObjC. Example: https://github.com/dotnet/runtime/blob/master/src/libraries/Native/Unix/System.Native/pal_log.m
cc @bartonjs

@bartonjs
Copy link
Member

BTW, can we use https://developer.apple.com/documentation/cryptokit on mac? ... ObjC

My understanding is that CryptoKit only has Swift bindings, no Objective C bindings. I found references to being able to call into "Swift-only" things from Objective C using some sort of Xcode-generated header, but nothing talking about how to do it outside of Xcode or what the calling conventions would look like for instance functions.

If someone can figure out how to call it from the Apple crypto shim, then sure!

@bartonjs
Copy link
Member

InitializeAppleCryptoSslShim

That function is in System.Security.Cryptography.Native.Apple, which doesn't link to OpenSSL.

On macOS both shims can be used. System.Security.Cryptography.Native.Apple is the only one used in the majority of programs that need any crypto. System.Security.Cryptography.Native.OpenSsl gets loaded when the AesGcm or AesCcm types are used, or any of the types whose name explicitly ends in "OpenSsl", (e.g. RSAOpenSsl). I don't really expect that anyone is using the *OpenSsl types from macOS, we might be able to drop support for them (from macOS) in 6 if we have reasons.

@VSadov
Copy link
Member Author

VSadov commented Jan 11, 2021

Right - the problem is not InitializeAppleCryptoSslShim, it is the InitializeOpenSSLShim

Both have __attribute__((constructor)), so I got confused. Initially I thought both functions cause the trouble.
Only InitializeOpenSSLShim aborts if the library is not found.

@VSadov
Copy link
Member Author

VSadov commented Jan 13, 2021

This was fixed in #46640

@VSadov VSadov closed this as completed Jan 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Single-File untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

5 participants