Remove cookie validation return value trick
authorMatt Caswell <matt@openssl.org>
Mon, 30 Nov 2015 10:38:54 +0000 (10:38 +0000)
committerMatt Caswell <matt@openssl.org>
Tue, 1 Dec 2015 15:23:42 +0000 (15:23 +0000)
In the DTLS ClientHello processing the return value is stored in |ret| which
by default is -1. |ret| is only updated to a positive value once we are past
all points where we could hit an error. We wish to return 1 on success or 2
on success *and* we have validated the DTLS cookie. Previously on successful
validation of the cookie we were setting |ret| to -2, and then once we were
past all error points we set |ret = -ret|. This is non-obvious behaviour and
could be error prone. This commit tries to make this a bit more intuitive.

Reviewed-by: Andy Polyakov <appro@openssl.org>
ssl/s3_srvr.c

index 424e50d13de50537cf3b76a8dcc50f41dbd01a31..ee83105b75bffa0a6ece0c33ac40ebe6f83356c5 100644 (file)
@@ -901,7 +901,7 @@ int ssl3_send_hello_request(SSL *s)
 
 int ssl3_get_client_hello(SSL *s)
 {
-    int i, j, ok, al = SSL_AD_INTERNAL_ERROR, ret = -1;
+    int i, j, ok, al = SSL_AD_INTERNAL_ERROR, ret = -1, cookie_valid = 0;
     unsigned int cookie_len;
     long n;
     unsigned long id;
@@ -1094,8 +1094,7 @@ int ssl3_get_client_hello(SSL *s)
                 SSLerr(SSL_F_SSL3_GET_CLIENT_HELLO, SSL_R_COOKIE_MISMATCH);
                 goto f_err;
             }
-            /* Set to -2 so if successful we return 2 */
-            ret = -2;
+            cookie_valid = 1;
         }
 
         p += cookie_len;
@@ -1465,8 +1464,7 @@ int ssl3_get_client_hello(SSL *s)
         }
     }
 
-    if (ret < 0)
-        ret = -ret;
+    ret = cookie_valid ? 2 : 1;
     if (0) {
  f_err:
         ssl3_send_alert(s, SSL3_AL_FATAL, al);
@@ -1476,7 +1474,7 @@ int ssl3_get_client_hello(SSL *s)
 
     if (ciphers != NULL)
         sk_SSL_CIPHER_free(ciphers);
-    return ret < 0 ? -1 : ret;
+    return ret;
 }
 
 int ssl3_send_server_hello(SSL *s)