Rework the decrypt ticket callback

Don't call the decrypt ticket callback if we've already encountered a
fatal error. Do call it if we have an empty ticket present.

Change the return code to have 5 distinct returns codes and separate it
from the input status value.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6198)

9 files changed, 336 insertions, 145 deletions

diff --git a/doc/man3/SSL_CTX_set_session_ticket_cb.pod b/doc/man3/SSL_CTX_set_session_ticket_cb.pod
index 3066534223..8f98c6f1c9 100644
--- a/doc/man3/SSL_CTX_set_session_ticket_cb.pod
+++ b/doc/man3/SSL_CTX_set_session_ticket_cb.pod
@@ -16,7 +16,7 @@ SSL_CTX_decrypt_session_ticket_fn - manage session ticket application data
  typedef SSL_TICKET_RETURN (*SSL_CTX_decrypt_session_ticket_fn)(SSL *s, SSL_SESSION *ss,
                                                                 const unsigned char *keyname,
                                                                 size_t keyname_len,
-                                                                SSL_TICKET_RETURN retv,
+                                                                SSL_TICKET_STATUS status,
                                                                 void *arg);
  int SSL_CTX_set_session_ticket_cb(SSL_CTX *ctx,
                                    SSL_CTX_generate_session_ticket_fn gen_cb,
@@ -39,13 +39,13 @@ is the same as that given to SSL_CTX_set_session_ticket_cb(). The B<gen_cb>
 callback is defined as type B<SSL_CTX_generate_session_ticket_fn>.
 
 B<dec_cb> is the application defined callback invoked after session ticket
-decryption has been attempted and any session ticket application data is available.
-The application can call SSL_SESSION_get_ticket_appdata() at this time to retrieve
-the application data. The value of B<arg> is the same as that given to
-SSL_CTX_set_session_ticket_cb(). The B<retv> argument is the result of the ticket
-decryption. The B<keyname> and B<keyname_len> identify the key used to decrypt the
-session ticket. The B<dec_cb> callback is defined as type
-B<SSL_CTX_decrypt_session_ticket_fn>.
+decryption has been attempted and any session ticket application data is
+available. If ticket decryption was successful then the B<ss> argument contains
+the session data. The B<keyname> and B<keyname_len> arguments identify the key
+used to decrypt the session ticket. The B<status> argument is the result of the
+ticket decryption. See the L<NOTES> section below for further details. The value
+of B<arg> is the same as that given to SSL_CTX_set_session_ticket_cb(). The
+B<dec_cb> callback is defined as type B<SSL_CTX_decrypt_session_ticket_fn>.
 
 SSL_SESSION_set1_ticket_appdata() sets the application data specified by
 B<data> and B<len> into B<ss> which is then placed into any generated session
@@ -66,73 +66,110 @@ application that a session ticket has just been decrypted.
 =head1 NOTES
 
 When the B<dec_cb> callback is invoked, the SSL_SESSION B<ss> has not yet been
-assigned to the SSL B<s>. The B<retv> indicates the result of the ticket
-decryption which can be modified by the callback before being returned. The
-callback must check the B<retv> value before performing any action, as it's
-called even if ticket decryption fails.
+assigned to the SSL B<s>. The B<status> indicates the result of the ticket
+decryption. The callback must check the B<status> value before performing any
+action, as it is called even if ticket decryption fails.
 
 The B<keyname> and B<keyname_len> arguments to B<dec_cb> may be used to identify
 the key that was used to encrypt the session ticket.
 
-When the B<gen_cb> callback is invoked, the SSL_get_session() function can be
-used to retrieve the SSL_SESSION for SSL_SESSION_set1_ticket_appdata().
+The B<status> argument can be any of these values:
 
-By default, in TLSv1.2 and below, a new session ticket is not issued on a
-successful resumption and therefore B<gen_cb> will not be called. In TLSv1.3 the
-default behaviour is to always issue a new ticket on resumption. In both cases
-this behaviour can be changed if a ticket key callback is in use (see
-L<SSL_CTX_set_tlsext_ticket_key_cb(3)>).
+=over 4
 
-=head1 RETURN VALUES
+=item SSL_TICKET_EMPTY
 
-The SSL_CTX_set_session_ticket_cb(), SSL_SESSION_set1_ticket_appdata() and
-SSL_SESSION_get0_ticket_appdata() functions return 1 on success and 0 on
-failure.
+Empty ticket present. No ticket data will be used and a new ticket should be
+sent to the client. This only occurs in TLSv1.2 or below. In TLSv1.3 it is not
+valid for a client to send an empty ticket.
 
-The B<gen_cb> callback must return 1 to continue the connection. A return of 0
-will terminate the connection with an INTERNAL_ERROR alert.
+=item SSL_TICKET_NO_DECRYPT
 
-The B<dec_cb> callback must return one of the following B<SSL_TICKET_RETURN>
-values. Under normal circumstances the B<retv> value is returned unmodified,
-but the callback can change the behavior of the post-ticket decryption code
-by returning something different. The B<dec_cb> callback must check the B<retv>
-value before performing any action.
+The ticket couldn't be decrypted. No ticket data will be used and a new ticket
+should be sent to the client.
 
- typedef int SSL_TICKET_RETURN;
+=item SSL_TICKET_SUCCESS
 
-=over 4
+A ticket was successfully decrypted, any session ticket application data should
+be available. A new ticket should not be sent to the client.
 
-=item SSL_TICKET_FATAL_ERR_MALLOC
+=item SSL_TICKET_SUCCESS_RENEW
 
-Fatal error, malloc failure.
+Same as B<SSL_TICKET_SUCCESS>, but a new ticket should be sent to the client.
 
-=item SSL_TICKET_FATAL_ERR_OTHER
+=back
 
-Fatal error, either from parsing or decrypting the ticket.
+The return value can be any of these values:
 
-=item SSL_TICKET_NONE
+=over 4
 
-No ticket present.
+=item SSL_TICKET_RETURN_ABORT
 
-=item SSL_TICKET_EMPTY
+The handshake should be aborted, either because of an error or because of some
+policy. Note that in TLSv1.3 a client may send more than one ticket in a single
+handshake. Therefore just because one ticket is unacceptable it does not mean
+that all of them are. For this reason this option should be used with caution.
 
-Empty ticket present.
+=item SSL_TICKET_RETURN_IGNORE
 
-=item SSL_TICKET_NO_DECRYPT
+Do not use a ticket (if one was available). Do not send a renewed ticket to the
+client.
 
-The ticket couldn't be decrypted.
+=item SSL_TICKET_RETURN_IGNORE_RENEW
 
-=item SSL_TICKET_SUCCESS
+Do not use a ticket (if one was available). Send a renewed ticket to the client.
 
-A ticket was successfully decrypted, any session ticket application data should
-be available.
+If the callback does not wish to change the default ticket behaviour then it
+should return this value if B<status> is B<SSL_TICKET_EMPTY> or
+B<SSL_TICKET_NO_DECRYPT>.
 
-=item TICKET_SUCCESS_RENEW
+=item SSL_TICKET_RETURN_USE
 
-Same as B<TICKET_SUCCESS>, but the ticket needs to be renewed.
+Use the ticket. Do not send a renewed ticket to the client. It is an error for
+the callback to return this value if B<status> has a value other than
+B<SSL_TICKET_SUCCESS> or B<SSL_TICKET_SUCCESS_RENEW>.
+
+If the callback does not wish to change the default ticket behaviour then it
+should return this value if B<status> is B<SSL_TICKET_SUCCESS>.
+
+=item SSL_TICKET_RETURN_USE_RENEW
+
+Use the ticket. Send a renewed ticket to the client. It is an error for the
+callback to return this value if B<status> has a value other than
+B<SSL_TICKET_SUCCESS> or B<SSL_TICKET_SUCCESS_RENEW>.
+
+If the callback does not wish to change the default ticket behaviour then it
+should return this value if B<status> is B<SSL_TICKET_SUCCESS_RENEW>.
 
 =back
 
+If B<status> has the value B<SSL_TICKET_EMPTY> or B<SSL_TICKET_NO_DECRYPT> then
+no session data will be available and the callback must not use the B<ss>
+argument. If B<status> has the value B<SSL_TICKET_SUCCESS> or
+B<SSL_TICKET_SUCCESS_RENEW> then the application can call
+SSL_SESSION_get0_ticket_appdata() using the session provided in the B<ss>
+argument to retrieve the application data.
+
+When the B<gen_cb> callback is invoked, the SSL_get_session() function can be
+used to retrieve the SSL_SESSION for SSL_SESSION_set1_ticket_appdata().
+
+By default, in TLSv1.2 and below, a new session ticket is not issued on a
+successful resumption and therefore B<gen_cb> will not be called. In TLSv1.3 the
+default behaviour is to always issue a new ticket on resumption. In both cases
+this behaviour can be changed if a ticket key callback is in use (see
+L<SSL_CTX_set_tlsext_ticket_key_cb(3)>).
+
+=head1 RETURN VALUES
+
+The SSL_CTX_set_session_ticket_cb(), SSL_SESSION_set1_ticket_appdata() and
+SSL_SESSION_get0_ticket_appdata() functions return 1 on success and 0 on
+failure.
+
+The B<gen_cb> callback must return 1 to continue the connection. A return of 0
+will terminate the connection with an INTERNAL_ERROR alert.
+
+The B<dec_cb> callback must return a value as described in L<NOTES> above.
+
 =head1 SEE ALSO
 
 L<ssl(7)>,
diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 4bd53fc24c..1f4f2616b6 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2330,8 +2330,9 @@ __owur const struct openssl_ssl_test_functions *SSL_test_functions(void);
 __owur int SSL_free_buffers(SSL *ssl);
 __owur int SSL_alloc_buffers(SSL *ssl);
 
-/* Return codes for tls_get_ticket_from_client() and tls_decrypt_ticket() */
-typedef int SSL_TICKET_RETURN;
+/* Status codes passed to the decrypt session ticket callback. Some of these
+ * are for internal use only and are never passed to the callback. */
+typedef int SSL_TICKET_STATUS;
 
 /* Support for ticket appdata */
 /* fatal error, malloc failure */
@@ -2349,11 +2350,25 @@ typedef int SSL_TICKET_RETURN;
 /* same as above but the ticket needs to be renewed */
 # define SSL_TICKET_SUCCESS_RENEW    6
 
+/* Return codes for the decrypt session ticket callback */
+typedef int SSL_TICKET_RETURN;
+
+/* An error occurred */
+#define SSL_TICKET_RETURN_ABORT             0
+/* Do not use the ticket, do not send a renewed ticket to the client */
+#define SSL_TICKET_RETURN_IGNORE            1
+/* Do not use the ticket, send a renewed ticket to the client */
+#define SSL_TICKET_RETURN_IGNORE_RENEW      2
+/* Use the ticket, do not send a renewed ticket to the client */
+#define SSL_TICKET_RETURN_USE               3
+/* Use the ticket, send a renewed ticket to the client */
+#define SSL_TICKET_RETURN_USE_RENEW         4
+
 typedef int (*SSL_CTX_generate_session_ticket_fn)(SSL *s, void *arg);
 typedef SSL_TICKET_RETURN (*SSL_CTX_decrypt_session_ticket_fn)(SSL *s, SSL_SESSION *ss,
                                                                const unsigned char *keyname,
                                                                size_t keyname_length,
-                                                               SSL_TICKET_RETURN retv,
+                                                               SSL_TICKET_STATUS status,
                                                                void *arg);
 int SSL_CTX_set_session_ticket_cb(SSL_CTX *ctx,
                                   SSL_CTX_generate_session_ticket_fn gen_cb,
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index b32b23bedf..c066e14b70 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2473,9 +2473,9 @@ void tls1_get_supported_groups(SSL *s, const uint16_t **pgroups,
 
 __owur int tls1_set_server_sigalgs(SSL *s);
 
-__owur SSL_TICKET_RETURN tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
+__owur SSL_TICKET_STATUS tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
                                                     SSL_SESSION **ret);
-__owur SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
+__owur SSL_TICKET_STATUS tls_decrypt_ticket(SSL *s, const unsigned char *etick,
                                             size_t eticklen,
                                             const unsigned char *sess_id,
                                             size_t sesslen, SSL_SESSION **psess);
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index f936cb687f..541f82a851 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -485,9 +485,14 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
     SSL_SESSION *ret = NULL;
     int fatal = 0, discard;
     int try_session_cache = 0;
-    SSL_TICKET_RETURN r;
+    SSL_TICKET_STATUS r;
 
     if (SSL_IS_TLS13(s)) {
+        /*
+         * By default we will send a new ticket. This can be overridden in the
+         * ticket processing.
+         */
+        s->ext.ticket_expected = 1;
         if (!tls_parse_extension(s, TLSEXT_IDX_psk_kex_modes,
                                  SSL_EXT_CLIENT_HELLO, hello->pre_proc_exts,
                                  NULL, 0)
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index adf63d80bf..ec4e1b8139 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1030,6 +1030,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
+    s->ext.ticket_expected = 0;
     for (id = 0; PACKET_remaining(&identities) != 0; id++) {
         PACKET identity;
         unsigned long ticket_agel;
@@ -1127,9 +1128,17 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                 s->ext.early_data_ok = 1;
         } else {
             uint32_t ticket_age = 0, now, agesec, agems;
-            int ret = tls_decrypt_ticket(s, PACKET_data(&identity),
-                                         PACKET_remaining(&identity), NULL, 0,
-                                         &sess);
+            int ret;
+
+            ret = tls_decrypt_ticket(s, PACKET_data(&identity),
+                                     PACKET_remaining(&identity), NULL, 0,
+                                     &sess);
+
+            if (ret == SSL_TICKET_EMPTY) {
+                SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_TLS_PARSE_CTOS_PSK,
+                         SSL_R_BAD_EXTENSION);
+                return 0;
+            }
 
             if (ret == SSL_TICKET_FATAL_ERR_MALLOC
                     || ret == SSL_TICKET_FATAL_ERR_OTHER) {
@@ -1137,7 +1146,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
                          SSL_F_TLS_PARSE_CTOS_PSK, ERR_R_INTERNAL_ERROR);
                 return 0;
             }
-            if (ret == SSL_TICKET_NO_DECRYPT)
+            if (ret == SSL_TICKET_NONE || ret == SSL_TICKET_NO_DECRYPT)
                 continue;
 
             /* Check for replay */
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index 3f56ff1389..22786bed13 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -489,10 +489,10 @@ static WRITE_TRAN ossl_statem_server13_write_transition(SSL *s)
         st->hand_state = TLS_ST_SW_SESSION_TICKET;
         if (s->post_handshake_auth == SSL_PHA_REQUESTED) {
             s->post_handshake_auth = SSL_PHA_EXT_RECEIVED;
-        } else if (s->hit && !s->ext.ticket_expected) {
+        } else if (!s->ext.ticket_expected) {
             /*
-             * If we resumed and we're not going to renew the ticket then we
-             * just finish the handshake at this point.
+             * If we're not going to renew the ticket then we just finish the
+             * handshake at this point.
              */
             st->hand_state = TLS_ST_OK;
         }
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 2e8de7a09c..b312a14fab 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1194,20 +1194,8 @@ int tls1_set_server_sigalgs(SSL *s)
  *   hello: The parsed ClientHello data
  *   ret: (output) on return, if a ticket was decrypted, then this is set to
  *       point to the resulting session.
- *
- * If s->tls_session_secret_cb is set then we are expecting a pre-shared key
- * ciphersuite, in which case we have no use for session tickets and one will
- * never be decrypted, nor will s->ext.ticket_expected be set to 1.
- *
- * Side effects:
- *   Sets s->ext.ticket_expected to 1 if the server will have to issue
- *   a new session ticket to the client because the client indicated support
- *   (and s->tls_session_secret_cb is NULL) but the client either doesn't have
- *   a session ticket or we couldn't use the one it gave us, or if
- *   s->ctx->ext.ticket_key_cb asked to renew the client's ticket.
- *   Otherwise, s->ext.ticket_expected is set to 0.
  */
-SSL_TICKET_RETURN tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
+SSL_TICKET_STATUS tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
                                              SSL_SESSION **ret)
 {
     size_t size;
@@ -1229,23 +1217,6 @@ SSL_TICKET_RETURN tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
         return SSL_TICKET_NONE;
 
     size = PACKET_remaining(&ticketext->data);
-    if (size == 0) {
-        /*
-         * The client will accept a ticket but doesn't currently have
-         * one.
-         */
-        s->ext.ticket_expected = 1;
-        return SSL_TICKET_EMPTY;
-    }
-    if (s->ext.session_secret_cb) {
-        /*
-         * Indicate that the ticket couldn't be decrypted rather than
-         * generating the session from ticket now, trigger
-         * abbreviated handshake based on external mechanism to
-         * calculate the master secret later.
-         */
-        return SSL_TICKET_NO_DECRYPT;
-    }
 
     return tls_decrypt_ticket(s, PACKET_data(&ticketext->data), size,
                               hello->session_id, hello->session_id_len, ret);
@@ -1254,6 +1225,19 @@ SSL_TICKET_RETURN tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
 /*-
  * tls_decrypt_ticket attempts to decrypt a session ticket.
  *
+ * If s->tls_session_secret_cb is set and we're not doing TLSv1.3 then we are
+ * expecting a pre-shared key ciphersuite, in which case we have no use for
+ * session tickets and one will never be decrypted, nor will
+ * s->ext.ticket_expected be set to 1.
+ *
+ * Side effects:
+ *   Sets s->ext.ticket_expected to 1 if the server will have to issue
+ *   a new session ticket to the client because the client indicated support
+ *   (and s->tls_session_secret_cb is NULL) but the client either doesn't have
+ *   a session ticket or we couldn't use the one it gave us, or if
+ *   s->ctx->ext.ticket_key_cb asked to renew the client's ticket.
+ *   Otherwise, s->ext.ticket_expected is set to 0.
+ *
  *   etick: points to the body of the session ticket extension.
  *   eticklen: the length of the session tickets extension.
  *   sess_id: points at the session ID.
@@ -1261,21 +1245,40 @@ SSL_TICKET_RETURN tls_get_ticket_from_client(SSL *s, CLIENTHELLO_MSG *hello,
  *   psess: (output) on return, if a ticket was decrypted, then this is set to
  *       point to the resulting session.
  */
-SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
+SSL_TICKET_STATUS 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;
+    SSL_SESSION *sess = NULL;
     unsigned char *sdec;
     const unsigned char *p;
     int slen, renew_ticket = 0, declen;
-    SSL_TICKET_RETURN ret = SSL_TICKET_FATAL_ERR_OTHER;
+    SSL_TICKET_STATUS ret = SSL_TICKET_FATAL_ERR_OTHER;
     size_t mlen;
     unsigned char tick_hmac[EVP_MAX_MD_SIZE];
     HMAC_CTX *hctx = NULL;
     EVP_CIPHER_CTX *ctx = NULL;
     SSL_CTX *tctx = s->session_ctx;
 
+    if (eticklen == 0) {
+        /*
+         * The client will accept a ticket but doesn't currently have
+         * one (TLSv1.2 and below), or treated as a fatal error in TLSv1.3
+         */
+        ret = SSL_TICKET_EMPTY;
+        goto end;
+    }
+    if (!SSL_IS_TLS13(s) && s->ext.session_secret_cb) {
+        /*
+         * Indicate that the ticket couldn't be decrypted rather than
+         * generating the session from ticket now, trigger
+         * abbreviated handshake based on external mechanism to
+         * calculate the master secret later.
+         */
+        ret = SSL_TICKET_NO_DECRYPT;
+        goto end;
+    }
+
     /* Need at least keyname + iv */
     if (eticklen < TLSEXT_KEYNAME_LENGTH + EVP_MAX_IV_LENGTH) {
         ret = SSL_TICKET_NO_DECRYPT;
@@ -1394,7 +1397,6 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
             memcpy(sess->session_id, sess_id, sesslen);
             sess->session_id_length = sesslen;
         }
-        *psess = sess;
         if (renew_ticket)
             ret = SSL_TICKET_SUCCESS_RENEW;
         else
@@ -1412,18 +1414,56 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     HMAC_CTX_free(hctx);
 
     /*
-     * If set, the decrypt_ticket_cb() is always called regardless of the
-     * return value determined above. The callback is responsible for checking
-     * |ret| before it performs any action
+     * If set, the decrypt_ticket_cb() is called unless a fatal error was
+     * detected above. The callback is responsible for checking |ret| before it
+     * performs any action
      */
-    if (s->session_ctx->decrypt_ticket_cb != NULL) {
+    if (s->session_ctx->decrypt_ticket_cb != NULL
+            && (ret == SSL_TICKET_EMPTY
+                || ret == SSL_TICKET_NO_DECRYPT
+                || ret == SSL_TICKET_SUCCESS
+                || ret == SSL_TICKET_SUCCESS_RENEW)) {
         size_t keyname_len = eticklen;
+        int retcb;
 
         if (keyname_len > TLSEXT_KEYNAME_LENGTH)
             keyname_len = TLSEXT_KEYNAME_LENGTH;
-        ret = s->session_ctx->decrypt_ticket_cb(s, *psess, etick, keyname_len,
-                                                ret,
-                                                s->session_ctx->ticket_cb_data);
+        retcb = s->session_ctx->decrypt_ticket_cb(s, sess, etick, keyname_len,
+                                                  ret,
+                                                  s->session_ctx->ticket_cb_data);
+        switch (retcb) {
+        case SSL_TICKET_RETURN_ABORT:
+            ret = SSL_TICKET_FATAL_ERR_OTHER;
+            break;
+
+        case SSL_TICKET_RETURN_IGNORE:
+            ret = SSL_TICKET_NONE;
+            SSL_SESSION_free(sess);
+            sess = NULL;
+            break;
+
+        case SSL_TICKET_RETURN_IGNORE_RENEW:
+            if (ret != SSL_TICKET_EMPTY && ret != SSL_TICKET_NO_DECRYPT)
+                ret = SSL_TICKET_NO_DECRYPT;
+            /* else the value of |ret| will already do the right thing */
+            SSL_SESSION_free(sess);
+            sess = NULL;
+            break;
+
+        case SSL_TICKET_RETURN_USE:
+        case SSL_TICKET_RETURN_USE_RENEW:
+            if (ret != SSL_TICKET_SUCCESS
+                    && ret != SSL_TICKET_SUCCESS_RENEW)
+                ret = SSL_TICKET_FATAL_ERR_OTHER;
+            else if (retcb == SSL_TICKET_RETURN_USE)
+                ret = SSL_TICKET_SUCCESS;
+            else
+                ret = SSL_TICKET_SUCCESS_RENEW;
+            break;
+
+        default:
+            ret = SSL_TICKET_FATAL_ERR_OTHER;
+        }
     }
 
     switch (ret) {
@@ -1431,13 +1471,11 @@ SSL_TICKET_RETURN tls_decrypt_ticket(SSL *s, const unsigned char *etick,
     case SSL_TICKET_SUCCESS_RENEW:
     case SSL_TICKET_EMPTY:
         s->ext.ticket_expected = 1;
-        /* Fall through */
-    case SSL_TICKET_SUCCESS:
-    case SSL_TICKET_NONE:
-        return ret;
     }
 
-    return SSL_TICKET_FATAL_ERR_OTHER;
+    *psess = sess;
+
+    return ret;
 }
 
 /* Check to see if a signature algorithm is allowed */
diff --git a/test/handshake_helper.c b/test/handshake_helper.c
index fc5fcd6f4f..b3d94bb1ee 100644
--- a/test/handshake_helper.c
+++ b/test/handshake_helper.c
@@ -469,12 +469,24 @@ static int generate_session_ticket_cb(SSL *s, void *arg)
     return SSL_SESSION_set1_ticket_appdata(ss, app_data, strlen(app_data));
 }
 
-static SSL_TICKET_RETURN decrypt_session_ticket_cb(SSL *s, SSL_SESSION *ss,
-                                                   const unsigned char *keyname,
-                                                   size_t keyname_len,
-                                                   SSL_TICKET_RETURN retv, void *arg)
+static int decrypt_session_ticket_cb(SSL *s, SSL_SESSION *ss,
+                                     const unsigned char *keyname,
+                                     size_t keyname_len,
+                                     SSL_TICKET_STATUS status,
+                                     void *arg)
 {
-    return retv;
+    switch (status) {
+    case SSL_TICKET_EMPTY:
+    case SSL_TICKET_NO_DECRYPT:
+        return SSL_TICKET_RETURN_IGNORE_RENEW;
+    case SSL_TICKET_SUCCESS:
+        return SSL_TICKET_RETURN_USE;
+    case SSL_TICKET_SUCCESS_RENEW:
+        return SSL_TICKET_RETURN_USE_RENEW;
+    default:
+        break;
+    }
+    return SSL_TICKET_RETURN_ABORT;
 }
 
 /*
diff --git a/test/sslapitest.c b/test/sslapitest.c
index efce84d2d9..fc7288d57d 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -4596,7 +4596,9 @@ static int test_ssl_get_shared_ciphers(int tst)
 }
 
 static const char *appdata = "Hello World";
-static int gen_tick_called, dec_tick_called;
+static int gen_tick_called, dec_tick_called, tick_key_cb_called;
+static int tick_key_renew = 0;
+static SSL_TICKET_RETURN tick_dec_ret = SSL_TICKET_RETURN_ABORT;
 
 static int gen_tick_cb(SSL *s, void *arg)
 {
@@ -4609,26 +4611,46 @@ static int gen_tick_cb(SSL *s, void *arg)
 static SSL_TICKET_RETURN dec_tick_cb(SSL *s, SSL_SESSION *ss,
                                      const unsigned char *keyname,
                                      size_t keyname_length,
-                                     SSL_TICKET_RETURN retv, void *arg)
+                                     SSL_TICKET_STATUS status,
+                                     void *arg)
 {
     void *tickdata;
     size_t tickdlen;
 
     dec_tick_called = 1;
 
-    if (!TEST_true(retv == SSL_TICKET_SUCCESS
-                   || retv == SSL_TICKET_SUCCESS_RENEW))
-        return retv;
+    if (status == SSL_TICKET_EMPTY)
+        return SSL_TICKET_RETURN_IGNORE_RENEW;
 
-    if (!TEST_true(SSL_SESSION_get0_ticket_appdata(ss, &tickdata, &tickdlen))
+    if (!TEST_true(status == SSL_TICKET_SUCCESS
+                   || status == SSL_TICKET_SUCCESS_RENEW))
+        return SSL_TICKET_RETURN_ABORT;
+
+    if (!TEST_true(SSL_SESSION_get0_ticket_appdata(ss, &tickdata,
+                                                   &tickdlen))
             || !TEST_size_t_eq(tickdlen, strlen(appdata))
             || !TEST_int_eq(memcmp(tickdata, appdata, tickdlen), 0))
-        return SSL_TICKET_FATAL_ERR_OTHER;
+        return SSL_TICKET_RETURN_ABORT;
 
-    return retv;
-}
+    if (tick_key_cb_called)  {
+        /* Don't change what the ticket key callback wanted to do */
+        switch (status) {
+        case SSL_TICKET_NO_DECRYPT:
+            return SSL_TICKET_RETURN_IGNORE_RENEW;
+
+        case SSL_TICKET_SUCCESS:
+            return SSL_TICKET_RETURN_USE;
 
-static int tick_renew = 0;
+        case SSL_TICKET_SUCCESS_RENEW:
+            return SSL_TICKET_RETURN_USE_RENEW;
+
+        default:
+            return SSL_TICKET_RETURN_ABORT;
+        }
+    }
+    return tick_dec_ret;
+
+}
 
 static int tick_key_cb(SSL *s, unsigned char key_name[16],
                        unsigned char iv[EVP_MAX_IV_LENGTH], EVP_CIPHER_CTX *ctx,
@@ -4637,6 +4659,7 @@ static int tick_key_cb(SSL *s, unsigned char key_name[16],
     const unsigned char tick_aes_key[16] = "0123456789abcdef";
     const unsigned char tick_hmac_key[16] = "0123456789abcdef";
 
+    tick_key_cb_called = 1;
     memset(iv, 0, AES_BLOCK_SIZE);
     memset(key_name, 0, 16);
     if (!EVP_CipherInit_ex(ctx, EVP_aes_128_cbc(), NULL, tick_aes_key, iv, enc)
@@ -4644,17 +4667,23 @@ static int tick_key_cb(SSL *s, unsigned char key_name[16],
                              EVP_sha256(), NULL))
         return -1;
 
-    return tick_renew ? 2 : 1;
+    return tick_key_renew ? 2 : 1;
 }
 
 /*
  * Test the various ticket callbacks
- * Test 0: TLSv1.2, no ticket key callback (default no ticket renewal)
- * Test 1: TLSv1.3, no ticket key callback (default ticket renewal)
- * Test 2: TLSv1.2, ticket key callback, no ticket renewal
- * Test 3: TLSv1.3, ticket key callback, no ticket renewal
- * Test 4: TLSv1.2, ticket key callback, ticket renewal
- * Test 5: TLSv1.3, ticket key callback, ticket renewal
+ * Test 0: TLSv1.2, no ticket key callback, no ticket, no renewal
+ * Test 1: TLSv1.3, no ticket key callback, no ticket, no renewal
+ * Test 2: TLSv1.2, no ticket key callback, no ticket, renewal
+ * Test 3: TLSv1.3, no ticket key callback, no ticket, renewal
+ * Test 4: TLSv1.2, no ticket key callback, ticket, no renewal
+ * Test 5: TLSv1.3, no ticket key callback, ticket, no renewal
+ * Test 6: TLSv1.2, no ticket key callback, ticket, renewal
+ * Test 7: TLSv1.3, no ticket key callback, ticket, renewal
+ * Test 8: TLSv1.2, ticket key callback, ticket, no renewal
+ * Test 9: TLSv1.3, ticket key callback, ticket, no renewal
+ * Test 10: TLSv1.2, ticket key callback, ticket, renewal
+ * Test 11: TLSv1.3, ticket key callback, ticket, renewal
  */
 static int test_ticket_callbacks(int tst)
 {
@@ -4672,27 +4701,60 @@ static int test_ticket_callbacks(int tst)
         return 1;
 #endif
 
-    gen_tick_called = dec_tick_called = 0;
+    gen_tick_called = dec_tick_called = tick_key_cb_called = 0;
 
     /* Which tests the ticket key callback should request renewal for */
-    if (tst == 4 || tst == 5)
-        tick_renew = 1;
+    if (tst == 10 || tst == 11)
+        tick_key_renew = 1;
     else
-        tick_renew = 0;
+        tick_key_renew = 0;
+
+    /* Which tests the decrypt ticket callback should request renewal for */
+    switch (tst) {
+    case 0:
+    case 1:
+        tick_dec_ret = SSL_TICKET_RETURN_IGNORE;
+        break;
+
+    case 2:
+    case 3:
+        tick_dec_ret = SSL_TICKET_RETURN_IGNORE_RENEW;
+        break;
+
+    case 4:
+    case 5:
+        tick_dec_ret = SSL_TICKET_RETURN_USE;
+        break;
+
+    case 6:
+    case 7:
+        tick_dec_ret = SSL_TICKET_RETURN_USE_RENEW;
+        break;
+
+    default:
+        tick_dec_ret = SSL_TICKET_RETURN_ABORT;
+    }
 
     if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(),
                                        TLS_client_method(),
                                        TLS1_VERSION,
-                                       tst == 0 ? TLS1_2_VERSION
-                                                : TLS1_3_VERSION,
+                                       ((tst % 2) == 0) ? TLS1_2_VERSION
+                                                        : TLS1_3_VERSION,
                                        &sctx, &cctx, cert, privkey)))
         goto end;
 
