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

SSL_new_session_ticket() support for QUIC #26

Merged
merged 3 commits into from
Apr 9, 2021
Merged

SSL_new_session_ticket() support for QUIC #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c47e725
Let SSL_new_session_ticket() work immediately
kaduk Apr 2, 2021
85fd72e
Test new SSL_new_session_ticket() functionality
kaduk Apr 9, 2021
6cf11be
Update SSL_new_session_ticket() manual for triggered send
kaduk Apr 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 16 additions & 11 deletions doc/man3/SSL_CTX_set_num_tickets.pod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,22 @@ sent.
To issue tickets after other events (such as application-layer changes),
SSL_new_session_ticket() is used by a server application to request that a new
ticket be sent when it is safe to do so. New tickets are only allowed to be
sent in this manner after the initial handshake has completed, and only for TLS
1.3 connections. The ticket generation and transmission are delayed until the
server is starting a new write operation, so that it is bundled with other
application data being written and properly aligned to a record boundary.
SSL_new_session_ticket() can be called more than once to request additional
tickets be sent; all such requests are queued and written together when it is
safe to do so. Note that a successful return from SSL_new_session_ticket()
indicates only that the request to send a ticket was processed, not that the
ticket itself was sent. To be notified when the ticket itself is sent, a
new-session callback can be registered with L<SSL_CTX_sess_set_new_cb(3)> that
will be invoked as the ticket or tickets are generated.
sent in this manner after the initial handshake has completed, and only for
TLS 1.3 connections. By default, the ticket generation and transmission are
delayed until the server is starting a new write operation, so that it is
bundled with other application data being written and properly aligned to a
record boundary. If the connection was at a record boundary when
SSL_new_session_ticket() was called, the ticket can be sent immediately
(without waiting for the next application write) by calling
SSL_do_handshake(). SSL_new_session_ticket() can be called more than once to
request additional tickets be sent; all such requests are queued and written
together when it is safe to do so and triggered by SSL_write() or
SSL_do_handshake(). Note that a successful return from
SSL_new_session_ticket() indicates only that the request to send a ticket was
processed, not that the ticket itself was sent. To be notified when the
ticket itself is sent, a new-session callback can be registered with
L<SSL_CTX_sess_set_new_cb(3)> that will be invoked as the ticket or tickets
are generated.

SSL_CTX_get_num_tickets() and SSL_get_num_tickets() return the number of
tickets set by a previous call to SSL_CTX_set_num_tickets() or
Expand Down
6 changes: 5 additions & 1 deletion ssl/ssl_lib.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2360,10 +2360,14 @@ int SSL_renegotiate_pending(const SSL *s)

int SSL_new_session_ticket(SSL *s)
{
if (SSL_in_init(s) || SSL_IS_FIRST_HANDSHAKE(s) || !s->server
/* If we are in init because we're sending tickets, okay to send more. */
if ((SSL_in_init(s) && s->ext.extra_tickets_expected == 0)
|| SSL_IS_FIRST_HANDSHAKE(s) || !s->server
|| !SSL_IS_TLS13(s))
return 0;
s->ext.extra_tickets_expected++;
if (s->rlayer.wbuf[0].left == 0 && !SSL_in_init(s))
ossl_statem_set_in_init(s, 1);
Copy link
Member

Choose a reason for hiding this comment

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

How does this impact non-QUIC?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same check for "are we at a record boundary" that's already used (for non-QUIC) at the start of ssl3_write_bytes() where we would otherwise be writing the tickets out. The point of this commit is to evaluate the condition when SSL_new_session_ticket() is called instead of waiting until an application-data write to check.

Copy link
Member

Choose a reason for hiding this comment

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

Just wanted to make sure nothing bad happened in that case!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a good question to ask!
I believe it is fine, though, and the test coverage here is pretty good.

return 1;
}

Expand Down
26 changes: 23 additions & 3 deletions test/sslapitest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2483,11 +2483,22 @@ static int test_extra_tickets(int idx)
|| !TEST_int_eq(4, new_called))
goto end;

/* Once more, but with SSL_do_handshake() to drive the ticket generation */
c = '4';
new_called = 0;
if (!TEST_true(SSL_new_session_ticket(serverssl))
|| !TEST_true(SSL_new_session_ticket(serverssl))
|| !TEST_true(SSL_do_handshake(serverssl))
|| !TEST_int_eq(2, new_called)
|| !TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf), &nbytes))
|| !TEST_int_eq(4, new_called))
goto end;

/*
* Use the always-retry BIO to exercise the logic that forces ticket
* generation to wait until a record boundary.
*/
c = '4';
c = '5';
new_called = 0;
tmp = SSL_get_wbio(serverssl);
if (!TEST_ptr(tmp) || !TEST_true(BIO_up_ref(tmp))) {
Expand All @@ -2503,9 +2514,14 @@ static int test_extra_tickets(int idx)
/* Restore a BIO that will let the write succeed */
SSL_set0_wbio(serverssl, tmp);
tmp = NULL;
/* These calls should just queue the request and not send anything. */
/*
* These calls should just queue the request and not send anything
* even if we explicitly try to hit the state machine.
*/
if (!TEST_true(SSL_new_session_ticket(serverssl))
|| !TEST_true(SSL_new_session_ticket(serverssl))
|| !TEST_int_eq(0, new_called)
|| !TEST_true(SSL_do_handshake(serverssl))
|| !TEST_int_eq(0, new_called))
goto end;
/* Re-do the write; still no tickets sent */
Expand All @@ -2518,8 +2534,12 @@ static int test_extra_tickets(int idx)
|| !TEST_int_eq(c, buf[0])
|| !TEST_false(SSL_read_ex(clientssl, buf, sizeof(buf), &nbytes)))
goto end;
/* Even trying to hit the state machine now will still not send tickets */
if (!TEST_true(SSL_do_handshake(serverssl))
|| !TEST_int_eq(0, new_called))
goto end;
/* Now the *next* write should send the tickets */
c = '5';
c = '6';
if (!TEST_true(SSL_write_ex(serverssl, &c, 1, &nbytes))
|| !TEST_size_t_eq(1, nbytes)
|| !TEST_int_eq(2, new_called)
Expand Down