Miscellaneous style tweaks based on feedback received

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

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);
     }
 
     /*

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)

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);
 

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]);
 }

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;
 

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 */

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);
 

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);

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;
     }

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) {
         /*

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;