Add a missing check on s->s3->tmp.pkey
[openssl.git] / ssl / statem / statem_srvr.c
index 5c59eb8b1eb3025bf690762aef5b5c147a1fedbc..ac5fd09134ffe96657ace45414ef66c81ab87008 100644 (file)
@@ -764,6 +764,22 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst)
     return WORK_FINISHED_CONTINUE;
 }
 
+static ossl_inline int conn_is_closed(void)
+{
+    switch (get_last_sys_error()) {
+#if defined(EPIPE)
+    case EPIPE:
+        return 1;
+#endif
+#if defined(ECONNRESET)
+    case ECONNRESET:
+        return 1;
+#endif
+    default:
+        return 0;
+    }
+}
+
 /*
  * Perform any work that needs to be done after sending a message from the
  * server to the client.
@@ -848,12 +864,7 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                 return WORK_MORE_A;
             break;
         }
-        /*
-         * TODO(TLS1.3): This actually causes a problem. We don't yet know
-         * whether the next record we are going to receive is an unencrypted
-         * alert, or an encrypted handshake message. We're going to need
-         * something clever in the record layer for this.
-         */
+
         if (SSL_IS_TLS13(s)) {
             if (!s->method->ssl3_enc->setup_key_block(s)
                 || !s->method->ssl3_enc->change_cipher_state(s,
@@ -868,6 +879,12 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
                 /* SSLfatal() already called */
                 return WORK_ERROR;
             }
+            /*
+             * We don't yet know whether the next record we are going to receive
+             * is an unencrypted alert, an encrypted alert, or an encrypted
+             * handshake message. We temporarily tolerate unencrypted alerts.
+             */
+            s->statem.enc_read_state = ENC_READ_STATE_ALLOW_PLAIN_ALERTS;
             break;
         }
 
@@ -938,8 +955,23 @@ WORK_STATE ossl_statem_server_post_work(SSL *s, WORK_STATE wst)
         break;
 
     case TLS_ST_SW_SESSION_TICKET:
-        if (SSL_IS_TLS13(s) && statem_flush(s) != 1)
+        clear_sys_error();
+        if (SSL_IS_TLS13(s) && statem_flush(s) != 1) {
+            if (SSL_get_error(s, 0) == SSL_ERROR_SYSCALL
+                    && conn_is_closed()) {
+                /*
+                 * We ignore connection closed errors in TLSv1.3 when sending a
+                 * NewSessionTicket and behave as if we were successful. This is
+                 * so that we are still able to read data sent to us by a client
+                 * that closes soon after the end of the handshake without
+                 * waiting to read our post-handshake NewSessionTickets.
+                 */
+                s->rwstate = SSL_NOTHING;
+                break;
+            }
+
             return WORK_MORE_A;
+        }
         break;
     }
 
@@ -2024,10 +2056,6 @@ static int tls_early_post_process_client_hello(SSL *s)
 #else
         s->session->compress_meth = (comp == NULL) ? 0 : comp->id;
 #endif
-        if (!tls1_set_server_sigalgs(s)) {
-            /* SSLfatal() already called */
-            goto err;
-        }
     }
 
     sk_SSL_CIPHER_free(ciphers);
