Skip to content

Conversation

@vcsjones
Copy link
Member

@vcsjones vcsjones commented Oct 24, 2025

This removes all logic from our native shim required to support < OpenSSL 1.1.1. This makes our OpenSSL floor 1.1.1.

The general changes are:

  1. apibridge.c is about creating OpenSSL 1.1.x-like functions from the non-opaque structs. We don't need to shim those anymore, so the file is gone.
  2. LEGACY_FUNCTIONs went away.
  3. FALLBACK_FUNCTION and REQUIRED_FUNCTION_110 are now REQUIRED_FUNCTIONs.
  4. Any code paths were for older OpenSSLs have been removed.
  5. OpenSSL 1.0 initialization has been removed. The portable build will no longer look for 1.0 versions.

@Copilot Copilot AI review requested due to automatic review settings October 24, 2025 18:30
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes support for OpenSSL versions 1.0.2 and 1.1.0, establishing OpenSSL 1.1.1 as the minimum required version. This cleanup simplifies the native interop layer by eliminating compatibility shims and conditional compilation.

Key changes:

  • Removed all API bridging code for OpenSSL < 1.1.1
  • Converted all FALLBACK_FUNCTION, LEGACY_FUNCTION, and REQUIRED_FUNCTION_110 declarations to REQUIRED_FUNCTION
  • Eliminated version-specific initialization and detection logic

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pal_ssl.c Removed OpenSSL 1.0-specific initialization, ECDH setup, and conditional API existence checks
pal_hmac.h Updated documentation to reflect direct use of 1.1.1+ APIs
pal_evp_cipher.h Updated documentation to reflect direct use of 1.1.1+ APIs
osslcompat_111.h Deleted - function prototypes for 1.1.x compatibility no longer needed
osslcompat_102.h Deleted - function prototypes for 1.0.2 compatibility no longer needed
opensslshim.h Removed 1.0.x macros, simplified function declarations, removed legacy includes
opensslshim.c Removed 1.0.x library loading and function resolution logic
openssl_1_0_structs.h Deleted - OpenSSL 1.0 struct definitions no longer needed
openssl.c Removed 1.0.x initialization code, threading locks, and version detection
apibridge.h Deleted - API bridge function declarations no longer needed
apibridge.c Deleted - implementation of OpenSSL 1.1.x-like functions for 1.0.x removed
CMakeLists.txt Removed apibridge.c from build sources
OpenSslX509ChainProcessor.cs Removed 1.0.2 error code mapping function
Interop.X509.cs Removed X509VerifyStatusCode102 enum

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@vcsjones vcsjones added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Oct 24, 2025
@vcsjones
Copy link
Member Author

Marking this as no-merge for now because this needs manual testing against different build configurations not represented in CI.

#else
ret = EnsureOpenSsl11Initialized();
#endif
int ret = EnsureOpenSsl11Initialized();
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to move this back to a split decl and assignment. There was recently a PR to fix the build on some version of a toolchain that seemed to be requiring C "variables may only be declared at the beginning of a block".

Copy link
Member Author

Choose a reason for hiding this comment

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

There was recently a PR to fix the build on some version of a toolchain that seemed to be requiring C "variables may only be declared at the beginning of a block".

That was not the intent of that pull request. It was about making stack-based initializations were not skipped by goto.

@bartonjs
Copy link
Member

Generally LGTM. I'm waiting to officially sign off until the other testing has been completed, since it sounds like there's a fair amount of room for changes required.

@vcsjones
Copy link
Member Author

@vcsjones-bot test 951ae32 with openssl-3.6.0
@vcsjones-bot test 951ae32 with openssl-3.6.0, nonportable
@vcsjones-bot test 951ae32 with OpenSSL_1_1_1w
@vcsjones-bot test 951ae32 with OpenSSL_1_1_1w, nonportable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Security NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants