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

Server-side resumption support #13

Merged
merged 16 commits into from
Apr 30, 2024
Merged

Server-side resumption support #13

merged 16 commits into from
Apr 30, 2024

Conversation

ctz
Copy link
Member

@ctz ctz commented Apr 26, 2024

This PR has as a goal getting the tests in the final commit to pass. Those extend the existing nginx integration test to start 4 different servers with varying resumption options (none, per-worker, per-server, and per-worker+server together) and see that resumption works in a basic sense.

Some of the earlier commits put into place infrastructure that isn't fully used until the last one (for example, the fields on SslSession).

@cpu cpu self-requested a review April 26, 2024 16:24
Copy link
Member

@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.

Nice work! I didn't have as much feedback through this branch. I think it was all fairly easy to understand 🚀

rustls-libssl/src/lib.rs Show resolved Hide resolved
rustls-libssl/src/lib.rs Outdated Show resolved Hide resolved
rustls-libssl/src/lib.rs Outdated Show resolved Hide resolved
rustls-libssl/src/lib.rs Outdated Show resolved Hide resolved
rustls-libssl/src/entry.rs Show resolved Hide resolved
rustls-libssl/src/cache.rs Outdated Show resolved Hide resolved
rustls-libssl/src/cache.rs Show resolved Hide resolved
rustls-libssl/src/cache.rs Outdated Show resolved Hide resolved
rustls-libssl/src/cache.rs Outdated Show resolved Hide resolved
rustls-libssl/src/cache.rs Show resolved Hide resolved
- `SSL_SESSION_get_id`
- `SSL_SESSION_up_ref`
- `SSL_SESSION_free`
- `d2i_SSL_SESSION`
- `i2d_SSL_SESSION`

This will only be used for server-side sessions presently.
(rustls client-side session types are not serializable.)

nb. as of this commit, it is not possible to get an `SSL_SESSION`
instance (except from deserialising one using `d2i_SSL_SESSION`,
except -- where did the serialisation come from?!)
@ctz ctz force-pushed the jbp-nginx-resumption branch from a8f3eb2 to d97d2dd Compare April 30, 2024 14:00
ctz added 15 commits April 30, 2024 15:14
This provides:

- `SSL_CTX_sess_set_cache_size`
- `SSL_CTX_sess_get_cache_size`
- `SSL_CTX_set_session_cache_mode`

(all of these are routed through `SSL_CTX_ctrl`).
This is very bare-bones: it respects the size, but not
the mode.
Just setter/getter for now.
This is needed later for it to be able to call `SSL_CTX_sess_remove_cb`.
Delete stub for `SSL_set_session_id_context`.
This has a `SSL_CTX`-scope object `ServerSessionStorage` that
does the actual storage, plus a `SSL`-scope object `SingleServerCache`
that delegates to that while tracking which `SSL_SESSION` was used
for that single connection.
This only works for server SSLs currently.
This is observable from openssl behaviour (even though it is
extremely unsound -- what happens if you `SSL_CTX_up_ref`
inside the callback? Instant UAF!)
Every 255 cache operations, clear out any expired sessions.
This means unlimited size.
@ctz ctz force-pushed the jbp-nginx-resumption branch from d97d2dd to 1b26307 Compare April 30, 2024 14:16
Copy link
Member

@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.

The updates LGTM. Thanks!

@ctz ctz merged commit 9a80ba5 into main Apr 30, 2024
20 checks passed
@ctz ctz deleted the jbp-nginx-resumption branch April 30, 2024 15:04
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.

2 participants