From 1e6510105e17f4686509b6cef5e4a607664dd5c0 Mon Sep 17 00:00:00 2001 From: tb <> Date: Tue, 1 Sep 2020 12:40:53 +0000 Subject: copy session id directly in ssl_get_prev_session ssl_get_prev_session() hands the session id down to tls_decrypt_ticket() which then copies it into the session pointer that it is about to return. It's a lot simpler to retrieve the session pointer and copy the session id inside ssl_get_prev_session(). Also, 'goto err' directly in TLS1_TICKET_NOT_DECRYPTED instead of skipping a couple of long if clauses before doing so. ok inoguchi jsing --- src/lib/libssl/ssl_locl.h | 5 ++--- src/lib/libssl/ssl_sess.c | 19 ++++++++++++++++--- src/lib/libssl/t1_lib.c | 26 +++++--------------------- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h index 2f8ba1fc09..bd210cdce5 100644 --- a/src/lib/libssl/ssl_locl.h +++ b/src/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.287 2020/09/01 05:32:11 tb Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.288 2020/09/01 12:40:53 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -1403,8 +1403,7 @@ int ssl_check_serverhello_tlsext(SSL *s); #define TLS1_TICKET_NOT_DECRYPTED 2 #define TLS1_TICKET_DECRYPTED 3 -int tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, - int *alert, SSL_SESSION **ret); +int tls1_process_ticket(SSL *s, CBS *ext_block, int *alert, SSL_SESSION **ret); long ssl_get_algorithm2(SSL *s); diff --git a/src/lib/libssl/ssl_sess.c b/src/lib/libssl/ssl_sess.c index b953580d65..460c5d85f1 100644 --- a/src/lib/libssl/ssl_sess.c +++ b/src/lib/libssl/ssl_sess.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_sess.c,v 1.91 2020/09/01 06:05:09 tb Exp $ */ +/* $OpenBSD: ssl_sess.c,v 1.92 2020/09/01 12:40:53 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -420,7 +420,6 @@ ssl_get_new_session(SSL *s, int session) * session_id: points at the session ID in the ClientHello. This code will * read past the end of this in order to parse out the session ticket * extension, if any. - * session_id_len: the length of the session ID. * ext_block: a CBS for the ClientHello extensions block. * * Returns: @@ -438,6 +437,7 @@ int ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert) { SSL_SESSION *sess = NULL; + size_t session_id_len; int alert_desc = SSL_AD_INTERNAL_ERROR, fatal = 0; int try_session_cache = 1; @@ -450,7 +450,7 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert) try_session_cache = 0; /* Sets s->internal->tlsext_ticket_expected. */ - switch (tls1_process_ticket(s, session_id, ext_block, &alert_desc, &sess)) { + switch (tls1_process_ticket(s, ext_block, &alert_desc, &sess)) { case TLS1_TICKET_FATAL_ERROR: fatal = 1; goto err; @@ -458,8 +458,21 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert) case TLS1_TICKET_EMPTY: break; /* Ok to carry on processing session id. */ case TLS1_TICKET_NOT_DECRYPTED: + try_session_cache = 0; + goto err; case TLS1_TICKET_DECRYPTED: try_session_cache = 0; + + /* + * The session ID is used by some clients to detect that the + * ticket has been accepted so we copy it into sess. + */ + if (!CBS_write_bytes(session_id, sess->session_id, + sizeof(sess->session_id), &session_id_len)) { + fatal = 1; + goto err; + } + sess->session_id_length = (unsigned int)session_id_len; break; default: SSLerror(s, ERR_R_INTERNAL_ERROR); diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c index 8162259c66..dc6ffae418 100644 --- a/src/lib/libssl/t1_lib.c +++ b/src/lib/libssl/t1_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_lib.c,v 1.173 2020/09/01 05:38:48 tb Exp $ */ +/* $OpenBSD: t1_lib.c,v 1.174 2020/09/01 12:40:53 tb Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -122,7 +122,7 @@ #include "ssl_sigalgs.h" #include "ssl_tlsext.h" -static int tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert, +static int tls_decrypt_ticket(SSL *s, CBS *ticket, int *alert, SSL_SESSION **psess); SSL3_ENC_METHOD TLSv1_enc_data = { @@ -755,7 +755,6 @@ ssl_check_serverhello_tlsext(SSL *s) * ClientHello, and other operations depend on the result, we need to handle * any TLS session ticket extension at the same time. * - * session_id: a CBS containing the session ID. * ext_block: a CBS for the ClientHello extensions block. * ret: (output) on return, if a ticket was decrypted, then this is set to * point to the resulting session. @@ -783,8 +782,7 @@ ssl_check_serverhello_tlsext(SSL *s) * Otherwise, s->internal->tlsext_ticket_expected is set to 0. */ int -tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert, - SSL_SESSION **ret) +tls1_process_ticket(SSL *s, CBS *ext_block, int *alert, SSL_SESSION **ret) { CBS extensions, ext_data; uint16_t ext_type = 0; @@ -844,12 +842,11 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert, return TLS1_TICKET_NOT_DECRYPTED; } - return tls_decrypt_ticket(s, session_id, &ext_data, alert, ret); + return tls_decrypt_ticket(s, &ext_data, alert, ret); } /* tls_decrypt_ticket attempts to decrypt a session ticket. * - * session_id: a CBS containing the session ID. * ticket: a CBS containing the body of the session ticket extension. * psess: (output) on return, if a ticket was decrypted, then this is set to * point to the resulting session. @@ -860,14 +857,12 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert, * TLS1_TICKET_DECRYPTED: a ticket was decrypted and *psess was set. */ static int -tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert, - SSL_SESSION **psess) +tls_decrypt_ticket(SSL *s, CBS *ticket, int *alert, SSL_SESSION **psess) { CBS ticket_name, ticket_iv, ticket_encdata, ticket_hmac; SSL_SESSION *sess = NULL; unsigned char *sdec = NULL; size_t sdec_len = 0; - size_t session_id_len; const unsigned char *p; unsigned char hmac[EVP_MAX_MD_SIZE]; HMAC_CTX *hctx = NULL; @@ -990,17 +985,6 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert, p = sdec; if ((sess = d2i_SSL_SESSION(NULL, &p, slen)) == NULL) goto derr; - - /* - * The session ID, if non-empty, is used by some clients to detect that - * the ticket has been accepted. So we copy it to the session structure. - * If it is empty set length to zero as required by standard. - */ - if (!CBS_write_bytes(session_id, sess->session_id, - sizeof(sess->session_id), &session_id_len)) - goto err; - sess->session_id_length = (unsigned int)session_id_len; - *psess = sess; sess = NULL; -- cgit v1.2.3-55-g6feb