From f14afcaa4227df12bc11a426a60f41005a71e95f Mon Sep 17 00:00:00 2001
From: Matt Caswell <matt@openssl.org>
Date: Tue, 14 Feb 2017 11:20:44 +0000
Subject: [PATCH] Updates following review feedback

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2609)
---
 apps/s_client.c          | 4 ++--
 ssl/ssl_lib.c            | 7 +++++--
 ssl/ssl_locl.h           | 2 --
 ssl/statem/statem_lib.c  | 4 +---
 ssl/statem/statem_locl.h | 3 +++
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/apps/s_client.c b/apps/s_client.c
index a2ededcf0c..7d6eb02515 100644
--- 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 927d70a6a6..cb5e0cfbc9 100644
--- 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 31afe10f10..cd948bd567 100644
--- 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 5e194e886a..c871c00c0c 100644
--- 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 6713dad2e2..595a803f30 100644
--- 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
-- 
2.25.1