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

refactor: add key as a parameter for s2n_resume_encrypt_session_ticket #5014

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

boquan-fang
Copy link
Contributor

Release Summary:

Resolved issues:

Partial for issue #4583 and #2756.

Description of changes:

This PR is to address a comment in PR#5001. Currently s2n_resume_encrypt_session_ticket() picks a session ticket key inside the function:

s2n-tls/tls/s2n_resume.c

Lines 818 to 825 in edf68e0

S2N_RESULT s2n_resume_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *to)
{
RESULT_ENSURE_REF(conn);
RESULT_ENSURE_REF(to);
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
/* No keys loaded by the user or the keys are either in decrypt-only or expired state */
RESULT_ENSURE(key != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);

The s2n_get_ticket_encrypt_decrypt_key() function is non-deterministic, so we might end up with different session ticket keys if we call them multiple times. We need to specify the session ticket key outside of the function, so that when we are calculating the session ticket lifetime, S2N will use the same session ticket key for both lifetime calculation and session ticket encryption.

Hence, this PR adds struct s2n_ticket_key *key as an additional parameter in s2n_resume_encrypt_session_ticket(), and chooses the key before calling s2n_resume_encrypt_session_ticket() every where in the codebase.

Call-outs:

  • I made s2n_get_ticket_encrypt_decrypt_key() function available in s2n_resume.h, because this function needs to be called in s2n_server_nst_send() and s2n_tls13_server_nst_write() functions.
  • I will need to merge this PR first before continue to work on PR fix: calculation of session ticket age #5001. That PR should serve for fixing the calculation only.

Testing:

This PR is tested locally and by the CI.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@github-actions github-actions bot added the s2n-core team label Jan 9, 2025
@boquan-fang boquan-fang marked this pull request as ready for review January 9, 2025 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant