Fix session handling.
authorBodo Möller <bodo@openssl.org>
Mon, 5 Sep 2011 13:36:23 +0000 (13:36 +0000)
committerBodo Möller <bodo@openssl.org>
Mon, 5 Sep 2011 13:36:23 +0000 (13:36 +0000)
CHANGES
ssl/d1_srvr.c
ssl/s3_clnt.c
ssl/s3_srvr.c
ssl/ssl.h
ssl/ssl_lib.c
ssl/ssl_sess.c
ssl/t1_lib.c
test/testssl

diff --git a/CHANGES b/CHANGES
index eb4f58d..b3d4c06 100644 (file)
--- a/CHANGES
+++ b/CHANGES
   
  Changes between 1.0.0e and 1.0.1  [xx XXX xxxx]
 
+  *) Session-handling fixes:
+     - Fix handling of connections that are resuming with a session ID,
+       but also support Session Tickets.
+     - Fix a bug that suppressed issuing of a new ticket if the client
+       presented a ticket with an expired session.
+     - Try to set the ticket lifetime hint to something reasonable.
+     - Make tickets shorter by excluding irrelevant information.
+     - On the client side, don't ignore renewed tickets.
+     [Adam Langley, Bodo Moeller (Google)]
+
   *) Fix PSK session representation.
      [Bodo Moeller]
 
