Implement Server side of PSK extension parsing

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)

include/openssl/ssl.h

@@ -2324,6 +2324,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_GET_MESSAGE_HEADER                     387
 # define SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT               449
 # define SSL_F_TLS_PARSE_CTOS_KEY_SHARE                   463
+# define SSL_F_TLS_PARSE_CTOS_PSK                         505
 # define SSL_F_TLS_PARSE_CTOS_RENEGOTIATE                 464
 # define SSL_F_TLS_PARSE_CTOS_USE_SRTP                    465
 # define SSL_F_TLS_PARSE_STOC_KEY_SHARE                   445
@@ -2361,6 +2362,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_F_TLS_PROCESS_SKE_ECDHE                      420
 # define SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE               421
 # define SSL_F_TLS_PROCESS_SKE_SRP                        422
+# define SSL_F_TLS_PSK_DO_BINDER                          506
 # define SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT                450
 # define SSL_F_TLS_SETUP_HANDSHAKE                        508
 # define SSL_F_USE_CERTIFICATE_CHAIN_FILE                 220

ssl/ssl_ciph.c

@@ -2018,3 +2018,14 @@ int ssl_cipher_get_overhead(const SSL_CIPHER *c, size_t *mac_overhead,
 
     return 1;
 }
+
+const EVP_MD *ssl_cipher_get_handshake_md(int cipher_id)
+{
+    const SSL_CIPHER *cipher = ssl3_get_cipher_by_id(cipher_id);
+    if (cipher == NULL) {
+        /* Don't recognise this cipher */
+        return NULL;
+    }
+
+    return ssl_md(cipher->algorithm2);
+}

ssl/ssl_err.c

@@ -370,6 +370,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_PARSE_CLIENTHELLO_TLSEXT),
      "tls_parse_clienthello_tlsext"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_KEY_SHARE), "tls_parse_ctos_key_share"},
+    {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_PSK), "tls_parse_ctos_psk"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_RENEGOTIATE),
      "tls_parse_ctos_renegotiate"},
     {ERR_FUNC(SSL_F_TLS_PARSE_CTOS_USE_SRTP), "tls_parse_ctos_use_srtp"},
@@ -423,6 +424,7 @@ static ERR_STRING_DATA SSL_str_functs[] = {
     {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_PSK_PREAMBLE),
      "tls_process_ske_psk_preamble"},
     {ERR_FUNC(SSL_F_TLS_PROCESS_SKE_SRP), "tls_process_ske_srp"},
+    {ERR_FUNC(SSL_F_TLS_PSK_DO_BINDER), "tls_psk_do_binder"},
     {ERR_FUNC(SSL_F_TLS_SCAN_CLIENTHELLO_TLSEXT),
      "tls_scan_clienthello_tlsext"},
     {ERR_FUNC(SSL_F_TLS_SETUP_HANDSHAKE), "tls_setup_handshake"},

ssl/ssl_locl.h

@@ -1956,6 +1956,7 @@ __owur int ssl_cipher_get_overhead(const SSL_CIPHER *c, size_t *mac_overhead,
 __owur int ssl_cipher_get_cert_index(const SSL_CIPHER *c);
 __owur const SSL_CIPHER *ssl_get_cipher_by_char(SSL *ssl,
                                                 const unsigned char *ptr);
+__owur const EVP_MD *ssl_cipher_get_handshake_md(int cipher_id);
 __owur int ssl_cert_set0_chain(SSL *s, SSL_CTX *ctx, STACK_OF(X509) *chain);
 __owur int ssl_cert_set1_chain(SSL *s, SSL_CTX *ctx, STACK_OF(X509) *chain);
 __owur int ssl_cert_add0_chain_cert(SSL *s, SSL_CTX *ctx, X509 *x);
@@ -2193,6 +2194,17 @@ void ssl_set_default_md(SSL *s);
 __owur int tls1_set_server_sigalgs(SSL *s);
 __owur int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
                                       SSL_SESSION **ret);
