Pass the session ID down to the session/ticket handling code as a CBS.

Convert ssl_get_prev_session(), tls1_process_ticket() and
tls1_decrypt_ticket() to handle the session ID from the client hello
as a CBS. While here also swap the order of arguments for
tls1_decrypt_ticket() so that it is consistent with the other functions.

ok tb@

4 files changed, 36 insertions, 35 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index b895de1fd3c..c3c762a5fa3 100644
--- a/lib/libssl/ssl_locl.h
+++ b/lib/libssl/ssl_locl.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_locl.h,v 1.246 2019/04/22 14:49:42 jsing Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.247 2019/04/22 15:12:20 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1096,8 +1096,7 @@ int ssl_cert_add1_chain_cert(CERT *c, X509 *cert);
 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, const unsigned char *session_id,
-    int session_id_len, CBS *ext_block);
+int ssl_get_prev_session(SSL *s, CBS *session_id, 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);
@@ -1327,8 +1326,8 @@ int ssl_check_clienthello_tlsext_early(SSL *s);
 int ssl_check_clienthello_tlsext_late(SSL *s);
 int ssl_check_serverhello_tlsext(SSL *s);
 
-int tls1_process_ticket(SSL *s, const unsigned char *session_id,
-    int session_id_len, CBS *ext_block, SSL_SESSION **ret);
+int tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block,
+    SSL_SESSION **ret);
 
 long ssl_get_algorithm2(SSL *s);
 
diff --git a/lib/libssl/ssl_sess.c b/lib/libssl/ssl_sess.c
index 7e8a1bc6700..16b4b75bc4a 100644
--- a/lib/libssl/ssl_sess.c
+++ b/lib/libssl/ssl_sess.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_sess.c,v 1.84 2019/04/04 14:32:49 jsing Exp $ */
+/* $OpenBSD: ssl_sess.c,v 1.85 2019/04/22 15:12:20 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -435,8 +435,7 @@ sess_id_done:
  *     to 1 if the server should issue a new session ticket (to 0 otherwise).
  */
 int