index 608502d..1cae330 100644 (file)
@@ -638,9 +638,6 @@ int dtls1_accept(SSL *s)
 
                        if (s->renegotiate == 2) /* skipped if we just sent a HelloRequest */
                                {
-                               /* actually not necessarily a 'new' session unless
-                                * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is set */
-                               
                                s->renegotiate=0;
                                s->new_session=0;
                                
index c821724..5597e13 100644 (file)
@@ -298,7 +298,16 @@ int ssl3_connect(SSL *s)
                        if (ret <= 0) goto end;
 
                        if (s->hit)
+                               {
                                s->state=SSL3_ST_CR_FINISHED_A;
+#ifndef OPENSSL_NO_TLSEXT
+                               if (s->tlsext_ticket_expected)
+                                       {
+                                       /* receive renewed session ticket */
+                                       s->state=SSL3_ST_CR_SESSION_TICKET_A;
+                                       }
+#endif
+                               }
                        else
                                s->state=SSL3_ST_CR_CERT_A;
                        s->init_num=0;
index 834f301..7551220 100644 (file)
@@ -695,14 +695,11 @@ int ssl3_accept(SSL *s)
                        ret=ssl3_get_finished(s,SSL3_ST_SR_FINISHED_A,
                                SSL3_ST_SR_FINISHED_B);
                        if (ret <= 0) goto end;
-#ifndef OPENSSL_NO_TLSEXT
-                       if (s->tlsext_ticket_expected)
-                               s->state=SSL3_ST_SW_SESSION_TICKET_A;
-                       else if (s->hit)
-                               s->state=SSL_ST_OK;
-#else
                        if (s->hit)
                                s->state=SSL_ST_OK;
+#ifndef OPENSSL_NO_TLSEXT
+                       else if (s->tlsext_ticket_expected)
+                               s->state=SSL3_ST_SW_SESSION_TICKET_A;
 #endif
                        else
                                s->state=SSL3_ST_SW_CHANGE_A;
@@ -789,9 +786,6 @@ int ssl3_accept(SSL *s)
 
                        if (s->renegotiate == 2) /* skipped if we just sent a HelloRequest */
                                {
-                               /* actually not necessarily a 'new' session unless
-                                * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is set */
-                               
                                s->renegotiate=0;
                                s->new_session=0;
                                
@@ -983,13 +977,16 @@ int ssl3_get_client_hello(SSL *s)
        j= *(p++);
 
        s->hit=0;
-       /* Versions before 0.9.7 always allow session reuse during renegotiation
-        * (i.e. when s->new_session is true), option
-        * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is new with 0.9.7.
-        * Maybe this optional behaviour should always have been the default,
-        * but we cannot safely change the default behaviour (or new applications
-        * might be written that become totally unsecure when compiled with
-        * an earlier library version)
+       /* Versions before 0.9.7 always allow clients to resume sessions in renegotiation.
+        * 0.9.7 and later allow this by default, but optionally ignore resumption requests
+        * with flag SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION (it's a new flag rather
+        * than a change to default behavior so that applications relying on this for security
+        * won't even compile against older library versions).
+        *
+        * 1.0.1 and later also have a function SSL_renegotiate_abbreviated() to request
+        * renegotiation but not a new session (s->new_session remains unset): for servers,
+        * this essentially just means that the SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION
+        * setting will be ignored.
         */
        if ((s->new_session && (s->options & SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION)))
                {
@@ -1444,20 +1441,20 @@ int ssl3_send_server_hello(SSL *s)
                memcpy(p,s->s3->server_random,SSL3_RANDOM_SIZE);
                p+=SSL3_RANDOM_SIZE;
 
-               /* now in theory we have 3 options to sending back the
-                * session id.  If it is a re-use, we send back the
-                * old session-id, if it is a new session, we send
-                * back the new session-id or we send back a 0 length
-                * session-id if we want it to be single use.
-                * Currently I will not implement the '0' length session-id
-                * 12-Jan-98 - I'll now support the '0' length stuff.
-                *
-                * We also have an additional case where stateless session
-                * resumption is successful: we always send back the old
-                * session id. In this case s->hit is non zero: this can
-                * only happen if stateless session resumption is succesful
-                * if session caching is disabled so existing functionality
-                * is unaffected.
+               /* There are several cases for the session ID to send
+                * back in the server hello:
+                * - For session reuse from the session cache,
+                *   we send back the old session ID.
+                * - If stateless session reuse (using a session ticket)
+                *   is successful, we send back the client's "session ID"
+                *   (which doesn't actually identify the session).
+                * - If it is a new session, we send back the new
+                *   session ID.
+                * - However, if we want the new session to be single-use,
+                *   we send back a 0-length session ID.
+                * s->hit is non-zero in either case of session reuse,
+                * so the following won't overwrite an ID that we're supposed
+                * to send back.
                 */
                if (s->session->not_resumable ||
                        (!(s->ctx->session_cache_mode & SSL_SESS_CACHE_SERVER)
@@ -3341,13 +3338,17 @@ int ssl3_send_server_certificate(SSL *s)
        /* SSL3_ST_SW_CERT_B */
        return(ssl3_do_write(s,SSL3_RT_HANDSHAKE));
        }
+
 #ifndef OPENSSL_NO_TLSEXT
+/* send a new session ticket (not necessarily for a new session) */
 int ssl3_send_newsession_ticket(SSL *s)
        {
        if (s->state == SSL3_ST_SW_SESSION_TICKET_A)
                {
                unsigned char *p, *senc, *macstart;
-               int len, slen;
+               const unsigned char *const_p;
+               int len, slen_full, slen;
+               SSL_SESSION *sess;
                unsigned int hlen;
                EVP_CIPHER_CTX ctx;
                HMAC_CTX hctx;
@@ -3356,12 +3357,38 @@ int ssl3_send_newsession_ticket(SSL *s)
                unsigned char key_name[16];
 
                /* get session encoding length */
-               slen = i2d_SSL_SESSION(s->session, NULL);
+               slen_full = i2d_SSL_SESSION(s->session, NULL);
                /* Some length values are 16 bits, so forget it if session is
                 * too long
                 */
-               if (slen > 0xFF00)
+               if (slen_full > 0xFF00)
+                       return -1;
+               senc = OPENSSL_malloc(slen_full);
+               if (!senc)
+                       return -1;
+               p = senc;
+               i2d_SSL_SESSION(s->session, &p);
+
+               /* create a fresh copy (not shared with other threads) to clean up */
+               const_p = senc;
+               sess = d2i_SSL_SESSION(NULL, &const_p, slen_full);
+               if (sess == NULL)
+                       {
+                       OPENSSL_free(senc);
                        return -1;
+                       }
+               sess->session_id_length = 0; /* ID is irrelevant for the ticket */
+
+               slen = i2d_SSL_SESSION(sess, NULL);
+               if (slen > slen_full) /* shouldn't ever happen */
+                       {
+                       OPENSSL_free(senc);
+                       return -1;
+                       }
+               p = senc;
+               i2d_SSL_SESSION(sess, &p);
+               SSL_SESSION_free(sess);
+
                /* Grow buffer if need be: the length calculation is as
                 * follows 1 (size of message name) + 3 (message length
                 * bytes) + 4 (ticket lifetime hint) + 2 (ticket length) +
@@ -3373,11 +3400,6 @@ int ssl3_send_newsession_ticket(SSL *s)
                        26 + EVP_MAX_IV_LENGTH + EVP_MAX_BLOCK_LENGTH +
                        EVP_MAX_MD_SIZE + slen))
                        return -1;
-               senc = OPENSSL_malloc(slen);
-               if (!senc)
-                       return -1;
-               p = senc;
-               i2d_SSL_SESSION(s->session, &p);
 
                p=(unsigned char *)s->init_buf->data;
                /* do the header */
@@ -3408,7 +3430,13 @@ int ssl3_send_newsession_ticket(SSL *s)
                                        tlsext_tick_md(), NULL);
                        memcpy(key_name, tctx->tlsext_tick_key_name, 16);
                        }
-               l2n(s->session->tlsext_tick_lifetime_hint, p);
+
+               /* Ticket lifetime hint (advisory only):
+                * We leave this unspecified for resumed session (for simplicity),
+                * and guess that tickets for new sessions will live as long
+                * as their sessions. */
+               l2n(s->hit ? 0 : s->session->timeout, p);
+
                /* Skip ticket length for now */
                p += 2;
                /* Output key name */
index 7e3fa04..7f15173 100644 (file)
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -1133,12 +1133,12 @@ struct ssl_st
        int server;     /* are we the server side? - mostly used by SSL_clear*/
 
        int new_session;/* Generate a new session or reuse an old one.
-                                        * NB: For servers, the 'new' session may actually be a previously
-                                        * cached session or even the previous session unless
-                                        * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is set */
+                        * NB: For servers, the 'new' session may actually be a previously
+                        * cached session or even the previous session unless
+                        * SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION is set */
        int renegotiate;/* 1 if we are renegotiating.
-                                        * 2 if we are a server and are inside a handshake
-                                        *   (i.e. not just sending a HelloRequest) */
+                        * 2 if we are a server and are inside a handshake
+                        * (i.e. not just sending a HelloRequest) */
 
        int quiet_shutdown;/* don't send shutdown packets */
        int shutdown;   /* we have shut things down, 0x01 sent, 0x02
index 22039fb..b75c260 100644 (file)
@@ -1027,14 +1027,14 @@ int SSL_renegotiate(SSL *s)
        }
 
 int SSL_renegotiate_abbreviated(SSL *s)
-{
+       {
        if (s->renegotiate == 0)
                s->renegotiate=1;
-       
+
        s->new_session=0;
-       
+
        return(s->method->ssl_renegotiate(s));
-}
+       }
 
 int SSL_renegotiate_pending(SSL *s)
        {
@@ -3241,4 +3241,3 @@ IMPLEMENT_STACK_OF(SSL_CIPHER)
 IMPLEMENT_STACK_OF(SSL_COMP)
 IMPLEMENT_OBJ_BSEARCH_GLOBAL_CMP_FN(SSL_CIPHER, SSL_CIPHER,
                                    ssl_cipher_id);
-
index cbb7e70..74e8f7b 100644 (file)
@@ -436,6 +436,25 @@ int ssl_get_new_session(SSL *s, int session)
        return(1);
        }
 
+/* ssl_get_prev attempts to find an SSL_SESSION to be used to resume this
+ * connection. It is only called by servers.
+ *
+ *   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.
+ *   len: the length of the session ID.
+ *   limit: a pointer to the first byte after the ClientHello.
+ *
+ * Returns:
+ *   -1: error
+ *    0: a session may have been found.
+ *
+ * Side effects:
+ *   - If a session is found then s->session is pointed at it (after freeing an
+ *     existing session if need be) and s->verify_result is set from the session.
+ *   - Both for new and resumed sessions, s->tlsext_ticket_expected is set to 1
+ *     if the server should issue a new session ticket (to 0 otherwise).
+ */
 int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
                        const unsigned char *limit)
        {
@@ -443,27 +462,39 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
 
        SSL_SESSION *ret=NULL;
        int fatal = 0;
+       int try_session_cache = 1;
 #ifndef OPENSSL_NO_TLSEXT
        int r;
 #endif
 
        if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
                goto err;
+
+       if (len == 0)
+               try_session_cache = 0;
+
 #ifndef OPENSSL_NO_TLSEXT
-       r = tls1_process_ticket(s, session_id, len, limit, &ret);
-       if (r == -1)
+       r = tls1_process_ticket(s, session_id, len, limit, &ret); /* sets s->tlsext_ticket_expected */
+       switch (r)
                {
+       case -1: /* Error during processing */
                fatal = 1;
                goto err;
+       case 0: /* No ticket found */
+       case 1: /* Zero length ticket found */
+               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();
                }
-       else if (r == 0 || (!ret && !len))
-               goto err;
-       else if (!ret && !(s->session_ctx->session_cache_mode & SSL_SESS_CACHE_NO_INTERNAL_LOOKUP))
-#else
-       if (len == 0)
-               goto err;
-       if (!(s->session_ctx->session_cache_mode & SSL_SESS_CACHE_NO_INTERNAL_LOOKUP))
 #endif
+
+       if (try_session_cache &&
+           ret == NULL &&
+           !(s->session_ctx->session_cache_mode & SSL_SESS_CACHE_NO_INTERNAL_LOOKUP))
                {
                SSL_SESSION data;
                data.ssl_version=s->version;
@@ -474,20 +505,22 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
                CRYPTO_r_lock(CRYPTO_LOCK_SSL_CTX);
                ret=lh_SSL_SESSION_retrieve(s->session_ctx->sessions,&data);
                if (ret != NULL)
-                   /* don't allow other threads to steal it: */
-                   CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION);
+                       {
+                       /* don't allow other threads to steal it: */
+                       CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION);
+                       }
                CRYPTO_r_unlock(CRYPTO_LOCK_SSL_CTX);
+               if (ret == NULL)
+                       s->session_ctx->stats.sess_miss++;
                }
 
-       if (ret == NULL)
+       if (try_session_cache &&
+           ret == NULL &&
+           s->session_ctx->get_session_cb != NULL)
                {
                int copy=1;
        
-               s->session_ctx->stats.sess_miss++;
-               ret=NULL;
-               if (s->session_ctx->get_session_cb != NULL
-                   && (ret=s->session_ctx->get_session_cb(s,session_id,len,&copy))
-                      != NULL)
+               if ((ret=s->session_ctx->get_session_cb(s,session_id,len,&copy)))
                        {
                        s->session_ctx->stats.sess_cb_hit++;
 
@@ -506,23 +539,18 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
                                 * things are very strange */
                                SSL_CTX_add_session(s->session_ctx,ret);
                        }
-               if (ret == NULL)
-                       goto err;
                }
 
-       /* Now ret is non-NULL, and we own one of its reference counts. */
+       if (ret == NULL)
+               goto err;
+
+       /* Now ret is non-NULL and we own one of its reference counts. */
 
        if (ret->sid_ctx_length != s->sid_ctx_length
            || memcmp(ret->sid_ctx,s->sid_ctx,ret->sid_ctx_length))
                {
-               /* We've found the session named by the client, but we don't
+               /* We have the session requested by the client, but we don't
                 * want to use it in this context. */
-
-#if 0 /* The client cannot always know when a session is not appropriate,
-       * so we shouldn't generate an error message. */
-
-               SSLerr(SSL_F_SSL_GET_PREV_SESSION,SSL_R_ATTEMPT_TO_REUSE_SESSION_IN_DIFFERENT_CONTEXT);
-#endif
                goto err; /* treat like cache miss */
                }
        
@@ -559,39 +587,36 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
                        goto err;
                }
 
-
-#if 0 /* This is way too late. */
-
-       /* If a thread got the session, then 'swaped', and another got
-        * it and then due to a time-out decided to 'OPENSSL_free' it we could
-        * be in trouble.  So I'll increment it now, then double decrement
-        * later - am I speaking rubbish?. */
-       CRYPTO_add(&ret->references,1,CRYPTO_LOCK_SSL_SESSION);
-#endif
-
        if (ret->timeout < (long)(time(NULL) - ret->time)) /* timeout */
                {
                s->session_ctx->stats.sess_timeout++;
-               /* remove it from the cache */
-               SSL_CTX_remove_session(s->session_ctx,ret);
+               if (try_session_cache)
+                       {
+                       /* session was from the cache, so remove it */
+                       SSL_CTX_remove_session(s->session_ctx,ret);
+                       }
                goto err;
                }
 
        s->session_ctx->stats.sess_hit++;
 
-       /* ret->time=time(NULL); */ /* rezero timeout? */
-       /* again, just leave the session 
-        * if it is the same session, we have just incremented and
-        * then decremented the reference count :-) */
        if (s->session != NULL)
                SSL_SESSION_free(s->session);
        s->session=ret;
        s->verify_result = s->session->verify_result;
-       return(1);
+       return 1;
 
  err:
        if (ret != NULL)
+               {
                SSL_SESSION_free(ret);
+               if (!try_session_cache)
+                       {
+                       /* The session was from a ticket, so we should
+                        * issue a ticket for the new session */
+                       s->tlsext_ticket_expected = 1;
+                       }
+               }
        if (fatal)
                return -1;
        else
@@ -770,10 +795,6 @@ int SSL_set_session(SSL *s, SSL_SESSION *session)
                        {
                        if (!SSL_set_ssl_method(s,meth))
                                return(0);
-                       if (s->ctx->session_timeout == 0)
-                               session->timeout=SSL_get_default_timeout(s);
-                       else
-                               session->timeout=s->ctx->session_timeout;
                        }
 
 #ifndef OPENSSL_NO_KRB5
@@ -858,17 +879,17 @@ X509 *SSL_SESSION_get0_peer(SSL_SESSION *s)
 
 int SSL_SESSION_set1_id_context(SSL_SESSION *s,const unsigned char *sid_ctx,
                               unsigned int sid_ctx_len)
-    {
-    if(sid_ctx_len > SSL_MAX_SID_CTX_LENGTH)
        {
-       SSLerr(SSL_F_SSL_SESSION_SET1_ID_CONTEXT,SSL_R_SSL_SESSION_ID_CONTEXT_TOO_LONG);
-       return 0;
-       }
-    s->sid_ctx_length=sid_ctx_len;
-    memcpy(s->sid_ctx,sid_ctx,sid_ctx_len);
+       if(sid_ctx_len > SSL_MAX_SID_CTX_LENGTH)
+               {
+               SSLerr(SSL_F_SSL_SESSION_SET1_ID_CONTEXT,SSL_R_SSL_SESSION_ID_CONTEXT_TOO_LONG);
+               return 0;
+               }
+       s->sid_ctx_length=sid_ctx_len;
+       memcpy(s->sid_ctx,sid_ctx,sid_ctx_len);
 
-    return 1;
-    }
+       return 1;
+       }
 
 long SSL_CTX_set_timeout(SSL_CTX *s, long t)
        {
index cfb3b54..93869fa 100644 (file)
@@ -1838,26 +1838,56 @@ int ssl_check_serverhello_tlsext(SSL *s)
                }
        }
 