+    /*
+     * We only want sessions to resume from tickets - not the session cache. So
+     * switch the cache off.
+     */
+    if (!TEST_true(SSL_CTX_set_session_cache_mode(sctx, SSL_SESS_CACHE_OFF)))
+        goto end;
+
     if (!TEST_true(SSL_CTX_set_session_ticket_cb(sctx, gen_tick_cb, dec_tick_cb,
                                                  NULL)))
         goto end;
 
-    if (tst >= 2
+    if (tst >= 8
             && !TEST_true(SSL_CTX_set_tlsext_ticket_key_cb(sctx, tick_key_cb)))
         goto end;
 
@@ -4702,8 +4764,15 @@ static int test_ticket_callbacks(int tst)
                                                 SSL_ERROR_NONE)))
         goto end;
 
+    /*
+     * The decrypt ticket key callback in TLSv1.2 should be called even though
+     * we have no ticket yet, because it gets called with a status of
+     * SSL_TICKET_EMPTY (the client indicates support for tickets but does not
+     * actually send any ticket data). This does not happen in TLSv1.3 because
+     * it is not valid to send empty ticket data in TLSv1.3.
+     */
     if (!TEST_int_eq(gen_tick_called, 1)
-            || !TEST_int_eq(dec_tick_called, 0))
+            || !TEST_int_eq(dec_tick_called, ((tst % 2) == 0) ? 1 : 0))
         goto end;
 
     gen_tick_called = dec_tick_called = 0;
