-
Notifications
You must be signed in to change notification settings - Fork 116
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
bound session cache #195
bound session cache #195
Conversation
When establishing new TLS sessions, servers may send multiple session tickets (RFC8446 4.6.1). hyper-boring caches tickets without placing a limit on how many tickets are cached. This leads to unbounded growth of hyper-boring's cache and leaves clients vulnerable to malicious servers who might send many session tickets to exhaust a client's available memory. This change bounds the cache to a default of 8 tickets.
hyper-boring/src/lib.rs
Outdated
/// | ||
/// The session cache configuration of `ssl` will be overwritten. Session capacity is per | ||
/// session key (domain). | ||
pub fn with_connector_and_capacity( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make it future-proof and use settings struct here:
struct HttpsLayerSettings {
session_cache_capacity: usize
}
and change this to:
pub fn with_connector_and_settings(...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing it that way adding a field to HttpsLayerSettings
is still a breaking change so it's not really future-proof. We may want a builder instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense, updated!
I think there was a discussion about what should the default max, but the discussion didn't reach a conclusion. Honestly I think we should just merge it. |
I'm in agreement here - let's just merge. |
I don't have permissions to do so, so this will need to be handled by someone else. |
Merged, thanks @ehaydenr |
When establishing new TLS sessions, servers may send multiple session tickets (RFC8446 4.6.1). hyper-boring caches tickets without placing a limit on how many tickets are cached. This leads to unbounded growth of hyper-boring's cache and leaves clients vulnerable to malicious servers who might send many session tickets to exhaust a client's available memory.
This change bounds the cache to a default of 8 tickets.