Return code tweaks for session ticket handlers

In tls1_process_ticket() and tls_decrypt_ticket() use #defines with
descriptive names instead of hardcoding -1 1 2 3 4 and occasionally
explaining the magic numbers with comments.

ok beck inoguchi

3 files changed, 51 insertions, 47 deletions

diff --git a/src/lib/libssl/ssl_locl.h b/src/lib/libssl/ssl_locl.h
index 036c1dacb2..18ff5b0c30 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.285 2020/08/31 14:04:51 tb Exp $ */
+/* $OpenBSD: ssl_locl.h,v 1.286 2020/08/31 14:34:01 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -1397,6 +1397,13 @@ int ssl_check_clienthello_tlsext_early(SSL *s);
 int ssl_check_clienthello_tlsext_late(SSL *s);
 int ssl_check_serverhello_tlsext(SSL *s);
 
+#define TLS1_TICKET_FATAL_ERROR         -1
+#define TLS1_TICKET_NONE                 0
+#define TLS1_TICKET_EMPTY                1
+#define TLS1_TICKET_NOT_DECRYPTED        2
+#define TLS1_TICKET_DECRYPTED            3
+#define TLS1_TICKET_DECRYPTED_RENEW      4
+
 int tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block,
     int *alert, SSL_SESSION **ret);
 
diff --git a/src/lib/libssl/ssl_sess.c b/src/lib/libssl/ssl_sess.c
index 827360176b..9e8edd93e8 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.86 2020/08/31 14:04:51 tb Exp $ */
+/* $OpenBSD: ssl_sess.c,v 1.87 2020/08/31 14:34:01 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -440,7 +440,6 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
         SSL_SESSION *ret = NULL;
         int alert_desc = SSL_AD_INTERNAL_ERROR, fatal = 0;
         int try_session_cache = 1;
-        int r;
 
         /* This is used only by servers. */
 
@@ -451,16 +450,15 @@ ssl_get_prev_session(SSL *s, CBS *session_id, CBS *ext_block, int *alert)
                 try_session_cache = 0;
 
         /* Sets s->internal->tlsext_ticket_expected. */
