Revise hash management for reading the Finished message.
Upstream originally sampled the Finished message's hash at ChangeCipherSpec, but our patches to add messages between the two complicated this. Move DTLS to this path, but use the new SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag to avoid special-casing message types in ssl3_get_message. Change-Id: I9c8ddd9cc500c94dff2ec2f696f89d50ab01b3ad Reviewed-on: https://boringssl-review.googlesource.com/1632 Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
parent
880b14e98c
commit
5b8f104ee8
@ -191,7 +191,6 @@ int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
|
||||
return ssl_do_write(s);
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_NO_NEXTPROTONEG
|
||||
/* ssl3_take_mac calculates the Finished MAC for the handshakes messages seen to far. */
|
||||
static void ssl3_take_mac(SSL *s)
|
||||
{
|
||||
@ -216,7 +215,6 @@ static void ssl3_take_mac(SSL *s)
|
||||
s->s3->tmp.peer_finish_md_len = s->method->ssl3_enc->final_finish_mac(s,
|
||||
sender,slen,s->s3->tmp.peer_finish_md);
|
||||
}
|
||||
#endif
|
||||
|
||||
int ssl3_get_finished(SSL *s, int a, int b)
|
||||
{
|
||||
@ -224,22 +222,20 @@ int ssl3_get_finished(SSL *s, int a, int b)
|
||||
long n;
|
||||
unsigned char *p;
|
||||
|
||||
#ifdef OPENSSL_NO_NEXTPROTONEG
|
||||
/* the mac has already been generated when we received the
|
||||
* change cipher spec message and is in s->s3->tmp.peer_finish_md
|
||||
*/
|
||||
#endif
|
||||
|
||||
n=s->method->ssl_get_message(s,
|
||||
a,
|
||||
b,
|
||||
SSL3_MT_FINISHED,
|
||||
64, /* should actually be 36+4 :-) */
|
||||
SSL_GET_MESSAGE_HASH_MESSAGE,
|
||||
SSL_GET_MESSAGE_DONT_HASH_MESSAGE,
|
||||
&ok);
|
||||
|
||||
if (!ok) return((int)n);
|
||||
|
||||
/* Snapshot the finished hash before incorporating the new message. */
|
||||
ssl3_take_mac(s);
|
||||
ssl3_hash_current_message(s);
|
||||
|
||||
/* If this occurs, we have missed a message.
|
||||
* TODO(davidben): Is this check now redundant with
|
||||
* SSL3_FLAGS_EXPECT_CCS? */
|
||||
@ -466,13 +462,6 @@ long ssl3_get_message(SSL *s, int st1, int stn, int mt, long max, int hash_messa
|
||||
n -= i;
|
||||
}
|
||||
|
||||
#ifndef OPENSSL_NO_NEXTPROTONEG
|
||||
/* If receiving Finished, record MAC of prior handshake messages for
|
||||
* Finished verification. */
|
||||
if (*s->init_buf->data == SSL3_MT_FINISHED)
|
||||
ssl3_take_mac(s);
|
||||
#endif
|
||||
|
||||
/* Feed this message into MAC computation. */
|
||||
if (hash_message != SSL_GET_MESSAGE_DONT_HASH_MESSAGE)
|
||||
ssl3_hash_current_message(s);
|
||||
|
25
ssl/s3_pkt.c
25
ssl/s3_pkt.c
@ -1426,8 +1426,6 @@ err:
|
||||
int ssl3_do_change_cipher_spec(SSL *s)
|
||||
{
|
||||
int i;
|
||||
const char *sender;
|
||||
int slen;
|
||||
|
||||
if (s->state & SSL_ST_ACCEPT)
|
||||
i=SSL3_CHANGE_CIPHER_SERVER_READ;
|
||||
@ -1450,29 +1448,6 @@ int ssl3_do_change_cipher_spec(SSL *s)
|
||||
if (!s->method->ssl3_enc->change_cipher_state(s,i))
|
||||
return(0);
|
||||
|
||||
/* we have to record the message digest at
|
||||
* this point so we can get it before we read
|
||||
* the finished message */
|
||||
if (s->state & SSL_ST_CONNECT)
|
||||
{
|
||||
sender=s->method->ssl3_enc->server_finished_label;
|
||||
slen=s->method->ssl3_enc->server_finished_label_len;
|
||||
}
|
||||
else
|
||||
{
|
||||
sender=s->method->ssl3_enc->client_finished_label;
|
||||
slen=s->method->ssl3_enc->client_finished_label_len;
|
||||
}
|
||||
|
||||
i = s->method->ssl3_enc->final_finish_mac(s,
|
||||
sender,slen,s->s3->tmp.peer_finish_md);
|
||||
if (i == 0)
|
||||
{
|
||||
OPENSSL_PUT_ERROR(SSL, ssl3_do_change_cipher_spec, ERR_R_INTERNAL_ERROR);
|
||||
return 0;
|
||||
}
|
||||
s->s3->tmp.peer_finish_md_len = i;
|
||||
|
||||
return(1);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user