Welcome to mirror list, hosted at ThFree Co, Russian Federation.

github.com/mono/boringssl.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2016-09-21Take the version parameter out of ssl_do_msg_callback.David Benjamin
This will make it a little easier to store the normalized version rather than the wire version. Also document the V2ClientHello behavior. Change-Id: I5ce9ccce44ca48be2e60ddf293c0fab6bba1356e Reviewed-on: https://boringssl-review.googlesource.com/11121 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-09-12Use C99 for size_t loops.David Benjamin
This was done just by grepping for 'size_t i;' and 'size_t j;'. I left everything in crypto/x509 and friends alone. There's some instances in gcm.c that are non-trivial and pulled into a separate CL for ease of review. Change-Id: I6515804e3097f7e90855f1e7610868ee87117223 Reviewed-on: https://boringssl-review.googlesource.com/10801 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-29Switch finish_handshake to release_current_message.David Benjamin
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-29Reject stray post-Finished messages in DTLS.David Benjamin
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-16Check for buffered handshake messages on cipher change in DTLS.David Benjamin
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-12Simplify ssl_get_message somewhat.David Benjamin
It still places the current message all over the place, but remove the bizarre init_num/error/ok split. Now callers get the message length out of init_num, which mirrors init_msg. Also fix some signedness. Change-Id: Ic2e97b6b99e234926504ff217b8aedae85ba6596 Reviewed-on: https://boringssl-review.googlesource.com/8690 Reviewed-by: David Benjamin <davidben@google.com>
2016-07-12Don't use init_buf in DTLS.David Benjamin
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-06-29Remove a/b parameters to send_change_cipher_spec.David Benjamin
They're not necessary. Change-Id: Ifeb3fae73a8b22f88019e6ef9f9ba5e64ed3cfab Reviewed-on: https://boringssl-review.googlesource.com/8543 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-28Group d1_both.c by sending and receiving handshake messages.David Benjamin
This file is still kind of a mess, but put the two halves together at least. Change-Id: Ib21d9c4a7f4864cf80e521f7d0ebec029e5955a1 Reviewed-on: https://boringssl-review.googlesource.com/8502 Reviewed-by: Adam Langley <agl@google.com>
2016-06-28Remove compatibility 'inline' define.David Benjamin
MSVC 2015 seems to support it just fine. Change-Id: I9c91c18c260031e6024480d1f57bbb334ed7118c Reviewed-on: https://boringssl-review.googlesource.com/8501 Reviewed-by: Adam Langley <agl@google.com>
2016-06-28Stop using the word 'buffer' everywhere.David Benjamin
buffer buffer buffer buffer buffer. At some point, words lose their meaning if they're used too many times. Notably, the DTLS code can't decide whether a "buffered message" is an incoming message to be reassembled or an outgoing message to be (re)transmitted. Change-Id: Ibdde5c00abb062c603d21be97aff49e1c422c755 Reviewed-on: https://boringssl-review.googlesource.com/8500 Reviewed-by: Adam Langley <agl@google.com>
2016-06-28Disconnect handshake message creation from init_buf.David Benjamin
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-27Replace the incoming message buffer with a ring buffer.David Benjamin
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-27Store only one handshake write sequence number.David Benjamin
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-27Rewrite DTLS outgoing message buffering.David Benjamin
Now that retransitting is a lot less stateful, a lot of surrounding code can lose statefulness too. Rather than this overcomplicated pqueue structure, hardcode that a handshake flight is capped at 7 messages (actually, DTLS can only get up to 6 because we don't support NPN or Channel ID in DTLS) and used a fixed size array. This also resolves several TODOs. Change-Id: I2b54c3441577a75ad5ca411d872b807d69aa08eb Reviewed-on: https://boringssl-review.googlesource.com/8435 Reviewed-by: Adam Langley <agl@google.com>
2016-06-27Make dtls1_do_handshake_write less stateful.David Benjamin
Now dtls1_do_handshake_write takes in a serialized form of the full message and writes it. It's a little weird to serialize and deserialize the header a bunch, but msg_callback requires that we keep the full one around in memory anyway. Between that and the handshake hash definition, DTLS really wants messages to mean the assembled header, redundancies and all, so we'll just put together messages that way. This also fixes a bug where ssl_do_msg_callback would get passed in garbage where the header was supposed to be. The buffered messages get sampled before writing the fragment rather than after. Change-Id: I4e3b8ce4aab4c4ab4502d5428dfb8f3f729c6ef9 Reviewed-on: https://boringssl-review.googlesource.com/8433 Reviewed-by: Adam Langley <agl@google.com>
2016-06-08Trim the DTLS write code slightly.David Benjamin
Change-Id: I0fb4152ed656a60fae3aa7922652df766d4978d7 Reviewed-on: https://boringssl-review.googlesource.com/8178 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08Don't use dtls1_read_bytes to read messages.David Benjamin
This was probably the worst offender of them all as read_bytes is the wrong abstraction to begin with. Note this is a slight change in how processing a record works. Rather than reading one fragment at a time, we process all fragments in a record and return. The intent here is so that all records are processed atomically since the connection eventually will not be able to retain a buffer holding the record. This loses a ton of (though not quite all yet) those a2b macros. Change-Id: Ibe4bbcc33c496328de08d272457d2282c411b38b Reviewed-on: https://boringssl-review.googlesource.com/8176 Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08Add helper functions for info_callback and msg_callback.David Benjamin
This is getting a little repetitive. Change-Id: Ib0fa8ab10149557c2d728b88648381b9368221d9 Reviewed-on: https://boringssl-review.googlesource.com/8126 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08Tidy up the DTLS code's blocking-mode retransmits.David Benjamin
Move this logic out of dtls1_read_bytes and into dtls1_get_record. Only trigger it when reading from the buffer fails. The other one shouldn't be necessary. This exists to handle the blocking BIO case when the BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT signal triggers, so we only need to do it when timeouts actually trigger. There also doesn't seem to be a need for most of the machinery. The BIO_set_flags call seems to be working around a deficiency in the underlying BIO. There also shouldn't be a need to check the handshake state as there wouldn't be a timer to restart otherwise. Change-Id: Ic901ccfb5b82aeb409d16a9d32c04741410ad6d7 Reviewed-on: https://boringssl-review.googlesource.com/8122 Reviewed-by: Steven Valdez <svaldez@google.com> Reviewed-by: Adam Langley <agl@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2016-05-23Never expose ssl->bbio in the public API.David Benjamin
OpenSSL's bbio logic is kind of crazy. It would be good to eventually do the buffering in a better way (notably, bbio is fragile, if not outright broken, for DTLS). In the meantime, this fixes a number of bugs where the existence of bbio was leaked in the public API and broke things. - SSL_get_wbio returned the bbio during the handshake. It must always return the BIO the consumer configured. In doing so, internal accesses of SSL_get_wbio should be switched to ssl->wbio since those want to see bbio. For consistency, do the same with rbio. - The logic in SSL_set_rfd, etc. (which I doubt is quite right since SSL_set_bio's lifetime is unclear) would get confused once wbio got wrapped. Those want to compare to SSL_get_wbio. - If SSL_set_bio was called mid-handshake, bbio would get disconnected and lose state. It forgets to reattach the bbio afterwards. Unfortunately, Conscrypt does this a lot. It just never ended up calling it at a point where the bbio would cause problems. - Make more explicit the invariant that any bbio's which exist are always attached. Simplify a few things as part of that. Change-Id: Ia02d6bdfb9aeb1e3021a8f82dcbd0629f5c7fb8d Reviewed-on: https://boringssl-review.googlesource.com/8023 Reviewed-by: Kenny Root <kroot@google.com> Reviewed-by: Adam Langley <agl@google.com>
2016-05-18Remove state parameters to ssl3_get_message.David Benjamin
They're completely unused now. The handshake message reassembly logic should not depend on the state machine. This should partially free it up (ugly as it is) to be shared with a future TLS 1.3 implementation while, in parallel, it and the layers below, get reworked. This also cuts down on the number of states significantly. Partially because I expect we'd want to get ssl_hash_message_t out of there too. Having it in common code is fine, but it needs to be in the (supposed to be) protocol-agnostic handshake state machine, not the protocol-specific handshake message layer. Change-Id: I12f9dc57bf433ceead0591106ab165d352ef6ee4 Reviewed-on: https://boringssl-review.googlesource.com/7949 Reviewed-by: Adam Langley <agl@google.com>
2016-05-18Simplify ssl3_get_message.David Benjamin
Rather than this confusing coordination with the handshake state machine and init_num changing meaning partway through, use the length field already in BUF_MEM. Like the new record layer parsing, is no need to keep track of whether we are reading the header or the body. Simply keep extending the handshake message until it's far enough along. ssl3_get_message still needs tons of work, but this allows us to disentangle it from the handshake state. Change-Id: Ic2b3e7cfe6152a7e28a04980317d3c7c396d9b08 Reviewed-on: https://boringssl-review.googlesource.com/7948 Reviewed-by: Adam Langley <agl@google.com>
2016-05-13Simplify handshake message size limits.David Benjamin
A handshake message can go up to 2^24 bytes = 16MB which is a little large for the peer to force us to buffer. Accordingly, we bound the size of a handshake message. Rather than have a global limit, the existing logic uses a different limit at each state in the handshake state machine and, for certificates, allows configuring the maximum certificate size. This is nice in that we engage larger limits iff the relevant state is reachable from the handshake. Servers without client auth get a tighter limit "for free". However, this doesn't work for DTLS due to out-of-order messages and we use a simpler scheme for DTLS. This scheme also is tricky on optional messages and makes the handshake <-> message layer communication complex. Apart from an ignored 20,000 byte limit on ServerHello, the largest non-certificate limit is the common 16k limit on ClientHello. So this complexity wasn't buying us anything. Unify everything on the DTLS scheme except, so as not to regress bounds on client-auth-less servers, also correctly check for whether client auth is configured. The value of 16k was chosen based on this value. (The 20,000 byte ServerHello limit makes no sense. We can easily bound the ServerHello because servers may not send extensions we don't implement. But it gets overshadowed by the certificate anyway.) Change-Id: I00309b16d809a3c2a1543f99fd29c4163e3add81 Reviewed-on: https://boringssl-review.googlesource.com/7941 Reviewed-by: David Benjamin <davidben@google.com>
2016-05-06Remove the push argument to ssl_init_wbio_buffer.David Benjamin
Having bbio be tri-state (not allocated, allocated but not active, and allocated and active) is confusing. The extra state is only used in the client handshake, where ClientHello is special-cased to not go through the buffer while everything else is. This dates to OpenSSL's initial commit and doesn't seem to do much. I do not believe it can affect renego as the buffer only affects writes; although OpenSSL accepted interleave on read (though this logic predates it slightly), it never sent application data while it believed a handshake was active. The handshake would always be driven to completion first. My guess is this was to save a copy since the ClientHello is a one-message flight so it wouldn't need to be buffered? This is probably not worth the extra variation in the state. (Especially with the DTLS state machine going through ClientHello twice and pushing the BIO in between the two. Though I suspect that was a mistake in itself. If the optimization guess is correct, there was no need to do that.) Change-Id: I6726f866e16ee7213cab0c3e6abb133981444d47 Reviewed-on: https://boringssl-review.googlesource.com/7873 Reviewed-by: Adam Langley <agl@google.com>
2016-05-06Check BIO_flush return value.David Benjamin
That we're ignoring the return value is clearly wrong when dtls1_retransmit_message has other code that doesn't ignore it, by way of dtls1_do_handshake_write. Change-Id: Ie3f8c0defdf1f5e709d67af4ca6fa4f0d83c76c9 Reviewed-on: https://boringssl-review.googlesource.com/7872 Reviewed-by: Adam Langley <agl@google.com>
2016-05-06Always buffer DTLS retransmits.David Benjamin
The DTLS bbio logic is rather problematic, but this shouldn't make things worse. In the in-handshake case, the new code merges the per-message (unchecked) BIO_flush calls into one call at the end but otherwise the BIO is treated as is. Otherwise any behavior around non-block writes should be preserved. In the post-handshake case, we now install the buffer when we didn't previously. On write error, the buffer will have garbage in it, but it will be discarded, so that will preserve any existing retry behavior. (Arguably the existing retry behavior is a bug, but that's another matter.) Add a test for all this, otherwise it is sure to regress. Testing for record-packing is a little fuzzy, but we can assert ChangeCipherSpec always shares a record with something. BUG=57 Change-Id: I8603f20811d502c71ded2943b0e72a8bdc4e46f2 Reviewed-on: https://boringssl-review.googlesource.com/7871 Reviewed-by: Adam Langley <agl@google.com>
2016-04-18Set rwstate consistently.David Benjamin
We reset it to SSL_NOTHING at the start of ever SSL_get_error-using operation. Then we only set it to a non-NOTHING value in the rest of the stack on error paths. Currently, ssl->rwstate is set all over the place. Sometimes the pattern is: ssl->rwstate = SSL_WRITING; if (BIO_write(...) <= 0) { goto err; } ssl->rwstate = SSL_NOTHING; Sometimes we only set it to the non-NOTHING value on error. if (BIO_write(...) <= 0) { ssl->rwstate = SSL_WRITING; } ssl->rwstate = SSL_NOTHING; Sometimes we just set it to SSL_NOTHING far from any callback in random places. The third case is arbitrary and clearly should be removed. But, in the second case, we sometimes forget to undo it afterwards. This is largely harmless since an error in the error queue overrides rwstate, but we don't always put something in the error queue (falling back to SSL_ERROR_SYSCALL for "I'm not sure why it failed. Perhaps it was one of your callbacks? Check your errno equivalent."), but in that case a stray rwstate value will cause it to be wrong. We could fix the cases where we fail to set SSL_NOTHING on success cases, but this doesn't account for there being multiple SSL_get_error operations. The consumer may have an SSL_read and an SSL_write running concurrently. Instead, it seems the best option is to lift the SSL_NOTHING reset to the operations and set SSL_WRITING and friends as in the second case. (Someday hopefully we can fix this to just be an enum that is internally returned. It can convert to something stateful at the API layer.) Change-Id: I54665ec066a64eb0e48a06e2fcd0d2681a42df7f Reviewed-on: https://boringssl-review.googlesource.com/7453 Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31Remove some easy obj.h dependencies.David Benjamin
A lot of consumers of obj.h only want the NID values. Others didn't need it at all. This also removes some OBJ_nid2sn and OBJ_nid2ln calls in EVP error paths which isn't worth pulling a large table in for. BUG=chromium:499653 Change-Id: Id6dff578f993012e35b740a13b8e4f9c2edc0744 Reviewed-on: https://boringssl-review.googlesource.com/7563 Reviewed-by: David Benjamin <davidben@google.com>
2016-03-17Remove a number of unnecessary stdio.h includes.David Benjamin
Change-Id: I6267c9bfb66940d0b6fe5368514210a058ebd3cc Reviewed-on: https://boringssl-review.googlesource.com/7494 Reviewed-by: Emily Stark (Dunn) <estark@google.com> Reviewed-by: David Benjamin <davidben@google.com>
2015-12-23Switch s to ssl everywhere.David Benjamin
That we're half and half is really confusing. Change-Id: I1c2632682e8a3e63d01dada8e0eb3b735ff709ce Reviewed-on: https://boringssl-review.googlesource.com/6785 Reviewed-by: Adam Langley <agl@google.com>
2015-11-07Rewrite DTLS handshake message sending logic.David Benjamin
This fixes a number of bugs with the original logic: - If handshake messages are fragmented and writes need to be retried, frag_off gets completely confused. - The BIO_flush call didn't set rwstate, so it wasn't resumable at that point. - The msg_callback call gets garbage because the fragment header would get scribbled over the handshake buffer. The original logic was also extremely confusing with how it handles init_off. (init_off gets rewound to make room for the fragment header. Depending on where you pause, resuming may or may not have already been rewound.) For simplicity, just allocate a new buffer to assemble the fragment in and avoid clobbering the old one. I don't think it's worth the complexity to optimize that. If we want to optimize this sort of thing, not clobbering seems better anyway because the message may need to be retransmitted. We could avoid doing a copy when buffering the outgoing message for retransmission later. We do still need to track how far we are in sending the current message via init_off, so I haven't opted to disconnect this function from init_{buf,off,num} yet. Test the fix to the retry + fragment case by having the splitHandshake option to the state machine tests, in DTLS, also clamp the MTU to force handshake fragmentation. Change-Id: I66f634d6c752ea63649db8ed2f898f9cc2b13908 Reviewed-on: https://boringssl-review.googlesource.com/6421 Reviewed-by: Adam Langley <agl@google.com>
2015-11-04Separate CCS and handshake writing in DTLS.David Benjamin
They run through completely different logic as only handshake is fragmented. This'll make it easier to rewrite the handshake logic in a follow-up. Change-Id: I9515feafc06bf069b261073873966e72fcbe13cb Reviewed-on: https://boringssl-review.googlesource.com/6420 Reviewed-by: Adam Langley <agl@google.com>
2015-10-28Fix all sign/unsigned warnings with Clang and GCC.Adam Langley
Change-Id: If2a83698236f7b0dcd46701ccd257a85463d6ce5 Reviewed-on: https://boringssl-review.googlesource.com/4992 Reviewed-by: Adam Langley <agl@google.com>
2015-09-16Align the SSL stack on #include style.David Benjamin
ssl.h should be first. Also two lines after includes and the rest of the file. Change-Id: Icb7586e00a3e64170082c96cf3f8bfbb2b7e1611 Reviewed-on: https://boringssl-review.googlesource.com/5892 Reviewed-by: Adam Langley <agl@google.com>
2015-09-15Tidy up dtls1_hm_fragment_new and fix (unreachable) memory leak.David Benjamin
clang scan-build found a memory leak if the overflow codepath in dtls1_hm_fragment is hit. Along the way, tidy up that function. Change-Id: I3c4b88916ee56ab3ab63f93d4a967ceae381d187 Reviewed-on: https://boringssl-review.googlesource.com/5870 Reviewed-by: Adam Langley <agl@google.com>
2015-07-16Remove the func parameter to OPENSSL_PUT_ERROR.David Benjamin
Much of this was done automatically with find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/' find . -name '*.c' | xargs sed -E -i '' -e 's/(OPENSSL_PUT_ERROR\([a-zA-Z_0-9]+, )[a-zA-Z_0-9]+, ([a-zA-Z_0-9]+\);)/\1\2/' BUG=468039 Change-Id: I4c75fd95dff85ab1d4a546b05e6aed1aeeb499d8 Reviewed-on: https://boringssl-review.googlesource.com/5276 Reviewed-by: Adam Langley <agl@google.com>
2015-07-01Add CBB_zero to set a CBB to the zero state.David Benjamin
One tedious thing about using CBB is that you can't safely CBB_cleanup until CBB_init is successful, which breaks the general 'goto err' style of cleanup. This makes it possible: CBB_zero ~ EVP_MD_CTX_init CBB_init ~ EVP_DigestInit CBB_cleanup ~ EVP_MD_CTX_cleanup Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52 Reviewed-on: https://boringssl-review.googlesource.com/5267 Reviewed-by: Adam Langley <agl@google.com>
2015-06-16DTLS fragments may not be split across two records.David Benjamin
See also upstream's 9dcab127e14467733523ff7626da8906e67eedd6. The root problem is dtls1_read_bytes is wrong, but we can get the right behavior now and add a regression test for it before cleaning it up. Change-Id: I4e5c39ab254a872d9f64242c9b77b020bdded6e6 Reviewed-on: https://boringssl-review.googlesource.com/5123 Reviewed-by: Adam Langley <agl@google.com>
2015-06-02Fold away SSL_PROTOCOL_METHOD hooks shared between TLS and DTLS.David Benjamin
The ctrl hooks are left alone since they should just go away. Simplifying the cipher story will happen in the next CL. BUG=468889 Change-Id: I979971c90f59c55cd5d17554f1253158b114f18b Reviewed-on: https://boringssl-review.googlesource.com/4957 Reviewed-by: Adam Langley <agl@google.com>
2015-06-02Split ssl_read_bytes hook into app_data and close_notify hooks.David Benjamin
This still needs significant work, especially the close_notify half, but clarify the interface and get *_read_bytes out of SSL_PROTOCOL_METHOD. read_bytes is an implementation detail of those two and get_message rather than both an implementation detail of get_message for handshake and a (wholly inappropriate) exposed interface for the other two. BUG=468889 Change-Id: I7dd23869e0b7c3532ceb2e9dd31ca25ea31128e7 Reviewed-on: https://boringssl-review.googlesource.com/4956 Reviewed-by: Adam Langley <agl@google.com>
2015-05-21Pass a dtls1_use_epoch enum down to dtls1_seal_record.David Benjamin
This is considerably less scary than swapping out connection state. It also fixes a minor bug where, if dtls1_do_write had an alert to dispatch and we happened to retry during a rexmit, it would use the wrong epoch. BUG=468889 Change-Id: I754b0d46bfd02f797f4c3f7cfde28d3e5f30c52b Reviewed-on: https://boringssl-review.googlesource.com/4793 Reviewed-by: Adam Langley <agl@google.com>
2015-05-21Factor SSL_AEAD_CTX into a dedicated type.David Benjamin
tls1_enc is now SSL_AEAD_CTX_{open,seal}. This starts tidying up a bit of the record-layer logic. This removes rr->input, as encrypting and decrypting records no longer refers to various globals. It also removes wrec altogether. SSL3_RECORD is now only used to maintain state about the current incoming record. Outgoing records go straight to the write buffer. This also removes the outgoing alignment memcpy and simply calls SSL_AEAD_CTX_seal with the parameters as appropriate. From bssl speed tests, this seems to be faster on non-ARM and a bit of a wash on ARM. Later it may be worth recasting these open/seal functions to write into a CBB (tweaked so it can be malloc-averse), but for now they take an out/out_len/max_out trio like their EVP_AEAD counterparts. BUG=468889 Change-Id: Ie9266a818cc053f695d35ef611fd74c5d4def6c3 Reviewed-on: https://boringssl-review.googlesource.com/4792 Reviewed-by: Adam Langley <agl@google.com>
2015-05-07Promote max_cert_list and max_send_fragment to functions.David Benjamin
Also size them based on the limits in the quantities they control (after checking bounds at the API boundary). BUG=404754 Change-Id: Id56ba45465a473a1a793244904310ef747f29b63 Reviewed-on: https://boringssl-review.googlesource.com/4559 Reviewed-by: Adam Langley <agl@google.com>
2015-05-07Promote all dtls1_ctrl hooks to functions.David Benjamin
BUG=404754 Change-Id: I5f11485fbafa07cddcf2612e2f616f90bf7c722d Reviewed-on: https://boringssl-review.googlesource.com/4554 Reviewed-by: Adam Langley <agl@google.com>
2015-05-05Remove unnecessary NULL checks, part 5.David Benjamin
Finally, the ssl stack. Change-Id: Iea10e302825947da36ad46eaf3e8e2bce060fde2 Reviewed-on: https://boringssl-review.googlesource.com/4518 Reviewed-by: Adam Langley <agl@google.com>
2015-05-05Require that FOO_free functions do nothing on NULL.David Benjamin
This is consistent with C's free function and upstream's convention. Change-Id: I83f6e2f5824e28f69a9916e580dc2d8cb3b94234 Reviewed-on: https://boringssl-review.googlesource.com/4512 Reviewed-by: Adam Langley <agl@google.com>
2015-04-11Rename ssl_locl.h to internal.hDavid Benjamin
Match the other internal headers. Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa Reviewed-on: https://boringssl-review.googlesource.com/4280 Reviewed-by: Adam Langley <agl@google.com>
2015-04-06Remove redundant SSL_READING lines after ssl_read_bytes.David Benjamin
These are redundant with the lower level ones in s3_pkt.c just before BIO_read. Only the operation which actually failed an operation on the BIO should set the wait state. Not all failure paths in ssl3_read_bytes and dtls1_read_bytes set SSL_READING, but those that don't leave the BIO in a retry state, so SSL_READING doesn't matter. Change-Id: I2ae064ecc8b2946cc8ae8f724be09dfe49e077b5 Reviewed-on: https://boringssl-review.googlesource.com/4230 Reviewed-by: Adam Langley <agl@google.com>
2015-03-10Fix Windows build.David Benjamin
signed/unsigned comparison. Just add a cast for now as in s3_both.c. Later we'll properly size_t it alongside other tightening of this interface. Change-Id: Idc8441d65e8ca65e39ab7172a8ec87d9ad710ed6 Reviewed-on: https://boringssl-review.googlesource.com/3860 Reviewed-by: Adam Langley <agl@google.com>