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

Fix and align ticket age check in ssl_ticket.c for TLS 1.2 and TLS 1.3 #8574

Merged
merged 16 commits into from
Feb 21, 2024
+233 −176
Merged

Fix and align ticket age check in ssl_ticket.c for TLS 1.2 and TLS 1.3 #8574

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
d1100b0
Disable ticket module when useless
ronald-cron-arm Nov 21, 2023
ce72763
ssl_ticket.c: Remove client code
ronald-cron-arm Nov 22, 2023
3c3e2e6
ssl_ticket.c: Remove TLS server guard
ronald-cron-arm Nov 22, 2023
e34f124
ssl_ticket.c: Remove pedantic server endpoint check
ronald-cron-arm Nov 22, 2023
ba5165e
ssl_ticket.c: Fix ticket lifetime enforcement
ronald-cron-arm Nov 21, 2023
17ef8df
ssl_session: Define unconditionally the endpoint field
ronald-cron-arm Nov 22, 2023
7b1921a
Add endpoint in TLS 1.2 session serialization data
ronald-cron-arm Nov 23, 2023
feb577a
Fix TLS 1.2 session serialization on server side
ronald-cron-arm Nov 23, 2023
d1c106c
Define ticket creation time in TLS 1.2 case as well
ronald-cron-arm Nov 22, 2023
c57f86e
Add ticket creation time to TLS 1.2 session serialization
ronald-cron-arm Nov 22, 2023
3c0072b
ssl_ticket.c: Base ticket age check on the ticket creation time
ronald-cron-arm Nov 22, 2023
40a4ab0
ssl_tls.c: Factorize save/load of endpoint and ciphersuite
ronald-cron-arm Jan 15, 2024
7fdee8b
ssl_session: Reorder some fields to reduce padding
ronald-cron-arm Feb 9, 2024
c7fa82e
tests: ssl: Improve test parameter sanity check
ronald-cron-arm Feb 9, 2024
7b0ac0b
Add change log for mbedtls_ssl_session_get_ticket_creation_time()
ronald-cron-arm Feb 9, 2024
a93e25e
tls12: Fix documentation of TLS 1.2 session serialized data
ronald-cron-arm Feb 9, 2024
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
4 changes: 4 additions & 0 deletions include/mbedtls/config_adjust_ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@
#undef MBEDTLS_SSL_PROTO_DTLS
#endif

#if !(defined(MBEDTLS_SSL_SRV_C) && defined(MBEDTLS_SSL_SESSION_TICKETS))
#undef MBEDTLS_SSL_TICKET_C
#endif

