tls: remove TODOs
authorPauli <pauli@openssl.org>
Mon, 31 May 2021 04:27:48 +0000 (14:27 +1000)
committerPauli <pauli@openssl.org>
Wed, 2 Jun 2021 06:30:15 +0000 (16:30 +1000)
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15539)

19 files changed:
ssl/build.info
ssl/d1_lib.c
ssl/record/rec_layer_s3.c
ssl/record/ssl3_record.c
ssl/record/ssl3_record_tls13.c
ssl/s3_cbc.c
ssl/s3_enc.c
ssl/ssl_ciph.c
ssl/ssl_lib.c
ssl/ssl_local.h
ssl/ssl_txt.c
ssl/statem/extensions.c
ssl/statem/extensions_clnt.c
ssl/statem/extensions_srvr.c
ssl/statem/statem_clnt.c
ssl/statem/statem_srvr.c
ssl/t1_enc.c
ssl/t1_lib.c
ssl/tls13_enc.c

index c17084b9adcfa348db5b0b70b9b564494091d65d..f2de0371ae75e92412c2df5fbdd0ba383ddda490 100644 (file)
@@ -15,10 +15,10 @@ IF[{- !$disabled{ktls} -}]
   $KTLSSRC=ktls.c
 ENDIF
 
-#TODO: For now we just include the libcrypto packet.c in libssl as well. We
-#      could either continue to do it like this, or export all the WPACKET
-#      symbols so that libssl can use them like any other. Probably would do
-#      this privately so it does not become part of the public API.
+# For now we just include the libcrypto packet.c in libssl as well. We
+# could either continue to do it like this, or export all the WPACKET
+# symbols so that libssl can use them like any other. Probably would do
+# this privately so it does not become part of the public API.
 SOURCE[../libssl]=\
         pqueue.c ../crypto/packet.c \
         statem/statem_srvr.c statem/statem_clnt.c  s3_lib.c  s3_enc.c record/rec_layer_s3.c \
index 5626b7f50659a9abb44c8d9558619cfe234dc2dd..f9ad4ed684a3448406e55c793646b7d3a2f44c10 100644 (file)
@@ -797,7 +797,6 @@ int DTLSv1_listen(SSL *s, BIO_ADDR *client)
             BIO_ADDR_free(tmpclient);
             tmpclient = NULL;
 