+
+/* Return codes for tls_decrypt_ticket */
+#define TICKET_FATAL_ERR_MALLOC     -2
+#define TICKET_FATAL_ERR_OTHER      -1
+#define TICKET_NO_DECRYPT            2
+#define TICKET_SUCCESS               3
+#define TICKET_SUCCESS_RENEW         4
+__owur int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
+                              size_t eticklen, const unsigned char *sess_id,
+                              size_t sesslen, SSL_SESSION **psess);
+
 __owur int tls_use_ticket(SSL *s);
 
 __owur int tls12_get_sigandhash(SSL *s, WPACKET *pkt, const EVP_PKEY *pk,

ssl/ssl_sess.c

@@ -39,6 +39,7 @@
 #include <openssl/rand.h>
 #include <openssl/engine.h>
 #include "ssl_locl.h"
+#include "statem/statem_locl.h"
 
 static void SSL_SESSION_list_remove(SSL_CTX *ctx, SSL_SESSION *s);
 static void SSL_SESSION_list_add(SSL_CTX *ctx, SSL_SESSION *s);
@@ -444,8 +445,9 @@ int ssl_get_new_session(SSL *s, int session)
  *   hello: The parsed ClientHello data
  *
  * Returns:
- *   -1: error
+ *   -1: fatal error
- *    0: a session may have been found.
+ *    0: no session found
+ *    1: a session may have been found.
  *
  * Side effects:
  *   - If a session is found then s->session is pointed at it (after freeing an
@@ -459,12 +461,18 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
 
     SSL_SESSION *ret = NULL;
     int fatal = 0;
-    int try_session_cache = 1;
+    int try_session_cache = 0;
     int r;
 
-    if (hello->session_id_len == 0)
+    if (SSL_IS_TLS13(s)) {
-        try_session_cache = 0;
+        int al;
 
+        if (!tls_parse_extension(s, TLSEXT_IDX_psk, EXT_CLIENT_HELLO,
+                                 hello->pre_proc_exts, NULL, 0, &al))
+            return -1;
+
+        ret = s->session;
+    } else {
         /* sets s->ext.ticket_expected */
         r = tls_get_ticket_from_client(s, hello, &ret);
         switch (r) {
@@ -473,14 +481,15 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
             goto err;
         case 0:                    /* No ticket found */
         case 1:                    /* Zero length ticket found */
+            try_session_cache = 1;
             break;                  /* Ok to carry on processing session id. */
         case 2:                    /* Ticket found but not decrypted. */
         case 3:                    /* Ticket decrypted, *ret has been set. */
-        try_session_cache = 0;
             break;
         default:
             abort();
         }
+    }
 
     if (try_session_cache &&
         ret == NULL &&
@@ -628,11 +637,15 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
         goto err;
     }
 
-    s->session_ctx->stats.sess_hit++;
+    if (!SSL_IS_TLS13(s)) {
-
+        /* We already did this for TLS1.3 */
         SSL_SESSION_free(s->session);
         s->session = ret;
+    }
+
+    s->session_ctx->stats.sess_hit++;
     s->verify_result = s->session->verify_result;
+
     return 1;
 
  err:

ssl/statem/extensions.c

@@ -279,7 +279,8 @@ static const EXTENSION_DEFINITION ext_defs[] = {
         TLSEXT_TYPE_psk,
         EXT_CLIENT_HELLO | EXT_TLS1_3_SERVER_HELLO | EXT_TLS_IMPLEMENTATION_ONLY
         | EXT_TLS1_3_ONLY,
-        NULL, NULL, tls_parse_stoc_psk, NULL, tls_construct_ctos_psk, NULL
+        NULL, tls_parse_ctos_psk, tls_parse_stoc_psk, NULL,
+        tls_construct_ctos_psk, NULL
     }
 };
 
@@ -1002,3 +1003,97 @@ static int init_psk_kex_modes(SSL *s, unsigned int context)
 
     return 1;
 }