@@ -4720,17 +4789,23 @@ static int test_ticket_callbacks(int tst)
                                       NULL))
             || !TEST_true(SSL_set_session(clientssl, clntsess))
             || !TEST_true(create_ssl_connection(serverssl, clientssl,
-                                                SSL_ERROR_NONE))
-            || !TEST_true(SSL_session_reused(clientssl)))
+                                                SSL_ERROR_NONE)))
         goto end;
 
-    /*
-     * In TLSv1.2 we default to not renewing the ticket everytime. In TLSv1.3
-     * we default to renewing. The defaults are overridden if a ticket key
-     * callback is in place.
-     */
+    if (tick_dec_ret == SSL_TICKET_RETURN_IGNORE
+            || tick_dec_ret == SSL_TICKET_RETURN_IGNORE_RENEW) {
+        if (!TEST_false(SSL_session_reused(clientssl)))
+            goto end;
+    } else {
+        if (!TEST_true(SSL_session_reused(clientssl)))
+            goto end;
+    }
+
     if (!TEST_int_eq(gen_tick_called,
-                     (tst == 0 || tst == 2 || tst == 3) ? 0 : 1)
+                     (tick_key_renew
+                      || tick_dec_ret == SSL_TICKET_RETURN_IGNORE_RENEW
+                      || tick_dec_ret == SSL_TICKET_RETURN_USE_RENEW)
+                     ? 1 : 0)
             || !TEST_int_eq(dec_tick_called, 1))
         goto end;
 
@@ -4839,7 +4914,7 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_info_callback, 6);
     ADD_ALL_TESTS(test_ssl_pending, 2);
     ADD_ALL_TESTS(test_ssl_get_shared_ciphers, OSSL_NELEM(shared_ciphers_data));
-    ADD_ALL_TESTS(test_ticket_callbacks, 6);
+    ADD_ALL_TESTS(test_ticket_callbacks, 12);
     return 1;
 }