-/* Since the server cache lookup is done early on in the processing of client
- * hello and other operations depend on the result we need to handle any TLS
- * session ticket extension at the same time.
+/* Since the server cache lookup is done early on in the processing of the
+ * 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. This code will
+ *       read past the end of this in order to parse out the session ticket
+ *       extension, if any.
+ *   len: the length of the session ID.
+ *   limit: a pointer to the first byte after the ClientHello.
+ *   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->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->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.
+ *
+ * Side effects:
+ *   Sets s->tlsext_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->tlsext_ticket_key_cb asked to renew the client's ticket.
+ *   Otherwise, s->tlsext_ticket_expected is set to 0.
  */
-
 int tls1_process_ticket(SSL *s, unsigned char *session_id, int len,
-                               const unsigned char *limit, SSL_SESSION **ret)
+                       const unsigned char *limit, SSL_SESSION **ret)
        {
        /* Point after session ID in client hello */
        const unsigned char *p = session_id + len;
        unsigned short i;
 
+       *ret = NULL;
+       s->tlsext_ticket_expected = 0;
+
        /* If tickets disabled behave as if no ticket present
-        * to permit stateful resumption.
-        */
+        * to permit stateful resumption.
+        */
        if (SSL_get_options(s) & SSL_OP_NO_TICKET)
-               return 1;
-
+               return 0;
        if ((s->version <= SSL3_VERSION) || !limit)
-               return 1;
+               return 0;
        if (p >= limit)
                return -1;
        /* Skip past DTLS cookie */
@@ -1880,7 +1910,7 @@ int tls1_process_ticket(SSL *s, unsigned char *session_id, int len,
                return -1;
        /* Now at start of extensions */
        if ((p + 2) >= limit)
-               return 1;
+               return 0;
        n2s(p, i);
        while ((p + 4) <= limit)
                {
@@ -1888,39 +1918,61 @@ int tls1_process_ticket(SSL *s, unsigned char *session_id, int len,
                n2s(p, type);
                n2s(p, size);
                if (p + size > limit)
-                       return 1;
+                       return 0;
                if (type == TLSEXT_TYPE_session_ticket)
                        {
-                       /* If tickets disabled indicate cache miss which will
-                        * trigger a full handshake
-                        */
-                       if (SSL_get_options(s) & SSL_OP_NO_TICKET)
-                               return 1;
-                       /* If zero length note client will accept a ticket
-                        * and indicate cache miss to trigger full handshake
-                        */
+                       int r;
                        if (size == 0)
                                {
+                               /* The client will accept a ticket but doesn't
+                                * currently have one. */
                                s->tlsext_ticket_expected = 1;
-                               return 0;       /* Cache miss */
+                               return 1;
                                }
                        if (s->tls_session_secret_cb)
                                {
-                               /* Indicate cache miss here and instead of
-                                * generating the session from ticket now,
-                                * trigger abbreviated handshake based on
-                                * external mechanism to calculate the master
-                                * secret later. */
-                               return 0;
+                               /* 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 2;
+                               }
+                       r = tls_decrypt_ticket(s, p, size, session_id, len, ret);
+                       switch (r)
+                               {
+                               case 2: /* ticket couldn't be decrypted */
+                                       s->tlsext_ticket_expected = 1;
+                                       return 2;
+                               case 3: /* ticket was decrypted */
+                                       return r;
+                               case 4: /* ticket decrypted but need to renew */
+                                       s->tlsext_ticket_expected = 1;
+                                       return 3;
+                               default: /* fatal error */
+                                       return -1;
                                }
-                       return tls_decrypt_ticket(s, p, size, session_id, len,
-                                                                       ret);
                        }
                p += size;
                }
-       return 1;
+       return 0;
        }
 
+/* tls_decrypt_ticket attempts to decrypt a session ticket.
+ *
+ *   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.
+ *
+ * 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.
+ */
 static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                                const unsigned char *sess_id, int sesslen,
                                SSL_SESSION **psess)
@@ -1935,7 +1987,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
        SSL_CTX *tctx = s->initial_ctx;
        /* Need at least keyname + iv + some encrypted data */
        if (eticklen < 48)
-               goto tickerr;
+               return 2;
        /* Initialize session ticket encryption and HMAC contexts */
        HMAC_CTX_init(&hctx);
        EVP_CIPHER_CTX_init(&ctx);
@@ -1947,7 +1999,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                if (rv < 0)
                        return -1;
                if (rv == 0)
-                       goto tickerr;
+                       return 2;
                if (rv == 2)
                        renew_ticket = 1;
                }
