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

Ubuntu 24.04 & Nginx 1.24.0 compat work #20

Merged
merged 10 commits into from
May 2, 2024

Conversation

cpu
Copy link
Member

@cpu cpu commented May 2, 2024

Ctz and I both riffed on this branch, working towards initial support for Nginx on Ubuntu 24.04.

At a high-level this involves:

  • Updating the testing infrastructure to add a Dockerfile and associated bits for testing on Ubuntu 24.04. This LTS release is brand spanking new and so there are no GitHub runner images available for it at this time.
  • Implementing some SSL_SESSION manipulation APIs
  • Implementing SSL_get_SSL_CTX
  • Implementing SSL_version
  • Stubbing out the SSL_CONF_* API surface (this will likely deserve revisiting soon to implement some config manipulation we could support)
  • Stubbing out SSL_sendfile (this is kTLS specific in the OpenSSL API)

With these changes in place the Nginx 1.24.0 install is operable in a basic form with Rustls acting as libssl.

@cpu cpu assigned cpu and unassigned cpu May 2, 2024
Copy link
Member

@ctz ctz left a comment

Choose a reason for hiding this comment

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

final 4 commits lgtm!

Copy link
Member Author

@cpu cpu left a comment

Choose a reason for hiding this comment

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

I can't "approve" since I opened the PR, but I've reviewed the early commits and do approve ☑️ Just one comment to ask about.

rustls-libssl/src/entry.rs Outdated Show resolved Hide resolved
ctz and others added 7 commits May 2, 2024 18:45
Ensure limitations on ID contexts are applied.
Similar to the already implemented `SSL_get_version()`, but returning
the IANA registered protocol identifier constant as an `int`, instead of
its name as a `*char`.
Trying to avoid a second test instance run after the first encountering
an error of the form:
```
Bind for 0.0.0.0:8443 failed: port is already allocated.
```
This is from an optional package.  Instead rely on sh and /etc/lsb-release.
@ctz ctz force-pushed the cpu-jbp-nginx-on-ubuntu-24 branch from e974b84 to 7e2202d Compare May 2, 2024 17:46
Copy link
Member Author

@cpu cpu left a comment

Choose a reason for hiding this comment

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

Latest diff LGTM

@ctz ctz merged commit 5394c7e into rustls:main May 2, 2024
11 checks passed
@cpu cpu deleted the cpu-jbp-nginx-on-ubuntu-24 branch May 2, 2024 18:26
@cpu cpu mentioned this pull request May 3, 2024
@simondeziel
Copy link

simondeziel commented May 30, 2024

Updating the testing infrastructure to add a Dockerfile and associated bits for testing on Ubuntu 24.04. This LTS release is brand spanking new and so there are no GitHub runner images available for it at this time.

ubuntu-24.04 is available now (actions/runner-images#9848). ubuntu-latest still points to ubuntu-22.04 for the time being though.

@cpu
Copy link
Member Author

cpu commented May 30, 2024

Thanks for the note, appreciated :-) We should look at adding that in to CI.

@cpu
Copy link
Member Author

cpu commented May 30, 2024

We should look at adding that in to CI.

I started on this, but I think I'm bumping into the issue described in actions/runner-images#9959

My 24.04 runs (exemplar) are dying in an early job step running sudo apt-get update && sudo apt-get install -y openssl libssl3 libssl-dev lld with:

Error: The operation was canceled.

I'll keep an eye on that upstream issue and try again once it has been resolved.

@cpu
Copy link
Member Author

cpu commented Jun 21, 2024

I'll keep an eye on that upstream issue and try again once it has been resolved.

FWIW I rolled this into #29 because I needed nginx 1.24+ for an integration test in that branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants