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

Make portable builds work across OpenSSL 1.0.2/1.1.1/3.0 #51155

Merged
merged 2 commits into from
Apr 14, 2021

Conversation

bartonjs
Copy link
Member

Overall structure of changes

  • Pull compatibility headers out into separate include files, because opensslshim.h is too big.
  • Use forward definition of EVP_PKEY_CTX_set_rsa_keygen_bits and friends.
    • These are in a new apibridge file because they're for bridging up to 3.0, and the existing one was for 1.1(.1)
    • Some constants needed for this file changed between 1.1 and 3.0, so there are a lot of asserts and redefines.
  • On OpenSSL 3.0, build a legacy version of ERR_put_error since it has the easier signature to work with.
  • FALLBACK_FUNCTION doesn't care which version it bound to, if it doesn't find it use a local_ function.
  • Renamed NEW_REQUIRED_FUNCTION to REQUIRED_FUNCTION_110 because "new" is now "sort of old".
  • There's a manual sanity test that either ERR_put_error or the three new functions that together replace it are found, so we don't end up in a state where we can't report shim-injected errors.

Portable build checker:

  • Built with OpenSSL 1.0.2 headers (Ubuntu 16.04 default libssl-dev)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)
  • Built with OpenSSL 1.1.1 headers (Ubuntu 18.04 default libssl-dev)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)
  • Built with OpenSSL 3.0 headers (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13 and some surgery to the extra_libs.cmake)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)

3.0 doesn't run error-free, but it runs with the same error rate from portable and direct builds. All verification was limited to the System.Security.Cryptography.Algorithms.Tests run, but that's generally representative of the bindings.

Contributes to #46526.

@bartonjs bartonjs added this to the 6.0.0 milestone Apr 13, 2021
@bartonjs bartonjs self-assigned this Apr 13, 2021
@ghost
Copy link

ghost commented Apr 13, 2021

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

Issue Details

Overall structure of changes

  • Pull compatibility headers out into separate include files, because opensslshim.h is too big.
  • Use forward definition of EVP_PKEY_CTX_set_rsa_keygen_bits and friends.
    • These are in a new apibridge file because they're for bridging up to 3.0, and the existing one was for 1.1(.1)
    • Some constants needed for this file changed between 1.1 and 3.0, so there are a lot of asserts and redefines.
  • On OpenSSL 3.0, build a legacy version of ERR_put_error since it has the easier signature to work with.
  • FALLBACK_FUNCTION doesn't care which version it bound to, if it doesn't find it use a local_ function.
  • Renamed NEW_REQUIRED_FUNCTION to REQUIRED_FUNCTION_110 because "new" is now "sort of old".
  • There's a manual sanity test that either ERR_put_error or the three new functions that together replace it are found, so we don't end up in a state where we can't report shim-injected errors.

Portable build checker:

  • Built with OpenSSL 1.0.2 headers (Ubuntu 16.04 default libssl-dev)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)
  • Built with OpenSSL 1.1.1 headers (Ubuntu 18.04 default libssl-dev)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)
  • Built with OpenSSL 3.0 headers (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13 and some surgery to the extra_libs.cmake)
    • Ran with 1.0.2 (Ubuntu 16.04 default libssl)
    • Ran with 1.1.1 (Ubuntu 18.04 default libssl)
    • Ran with 3.0 (Ubuntu 16.04 with local build of OpenSSL 3.0 alpha 13)

3.0 doesn't run error-free, but it runs with the same error rate from portable and direct builds. All verification was limited to the System.Security.Cryptography.Algorithms.Tests run, but that's generally representative of the bindings.

Contributes to #46526.

Author: bartonjs
Assignees: bartonjs
Labels:

area-System.Security

Milestone: 6.0.0

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@bartonjs bartonjs merged commit 7c148c4 into dotnet:main Apr 14, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 15, 2021
@bartonjs bartonjs removed their assignment Jul 26, 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.

2 participants