diff options
author | Matt Caswell <matt@openssl.org> | 2021-04-19 15:21:54 +0100 |
---|---|---|
committer | Matt Caswell <matt@openssl.org> | 2021-04-28 16:23:08 +0100 |
commit | f42e68dc473081393835b0ae7dad19d393ee589d (patch) | |
tree | 1042d31d7f64c5a06c8a2cbff56a5bef7f68433c /ssl/statem/statem.c | |
parent | 460d2fbcd75bef492638b54c17aa5f5bca7eec2a (diff) |
Defer Finished MAC handling until after state transition
In TLS we process received messages like this:
1) Read Message Header
2) Validate and transition state based on received message type
3) Read Message Body
4) Process Message
In DTLS we read messages like this:
1) Read Message Header and Body
2) Validate and transition state based on received message type
3) Process Message
The difference is because of the stream vs datagram semantics of the
underlying transport.
In both TLS and DTLS we were doing finished MAC processing as part of
reading the message body. This means that in DTLS this was occurring
*before* the state transition has been validated. A crash was occurring
in DTLS if a Finished message was sent in an invalid state due to
assumptions in the code that certain variables would have been setup by
the time a Finished message arrives.
To avoid this problem we shift the finished MAC processing to be after
the state transition in DTLS.
Thanks to github user @bathooman for reporting this issue.
Fixes #14906
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14930)
Diffstat (limited to 'ssl/statem/statem.c')
-rw-r--r-- | ssl/statem/statem.c | 19 |
1 files changed, 12 insertions, 7 deletions
diff --git a/ssl/statem/statem.c b/ssl/statem/statem.c index 3b6e78e3f8..4c463974ea 100644 --- a/ssl/statem/statem.c +++ b/ssl/statem/statem.c @@ -582,7 +582,7 @@ static SUB_STATE_RETURN read_state_machine(SSL *s) /* * In DTLS we get the whole message in one go - header and body */ - ret = dtls_get_message(s, &mt, &len); + ret = dtls_get_message(s, &mt); } else { ret = tls_get_message_header(s, &mt); } @@ -625,13 +625,18 @@ static SUB_STATE_RETURN read_state_machine(SSL *s) /* Fall through */ case READ_STATE_BODY: - if (!SSL_IS_DTLS(s)) { - /* We already got this above for DTLS */ + if (SSL_IS_DTLS(s)) { + /* + * Actually we already have the body, but we give DTLS the + * opportunity to do any further processing. + */ + ret = dtls_get_message_body(s, &len); + } else { ret = tls_get_message_body(s, &len); - if (ret == 0) { - /* Could be non-blocking IO */ - return SUB_STATE_ERROR; - } + } + if (ret == 0) { + /* Could be non-blocking IO */ + return SUB_STATE_ERROR; } s->first_packet = 0; |