Updates following review feedback

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

diff --git a/apps/s_client.c b/apps/s_client.c
index a2ededcf0c02367569c92194a8f1f09521bb9bd3..7d6eb025155f16440f28132cd6ef8b2832a719b6 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -2608,8 +2608,8 @@ int s_client_main(int argc, char **argv)
                 cbuf_len = 0;
             }
 
-            if ((!c_ign_eof) && ((cbuf[0] == 'K' || cbuf[0] == 'k' )
-                                 && cmdletters)) {
+            if (!c_ign_eof && (cbuf[0] == 'K' || cbuf[0] == 'k' )
+                    && cmdletters) {
                 BIO_printf(bio_err, "KEYUPDATE\n");
                 SSL_key_update(con,
                                cbuf[0] == 'K' ? SSL_KEY_UPDATE_REQUESTED

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 927d70a6a6011c6928f27e883b907b0963d03aca..cb5e0cfbc9bce8b67e8013974fff44214b5de339 100644 (file)
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1720,6 +1720,11 @@ int SSL_shutdown(SSL *s)
 
 int SSL_key_update(SSL *s, SSL_KEY_UPDATE 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)) {
         SSLerr(SSL_F_SSL_KEY_UPDATE, SSL_R_WRONG_SSL_VERSION);
         return 0;
@@ -1737,9 +1742,7 @@ int SSL_key_update(SSL *s, SSL_KEY_UPDATE updatetype)
     }
 
     ossl_statem_set_in_init(s, 1);
-
     s->key_update = updatetype;
-
     return 1;
 }
 

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 31afe10f102796c181c070e433ba79da6f6060f6..cd948bd567926797bf7621f4580f35949d31f4be 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -996,10 +996,8 @@ struct ssl_st {
     EVP_CIPHER_CTX *enc_write_ctx; /* cryptographic state */
     unsigned char write_iv[EVP_MAX_IV_LENGTH]; /* TLSv1.3 static write IV */
     EVP_MD_CTX *write_hash;     /* used for mac generation */
-
     /* Count of how many KeyUpdate messages we have received */
     unsigned int key_update_count;
-
     /* session info */
     /* client cert? */
     /* This is used to hold the server certificate used */

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 5e194e886afe73d2be23447a4a68b6ee955e3668..c871c00c0c72caab9e62342efe68ca4dee29ca2d 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -503,15 +503,13 @@ int tls_construct_key_update(SSL *s, WPACKET *pkt)
     }
 
     s->key_update = SSL_KEY_UPDATE_NONE;
-
     return 1;
+
  err:
     ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
     return 0;
 }
 
-#define MAX_KEY_UPDATE_MESSAGES     32
-
 MSG_PROCESS_RETURN tls_process_key_update(SSL *s, PACKET *pkt)
 {
     int al;

diff --git a/ssl/statem/statem_locl.h b/ssl/statem/statem_locl.h
index 6713dad2e2c39f0e1ef2fd1eb9f22327792df9f3..595a803f3062c73584c3e542a425c279895f1f56 100644 (file)
--- a/ssl/statem/statem_locl.h
+++ b/ssl/statem/statem_locl.h
@@ -28,6 +28,9 @@
 /* Max should actually be 36 but we are generous */
 #define FINISHED_MAX_LENGTH             64
 
+/* The maximum number of incoming KeyUpdate messages we will accept */
+#define MAX_KEY_UPDATE_MESSAGES     32
+
 /* Extension context codes */
 /* This extension is only allowed in TLS */
 #define EXT_TLS_ONLY                        0x0001