Miscellaneous style tweaks based on feedback received

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

diff --git a/apps/s_client.c b/apps/s_client.c
index 362f005cc5eb6e0132284e368b90d04a7d2c658e..99770b9b978a3e62548d952416a4304baaa90880 100644 (file)
--- a/apps/s_client.c
+++ b/apps/s_client.c
@@ -784,11 +784,11 @@ static int new_session_cb(SSL *S, SSL_SESSION *sess)
 {
     BIO *stmp = BIO_new_file(sess_out, "w");
 
-    if (stmp != NULL) {
+    if (stmp == NULL) {
+        BIO_printf(bio_err, "Error writing session file %s\n", sess_out);
+    } else {
         PEM_write_bio_SSL_SESSION(stmp, sess);
         BIO_free(stmp);
-    } else {
-        BIO_printf(bio_err, "Error writing session file %s\n", sess_out);
     }
 
     /*

diff --git a/include/openssl/ssl3.h b/include/openssl/ssl3.h
index 8d146be19bcf886d8310f1178dd56211e918a38a..79f9b7eac2a361896e797fb1297edf2b1caf2476 100644 (file)
--- a/include/openssl/ssl3.h
+++ b/include/openssl/ssl3.h
@@ -25,6 +25,9 @@
 extern "C" {
 #endif
 
+/* Flag used on OpenSSL ciphersuite ids to indicate they are for SSLv3+ */
+# define SSL3_CK_CIPHERSUITE_FLAG                0x03000000
+
 /*
  * Signalling cipher suite value from RFC 5746
  * (TLS_EMPTY_RENEGOTIATION_INFO_SCSV)

diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h
index cd702653370f61c561dc4938a2363e63cfc17b3d..67b49994f7d27c911a6adfd72c0b74a5e9b37c43 100644 (file)
--- a/ssl/packet_locl.h
+++ b/ssl/packet_locl.h
@@ -700,11 +700,11 @@ int WPACKET_close(WPACKET *pkt);
 int WPACKET_finish(WPACKET *pkt);
 
 /*
- * Iterates through all the sub-packets and writes out their lengths as if they
+ * Iterate through all the sub-packets and write out their lengths as if they
  * were being closed. The lengths will be overwritten with the final lengths
  * when the sub-packets are eventually closed (which may be different if more
- * data is added to the WPACKET). This function will fail if a sub-packet is of
- * 0 length and WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH is used.
+ * data is added to the WPACKET). This function fails if a sub-packet is of 0
+ * length and WPACKET_FLAGS_ABANDON_ON_ZERO_LENGTH is set.
  */
 int WPACKET_fill_lengths(WPACKET *pkt);
 

diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 35684f47905da17006d5d9717771377536e6e3fa..bdb4cb41444d3277adf0581f3ca85837834dc895 100644 (file)
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -3559,7 +3559,7 @@ const SSL_CIPHER *ssl3_get_cipher_by_id(uint32_t id)
  */
 const SSL_CIPHER *ssl3_get_cipher_by_char(const unsigned char *p)
 {
-    return ssl3_get_cipher_by_id(0x03000000
+    return ssl3_get_cipher_by_id(SSL3_CK_CIPHERSUITE_FLAG
                                  | ((uint32_t)p[0] << 8L)
                                  | (uint32_t)p[1]);
 }

diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 77c917f38a0d6d9f5dc69c52ddb60a0d97fb84ec..2ef0006649dea2db56360f4fdb314e95ad8652db 100644 (file)
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -620,13 +620,12 @@ int ssl_get_prev_session(SSL *s, CLIENTHELLO_MSG *hello)
 
     s->session_ctx->stats.sess_hit++;
     s->verify_result = s->session->verify_result;
-
     return 1;
 
  err:
     if (ret != NULL) {
         SSL_SESSION_free(ret);
-        /* In TLSv1.3 we already set s->session, so better NULL it out */
+        /* In TLSv1.3 s->session was already set to ret, so we NULL it out */
         if (SSL_IS_TLS13(s))
             s->session = NULL;
 

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index c9e7d30eaae150454dbe15a77938e91b20551ba9..bce31a2c17e9ee27c1c41d72d3a7d3f4d83effbb 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1000,7 +1000,6 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
 static int init_psk_kex_modes(SSL *s, unsigned int context)
 {
     s->ext.psk_kex_mode = TLSEXT_KEX_MODE_FLAG_NONE;
-
     return 1;
 }
 
@@ -1014,7 +1013,7 @@ int tls_psk_do_binder(SSL *s, const EVP_MD *md, const unsigned char *msgstart,
     unsigned char hash[EVP_MAX_MD_SIZE], binderkey[EVP_MAX_MD_SIZE];
     unsigned char finishedkey[EVP_MAX_MD_SIZE], tmpbinder[EVP_MAX_MD_SIZE];
     const char resumption_label[] = "resumption psk binder key";
-    size_t hashsize = EVP_MD_size(md), bindersize;
+    size_t bindersize, hashsize = EVP_MD_size(md);
     int ret = -1;
 
     /* Generate the early_secret */

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 368f1968d8b1bf0813de543369fe05eab1eeb0be..738ab6b054a6014f868fd9de2622b59996e46465 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -682,6 +682,12 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
             || s->session->ext.ticklen == 0)
         return 1;
 
+    md = ssl_md(s->session->cipher->algorithm2);
+    if (md == NULL) {
+        /* Don't recognise this cipher so we can't use the session. Ignore it */
+        return 1;
+    }
+
     /*
      * Technically the C standard just says time() returns a time_t and says
      * nothing about the encoding of that type. In practice most implementations
@@ -721,11 +727,6 @@ int tls_construct_ctos_psk(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
         SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_PSK, ERR_R_INTERNAL_ERROR);
         goto err;
     }
-    md = ssl_md(s->session->cipher->algorithm2);
-    if (md == NULL) {
-        /* Don't recognise this cipher so we can't use the session. Ignore it */
-        return 1;
-    }
 
     hashsize = EVP_MD_size(md);
 

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 088dcbc3b65a6c54d406682693de9048c0b7cc2e..407b48c6715c09a5d34c8686b90adfafc71cb517 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -735,7 +735,6 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al)
         return 1;
 
     binderoffset = PACKET_data(pkt) - (const unsigned char *)s->init_buf->data;
-
     hashsize = EVP_MD_size(md);
 
     if (!PACKET_get_length_prefixed_2(pkt, &binders)) {
@@ -763,7 +762,6 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, X509 *x, size_t chainidx, int *al)
     sess->ext.tick_identity = id;
     SSL_SESSION_free(s->session);
     s->session = sess;
-
     return 1;
 err:
     return 0;
@@ -1007,7 +1005,7 @@ int tls_construct_stoc_key_share(SSL *s, WPACKET *pkt, X509 *x, size_t chainidx,
     EVP_PKEY *ckey = s->s3->peer_tmp, *skey = NULL;
 
     if (ckey == NULL) {
-        /* No key_share received from client, must be resuming. */
+        /* No key_share received from client; must be resuming. */
         if (!s->hit || !tls13_generate_handshake_secret(s, NULL, 0)) {
             *al = SSL_AD_INTERNAL_ERROR;
             SSLerr(SSL_F_TLS_CONSTRUCT_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);

diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c
index 53e16f87ef5656b16a77dae229213042eb9fd8f0..cfc104726757e5a1cb0a449b3221c1a241afb86c 100644 (file)
--- a/ssl/statem/statem_clnt.c
+++ b/ssl/statem/statem_clnt.c
@@ -2220,8 +2220,8 @@ MSG_PROCESS_RETURN tls_process_new_session_ticket(SSL *s, PACKET *pkt)
         || (SSL_IS_TLS13(s) && !PACKET_get_net_4(pkt, &age_add))
         || !PACKET_get_net_2(pkt, &ticklen)
         || (!SSL_IS_TLS13(s) && PACKET_remaining(pkt) != ticklen)
-        || (SSL_IS_TLS13(s) && (ticklen == 0
-                                || PACKET_remaining(pkt) < ticklen))) {
+        || (SSL_IS_TLS13(s)
+            && (ticklen == 0 || PACKET_remaining(pkt) < ticklen))) {
         SSLerr(SSL_F_TLS_PROCESS_NEW_SESSION_TICKET, SSL_R_LENGTH_MISMATCH);
         goto f_err;
     }

diff --git a/ssl/statem/statem_dtls.c b/ssl/statem/statem_dtls.c
index 1bc82d1625a9f1d0e2ef3394d5d9d827074a0c81..34964dbd5d791618a7c080c0c0fcd5174dcdf89c 100644 (file)
--- a/ssl/statem/statem_dtls.c
+++ b/ssl/statem/statem_dtls.c
@@ -788,7 +788,8 @@ static int dtls_get_reassembled_message(SSL *s, int *errtype, size_t *len)
         return 0;
     }
 
-    if (!s->server && s->d1->r_msg_hdr.frag_off == 0
+    if (!s->server
+            && s->d1->r_msg_hdr.frag_off == 0
             && s->statem.hand_state != TLS_ST_OK
             && wire[0] == SSL3_MT_HELLO_REQUEST) {
         /*

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 3a6b672a0ffff82799cc30ecb02a32018c71954f..c8b1469d1200621aa013417745cc225ab6325d94 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -72,7 +72,8 @@ int tls_close_construct_packet(SSL *s, WPACKET *pkt, int htype)
     return 1;
 }
 
-int tls_setup_handshake(SSL *s) {
+int tls_setup_handshake(SSL *s)
+{
     if (!ssl3_init_finished_mac(s))
         return 0;
 
@@ -107,9 +108,8 @@ int tls_setup_handshake(SSL *s) {
 
         s->s3->tmp.cert_req = 0;
 
-        if (SSL_IS_DTLS(s)) {
+        if (SSL_IS_DTLS(s))
             s->statem.use_timer = 1;
-        }
     }
 
     return 1;