-            /* TODO(size_t): convert this call */
             if (BIO_write(wbio, wbuf, wreclen) < (int)wreclen) {
                 if (BIO_should_retry(wbio)) {
                     /*
index a217db772ab9f4b5a1b48c3d0f8483ad9c3db45a..aacd5694fcb1f569d9ae79053549c6ab9bc38223 100644 (file)
@@ -295,7 +295,6 @@ int ssl3_read_n(SSL *s, size_t n, size_t max, int extend, int clearold,
         clear_sys_error();
         if (s->rbio != NULL) {
             s->rwstate = SSL_READING;
-            /* TODO(size_t): Convert this function */
             ret = BIO_read(s->rbio, pkt + len + left, max - left);
             if (ret >= 0)
                 bioread = ret;
@@ -722,7 +721,6 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
         clear = s->enc_write_ctx ? 0 : 1; /* must be AEAD cipher */
         mac_size = 0;
     } else {
-        /* TODO(siz_t): Convert me */
         mac_size = EVP_MD_CTX_get_size(s->write_hash);
         if (mac_size < 0) {
             SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
@@ -833,7 +831,6 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
     if (s->enc_write_ctx && SSL_USE_EXPLICIT_IV(s) && !SSL_TREAT_AS_TLS13(s)) {
         int mode = EVP_CIPHER_CTX_get_mode(s->enc_write_ctx);
         if (mode == EVP_CIPH_CBC_MODE) {
-            /* TODO(size_t): Convert me */
             eivlen = EVP_CIPHER_CTX_get_iv_length(s->enc_write_ctx);
             if (eivlen <= 1)
                 eivlen = 0;
@@ -1195,7 +1192,6 @@ int ssl3_write_pending(SSL *s, int type, const unsigned char *buf, size_t len,
                     return i;
                 BIO_set_ktls_ctrl_msg(s->wbio, type);
             }
-            /* TODO(size_t): Convert this call */
             i = BIO_write(s->wbio, (char *)
                           &(SSL3_BUFFER_get_buf(&wb[currbuf])
                             [SSL3_BUFFER_get_offset(&wb[currbuf])]),
index 8c4ff01dd180fa31237953fefdfd96ce56f55657..4275c19cffe24a29e4f743d68c5f63f1fcb3716c 100644 (file)
@@ -521,7 +521,6 @@ int ssl3_get_record(SSL *s)
     if (BIO_get_ktls_recv(s->rbio) && !is_ktls_left)
         goto skip_decryption;
 
-    /* TODO(size_t): convert this to do size_t properly */
     if (s->read_hash != NULL) {
         const EVP_MD *tmpmd = EVP_MD_CTX_get0_md(s->read_hash);
 
@@ -782,7 +781,6 @@ int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr)
     if (rr->comp == NULL)
         return 0;
 
-    /* TODO(size_t): Convert this call */
     i = COMP_expand_block(ssl->expand, rr->comp,
                           SSL3_RT_MAX_PLAIN_LENGTH, rr->data, (int)rr->length);
     if (i < 0)
@@ -799,7 +797,6 @@ int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr)
 #ifndef OPENSSL_NO_COMP
     int i;
 
-    /* TODO(size_t): Convert this call */
     i = COMP_compress_block(ssl->compress, wr->data,
                             (int)(wr->length + SSL3_RT_MAX_COMPRESSED_OVERHEAD),
                             wr->input, (int)wr->length);
@@ -858,7 +855,6 @@ int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending,
         int provided = (EVP_CIPHER_get0_provider(enc) != NULL);
 
         l = rec->length;
-        /* TODO(size_t): Convert this call */
         bs = EVP_CIPHER_CTX_get_block_size(ds);
 
         /* COMPRESS */
@@ -916,7 +912,6 @@ int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending,
                 }
             }
         } else {
-            /* TODO(size_t): Convert this call */
             if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) {
                 /* Shouldn't happen */
                 SSLfatal(s, SSL_AD_BAD_RECORD_MAC, ERR_R_INTERNAL_ERROR);
@@ -1212,7 +1207,6 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
         } else {
             /* Legacy cipher */
 
-            /* TODO(size_t): Convert this call */
             tmpr = EVP_Cipher(ds, recs[0].data, recs[0].input,
                               (unsigned int)reclen[0]);
             if ((EVP_CIPHER_get_flags(EVP_CIPHER_CTX_get0_cipher(ds))
@@ -1471,7 +1465,6 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
             return 0;
     }
 
-    /* TODO(size_t): Convert these calls */
     if (EVP_DigestSignUpdate(mac_ctx, header, sizeof(header)) <= 0
         || EVP_DigestSignUpdate(mac_ctx, rec->input, rec->length) <= 0
         || EVP_DigestSignFinal(mac_ctx, md, &md_size) <= 0) {
@@ -1546,7 +1539,6 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
     rr->data = rr->input;
     rr->orig_len = rr->length;
 
-    /* TODO(size_t): convert this to do size_t properly */
     if (s->read_hash != NULL) {
         const EVP_MD *tmpmd = EVP_MD_CTX_get0_md(s->read_hash);
 
@@ -1850,10 +1842,6 @@ int dtls1_get_record(SSL *s)
     if (!BIO_dgram_is_sctp(SSL_get_rbio(s))) {
 #endif
         /* Check whether this is a repeat, or aged record. */
-        /*
-         * TODO: Does it make sense to have replay protection in epoch 0 where
-         * we have no integrity negotiated yet?
-         */
         if (!dtls1_record_replay_check(s, bitmap)) {
             rr->length = 0;
             rr->read = 1;
index 13c007ae2363446b89130307f4ad8a8d971d9828..3d350718479c59e92ae7ef06d90c984ed14922b8 100644 (file)
@@ -35,7 +35,6 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
 
     if (n_recs != 1) {
         /* Should not happen */
-        /* TODO(TLS1.3): Support pipelining */
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
     }
@@ -139,7 +138,6 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
         return 0;
     }
 
-    /* TODO(size_t): lenu/lenf should be a size_t but EVP doesn't support it */
     if (EVP_CipherInit_ex(ctx, NULL, NULL, NULL, iv, sending) <= 0
             || (!sending && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
                                              taglen,
index 8e1c779ddb53049d46b80a2e65926c546e526b18..b0e3496ba225faf10fec19df3e8766126994ed55 100644 (file)
@@ -500,7 +500,6 @@ int ssl3_cbc_digest_record(const EVP_MD *md,
             || EVP_DigestUpdate(md_ctx, mac_out, md_size) <= 0)
             goto err;
     }
-    /* TODO(size_t): Convert me */
     ret = EVP_DigestFinal(md_ctx, md_out, &md_out_size_u);
     if (ret && md_out_size)
         *md_out_size = md_out_size_u;
index 64b246eb65acca683f6f78ce297392ba7f77be99..2ca3f74ae77102a121169f1abd563c17e8e0254f 100644 (file)
@@ -499,7 +499,6 @@ int ssl3_generate_master_secret(SSL *s, unsigned char *out, unsigned char *p,
                                 SSL3_RANDOM_SIZE) <= 0
             || EVP_DigestUpdate(ctx, &(s->s3.server_random[0]),
                                 SSL3_RANDOM_SIZE) <= 0
-               /* TODO(size_t) : convert me */
             || EVP_DigestFinal_ex(ctx, buf, &n) <= 0
             || EVP_DigestInit_ex(ctx, s->ctx->md5, NULL) <= 0
             || EVP_DigestUpdate(ctx, p, len) <= 0
index d7c19feedf90b8a6dd7de951135ee1d097358a12..dd22e57c59f0301463c4b6c6615cc0b04b138b05 100644 (file)
@@ -1543,7 +1543,6 @@ STACK_OF(SSL_CIPHER) *ssl_create_cipher_list(SSL_CTX *ctx,
 
     /*
      * Partially overrule strength sort to prefer TLS 1.2 ciphers/PRFs.
-     * TODO(openssl-team): is there an easier way to accomplish all this?
      */
     ssl_cipher_apply_rule(0, 0, 0, 0, 0, TLS1_2_VERSION, 0, CIPHER_BUMP, -1,
                           &head, &tail);
index 063134015a1937a0ff7fb053beef4145f73265c8..c1e8e41f02bc68581a110323c3e444cc6cd1bc6b 100644 (file)
@@ -2246,11 +2246,6 @@ int SSL_shutdown(SSL *s)
 
 int SSL_key_update(SSL *s, int updatetype)
 {
-    /*
-     * TODO(TLS1.3): How will applications know whether TLSv1.3 has been
-     * negotiated, and that it is appropriate to call SSL_key_update() instead
-     * of SSL_renegotiate().
-     */
     if (!SSL_IS_TLS13(s)) {
         ERR_raise(ERR_LIB_SSL, SSL_R_WRONG_SSL_VERSION);
         return 0;
index 28603a81adb1affad4afc4a43bd5755e88982e82..b222fc6a2d5c10c15155a969a0c16462df3b25b3 100644 (file)
@@ -1379,7 +1379,7 @@ struct ssl_st {
         size_t previous_client_finished_len;
         unsigned char previous_server_finished[EVP_MAX_MD_SIZE];
         size_t previous_server_finished_len;
-        int send_connection_binding; /* TODOEKR */
+        int send_connection_binding;
 
 # ifndef OPENSSL_NO_NEXTPROTONEG
         /*
index 8dc418ca48171e7d733cdd1b84e566eb87df681e..01871dca8ce04131038abd0ca86631685c45a126 100644 (file)
@@ -107,7 +107,6 @@ int SSL_SESSION_print(BIO *bp, const SSL_SESSION *x)
     if (x->ext.tick) {
         if (BIO_puts(bp, "\n    TLS session ticket:\n") <= 0)
             goto err;
-        /* TODO(size_t): Convert this call */
         if (BIO_dump_indent
             (bp, (const char *)x->ext.tick, (int)x->ext.ticklen, 4)
             <= 0)
index d12e940704cbb14e33bdba86c0ca4d17a55b6dc8..f58111c95c662e79150f45ce330aac3dc6a300bb 100644 (file)
@@ -115,8 +115,6 @@ typedef struct extensions_definition_st {
  * messages the extension is relevant to. These flags also specify whether the
  * extension is relevant to a particular protocol or protocol version.
  *
- * TODO(TLS1.3): Make sure we have a test to check the consistency of these
- *
  * NOTE: WebSphere Application Server 7+ cannot handle empty extensions at
  * the end, keep these extensions before signature_algorithm.
  */
index 545b2d034fe96cbf0805c49682b2b6c9f85b0734..78cc226064030be4ef8bb485032372c473ec29bc 100644 (file)
@@ -629,7 +629,7 @@ static int add_key_share(SSL *s, WPACKET *pkt, unsigned int curve_id)
     }
 
     /*
-     * TODO(TLS1.3): When changing to send more than one key_share we're
+     * When changing to send more than one key_share we're
      * going to need to be able to save more than one EVP_PKEY. For now
      * we reuse the existing tmp.pkey
      */
@@ -668,8 +668,8 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
     tls1_get_supported_groups(s, &pgroups, &num_groups);
 
     /*
-     * TODO(TLS1.3): Make the number of key_shares sent configurable. For
-     * now, just send one
+     * Make the number of key_shares sent configurable. For
+     * now, we just send one
      */
     if (s->s3.group_id != 0) {
         curve_id = s->s3.group_id;
@@ -1387,7 +1387,6 @@ int tls_parse_stoc_status_request(SSL *s, PACKET *pkt, unsigned int context,
 {
     if (context == SSL_EXT_TLS1_3_CERTIFICATE_REQUEST) {
         /* We ignore this if the server sends a CertificateRequest */
-        /* TODO(TLS1.3): Add support for this */
         return 1;
     }
 
@@ -1429,7 +1428,6 @@ int tls_parse_stoc_sct(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 {
     if (context == SSL_EXT_TLS1_3_CERTIFICATE_REQUEST) {
         /* We ignore this if the server sends it in a CertificateRequest */
-        /* TODO(TLS1.3): Add support for this */
         return 1;
     }
 
index 51c3251635062838a3bf72edeaa726c96c3feaae..e8e57cd5d9f347cde19bc54e8d051ed9419a1c9c 100644 (file)
@@ -155,10 +155,6 @@ int tls_parse_ctos_server_name(SSL *s, PACKET *pkt, unsigned int context,
          * the initial handshake and the resumption. In TLSv1.3 SNI is not
          * associated with the session.
          */
-        /*
-         * TODO(openssl-team): if the SNI doesn't match, we MUST
-         * fall back to a full handshake.
-         */
         s->servername_done = (s->session->ext.hostname != NULL)
             && PACKET_equal(&hostname, s->session->ext.hostname,
                             strlen(s->session->ext.hostname));
@@ -215,10 +211,6 @@ int tls_parse_ctos_srp(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
-    /*
-     * TODO(openssl-team): currently, we re-authenticate the user
-     * upon resumption. Instead, we MUST ignore the login.
-     */
     if (!PACKET_strndup(&srp_I, &s->srp_ctx.login)) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return 0;
@@ -364,7 +356,6 @@ int tls_parse_ctos_status_request(SSL *s, PACKET *pkt, unsigned int context,
         }
 
         id_data = PACKET_data(&responder_id);
-        /* TODO(size_t): Convert d2i_* to size_t */
         id = d2i_OCSP_RESPID(NULL, &id_data,
                              (int)PACKET_remaining(&responder_id));
         if (id == NULL) {
index 88b34c6ad104d12b66d3fdecf4459350167ca94d..e8e9f94651bf998182355582f154cea4871d1e9f 100644 (file)
@@ -168,7 +168,8 @@ static int ossl_statem_client13_read_transition(SSL *s, int mt)
         }
         if (mt == SSL3_MT_CERTIFICATE_REQUEST) {
 #if DTLS_MAX_VERSION_INTERNAL != DTLS1_2_VERSION
-# error TODO(DTLS1.3): Restore digest for PHA before adding message.
+            /* Restore digest for PHA before adding message.*/
+# error Internal DTLS version error
 #endif
             if (!SSL_IS_DTLS(s) && s->post_handshake_auth == SSL_PHA_EXT_SENT) {
                 s->post_handshake_auth = SSL_PHA_REQUESTED;
@@ -1985,7 +1986,6 @@ static int tls_process_ske_srp(SSL *s, PACKET *pkt, EVP_PKEY **pkey)
         return 0;
     }
 
-    /* TODO(size_t): Convert BN_bin2bn() calls */
     if ((s->srp_ctx.N =
          BN_bin2bn(PACKET_data(&prime),
                    (int)PACKET_remaining(&prime), NULL)) == NULL
@@ -2035,7 +2035,6 @@ static int tls_process_ske_dhe(SSL *s, PACKET *pkt, EVP_PKEY **pkey)
         return 0;
     }
 
-    /* TODO(size_t): Convert these calls */
     p = BN_bin2bn(PACKET_data(&prime), (int)PACKET_remaining(&prime), NULL);
     g = BN_bin2bn(PACKET_data(&generator), (int)PACKET_remaining(&generator),
                   NULL);
@@ -2573,7 +2572,7 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
         goto err;
     }
     /*
-     * TODO(size_t): we use sess_len here because EVP_Digest expects an int
+     * We use sess_len here because EVP_Digest expects an int
      * but s->session->session_id_length is a size_t
      */
     if (!EVP_Digest(s->session->ext.tick, ticklen,
@@ -2853,7 +2852,6 @@ static int tls_construct_cke_rsa(SSL *s, WPACKET *pkt)
 
     pms[0] = s->client_version >> 8;
     pms[1] = s->client_version & 0xff;
-    /* TODO(size_t): Convert this function */
     if (RAND_bytes_ex(s->ctx->libctx, pms + 2, pmslen - 2, 0) <= 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_MALLOC_FAILURE);
         goto err;
@@ -3059,7 +3057,6 @@ static int tls_construct_cke_gost(SSL *s, WPACKET *pkt)
 
     if (EVP_PKEY_encrypt_init(pkey_ctx) <= 0
         /* Generate session key
-         * TODO(size_t): Convert this function
          */
         || RAND_bytes_ex(s->ctx->libctx, pms, pmslen, 0) <= 0) {
         SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
index a954097a390d188a40a2e8c255dd944d2483b2d4..c1c0d455e1dfab1d45e3611885510b0b905a0a2d 100644 (file)
@@ -1765,7 +1765,7 @@ static int tls_early_post_process_client_hello(SSL *s)
 
     /*
      * We don't allow resumption in a backwards compatible ClientHello.
-     * TODO(openssl-team): in TLS1.1+, session_id MUST be empty.
+     * In TLS1.1+, session_id MUST be empty.
      *
      * Versions before 0.9.7 always allow clients to resume sessions in
      * renegotiation. 0.9.7 and later allow this by default, but optionally
index 03a83ee9a04064763962d6ccf07db5e1aa285d41..51688d4f2eac70f469118ed79d2486130231aa27 100644 (file)
@@ -333,7 +333,6 @@ int tls1_change_cipher_state(SSL *s, int which)
     p = s->s3.tmp.key_block;
     i = *mac_secret_size = s->s3.tmp.new_mac_secret_size;
 
-    /* TODO(size_t): convert me */
     cl = EVP_CIPHER_get_key_length(c);
     j = cl;
     k = tls_iv_length_within_key_block(c);
index d22a794d3717113c4eb2efe957df30bbb598c980..3bc424acefe5db8d1252b9a820467bb3086152f9 100644 (file)
@@ -3079,7 +3079,7 @@ static int check_cert_usable(SSL *s, const SIGALG_LOOKUP *sig, X509 *x,
                 continue;
 
             /*
-             * TODO this does not differentiate between the
+             * This does not differentiate between the
              * rsa_pss_pss_* and rsa_pss_rsae_* schemes since we do not
              * have a chain here that lets us look at the key OID in the
              * signing certificate.
index 53aeea446b8aaa32cb9c6344eb23e59a3bab8d2b..11e39715d81296bcb3443c667b01153f431940fc 100644 (file)
@@ -402,7 +402,6 @@ static int derive_secret_key_and_iv(SSL *s, int sending, const EVP_MD *md,
         return 0;
     }
 
-    /* TODO(size_t): convert me */
     keylen = EVP_CIPHER_get_key_length(ciph);
     if (EVP_CIPHER_get_mode(ciph) == EVP_CIPH_CCM_MODE) {
         uint32_t algenc;