#if !defined(MBEDTLS_SSL_PROTO_DTLS)
#undef MBEDTLS_SSL_DTLS_ANTI_REPLAY
#undef MBEDTLS_SSL_DTLS_CONNECTION_ID
Expand Down
62 changes: 53 additions & 9 deletions include/mbedtls/ssl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,7 @@ struct mbedtls_ssl_session {
* or resuming a session instead of the configured minor TLS version.
*/
mbedtls_ssl_protocol_version MBEDTLS_PRIVATE(tls_version);
uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */

#if defined(MBEDTLS_HAVE_TIME)
mbedtls_time_t MBEDTLS_PRIVATE(start); /*!< start time of current session */
Expand Down Expand Up @@ -1227,8 +1228,28 @@ struct mbedtls_ssl_session {
uint32_t MBEDTLS_PRIVATE(ticket_lifetime); /*!< ticket lifetime hint */
#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_CLI_C */

#if defined(MBEDTLS_SSL_SESSION_TICKETS) && defined(MBEDTLS_SSL_SRV_C) && \
defined(MBEDTLS_HAVE_TIME)
/*! When a ticket is created by a TLS server as part of an established TLS
* session, the ticket creation time may need to be saved for the ticket
* module to be able to check the ticket age when the ticket is used.
* That's the purpose of this field.
* Before creating a new ticket, an Mbed TLS server set this field with
* its current time in milliseconds. This time may then be saved in the
* session ticket data by the session ticket writing function and
* recovered by the ticket parsing function later when the ticket is used.
Comment on lines +1237 to +1240
Copy link
Contributor

Choose a reason for hiding this comment

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

Info: why do it this way, rather than letting the ticket writing function query the current time if it wants? That would seem more natural to me.

If this was a session creation time used for multiple purposes, this approach would be necessary, since there would have to be a single point where the official session creation time is decided. But for the ticket creation time, the (single) ticket writing function is the natural place.

(I realize that this is not new from this pull request: it's from #6788, which isn't released yet, and which AFAICT wasn't part of the public API until this PR which exposes it via mbedtls_ssl_session_get_ticket_creation_time.)

Copy link
Contributor Author

@ronald-cron-arm ronald-cron-arm Feb 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

First, thanks for the review.

Info: why do it this way, rather than letting the ticket writing function query the current time if it wants?

One important point is that when using the ticket the TLS 1.3 server needs the ticket_creation_time, it cannot be only a ticket module data. Thus it means that the ticket_creation_time needs to be in the ticket data.
In our implementation of mbedtls_ssl_ticket_write_t (and it is expected to be so in third party ones) the creation of the ticket data is based on mbedtls_ssl_session_save() thus when mbedtls_ssl_session_save() is called the
ticket_creation_time of the mbedtls_ssl_session structure must be set.

We could do it in mbedtls_ssl_ticket_write() but it is not straightforward as the session input parameter is a const: const mbedtls_ssl_session *session (we would need to copy the session ...). And if we do it that way with another implementation of mbedtls_ssl_ticket_write_t based on mbedtls_ssl_session_save() we would not add the ticket_creation_time in the ticket data.

* The ticket module may then use this time to compute the ticket age and
* determine if it has expired or not.
* The Mbed TLS implementations of the session ticket writing and parsing
* functions save and retrieve the ticket creation time as part of the
* session ticket data. The session ticket parsing function relies on
* the mbedtls_ssl_session_get_ticket_creation_time() API to get the
* ticket creation time from the session ticket data.
*/
mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time);
#endif

#if defined(MBEDTLS_SSL_PROTO_TLS1_3) && defined(MBEDTLS_SSL_SESSION_TICKETS)
uint8_t MBEDTLS_PRIVATE(endpoint); /*!< 0: client, 1: server */
uint8_t MBEDTLS_PRIVATE(ticket_flags); /*!< Ticket flags */
uint32_t MBEDTLS_PRIVATE(ticket_age_add); /*!< Randomly generated value used to obscure the age of the ticket */
uint8_t MBEDTLS_PRIVATE(resumption_key_len); /*!< resumption_key length */
Expand All @@ -1238,15 +1259,10 @@ struct mbedtls_ssl_session {
char *MBEDTLS_PRIVATE(hostname); /*!< host name binded with tickets */
#endif /* MBEDTLS_SSL_SERVER_NAME_INDICATION && MBEDTLS_SSL_CLI_C */

#if defined(MBEDTLS_HAVE_TIME)
#if defined(MBEDTLS_SSL_CLI_C)
mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time); /*!< time when ticket was received. */
#if defined(MBEDTLS_HAVE_TIME) && defined(MBEDTLS_SSL_CLI_C)
/*! Time in milliseconds when the last ticket was received. */
mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_reception_time);
#endif
#if defined(MBEDTLS_SSL_SRV_C)
mbedtls_ms_time_t MBEDTLS_PRIVATE(ticket_creation_time); /*!< time when ticket was created. */
#endif
#endif /* MBEDTLS_HAVE_TIME */

#endif /* MBEDTLS_SSL_PROTO_TLS1_3 && MBEDTLS_SSL_SESSION_TICKETS */

#if defined(MBEDTLS_SSL_EARLY_DATA)
Expand Down Expand Up @@ -2572,6 +2588,34 @@ void mbedtls_ssl_conf_session_tickets_cb(mbedtls_ssl_config *conf,
mbedtls_ssl_ticket_write_t *f_ticket_write,
mbedtls_ssl_ticket_parse_t *f_ticket_parse,
void *p_ticket);

#if defined(MBEDTLS_HAVE_TIME)
/**
* \brief Get the creation time of a session ticket.
*
* \note See the documentation of \c ticket_creation_time for information about
* the intended usage of this function.
*
* \param session SSL session
* \param ticket_creation_time On exit, holds the ticket creation time in
* milliseconds.
*
* \return 0 on success,
* MBEDTLS_ERR_SSL_BAD_INPUT_DATA if an input is not valid.
*/
static inline int mbedtls_ssl_session_get_ticket_creation_time(
mbedtls_ssl_session *session, mbedtls_ms_time_t *ticket_creation_time)
{
if (session == NULL || ticket_creation_time == NULL ||
session->MBEDTLS_PRIVATE(endpoint) != MBEDTLS_SSL_IS_SERVER) {
return MBEDTLS_ERR_SSL_BAD_INPUT_DATA;
}

*ticket_creation_time = session->MBEDTLS_PRIVATE(ticket_creation_time);

return 0;
}
#endif /* MBEDTLS_HAVE_TIME */
#endif /* MBEDTLS_SSL_SESSION_TICKETS && MBEDTLS_SSL_SRV_C */

