Commit Graph

8 Commits

Author SHA1 Message Date
David Benjamin
4497e58961 Switch finish_handshake to release_current_message.
With the previous DTLS change, the dispatch layer only cares about the
end of the handshake to know when to drop the current message. TLS 1.3
post-handshake messages will need a similar hook, so convert it to this
lower-level one.

BUG=83

Change-Id: I4c8c3ba55ba793afa065bf261a7bccac8816c348
Reviewed-on: https://boringssl-review.googlesource.com/8989
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 22:59:18 +00:00
David Benjamin
02edcd0098 Reject stray post-Finished messages in DTLS.
This is in preparation for switching finish_handshake to a
release_current_message hook. finish_handshake in DTLS is also
responsible for releasing any memory associated with extra messages in
the handshake.

Except that's not right and we need to make it an error anyway. Given
that the rest of the DTLS dispatch layer already strongly assumes there
is only one message in epoch one, putting the check in the fragment
processing works fine enough. Add tests for this.

This will certainly need revising when DTLS 1.3 happens (perhaps just a
version check, perhaps bringing finish_handshake back as a function that
can fail... which means we need a state just before SSL_ST_OK), but DTLS
1.3 post-handshake messages haven't really been written down, so let's
do the easy thing for now and add a test for when it gets more
interesting.

This removes the sequence number reset in the DTLS code. That reset
never did anything becase we don't and never will renego. We should make
sure DTLS 1.3 does not bring the reset back for post-handshake stuff.
(It was wrong in 1.2 too. Penultimate-flight retransmits and renego
requests are ambiguous in DTLS.)

BUG=83

Change-Id: I33d645a8550f73e74606030b9815fdac0c9fb682
Reviewed-on: https://boringssl-review.googlesource.com/8988
Reviewed-by: Adam Langley <agl@google.com>
2016-07-28 22:53:04 +00:00
David Benjamin
481b9d2047 Remove begin_handshake and allocate init_buf lazily.
For TLS 1.3, we will need to process more complex post-handshake
messages. It is simplest if we use the same mechanism. In preparation,
allow ssl3_get_message to be called at any point.

Note that this stops reserving SSL3_RT_MAX_PLAIN_LENGTH in init_buf
right off the bat. Instead it will grow as-needed to accomodate the
handshake. SSL3_RT_MAX_PLAIN_LENGTH is rather larger than we probably
need to receive, particularly as a server, so this seems a good plan.

BUG=83

Change-Id: Id7f4024afc4c8a713b46b0d1625432315594350e
Reviewed-on: https://boringssl-review.googlesource.com/8985
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 22:07:28 +00:00
David Benjamin
49054544d3 Clear init_msg/init_num whenever we clear the backing store.
This API needs to be improved but, for the time being, keep the
invariant reasonable.

Change-Id: If94d41e7e7936e44de5ecb36da45f89f80df7784
Reviewed-on: https://boringssl-review.googlesource.com/8984
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-28 21:44:39 +00:00
David Benjamin
61672818ef Check for buffered handshake messages on cipher change in DTLS.
This is the equivalent of FragmentAcrossChangeCipherSuite for DTLS. It
is possible for us to, while receiving pre-CCS handshake messages, to
buffer up a message with sequence number meant for a post-CCS Finished.
When we then get to the new epoch and attempt to read the Finished, we
will process the buffered Finished although it was sent with the wrong
encryption.

Move ssl_set_{read,write}_state to SSL_PROTOCOL_METHOD hooks as this is
a property of the transport. Notably, read_state may fail. In DTLS
check the handshake buffer size. We could place this check in
read_change_cipher_spec, but TLS 1.3 has no ChangeCipherSpec message, so
we will need to implement this at the cipher change point anyway. (For
now, there is only an assert on the TLS side. This will be replaced with
a proper check in TLS 1.3.)

Change-Id: Ia52b0b81e7db53e9ed2d4f6d334a1cce13e93297
Reviewed-on: https://boringssl-review.googlesource.com/8790
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2016-07-16 08:25:02 +00:00
David Benjamin
528bd26dd9 Don't use init_buf in DTLS.
This machinery is so different between TLS and DTLS that there is no
sense in having them share structures. This switches us to maintaining
the full reassembled message in hm_fragment and get_message just lets
the caller read out of that when ready.

This removes the last direct handshake dependency on init_buf,
ssl3_hash_message.

Change-Id: I4eccfb6e6021116255daead5359a0aa3f4d5be7b
Reviewed-on: https://boringssl-review.googlesource.com/8667
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-11 23:01:11 +00:00
David Benjamin
97718f1437 Move references to init_buf into SSL_PROTOCOL_METHOD.
Both DTLS and TLS still use it, but that will change in the following
commit. This also removes the handshake's knowledge of the
dtls_clear_incoming_messages function.

(It's possible we'll want to get rid of begin_handshake in favor of
allocating it lazily depending on how TLS 1.3 post-handshake messages
end up working out. But this should work for now.)

Change-Id: I0f512788bbc330ab2c947890939c73e0a1aca18b
Reviewed-on: https://boringssl-review.googlesource.com/8666
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-11 18:20:33 +00:00
David Benjamin
a2c42d7685 Rename (s3,d1)_meth.c.
These are where the DTLS- and TLS-specific transport layer hooks will be
defined. Later we can probably move much of the implementations of these
hooks into these files so those functions can be static.

While I'm here, fix up the naming of some constants.

Change-Id: I1009dd9fdc3cc4fd49fbff0802f6289931abec3d
Reviewed-on: https://boringssl-review.googlesource.com/8665
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-07-11 17:22:35 +00:00