+
+int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
+                      size_t binderoffset, const unsigned char *binderin,
+                      unsigned char *binderout,
+                      SSL_SESSION *sess, int sign)
+{
+    EVP_PKEY *mackey = NULL;
+    EVP_MD_CTX *mctx = NULL;
+    unsigned char hash[EVP_MAX_MD_SIZE], binderkey[EVP_MAX_MD_SIZE];
+    unsigned char finishedkey[EVP_MAX_MD_SIZE], tmpbinder[EVP_MAX_MD_SIZE];
+    const char resumption_label[] = "resumption psk binder key";
+    size_t hashsize = EVP_MD_size(md), bindersize;
+    int ret = -1;
+
+    /* Generate the early_secret */
+    if (!tls13_generate_secret(s, md, NULL, sess->master_key,
+                               sess->master_key_length,
+                               (unsigned char *)&s->early_secret)) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    /*
+     * Create the handshake hash for the binder key...the messages so far are
+     * empty!
+     */
+    mctx = EVP_MD_CTX_new();
+    if (mctx == NULL
+            || EVP_DigestInit_ex(mctx, md, NULL) <= 0
+            || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    /* Generate the binder key */
+    if (!tls13_hkdf_expand(s, md, s->early_secret,
+                           (unsigned char *)resumption_label,
+                           sizeof(resumption_label) - 1, hash, binderkey,
+                           hashsize)) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    /* Generate the finished key */
+    if (!tls13_derive_finishedkey(s, md, binderkey, finishedkey, hashsize)) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    /*
+     * Get a hash of the ClientHello up to the start of the binders.
+     * TODO(TLS1.3): This will need to be tweaked when we implement
+     * HelloRetryRequest to include the digest of the previous messages here.
+     */
+    if (EVP_DigestInit_ex(mctx, md, NULL) <= 0
+            || EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
+            || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    mackey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, finishedkey, hashsize);
+    if (mackey == NULL) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    if (!sign)
+        binderout = tmpbinder;
+
+    bindersize = hashsize;
+    if (EVP_DigestSignInit(mctx, NULL, md, NULL, mackey) <= 0
+            || EVP_DigestSignUpdate(mctx, hash, hashsize) <= 0
+            || EVP_DigestSignFinal(mctx, binderout, &bindersize) <= 0
+            || bindersize != hashsize) {
+        SSLerr(SSL_F_TLS_PSK_DO_BINDER, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    if (sign) {
+        ret = 1;
+    } else {
+        /* HMAC keys can't do EVP_DigestVerify* - use CRYPTO_memcmp instead */
+        ret = (CRYPTO_memcmp(binderin, binderout, hashsize) == 0);
+    }
+
+ err:
+    OPENSSL_cleanse(binderkey, sizeof(binderkey));
+    OPENSSL_cleanse(finishedkey, sizeof(finishedkey));
+    EVP_PKEY_free(mackey);
+    EVP_MD_CTX_free(mctx);
+
+    return ret;
+}

ssl/statem/extensions_clnt.c

@@ -663,16 +663,10 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
                            int *al)
 {
 #ifndef OPENSSL_NO_TLS1_3
-    const SSL_CIPHER *cipher;
     uint32_t now, ages, agems;
-    size_t hashsize, bindersize, binderoffset, msglen;
+    size_t hashsize, binderoffset, msglen;
     unsigned char *binder = NULL, *msgstart = NULL;
-    EVP_PKEY *mackey = NULL;
     const EVP_MD *md;
-    EVP_MD_CTX *mctx = NULL;
-    unsigned char hash[EVP_MAX_MD_SIZE], binderkey[EVP_MAX_MD_SIZE];
-    unsigned char finishedkey[EVP_MAX_MD_SIZE];
-    const char resumption_label[] = "resumption psk binder key";
     int ret = 0;
 
     s->session->ext.tick_identity = TLSEXT_PSK_BAD_IDENTITY;
@@ -719,17 +713,12 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
      */
     agems += s->session->ext.tick_age_add;
 
-    cipher = ssl3_get_cipher_by_id(s->session->cipher_id);
+    md = ssl_cipher_get_handshake_md(s->session->cipher_id);
-    if (cipher == NULL) {
+    if (md == NULL) {
         /* Don't recognise this cipher so we can't use the session. Ignore it */
         return 1;
     }
-    md = ssl_md(cipher->algorithm2);
+
-    if (md == NULL) {
-        /* Shouldn't happen!! */
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
     hashsize = EVP_MD_size(md);
 
     /* Create the extension, but skip over the binder for now */
@@ -757,60 +746,8 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
 
     msgstart = WPACKET_get_curr(pkt) - msglen;
 
-    /* Generate the early_secret */
+    if (tls_psk_do_binder(s, md, msgstart, binderoffset, NULL, binder,
-    if (!tls13_generate_secret(s, md, NULL, s->session->master_key,
+                          s->session, 1) != 1) {
-                               s->session->master_key_length,
-                               (unsigned char *)&s->early_secret)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    /*
-     * Create the handshake hash for the binder key...the messages so far are
-     * empty!
-     */
-    mctx = EVP_MD_CTX_new();
-    if (mctx == NULL
-            || EVP_DigestInit_ex(mctx, md, NULL) <= 0
-            || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    /* Generate the binder key */
-    if (!tls13_hkdf_expand(s, md, s->early_secret,
-                           (unsigned char *)resumption_label,
-                           sizeof(resumption_label) - 1, hash, binderkey,
-                           hashsize)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    /* Generate the finished key */
-    if (!tls13_derive_finishedkey(s, md, binderkey, finishedkey, hashsize)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    /*
-     * Get a hash of the ClientHello up to the start of the binders.
-     * TODO(TLS1.3): This will need to be tweaked when we implement
-     * HelloRetryRequest to include the digest of the previous messages here.
-     */
-    if (EVP_DigestInit_ex(mctx, md, NULL) <= 0
-            || EVP_DigestUpdate(mctx, msgstart, binderoffset) <= 0
-            || EVP_DigestFinal_ex(mctx, hash, NULL) <= 0) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
-        goto err;
-    }
-
-    mackey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, finishedkey, hashsize);
-    bindersize = hashsize;
-    if (binderkey == NULL
-            || EVP_DigestSignInit(mctx, NULL, md, NULL, mackey) <= 0
-            || EVP_DigestSignUpdate(mctx, hash, hashsize) <= 0
-            || EVP_DigestSignFinal(mctx, binder, &bindersize) <= 0
-            || bindersize != hashsize) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
         goto err;
     }
@@ -819,11 +756,6 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
 
     ret = 1;
  err:
-    OPENSSL_cleanse(binderkey, sizeof(binderkey));
-    OPENSSL_cleanse(finishedkey, sizeof(finishedkey));
-    EVP_PKEY_free(mackey);
-    EVP_MD_CTX_free(mctx);
-
     return ret;
 #else
     return 1;

ssl/statem/extensions_srvr.c

@@ -655,8 +655,7 @@ int tls_parse_ctos_supported_groups(SSL *s, PACKET *pkt, X509 *x,
         return 0;
     }
 
-    if (!s->hit
+    if (!PACKET_memdup(&supported_groups_list,
-            && !PACKET_memdup(&supported_groups_list,
                        &s->session->ext.supportedgroups,
                        &s->session->ext.supportedgroups_len)) {
         *al = SSL_AD_DECODE_ERROR;
@@ -680,6 +679,96 @@ int tls_parse_ctos_ems(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al)
     return 1;
 }
 
+int tls_parse_ctos_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al)
+{
+    PACKET identities, binders, binder;
+    size_t binderoffset, hashsize;
+    SSL_SESSION *sess = NULL;
+    unsigned int id, i;
+    const EVP_MD *md = NULL;
+
+    if (!PACKET_get_length_prefixed_2(pkt, &identities)) {
+        *al = SSL_AD_DECODE_ERROR;
+        return 0;
+    }
+
+    for (id = 0; PACKET_remaining(&identities) != 0; id++) {
+        PACKET identity;
+        unsigned long ticket_age;
+        int ret;
+
+        if (!PACKET_get_length_prefixed_2(&identities, &identity)
+                || !PACKET_get_net_4(&identities, &ticket_age)) {
+            *al = SSL_AD_DECODE_ERROR;
+            return 0;
+        }
+
+        ret = tls_decrypt_ticket(s, PACKET_data(&identity),
+                                 PACKET_remaining(&identity), NULL, 0, &sess);
+        if (ret == TICKET_FATAL_ERR_MALLOC || ret == TICKET_FATAL_ERR_OTHER) {
+            *al = SSL_AD_INTERNAL_ERROR;
+            return 0;
+        }
+        if (ret == TICKET_NO_DECRYPT)
+            continue;
+
+        md = ssl_cipher_get_handshake_md(sess->cipher_id);
+        if (md == NULL) {
+            /*
+             * Don't recognise this cipher so we can't use the session.
+             * Ignore it
+             */
+            SSL_SESSION_free(sess);
+            sess = NULL;
+            continue;
+        }
+
+        /*
+         * TODO(TLS1.3): Somehow we need to handle the case of a ticket renewal.
+         * Ignored for now
+         */
+
+        break;
+    }
+
+    if (sess == NULL)
+        return 1;
+
+    binderoffset = PACKET_data(pkt) - (const unsigned char *)s->init_buf->data;
+
+    hashsize = EVP_MD_size(md);
+
+    if (!PACKET_get_length_prefixed_2(pkt, &binders)) {
+        *al = SSL_AD_DECODE_ERROR;
+        goto err;
+    }
+
+    for (i = 0; i <= id; i++) {
+        if (!PACKET_get_length_prefixed_1(&binders, &binder)) {
+            *al = SSL_AD_DECODE_ERROR;
+            goto err;
+        }
+    }
+
+    if (PACKET_remaining(&binder) != hashsize
+            || tls_psk_do_binder(s, md,
+                                 (const unsigned char *)s->init_buf->data,
+                                 binderoffset, PACKET_data(&binder), NULL,
+                                 sess, 0) != 1) {
+        *al = SSL_AD_DECODE_ERROR;
+        SSLerr(SSL_F_TLS_PARSE_CTOS_PSK, ERR_R_INTERNAL_ERROR);
+        goto err;
+    }
+
+    sess->ext.tick_identity = id;
+    SSL_SESSION_free(s->session);
+    s->session = sess;
+
+    return 1;
+err:
+    return 0;
+}
+
 /*
  * Add the server's renegotiation binding
  */

ssl/statem/statem_locl.h

@@ -166,6 +166,12 @@ __owur int tls_parse_all_extensions(SSL *s, int context, RAW_EXTENSION *exts,
 __owur int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
                                     X509 *x, size_t chainidx, int *al);
 
+__owur int tls_psk_do_binder(SSL *s, const EVP_MD *md,
+                             const unsigned char *msgstart,
+                             size_t binderoffset, const unsigned char *binderin,
+                             unsigned char *binderout,
+                             SSL_SESSION *sess, int sign);
+
 /* Server Extension processing */
 int tls_parse_ctos_renegotiate(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
                                int *al);
@@ -202,6 +208,7 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
 int tls_parse_ctos_ems(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);
 int tls_parse_ctos_psk_kex_modes(SSL *s, PACKET *pkt, X509 *x, size_t chainidx,
                                  int *al);
+int tls_parse_ctos_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al);
 
 int tls_construct_stoc_renegotiate(SSL *s, WPACKET *pkt, X509 *x,
                                    size_t chainidx, int *al);

ssl/t1_lib.c

@@ -20,10 +20,6 @@
 #include "ssl_locl.h"
 #include <openssl/ct.h>
 
-static int tls_decrypt_ticket(SSL *s, const unsigned char *tick, size_t ticklen,
-                              const unsigned char *sess_id, size_t sesslen,
-                              SSL_SESSION **psess);
-
 SSL3_ENC_METHOD const TLSv1_enc_data = {
     tls1_enc,
     tls1_mac,
@@ -1097,14 +1093,14 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
     retv = tls_decrypt_ticket(s, PACKET_data(&ticketext->data), size,
                               hello->session_id, hello->session_id_len, ret);
     switch (retv) {
-    case 2:            /* ticket couldn't be decrypted */
+    case TICKET_NO_DECRYPT: /* ticket couldn't be decrypted */
         s->ext.ticket_expected = 1;
         return 2;
 
-    case 3:            /* ticket was decrypted */
+    case TICKET_SUCCESS: /* ticket was decrypted */
         return 3;
 
-    case 4:            /* ticket decrypted but need to renew */
+    case TICKET_SUCCESS_RENEW: /* ticket decrypted but need to renew */
         s->ext.ticket_expected = 1;
         return 3;
 
@@ -1124,20 +1120,27 @@ int tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
  *       point to the resulting session.
  *
  * Returns:
- *   -2: fatal error, malloc failure.
+ *   TICKET_FATAL_ERR_MALLOC: fatal error, malloc failure.
- *   -1: fatal error, either from parsing or decrypting the ticket.
+ *   TICKET_FATAL_ERR_OTHER:  fatal error, either from parsing or decrypting the
- *    2: the ticket couldn't be decrypted.
+ *                            ticket.
- *    3: a ticket was successfully decrypted and *psess was set.
+ *   TICKET_NO_DECRYPT:       the ticket couldn't be decrypted.
- *    4: same as 3, but the ticket needs to be renewed.
+ *   TICKET_SUCCESS:          a ticket was successfully decrypted and *psess was
+ *                            set.
+ *   TICKET_SUCCESS_RENEW:    same as 3, but the ticket needs to be renewed
  */
-static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
+#define TICKET_FATAL_ERR_MALLOC     -2
-                              size_t eticklen, const unsigned char *sess_id,
+#define TICKET_FATAL_ERR_OTHER      -1
-                              size_t sesslen, SSL_SESSION **psess)
+#define TICKET_NO_DECRYPT            2
+#define TICKET_SUCCESS               3
+#define TICKET_SUCCESS_RENEW         4
+int tls_decrypt_ticket(SSL *s, const unsigned char *etick, size_t eticklen,
+                       const unsigned char *sess_id, size_t sesslen,
+                       SSL_SESSION **psess)
 {
     SSL_SESSION *sess;
     unsigned char *sdec;
     const unsigned char *p;
-    int slen, renew_ticket = 0, ret = -1, declen;
+    int slen, renew_ticket = 0, ret = TICKET_FATAL_ERR_OTHER, declen;
     size_t mlen;
     unsigned char tick_hmac[EVP_MAX_MD_SIZE];
     HMAC_CTX *hctx = NULL;
@@ -1147,10 +1150,10 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     /* Initialize session ticket encryption and HMAC contexts */
     hctx = HMAC_CTX_new();
     if (hctx == NULL)
-        return -2;
+        return TICKET_FATAL_ERR_MALLOC;
     ctx = EVP_CIPHER_CTX_new();
     if (ctx == NULL) {
-        ret = -2;
+        ret = TICKET_FATAL_ERR_MALLOC;
         goto err;
     }
     if (tctx->ext.ticket_key_cb) {
@@ -1160,7 +1163,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
         if (rv < 0)
             goto err;
         if (rv == 0) {
-            ret = 2;
+            ret = TICKET_NO_DECRYPT;
             goto err;
         }
         if (rv == 2)
@@ -1169,7 +1172,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
         /* Check key name matches */
         if (memcmp(etick, tctx->ext.tick_key_name,
                    sizeof(tctx->ext.tick_key_name)) != 0) {
-            ret = 2;
+            ret = TICKET_NO_DECRYPT;
             goto err;
         }
         if (HMAC_Init_ex(hctx, tctx->ext.tick_hmac_key,
@@ -1177,8 +1180,8 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
                          EVP_sha256(), NULL) <= 0
             || EVP_DecryptInit_ex(ctx, EVP_aes_256_cbc(), NULL,
                                   tctx->ext.tick_aes_key,
-                                  etick + sizeof(tctx->ext.tick_key_name)) <=
+                                  etick
-            0) {
+                                  + sizeof(tctx->ext.tick_key_name)) <= 0) {
             goto err;
         }
     }
@@ -1193,7 +1196,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     /* Sanity check ticket length: must exceed keyname + IV + HMAC */
     if (eticklen <=
         TLSEXT_KEYNAME_LENGTH + EVP_CIPHER_CTX_iv_length(ctx) + mlen) {
-        ret = 2;
+        ret = TICKET_NO_DECRYPT;
         goto err;
     }
     eticklen -= mlen;
@@ -1205,7 +1208,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     HMAC_CTX_free(hctx);
     if (CRYPTO_memcmp(tick_hmac, etick + eticklen, mlen)) {
         EVP_CIPHER_CTX_free(ctx);
-        return 2;
+        return TICKET_NO_DECRYPT;
     }
     /* Attempt to decrypt session data */
     /* Move p after IV to start of encrypted ticket, update length */
@@ -1216,12 +1219,12 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
                                           (int)eticklen) <= 0) {
         EVP_CIPHER_CTX_free(ctx);
         OPENSSL_free(sdec);
-        return -1;
+        return TICKET_FATAL_ERR_OTHER;
     }
     if (EVP_DecryptFinal(ctx, sdec + slen, &declen) <= 0) {
         EVP_CIPHER_CTX_free(ctx);
         OPENSSL_free(sdec);
-        return 2;
+        return TICKET_NO_DECRYPT;
     }
     slen += declen;
     EVP_CIPHER_CTX_free(ctx);
@@ -1242,15 +1245,15 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick,
         sess->session_id_length = sesslen;
         *psess = sess;
         if (renew_ticket)
-            return 4;
+            return TICKET_SUCCESS_RENEW;
         else
-            return 3;
+            return TICKET_SUCCESS;
     }
     ERR_clear_error();
     /*
      * For session parse failure, indicate that we need to send a new ticket.
      */
-    return 2;
+    return TICKET_NO_DECRYPT;
  err:
     EVP_CIPHER_CTX_free(ctx);
     HMAC_CTX_free(hctx);