Commit Graph

16 Commits

Author SHA1 Message Date
David Benjamin
18a3518e5a Don't allocate a group/curve ID for CECPQ1.
We ended up switching this from a curve to a cipher suite, so the group
ID isn't used. This is in preparation for adding an API for the curve
ID, at which point leaving the protocol constants undefined seems
somewhat bad manners.

Change-Id: Icb8bf4594879dbbc24177551868ecfe89bc2f8c3
Reviewed-on: https://boringssl-review.googlesource.com/8563
Reviewed-by: Adam Langley <agl@google.com>
2016-06-30 22:28:37 +00:00
David Benjamin
b6a0a518a3 Simplify version configuration.
OpenSSL's SSL_OP_NO_* flags allow discontinuous version ranges. This is a
nuisance for two reasons. First it makes it unnecessarily difficult to answer
"are any versions below TLS 1.3 enabled?". Second the protocol does not allow
discontinuous version ranges on the client anyway. OpenSSL instead picks the
first continous range of enabled versions on the client, but not the server.

This is bizarrely inconsistent. It also doesn't quite do this as the
ClientHello sending logic does this, but not the ServerHello processing logic.
So we actually break some invariants slightly. The logic is also cumbersome in
DTLS which kindly inverts the comparison logic.

First, switch min_version/max_version's storage to normalized versions. Next
replace all the ad-hoc version-related functions with a single
ssl_get_version_range function. Client and server now consistently pick a
contiguous range of versions. Note this is a slight behavior change for
servers. Version-range-sensitive logic is rewritten to use this new function.

BUG=66

Change-Id: Iad0d64f2b7a917603fc7da54c9fc6656c5fbdb24
Reviewed-on: https://boringssl-review.googlesource.com/8513
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-30 21:56:01 +00:00
Steven Valdez
f0451ca37d Cleaning up internal use of Signature Algorithms.
The signing logic itself still depends on pre-hashed messages and will be fixed
in later commits.

Change-Id: I901b0d99917c311653d44efa34a044bbb9f11e57
Reviewed-on: https://boringssl-review.googlesource.com/8545
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-29 21:22:25 +00:00
David Benjamin
352d0a9c6c Remove a/b parameters to send_change_cipher_spec.
They're not necessary.

Change-Id: Ifeb3fae73a8b22f88019e6ef9f9ba5e64ed3cfab
Reviewed-on: https://boringssl-review.googlesource.com/8543
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-29 18:50:47 +00:00
Steven Valdez
025638597a Changing representation of signature/hash to use SignatureScheme.
As part of the SignatureAlgorithm change in the TLS 1.3 specification,
the existing signature/hash combinations are replaced with a combined
signature algorithm identifier. This change maintains the existing APIs
while fixing the internal representations. The signing code currently
still treats the SignatureAlgorithm as a decomposed value, which will be
fixed as part of a separate CL.

Change-Id: I0cd1660d74ad9bcf55ce5da4449bf2922660be36
Reviewed-on: https://boringssl-review.googlesource.com/8480
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-28 14:18:53 +00:00
David Benjamin
7583643569 Disconnect handshake message creation from init_buf.
This allows us to use CBB for all handshake messages. Now, SSL_PROTOCOL_METHOD
is responsible for implementing a trio of CBB-related hooks to assemble
handshake messages.

Change-Id: I144d3cac4f05b6637bf45d3f838673fc5c854405
Reviewed-on: https://boringssl-review.googlesource.com/8440
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 22:15:01 +00:00
David Benjamin
ec847cea9b Replace the incoming message buffer with a ring buffer.
It has size 7. There's no need for a priority queue structure, especially one
that's O(N^2) anyway.

Change-Id: I7609794aac1925c9bbf3015744cae266dcb79bff
Reviewed-on: https://boringssl-review.googlesource.com/8437
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 20:12:03 +00:00
David Benjamin
778f57e511 Store only one handshake write sequence number.
The pair was a remnant of some weird statefulness and also ChangeCipherSpec
having a "sequence number" to make the pqueue turn into an array.