@@ -2195,19 +2223,25 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst)
     if (wst == WORK_MORE_B) {
         if (!s->hit || SSL_IS_TLS13(s)) {
             /* Let cert callback update server certificates if required */
-            if (!s->hit && s->cert->cert_cb != NULL) {
-                int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg);
-                if (rv == 0) {
-                    SSLfatal(s, SSL_AD_INTERNAL_ERROR,
-                             SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
-                             SSL_R_CERT_CB_ERROR);
-                    goto err;
+            if (!s->hit) {
+                if (s->cert->cert_cb != NULL) {
+                    int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg);
+                    if (rv == 0) {
+                        SSLfatal(s, SSL_AD_INTERNAL_ERROR,
+                                 SSL_F_TLS_POST_PROCESS_CLIENT_HELLO,
+                                 SSL_R_CERT_CB_ERROR);
+                        goto err;
+                    }
+                    if (rv < 0) {
+                        s->rwstate = SSL_X509_LOOKUP;
+                        return WORK_MORE_B;
+                    }
+                    s->rwstate = SSL_NOTHING;
                 }
-                if (rv < 0) {
-                    s->rwstate = SSL_X509_LOOKUP;
-                    return WORK_MORE_B;
+                if (!tls1_set_server_sigalgs(s)) {
+                    /* SSLfatal already called */
+                    goto err;
                 }
-                s->rwstate = SSL_NOTHING;
             }
 
             /* In TLSv1.3 we selected the ciphersuite before resumption */
@@ -2370,15 +2404,19 @@ int tls_construct_server_hello(SSL *s, WPACKET *pkt)
 
     if (!WPACKET_sub_memcpy_u8(pkt, session_id, sl)
             || !s->method->put_cipher_by_char(s->s3->tmp.new_cipher, pkt, &len)
-            || !WPACKET_put_bytes_u8(pkt, compm)
-            || !tls_construct_extensions(s, pkt,
-                                         s->hello_retry_request
-                                            == SSL_HRR_PENDING
-                                            ? SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
-                                            : (SSL_IS_TLS13(s)
-                                                ? SSL_EXT_TLS1_3_SERVER_HELLO
-                                                : SSL_EXT_TLS1_2_SERVER_HELLO),
-                                         NULL, 0)) {
+            || !WPACKET_put_bytes_u8(pkt, compm)) {
+        SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_CONSTRUCT_SERVER_HELLO,
+                 ERR_R_INTERNAL_ERROR);
+        return 0;
+    }
+
+    if (!tls_construct_extensions(s, pkt,
+                                  s->hello_retry_request == SSL_HRR_PENDING
+                                      ? SSL_EXT_TLS1_3_HELLO_RETRY_REQUEST
+                                      : (SSL_IS_TLS13(s)
+                                          ? SSL_EXT_TLS1_3_SERVER_HELLO
+                                          : SSL_EXT_TLS1_2_SERVER_HELLO),
+                                  NULL, 0)) {
         /* SSLfatal() already called */
         return 0;
     }
@@ -3186,6 +3224,12 @@ static int tls_process_cke_ecdhe(SSL *s, PACKET *pkt)
                      SSL_R_LENGTH_MISMATCH);
             goto err;
         }
+        if (skey == NULL) {
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CKE_ECDHE,
+                     SSL_R_MISSING_TMP_ECDH_KEY);
+            goto err;
+        }
+
         ckey = EVP_PKEY_new();
         if (ckey == NULL || EVP_PKEY_copy_parameters(ckey, skey) <= 0) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CKE_ECDHE,
@@ -3519,6 +3563,13 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
     size_t chainidx;
     SSL_SESSION *new_sess = NULL;
 
+    /*
+     * To get this far we must have read encrypted data from the client. We no
+     * longer tolerate unencrypted alerts. This value is ignored if less than
+     * TLSv1.3
+     */
+    s->statem.enc_read_state = ENC_READ_STATE_VALID;
+
     if ((sk = sk_X509_new_null()) == NULL) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE,
                  ERR_R_MALLOC_FAILURE);
@@ -3648,8 +3699,6 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
      */
 
     if (s->post_handshake_auth == SSL_PHA_REQUESTED) {
-        int m = s->session_ctx->session_cache_mode;
-
         if ((new_sess = ssl_session_dup(s->session, 0)) == 0) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR,
                      SSL_F_TLS_PROCESS_CLIENT_CERTIFICATE,
@@ -3657,13 +3706,6 @@ MSG_PROCESS_RETURN tls_process_client_certificate(SSL *s, PACKET *pkt)
             goto err;
         }
 
-        if (m & SSL_SESS_CACHE_SERVER) {
-            /*
-             * Remove the old session from the cache. We carry on if this fails
-             */
-            SSL_CTX_remove_session(s->session_ctx, s->session);
-        }
-
         SSL_SESSION_free(s->session);
         s->session = new_sess;
     }