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

3 files changed, 23 insertions, 27 deletions

diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h
index 2f8ba1fc091..bd210cdce52 100644
--- a/lib/libssl/ssl_locl.h
+++ b/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/lib/libssl/ssl_sess.c b/lib/libssl/ssl_sess.c
index b953580d652..460c5d85f1f 100644
--- a/lib/libssl/ssl_sess.c
+++ b/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/lib/libssl/t1_lib.c b/lib/libssl/t1_lib.c
index 8162259c66e..dc6ffae4183 100644
--- a/lib/libssl/t1_lib.c
+++ b/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;