refactor: prevent sending zero lifetime new session ticket

[boquan-fang]

Release Summary:

Resolved issues:

resolves #2756.

Description of changes:

This PR resolves the concerns is issue #2756, which asks for

We should also probably handle the case where the result is 0-- should we still send the ticket?

If a new session ticket has zero lifetime, then we shouldn't send it. Hence, this PR will prevent sending new session tickets which have zero lifetime.

Add a check for ticket_lifetime_in_sec in s2n_tls13_server_nst_write() function.

We send new session ticket with this logic:

s2n-tls/tls/s2n_server_new_session_ticket.c

Lines 176 to 182 in fd41da0

	while (conn->tickets_to_send - conn->tickets_sent > 0) {
	if (s2n_result_is_error(s2n_tls13_server_nst_write(conn, nst_stuffer))) {
	return S2N_RESULT_OK;
	}
	RESULT_GUARD(s2n_post_handshake_write_records(conn, blocked));
	}

By adding a checking logic for zero session ticket lifetime in s2n_tls13_server_nst_write(), the s2n_tls13_server_nst_send() function can capture the error and stop sending the zero lifetime new session ticket by returning S2N_RESULT_OK.

Add one more tests in s2n_server_new_session_ticket_test.c to test sending zero lifetime new session ticket.

I intentionally set the tickets_to_send variable to one, so that the s2n_tls13_server_nst_send() function will attempt to send the ticket. I also set the ticket lifetime to zero, so that s2n_generate_ticket_lifetime() function will make the ticket age to be zero. Then in the test, we check for the following:

1. There is zero ticket sent.
2. The stuffer associated with the connection has nothing written in it.

The test logic is similar to the one already in s2n_server_new_session_ticket_test.c.

s2n-tls/tests/unit/s2n_server_new_session_ticket_test.c

Lines 984 to 1017 in fd41da0

	{
	struct s2n_config *config = NULL;
	struct s2n_connection *conn = NULL;
	EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER));
	EXPECT_NOT_NULL(config = s2n_config_new());
	EXPECT_OK(s2n_resumption_test_ticket_key_setup(config));
	EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
	conn->actual_protocol_version = S2N_TLS13;
	conn->secure->cipher_suite = &s2n_tls13_aes_128_gcm_sha256;
	conn->tickets_to_send = 1;
	EXPECT_NOT_EQUAL(s2n_stuffer_space_remaining(&conn->handshake.io), 0);
	/* Setup io */
	struct s2n_stuffer stuffer = { 0 };
	EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));
	EXPECT_SUCCESS(s2n_connection_set_io_stuffers(&stuffer, &stuffer, conn));
	s2n_blocked_status blocked = 0;
	EXPECT_OK(s2n_tls13_server_nst_send(conn, &blocked));
	EXPECT_TICKETS_SENT(conn, 1);
	/* Check only one record was written */
	uint16_t record_len = 0;
	EXPECT_SUCCESS(s2n_stuffer_skip_read(&stuffer, RECORD_LEN_MARKER));
	EXPECT_SUCCESS(s2n_stuffer_read_uint16(&stuffer, &record_len));
	EXPECT_TRUE(record_len > 0);
	EXPECT_SUCCESS(s2n_stuffer_skip_read(&stuffer, record_len));
	EXPECT_EQUAL(s2n_stuffer_data_available(&stuffer), 0);
	EXPECT_SUCCESS(s2n_stuffer_free(&stuffer));
	EXPECT_SUCCESS(s2n_connection_free(conn));
	EXPECT_SUCCESS(s2n_config_free(config));
	};

Call-outs:

• Our current way of calculating session ticket age is problematic which is mentioned in issue #4583 and issue #2756. This problem will be fixed in PR #5001.

Testing:

I have mentioned how I add test in the Description of changes section. This PR is tested both locally and in the CI.

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