/**
Expand Down
4 changes: 4 additions & 0 deletions include/mbedtls/ssl_ticket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ typedef struct mbedtls_ssl_ticket_key {
#if defined(MBEDTLS_HAVE_TIME)
mbedtls_time_t MBEDTLS_PRIVATE(generation_time); /*!< key generation timestamp (seconds) */
#endif
/*! Lifetime of the key in seconds. This is also the lifetime of the
* tickets created under that key.
*/
uint32_t MBEDTLS_PRIVATE(lifetime);
#if !defined(MBEDTLS_USE_PSA_CRYPTO)
mbedtls_cipher_context_t MBEDTLS_PRIVATE(ctx); /*!< context for auth enc/decryption */
#else
Expand Down
60 changes: 23 additions & 37 deletions library/ssl_ticket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ static int ssl_ticket_gen_key(mbedtls_ssl_ticket_context *ctx,
#if defined(MBEDTLS_HAVE_TIME)
key->generation_time = mbedtls_time(NULL);
#endif
/* The lifetime of a key is the configured lifetime of the tickets when
* the key is created.
*/
key->lifetime = ctx->ticket_lifetime;

if ((ret = ctx->f_rng(ctx->p_rng, key->name, sizeof(key->name))) != 0) {
return ret;
Expand Down Expand Up @@ -116,16 +120,17 @@ static int ssl_ticket_update_keys(mbedtls_ssl_ticket_context *ctx)
#if !defined(MBEDTLS_HAVE_TIME)
((void) ctx);
#else
if (ctx->ticket_lifetime != 0) {
mbedtls_ssl_ticket_key * const key = ctx->keys + ctx->active;
if (key->lifetime != 0) {
mbedtls_time_t current_time = mbedtls_time(NULL);
mbedtls_time_t key_time = ctx->keys[ctx->active].generation_time;
mbedtls_time_t key_time = key->generation_time;

#if defined(MBEDTLS_USE_PSA_CRYPTO)
psa_status_t status = PSA_ERROR_CORRUPTION_DETECTED;
#endif

if (current_time >= key_time &&
(uint64_t) (current_time - key_time) < ctx->ticket_lifetime) {
(uint64_t) (current_time - key_time) < key->lifetime) {
return 0;
}

Expand Down Expand Up @@ -198,6 +203,8 @@ int mbedtls_ssl_ticket_rotate(mbedtls_ssl_ticket_context *ctx,
#if defined(MBEDTLS_HAVE_TIME)
key->generation_time = mbedtls_time(NULL);
#endif
key->lifetime = lifetime;

return 0;
}

Expand Down Expand Up @@ -331,7 +338,7 @@ int mbedtls_ssl_ticket_write(void *p_ticket,

key = &ctx->keys[ctx->active];

*ticket_lifetime = ctx->ticket_lifetime;
*ticket_lifetime = key->lifetime;

memcpy(key_name, key->name, TICKET_KEY_NAME_BYTES);

Expand Down Expand Up @@ -495,43 +502,22 @@ int mbedtls_ssl_ticket_parse(void *p_ticket,
}

#if defined(MBEDTLS_HAVE_TIME)
#if defined(MBEDTLS_SSL_PROTO_TLS1_3)
if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_3) {
/* Check for expiration */
mbedtls_ms_time_t ticket_age = -1;
#if defined(MBEDTLS_SSL_SRV_C)
if (session->endpoint == MBEDTLS_SSL_IS_SERVER) {
ticket_age = mbedtls_ms_time() - session->ticket_creation_time;
}
#endif
#if defined(MBEDTLS_SSL_CLI_C)
if (session->endpoint == MBEDTLS_SSL_IS_CLIENT) {
ticket_age = mbedtls_ms_time() - session->ticket_reception_time;
}
#endif

mbedtls_ms_time_t ticket_lifetime =
(mbedtls_ms_time_t) ctx->ticket_lifetime * 1000;
mbedtls_ms_time_t ticket_creation_time, ticket_age;
mbedtls_ms_time_t ticket_lifetime =
(mbedtls_ms_time_t) ctx->ticket_lifetime * 1000;

if (ticket_age < 0 || ticket_age > ticket_lifetime) {
ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED;
goto cleanup;
}
ret = mbedtls_ssl_session_get_ticket_creation_time(session,
&ticket_creation_time);
if (ret != 0) {
goto cleanup;
}
#endif /* MBEDTLS_SSL_PROTO_TLS1_3 */
#if defined(MBEDTLS_SSL_PROTO_TLS1_2)
if (session->tls_version == MBEDTLS_SSL_VERSION_TLS1_2) {
/* Check for expiration */
mbedtls_time_t current_time = mbedtls_time(NULL);

if (current_time < session->start ||
(uint32_t) (current_time - session->start) > ctx->ticket_lifetime) {
ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED;
goto cleanup;
}
ticket_age = mbedtls_ms_time() - ticket_creation_time;
if (ticket_age < 0 || ticket_age > ticket_lifetime) {
ret = MBEDTLS_ERR_SSL_SESSION_TICKET_EXPIRED;
goto cleanup;
}
#endif /* MBEDTLS_SSL_PROTO_TLS1_2 */
#endif /* MBEDTLS_HAVE_TIME */
#endif

cleanup:
#if defined(MBEDTLS_THREADING_C)
Expand Down
Loading