@@ -1955,15 +2007,15 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                {
                /* Check key name matches */
                if (memcmp(etick, tctx->tlsext_tick_key_name, 16))
-                       goto tickerr;
+                       return 2;
                HMAC_Init_ex(&hctx, tctx->tlsext_tick_hmac_key, 16,
                                        tlsext_tick_md(), NULL);
                EVP_DecryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL,
                                tctx->tlsext_tick_aes_key, etick + 16);
                }
        /* Attempt to process session ticket, first conduct sanity and
-        * integrity checks on ticket.
-        */
+        * integrity checks on ticket.
+        */
        mlen = HMAC_size(&hctx);
        if (mlen < 0)
                {
@@ -1976,7 +2028,7 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
        HMAC_Final(&hctx, tick_hmac, NULL);
        HMAC_CTX_cleanup(&hctx);
        if (memcmp(tick_hmac, etick + eticklen, mlen))
-               goto tickerr;
+               return 2;
        /* Attempt to decrypt session data */
        /* Move p after IV to start of encrypted ticket, update length */
        p = etick + 16 + EVP_CIPHER_CTX_iv_length(&ctx);
@@ -1989,33 +2041,33 @@ static int tls_decrypt_ticket(SSL *s, const unsigned char *etick, int eticklen,
                }
        EVP_DecryptUpdate(&ctx, sdec, &slen, p, eticklen);
        if (EVP_DecryptFinal(&ctx, sdec + slen, &mlen) <= 0)
-               goto tickerr;
+               return 2;
        slen += mlen;
        EVP_CIPHER_CTX_cleanup(&ctx);
        p = sdec;
-               
+
        sess = d2i_SSL_SESSION(NULL, &p, slen);
        OPENSSL_free(sdec);
        if (sess)
                {
-               /* 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.
-                */
+               /* 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 (sesslen)
                        memcpy(sess->session_id, sess_id, sesslen);
                sess->session_id_length = sesslen;
                *psess = sess;
-               s->tlsext_ticket_expected = renew_ticket;
-               return 1;
+               if (renew_ticket)
+                       return 4;
+               else
+                       return 3;
                }
-       /* If session decrypt failure indicate a cache miss and set state to
-        * send a new ticket
-        */
-       tickerr:        
-       s->tlsext_ticket_expected = 1;
-       return 0;
+        ERR_clear_error();
+       /* For session parse failure, indicate that we need to send a new
+        * ticket. */
+       return 2;
        }
 
 /* Tables to translate from NIDs to TLS v1.2 ids */
index 530a252..2ae1e8b 100644 (file)
@@ -142,8 +142,8 @@ else
   fi
 fi
 
-echo test tls1 with PSK
-$ssltest -tls1 -cipher PSK -psk abc123 $extra || exit 1
+echo test tls1 with PSK
+$ssltest -tls1 -cipher PSK -psk abc123 $extra || exit 1
 
 echo test tls1 with PSK via BIO pair
 $ssltest -bio_pair -tls1 -cipher PSK -psk abc123 $extra || exit 1