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.

Sign up for GitHub

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

Jump to bottom

openssl: support OpenSSL 3.0 and above #1349

Merged
merged 1 commit into from
Jun 18, 2024
Merged

openssl: support OpenSSL 3.0 and above #1349

merged 1 commit into from
Jun 18, 2024

Conversation

alaviss
Copy link
Contributor

@alaviss alaviss commented Jun 17, 2024 •
edited by github-actions bot
Loading

Summary

The only ABI change between version 1.1.1 and 3.0 for our usage is
SSL_get_peer_certificate being splitted into
SSL_get1_peer_certificate (which is compatible with the prior symbol)
and SSL_get0_peer_certificate .

This PR modifies SSL_get_peer_certificate in the wrapper to use the
new symbol if available. No changes in other programs are required.

Details

  • SSL_get_peer_certificate will now select either
    SSL_get1_peer_certificate or SSL_get_peer_certificate depending
    on which symbols are available.
  • DLL names for OpenSSL 3.x has been added for macOS, Windows and
    Linux.
  • The symbols used for certificate verification are no longer
    unconditionally hidden on Windows. They were hidden previously as Nim
    ships old OpenSSL 1.0 which did not have these symbols.

Fixes #1160

@alaviss
openssl: support OpenSSL 3.0 and above
293e280 
The only ABI change is SSL_get_peer_certificate being splitted into
SSL_get1_peer_certificate (which is compatible with the prior symbol)
and SSL_get0_peer_certificate.

SSL_get_peer_certificate in the wrapper has been modified to use the new
symbol if available. No changes in other programs are required.
zerbina
zerbina reviewed Jun 18, 2024
View reviewed changes
Copy link
Collaborator

@zerbina zerbina left a comment

Choose a reason for hiding this comment

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

I left a question regarding the removal of the not defined(windows) condition, but otherwise this looks good to me.

lib/wrappers/openssl.nim Show resolved Hide resolved
@zerbina
Copy link
Collaborator

zerbina commented Jun 18, 2024

/merge

@github-actions GitHub Actions
Copy link

Merge requested by: @zerbina

Contents after the first section break of the PR description has been removed and preserved below:

Notes for Reviewers

  • Compared to openssl: new wrapper targeting OpenSSL 3+ #1336, this PR does not attempt to improve the correctness in library usage within the stdlib, nor is the wrapper made to link with OpenSSL at build time.
  • We are still using a fair share of deprecated symbols, but they are not likely to be removed until OpenSSL 4.

@chore-runner chore-runner bot added this pull request to the merge queue Jun 18, 2024
@zerbina zerbina added enhancement New feature or request stdlib Standard library labels Jun 18, 2024
Merged via the queue into nim-works:devel with commit cdcf8f7 Jun 18, 2024
31 checks passed
@alaviss alaviss deleted the push-vynxrlyullrk branch June 18, 2024 21:05
@alaviss alaviss mentioned this pull request Jun 18, 2024
Closed
@alaviss alaviss mentioned this pull request Jul 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zerbina zerbina zerbina approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request stdlib Standard library
Projects
None yet
Milestone
Standard library additions or cleanup
Development

Successfully merging this pull request may close these issues.

add OpenSSL 3.0+ support
2 participants
@alaviss @zerbina