mischief/openbsd-src
1
0
Fork 0
mirror of https://github.com/openbsd/src.git synced 2025-01-10 06:47:55 -08:00
Code

Simplify session ticket parsing/handling.

Browse Source 
The original implementation is rather crazy and means that we effectively
have two lots of code that parse a ClientHello and two lots of code that
parse TLS extensions. Partially simplify this by passing a CBS containing
the extension block through to the session handling functions, removing the
need to reimplement the ClientHello parsing.

While here standarise on naming for session_id and session_id_len.

ok inoguchi@ tb@
This commit is contained in:
jsing 2018-08-24 18:10:25 +00:00
parent 5a15326f4f
commit 307759ee4c
4 changed files with 46 additions and 69 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
lib/libssl/ssl_locl.h
View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssl_locl.h,v 1.208 2018/08/24 17:30:32 jsing Exp $ */
/* $OpenBSD: ssl_locl.h,v 1.209 2018/08/24 18:10:25 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -1056,10 +1056,11 @@ void ssl_cert_free(CERT *c);
SESS_CERT *ssl_sess_cert_new(void);
void ssl_sess_cert_free(SESS_CERT *sc);
int ssl_get_new_session(SSL *s, int session);
int ssl_get_prev_session(SSL *s, unsigned char *session, int len,
const unsigned char *limit);
int ssl_get_prev_session(SSL *s, const unsigned char *session_id,
int session_id_len, CBS *ext_block);
int ssl_cipher_id_cmp(const SSL_CIPHER *a, const SSL_CIPHER *b);
SSL_CIPHER *OBJ_bsearch_ssl_cipher_id(SSL_CIPHER *key, SSL_CIPHER const *base, int num);
SSL_CIPHER *OBJ_bsearch_ssl_cipher_id(SSL_CIPHER *key, SSL_CIPHER const *base,
int num);
int ssl_cipher_ptr_id_cmp(const SSL_CIPHER * const *ap,
const SSL_CIPHER * const *bp);
int ssl_cipher_list_to_bytes(SSL *s, STACK_OF(SSL_CIPHER) *ciphers, CBB *cbb);
@ -1278,8 +1279,8 @@ int ssl_check_clienthello_tlsext_late(SSL *s);
int ssl_check_serverhello_tlsext(SSL *s);
#define tlsext_tick_md EVP_sha256
int tls1_process_ticket(SSL *s, const unsigned char *session_id, int len,
const unsigned char *limit, SSL_SESSION **ret);
int tls1_process_ticket(SSL *s, const unsigned char *session_id,
int session_id_len, CBS *ext_block, SSL_SESSION **ret);
int tls12_get_hashid(const EVP_MD *md);
int tls12_get_sigid(const EVP_PKEY *pk);
int tls12_get_hashandsig(CBB *cbb, const EVP_PKEY *pk, const EVP_MD *md);

22
lib/libssl/ssl_sess.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssl_sess.c,v 1.80 2018/04/25 07:10:39 tb Exp $ */
/* $OpenBSD: ssl_sess.c,v 1.81 2018/08/24 18:10:25 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -420,8 +420,8 @@ sess_id_done:
* 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.
* len: the length of the session ID.
* limit: a pointer to the first byte after the ClientHello.
* session_id_len: the length of the session ID.
* ext_block: a CBS for the ClientHello extensions block.
*
* Returns:
* -1: error
@ -435,8 +435,8 @@ sess_id_done:
* to 1 if the server should issue a new session ticket (to 0 otherwise).
*/
int
ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
const unsigned char *limit)
ssl_get_prev_session(SSL *s, const unsigned char *session_id,
int session_id_len, CBS *ext_block)
{
SSL_SESSION *ret = NULL;
int fatal = 0;
@ -445,14 +445,14 @@ ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
/* This is used only by servers. */
if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
if (session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH)
goto err;
if (len == 0)
if (session_id_len == 0)
try_session_cache = 0;
/* Sets s->internal->tlsext_ticket_expected. */
r = tls1_process_ticket(s, session_id, len, limit, &ret);
r = tls1_process_ticket(s, session_id, session_id_len, ext_block, &ret);
switch (r) {
case -1: /* Error during processing */
fatal = 1;
@ -473,8 +473,8 @@ ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
SSL_SESSION data;
data.ssl_version = s->version;
data.session_id_length = len;
memcpy(data.session_id, session_id, len);
data.session_id_length = session_id_len;
memcpy(data.session_id, session_id, session_id_len);
CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
ret = lh_SSL_SESSION_retrieve(s->session_ctx->internal->sessions, &data);
@ -494,7 +494,7 @@ ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
int copy = 1;
if ((ret = s->session_ctx->internal->get_session_cb(s,
session_id, len, &copy))) {
session_id, session_id_len, &copy))) {
s->session_ctx->internal->stats.sess_cb_hit++;
/*

15
lib/libssl/ssl_srvr.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: ssl_srvr.c,v 1.44 2018/08/24 17:44:22 jsing Exp $ */
/* $OpenBSD: ssl_srvr.c,v 1.45 2018/08/24 18:10:25 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -818,7 +818,6 @@ ssl3_get_client_hello(SSL *s)
unsigned long alg_k;
const SSL_METHOD *method;
uint16_t shared_version;
unsigned char *end;
/*
* We do this so that we will respond with our native type.
@ -842,8 +841,6 @@ ssl3_get_client_hello(SSL *s)
if (n < 0)
goto err;
end = (unsigned char *)s->internal->init_msg + n;
CBS_init(&cbs, s->internal->init_msg, n);
/* Parse client hello up until the extensions (if any). */
@ -928,10 +925,12 @@ ssl3_get_client_hello(SSL *s)
if (!ssl_get_new_session(s, 1))
goto err;
} else {
/* XXX - pass CBS through instead... */
i = ssl_get_prev_session(s,
(unsigned char *)CBS_data(&session_id),
CBS_len(&session_id), end);
CBS ext_block;
CBS_dup(&cbs, &ext_block);
i = ssl_get_prev_session(s, CBS_data(&session_id),
CBS_len(&session_id), &ext_block);
if (i == 1) { /* previous session */
s->internal->hit = 1;
} else if (i == -1)

65
lib/libssl/t1_lib.c
View File

@ -1,4 +1,4 @@
/* $OpenBSD: t1_lib.c,v 1.143 2018/08/19 15:38:03 jsing Exp $ */
/* $OpenBSD: t1_lib.c,v 1.144 2018/08/24 18:10:25 jsing Exp $ */
/* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
* All rights reserved.
*
@ -815,11 +815,9 @@ 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: 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.
* len: the length of the session ID.
* limit: a pointer to the first byte after the ClientHello.
* session_id: points at the session ID in the ClientHello.
* session_id_len: the length of 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.
*
@ -845,55 +843,34 @@ ssl_check_serverhello_tlsext(SSL *s)
* Otherwise, s->internal->tlsext_ticket_expected is set to 0.
*/
int
tls1_process_ticket(SSL *s, const unsigned char *session, int session_len,
const unsigned char *limit, SSL_SESSION **ret)
tls1_process_ticket(SSL *s, const unsigned char *session_id, int session_id_len,
CBS *ext_block, SSL_SESSION **ret)
{
/* Point after session ID in client hello */
CBS session_id, cookie, cipher_list, compress_algo, extensions;
CBS extensions;
*ret = NULL;
s->internal->tlsext_ticket_expected = 0;
*ret = NULL;
/* If tickets disabled behave as if no ticket present
* to permit stateful resumption.
/*
* If tickets disabled behave as if no ticket present to permit stateful
* resumption.
*/
if (SSL_get_options(s) & SSL_OP_NO_TICKET)
return 0;
if (!limit)
/*
* An empty extensions block is valid, but obviously does not contain
* a session ticket.
*/
if (CBS_len(ext_block) == 0)
return 0;
if (limit < session)
return -1;
CBS_init(&session_id, session, limit - session);
/* Skip past the session id */
if (!CBS_skip(&session_id, session_len))
return -1;
/* Skip past DTLS cookie */
if (SSL_IS_DTLS(s)) {
if (!CBS_get_u8_length_prefixed(&session_id, &cookie))
return -1;
}
/* Skip past cipher list */
if (!CBS_get_u16_length_prefixed(&session_id, &cipher_list))
return -1;
/* Skip past compression algorithm list */
if (!CBS_get_u8_length_prefixed(&session_id, &compress_algo))
return -1;
/* Now at start of extensions */
if (CBS_len(&session_id) == 0)
return 0;
if (!CBS_get_u16_length_prefixed(&session_id, &extensions))
if (!CBS_get_u16_length_prefixed(ext_block, &extensions))
return -1;
while (CBS_len(&extensions) > 0) {
CBS ext_data;
uint16_t ext_type;
CBS ext_data;
if (!CBS_get_u16(&extensions, &ext_type) ||
!CBS_get_u16_length_prefixed(&extensions, &ext_data))
@ -907,7 +884,7 @@ tls1_process_ticket(SSL *s, const unsigned char *session, int session_len,
s->internal->tlsext_ticket_expected = 1;
return 1;
}
if (s->internal->tls_session_secret_cb) {
if (s->internal->tls_session_secret_cb != NULL) {
/* Indicate that the ticket couldn't be
* decrypted rather than generating the session
* from ticket now, trigger abbreviated
@ -917,7 +894,7 @@ tls1_process_ticket(SSL *s, const unsigned char *session, int session_len,
}
r = tls_decrypt_ticket(s, CBS_data(&ext_data),
CBS_len(&ext_data), session, session_len, ret);
CBS_len(&ext_data), session_id, session_id_len, ret);
switch (r) {
case 2: /* ticket couldn't be decrypted */