-        r = tls1_process_ticket(s, session_id, ext_block, &alert_desc, &ret);
-        switch (r) {
-        case -1: /* Error during processing */
+        switch (tls1_process_ticket(s, session_id, ext_block, &alert_desc, &ret)) {
+        case TLS1_TICKET_FATAL_ERROR:
                 fatal = 1;
                 goto err;
-        case 0: /* No ticket found */
-        case 1: /* Zero length ticket found */
+        case TLS1_TICKET_NONE:
+        case TLS1_TICKET_EMPTY:
                 break; /* Ok to carry on processing session id. */
-        case 2: /* Ticket found but not decrypted. */
-        case 3: /* Ticket decrypted, *ret has been set. */
+        case TLS1_TICKET_NOT_DECRYPTED:
+        case TLS1_TICKET_DECRYPTED:
                 try_session_cache = 0;
                 break;
         default:
diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index 59146eb767..b0fc630236 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.170 2020/08/31 14:04:51 tb Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.171 2020/08/31 14:34:01 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -765,13 +765,14 @@ ssl_check_serverhello_tlsext(SSL *s)
  * never be decrypted, nor will s->internal->tlsext_ticket_expected be set to 1.
  *
  * Returns:
- *   -1: fatal error, either from parsing or decrypting the ticket.
- *    0: no ticket was found (or was ignored, based on settings).
- *    1: a zero length extension was found, indicating that the client supports
- *       session tickets but doesn't currently have one to offer.
- *    2: either s->internal->tls_session_secret_cb was set, or a ticket was offered but
- *       couldn't be decrypted because of a non-fatal error.
- *    3: a ticket was successfully decrypted and *ret was set.
+ *    TLS1_TICKET_FATAL_ERROR: error from parsing or decrypting the ticket.
+ *    TLS1_TICKET_NONE: no ticket was found (or was ignored, based on settings).
+ *    TLS1_TICKET_EMPTY: a zero length extension was found, indicating that the
+ *       client supports session tickets but doesn't currently have one to offer.
+ *    TLS1_TICKET_NOT_DECRYPTED: either s->internal->tls_session_secret_cb was
+ *       set, or a ticket was offered but couldn't be decrypted because of a
+ *       non-fatal error.
+ *    TLS1_TICKET_DECRYPTED: a ticket was successfully decrypted and *ret was set.
  *
  * Side effects:
  *   Sets s->internal->tlsext_ticket_expected to 1 if the server will have to issue
@@ -787,7 +788,6 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
 {
         CBS extensions, ext_data;
         uint16_t ext_type = 0;
-        int r;
 
         s->internal->tlsext_ticket_expected = 0;
         *ret = NULL;
@@ -797,25 +797,25 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
          * resumption.
          */
         if (SSL_get_options(s) & SSL_OP_NO_TICKET)
-                return 0;
+                return TLS1_TICKET_NONE;
 
         /*
          * An empty extensions block is valid, but obviously does not contain
          * a session ticket.
          */
         if (CBS_len(ext_block) == 0)
-                return 0;
+                return TLS1_TICKET_NONE;
 
         if (!CBS_get_u16_length_prefixed(ext_block, &extensions)) {
                 *alert = SSL_AD_DECODE_ERROR;
-                return -1;
+                return TLS1_TICKET_FATAL_ERROR;
         }
 
         while (CBS_len(&extensions) > 0) {
                 if (!CBS_get_u16(&extensions, &ext_type) ||
                     !CBS_get_u16_length_prefixed(&extensions, &ext_data)) {
                         *alert = SSL_AD_DECODE_ERROR;
-                        return -1;
+                        return TLS1_TICKET_FATAL_ERROR;
                 }
 
                 if (ext_type == TLSEXT_TYPE_session_ticket)
@@ -823,7 +823,7 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
         }
 
         if (ext_type != TLSEXT_TYPE_session_ticket)
-                return 0;
+                return TLS1_TICKET_NONE;
 
         if (CBS_len(&ext_data) == 0) {
                 /*
@@ -831,7 +831,7 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
                  * have one.
                  */
                 s->internal->tlsext_ticket_expected = 1;
-                return 1;
+                return TLS1_TICKET_EMPTY;
         }
 
         if (s->internal->tls_session_secret_cb != NULL) {
@@ -841,21 +841,20 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
                  * handshake based on external mechanism to calculate the master
                  * secret later.
                  */
-                return 2;
+                return TLS1_TICKET_NOT_DECRYPTED;
         }
 
-        r = tls_decrypt_ticket(s, session_id, &ext_data, alert, ret);
-        switch (r) {
-        case 2: /* ticket couldn't be decrypted */
+        switch (tls_decrypt_ticket(s, session_id, &ext_data, alert, ret)) {
+        case TLS1_TICKET_NOT_DECRYPTED:
                 s->internal->tlsext_ticket_expected = 1;
-                return 2;
-        case 3: /* ticket was decrypted */
-                return r;
-        case 4: /* ticket decrypted but need to renew */
+                return TLS1_TICKET_NOT_DECRYPTED;
+        case TLS1_TICKET_DECRYPTED:
+                return TLS1_TICKET_DECRYPTED;
+        case TLS1_TICKET_DECRYPTED_RENEW:
                 s->internal->tlsext_ticket_expected = 1;
-                return 3;
-        default: /* fatal error */
-                return -1;
+                return TLS1_TICKET_DECRYPTED;
+        default:
+                return TLS1_TICKET_FATAL_ERROR;
         }
 }
 
@@ -867,10 +866,10 @@ tls1_process_ticket(SSL *s, CBS *session_id, CBS *ext_block, int *alert,
  *       point to the resulting session.
  *
  * Returns:
- *   -1: fatal error, either from parsing or decrypting the ticket.
- *    2: the ticket couldn't be decrypted.
- *    3: a ticket was successfully decrypted and *psess was set.
- *    4: same as 3, but the ticket needs to be renewed.
+ *    TLS1_TICKET_FATAL_ERROR: error from parsing or decrypting the ticket.
+ *    TLS1_TICKET_NOT_DECRYPTED: the ticket couldn't be decrypted.
+ *    TLS1_TICKET_DECRYPTED: a ticket was decrypted and *psess was set.
+ *    TLS1_TICKET_DECRYPTED_RENEW: same as 3, but the ticket needs to be renewed.
  */
 static int
 tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
@@ -887,9 +886,9 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
         EVP_CIPHER_CTX *cctx = NULL;
         SSL_CTX *tctx = s->initial_ctx;
         int slen, hlen;
-        int renew_ticket = 0;
-        int ret = -1;
         int alert_desc = SSL_AD_INTERNAL_ERROR;
+        int renew_ticket = 0;
+        int ret = TLS1_TICKET_FATAL_ERROR;
 
         *psess = NULL;
 
@@ -1018,19 +1017,19 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
         sess = NULL;
 
         if (renew_ticket)
-                ret = 4;
+                ret = TLS1_TICKET_DECRYPTED_RENEW;
         else
-                ret = 3;
+                ret = TLS1_TICKET_DECRYPTED;
 
         goto done;
 
  derr:
-        ret = 2;
+        ret = TLS1_TICKET_NOT_DECRYPTED;
         goto done;
 
  err:
         *alert = alert_desc;
-        ret = -1;
+        ret = TLS1_TICKET_FATAL_ERROR;
         goto done;
 
  done:
@@ -1039,7 +1038,7 @@ tls_decrypt_ticket(SSL *s, CBS *session_id, CBS *ticket, int *alert,
         HMAC_CTX_free(hctx);
         SSL_SESSION_free(sess);
 
-        if (ret == 2)
+        if (ret == TLS1_TICKET_NOT_DECRYPTED)
                 ERR_clear_error();
 
         return ret;