-ssl_get_prev_session(SSL *s, const unsigned char *session_id,
-    int session_id_len, CBS *ext_block)
+ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block)
 {
 	SSL_SESSION *ret = NULL;
 	int fatal = 0;
@@ -445,14 +444,14 @@ ssl_get_prev_session(SSL *s, const unsigned char *session_id,
 
 	/* This is used only by servers. */
 
-	if (session_id_len > SSL_MAX_SSL_SESSION_ID_LENGTH)
+	if (CBS_len(session_id) > SSL_MAX_SSL_SESSION_ID_LENGTH)
 		goto err;
 
-	if (session_id_len == 0)
+	if (CBS_len(session_id) == 0)
 		try_session_cache = 0;
 
 	/* Sets s->internal->tlsext_ticket_expected. */
-	r = tls1_process_ticket(s, session_id, session_id_len, ext_block, &ret);
+	r = tls1_process_ticket(s, session_id, ext_block, &ret);
 	switch (r) {
 	case -1: /* Error during processing */
 		fatal = 1;
@@ -474,9 +473,11 @@ ssl_get_prev_session(SSL *s, const unsigned char *session_id,
 	    !(s->session_ctx->internal->session_cache_mode &
 	     SSL_SESS_CACHE_NO_INTERNAL_LOOKUP)) {
 		SSL_SESSION data;
+
 		data.ssl_version = s->version;
-		data.session_id_length = session_id_len;
-		memcpy(data.session_id, session_id, session_id_len);
+		data.session_id_length = CBS_len(session_id);
+		memcpy(data.session_id, CBS_data(session_id),
+		    CBS_len(session_id));
 
 		CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
 		ret = lh_SSL_SESSION_retrieve(s->session_ctx->internal->sessions, &data);
@@ -496,7 +497,7 @@ ssl_get_prev_session(SSL *s, const unsigned char *session_id,
 		int copy = 1;
 
 		if ((ret = s->session_ctx->internal->get_session_cb(s,
-		    session_id, session_id_len, &copy))) {
+		    CBS_data(session_id), CBS_len(session_id), &copy))) {
 			s->session_ctx->internal->stats.sess_cb_hit++;
 
 			/*
diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c
index 0a533430582..809f589653c 100644
--- a/lib/libssl/ssl_srvr.c
+++ b/lib/libssl/ssl_srvr.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssl_srvr.c,v 1.67 2019/04/22 14:49:42 jsing Exp $ */
+/* $OpenBSD: ssl_srvr.c,v 1.68 2019/04/22 15:12:20 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -913,8 +913,7 @@ ssl3_get_client_hello(SSL *s)
 
 		CBS_dup(&cbs, &ext_block);
 
-		i = ssl_get_prev_session(s, CBS_data(&session_id),
-		    CBS_len(&session_id), &ext_block);
+		i = ssl_get_prev_session(s, &session_id, &ext_block);
 		if (i == 1) { /* previous session */
 			s->internal->hit = 1;
 		} else if (i == -1)
diff --git a/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index b8aa9894130..6af6d77eddb 100644
--- a/lib/libssl/t1_lib.c
+++ b/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.158 2019/04/22 14:49:42 jsing Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.159 2019/04/22 15:12:20 jsing Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -122,9 +122,8 @@
 #include "ssl_sigalgs.h"
 #include "ssl_tlsext.h"
 
-static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, int ticklen,
-    const unsigned char *sess_id, int sesslen,
-    SSL_SESSION **psess);
+static int tls_decrypt_ticket(SSL *s, CBS *session_id,
+    const unsigned char *tick, int ticklen, SSL_SESSION **psess);
 
 SSL3_ENC_METHOD TLSv1_enc_data = {
 	.enc = tls1_enc,
@@ -759,8 +758,7 @@ 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.
- *   session_id_len: the length of the session ID.
+ *   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.
@@ -787,8 +785,7 @@ 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_id, int session_id_len,
-    CBS *ext_block, SSL_SESSION **ret)
+tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, SSL_SESSION **ret)
 {
 	CBS extensions, ext_data;
 	uint16_t ext_type = 0;
@@ -845,8 +842,8 @@ tls1_process_ticket(SSL *s, const unsigned char *session_id, int session_id_len,
 		return 2;
 	}
 
-	r = tls_decrypt_ticket(s, CBS_data(&ext_data), CBS_len(&ext_data),
-	    session_id, session_id_len, ret);
+	r = tls_decrypt_ticket(s, session_id, CBS_data(&ext_data),
+	    CBS_len(&ext_data), ret);
 	switch (r) {
 	case 2: /* ticket couldn't be decrypted */
 		s->internal->tlsext_ticket_expected = 1;
@@ -863,10 +860,9 @@ tls1_process_ticket(SSL *s, const unsigned char *session_id, int session_id_len,
 
 /* tls_decrypt_ticket attempts to decrypt a session ticket.
  *
+ *   session_id: a CBS containing the session ID.
  *   etick: points to the body of the session ticket extension.
  *   eticklen: the length of the session tickets extenion.
- *   sess_id: points at the session ID.
- *   sesslen: the length of the session ID.
  *   psess: (output) on return, if a ticket was decrypted, then this is set to
  *       point to the resulting session.
  *
@@ -877,10 +873,11 @@ tls1_process_ticket(SSL *s, const unsigned char *session_id, int session_id_len,
  *    4: same as 3, but the ticket needs to be renewed.
  */
 static int
-tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
-    const unsigned char *sess_id, int sesslen, SSL_SESSION **psess)
+tls_decrypt_ticket(SSL *s, CBS *session_id, const unsigned char *etick,
+    int eticklen, SSL_SESSION **psess)
 {
-	SSL_SESSION *sess;
+	SSL_SESSION *sess = NULL;
+	size_t session_id_len = 0;
 	unsigned char *sdec = NULL;
 	const unsigned char *p;
 	int slen, mlen, renew_ticket = 0;
@@ -988,10 +985,14 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
 	 * 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 (sesslen)
-		memcpy(sess->session_id, sess_id, sesslen);
-	sess->session_id_length = sesslen;
+	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;
+
 	if (renew_ticket)
 		ret = 4;
 	else
@@ -1006,6 +1007,7 @@ tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
 	free(sdec);
 	HMAC_CTX_cleanup(&hctx);
 	EVP_CIPHER_CTX_cleanup(&ctx);
+	SSL_SESSION_free(sess);
 
 	if (ret == 2)
 		ERR_clear_error();