Change-Id: Iffd82594314df43934073bd141faee0fc167ed5f
Reviewed-on: https://boringssl-review.googlesource.com/8436
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 20:11:19 +00:00
David Benjamin
f182ee1bba Always release init_buf after the handshake.
Post-handshake retransmit in DTLS no longer needs that scratch space.

Change-Id: I2f070675d72426e61b19dab5bcac40bf62b8fd8d
Reviewed-on: https://boringssl-review.googlesource.com/8434
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 20:09:47 +00:00
David Benjamin
c42acee63d Stash a copy of the SKX params rather mess with init_buf.
It is an explicit copy of something, but it's a lot easier to reason about than
the init_buf/init_num gynmastics we were previously doing. This is along the
way to getting init_buf out of here.

Change-Id: Ia1819ba9db60ef6db09dd60d208dbc95fcfb4bd2
Reviewed-on: https://boringssl-review.googlesource.com/8432
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 20:07:42 +00:00
David Benjamin
f0ee907942 Remove the 'ssl_' prefix on most SSL_PROTOCOL_METHOD hooks.
It doesn't really convey anything useful. Leave ssl_get_message alone for now
since it's called everywhere in the handshake and I'm about to tweak it
further.

Change-Id: I6f3a74c170e818f624be8fbe5cf6b796353406df
Reviewed-on: https://boringssl-review.googlesource.com/8430
Reviewed-by: Adam Langley <agl@google.com>
2016-06-27 18:43:33 +00:00
David Benjamin
8e710ca1e2 Remove unnecessary check and comments.
The payload comments aren't necessary now that our parsing code is readable in
itself. The check is impossible to hit.

Change-Id: Ib41ad606babda903a9fab50de3189f97e99cac2f
Reviewed-on: https://boringssl-review.googlesource.com/8248
Reviewed-by: Adam Langley <agl@google.com>
2016-06-22 20:22:39 +00:00
David Benjamin
47749a6a29 Make the handshake state machines more linear.
TLS 1.2 has a long series of optional messages within a flight. We really
should send and process these synchronously. In the meantime, the 'skip'
pattern is probably the best we can get away with. Otherwise we have too many
state transitions to think about. (The business with CCS, NPN, and ChannelID is
particularly a headache. Session tickets aren't great either.)

Change-Id: I84e391a6410046372cf9c6989be056a27606ad19
Reviewed-on: https://boringssl-review.googlesource.com/8246
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2016-06-22 20:14:10 +00:00
David Benjamin
bde00394f0 Stop messing with ssl->version before sending protocol_version.
This is the only codepath where ssl->version can get a garbage value, which is
a little concerning. Since, in all these cases, the peer is failing to connect
and speaks so low a version we don't even accept it anymore, there is probably
not much value in letting them distinguish protocol_version from a record-layer
version number mismatch, where enforced (which will give a version-related
error anyway).

Should we get a decode_error or so just before version negotiation, we'd have
this behavior already.

Change-Id: I9b3e5685ab9c9ad32a7b7e3129363cd1d4cdaaf4
Reviewed-on: https://boringssl-review.googlesource.com/8420
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-22 13:59:16 +00:00
David Benjamin
3dcec458f1 Rename SERVER_DONE to SERVER_HELLO_DONE.
Match the actual name of the type.

Change-Id: I0ad27196ee2876ce0690d13068fa95f68b05b0da
Reviewed-on: https://boringssl-review.googlesource.com/8187
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:26:59 +00:00
David Benjamin
cfec7c60b9 Rename s3_{clnt,srvr}.c
Give them much more reasonable names.

Change-Id: Id14d983ab3231da21a4f987e662c2e01af7a2cd6
Reviewed-on: https://boringssl-review.googlesource.com/8185
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:25:31 +00:00