On 20 Jun 2014, at 7:35, Ted Unangst <[email protected]> wrote:
> Always explicitly compare memcmp with 0. I find this adds clarity.
i agree.
ok by me if that has any value in this part of the tree.
>
> Index: s3_clnt.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/src/ssl/s3_clnt.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 s3_clnt.c
> --- s3_clnt.c 19 Jun 2014 21:29:51 -0000 1.71
> +++ s3_clnt.c 19 Jun 2014 21:33:47 -0000
> @@ -886,7 +886,7 @@ ssl3_get_server_hello(SSL *s)
> timingsafe_memcmp(p, s->session->session_id, j) == 0) {
> if (s->sid_ctx_length != s->session->sid_ctx_length ||
> timingsafe_memcmp(s->session->sid_ctx,
> - s->sid_ctx, s->sid_ctx_length)) {
> + s->sid_ctx, s->sid_ctx_length) != 0) {
> /* actually a client application bug */
> al = SSL_AD_ILLEGAL_PARAMETER;
> SSLerr(SSL_F_SSL3_GET_SERVER_HELLO,
> Index: ssl_sess.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/src/ssl/ssl_sess.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 ssl_sess.c
> --- ssl_sess.c 19 Jun 2014 21:29:51 -0000 1.33
> +++ ssl_sess.c 19 Jun 2014 21:33:47 -0000
> @@ -498,7 +498,7 @@ ssl_get_prev_session(SSL *s, unsigned ch
> /* Now ret is non-NULL and we own one of its reference counts. */
>
> if (ret->sid_ctx_length != s->sid_ctx_length
> - || timingsafe_memcmp(ret->sid_ctx, s->sid_ctx,
> ret->sid_ctx_length)) {
> + || timingsafe_memcmp(ret->sid_ctx, s->sid_ctx, ret->sid_ctx_length)
> != 0) {
> /* We have the session requested by the client, but we don't
> * want to use it in this context. */
> goto err; /* treat like cache miss */
> Index: t1_reneg.c
> ===================================================================
> RCS file: /cvs/src/lib/libssl/src/ssl/t1_reneg.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 t1_reneg.c
> --- t1_reneg.c 19 Jun 2014 21:29:51 -0000 1.7
> +++ t1_reneg.c 19 Jun 2014 21:33:47 -0000
> @@ -173,7 +173,7 @@ ssl_parse_clienthello_renegotiate_ext(SS
> }
>
> if (timingsafe_memcmp(d, s->s3->previous_client_finished,
> - s->s3->previous_client_finished_len)) {
> + s->s3->previous_client_finished_len) != 0) {
> SSLerr(SSL_F_SSL_PARSE_CLIENTHELLO_RENEGOTIATE_EXT,
> SSL_R_RENEGOTIATION_MISMATCH);
> *al = SSL_AD_HANDSHAKE_FAILURE;
> @@ -260,7 +260,7 @@ ssl_parse_serverhello_renegotiate_ext(SS
> }
>
> if (timingsafe_memcmp(d, s->s3->previous_client_finished,
> - s->s3->previous_client_finished_len)) {
> + s->s3->previous_client_finished_len) != 0) {
> SSLerr(SSL_F_SSL_PARSE_SERVERHELLO_RENEGOTIATE_EXT,
> SSL_R_RENEGOTIATION_MISMATCH);
> *al = SSL_AD_HANDSHAKE_FAILURE;
>
