Some things were misindented in the reformatting.
Change-Id: I97642000452ce4d5b4c8a39b794cec13097d8760
Reviewed-on: https://boringssl-review.googlesource.com/3870
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's 687eaf27a7e4bdfc58dd455e2566b915a7a25c20. I don't think any
of the *Update functions can actually fail (we should verify this and, if
accurate, document it), but HMAC_Final can. It internally copies an EVP_MD_CTX.
Change-Id: I318cb9d0771d536249a26b61d34fe0413a4d3a10
Reviewed-on: https://boringssl-review.googlesource.com/3830
Reviewed-by: Adam Langley <agl@google.com>
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>
Imported from upstream's 918bb8652969fd53f0c390c1cd909265ed502c7e. (The
remainder is in code we've since deleted.)
Change-Id: Ie878272114086ba60a0fd5eae059b641b00ec0c4
Reviewed-on: https://boringssl-review.googlesource.com/3831
Reviewed-by: Adam Langley <agl@google.com>
The assert was supposed to be *added* in fcf25833 but instead replaced
the check.
BUG=465557
Change-Id: I0d3db5038515021e5bdd1ccb9ff08d4f78552621
Reviewed-on: https://boringssl-review.googlesource.com/3850
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's never called or implemented.
Change-Id: Id41c2fbd23d27cc440e8a23ac1b2d590e50ff20f
Reviewed-on: https://boringssl-review.googlesource.com/3770
Reviewed-by: Adam Langley <agl@google.com>
None of these are version-specific. SSL_PROTOCOL_METHOD's interface will change
later, but this gets us closer to folding away SSL3_ENC_METHOD.
Change-Id: Ib427cdff32d0701a18fe42a52cdbf798f82ba956
Reviewed-on: https://boringssl-review.googlesource.com/3769
Reviewed-by: Adam Langley <agl@google.com>
Turn them into static functions that take in an hm_fragment. It's not
immediately obvious that the frag_off/frag_len bounds checks and the msg_len
consistency check are critical to avoiding an out-of-bounds write. Better to
have dtls1_hm_fragment_mark also check internally.
Also rework the bitmask logic to be clearer and avoid a table.
Change-Id: Ica54e98f66295efb323e033cb6c67ab21e7d6cbc
Reviewed-on: https://boringssl-review.googlesource.com/3765
Reviewed-by: Adam Langley <agl@google.com>
Replace unsigned long with the appropriate sized integer type.
Change-Id: I7b4641d84568f6c11efa25350a9e488a556fc92e
Reviewed-on: https://boringssl-review.googlesource.com/3766
Reviewed-by: Adam Langley <agl@google.com>
Notably, drop all special cases around receiving a message in order and
receiving a full message. It makes things more complicated and was the source
of bugs (the MixCompleteMessageWithFragments tests added in this CL did not
pass before). Instead, every message goes through an hm_fragment, and
dtls1_get_message always checks buffered_messages to see if the next is
complete.
The downside is that we pay one more copy of the message data in the common
case. This is only during connection setup, so I think it's worth the
simplicity. (If we want to optimize later, we could either tighten
ssl3_get_message's interface to allow the handshake data being in the
hm_fragment's backing store rather than s->init_buf or swap out s->init_buf
with the hm_fragment's backing store when a mesasge completes.
This CL does not address ssl_read_bytes being an inappropriate API for DTLS.
Future work will revise the handshake/transport boundary to align better with
DTLS's needs. Also other problems that I've left as TODOs.
Change-Id: Ib4570d45634b5181ecf192894d735e8699b1c86b
Reviewed-on: https://boringssl-review.googlesource.com/3764
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I582eaa2ff922bbf1baf298a5c6857543524a8d4e
Reviewed-on: https://boringssl-review.googlesource.com/3810
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
It's unclear why .extern was being suppressed, it's also a little
unclear how the Chromium build was working without this. None the less,
it's causing problems with Android and it's more obviously correct to
make these symbols as hidden.
Change-Id: Id13ec238b80b8bd08d8ae923ac659835450e77f8
Reviewed-on: https://boringssl-review.googlesource.com/3800
Reviewed-by: Adam Langley <agl@google.com>
This avoids cluttering up the diff and making merge conflicts a pain. It does,
however, mean we need to generate err_data.c ahead of time in Chromium and
likely other downstream builds. It also adds a build dependency on Go.
Change-Id: I6e0513ed9f50cfb030f7a523ea28519590977104
Reviewed-on: https://boringssl-review.googlesource.com/3790
Reviewed-by: Adam Langley <agl@google.com>
It happens to give the same value anyway (64 + 16), but only on accident.
Change-Id: I1415f4015e3de472dbeb9ada0d92607c9d1bcd40
Reviewed-on: https://boringssl-review.googlesource.com/3780
Reviewed-by: Adam Langley <agl@google.com>
We actually don't really care about this special-case since we only test client
full handshakes where the runner sends the second Finished not the shim
(otherwise the overlap logic and retransmitting on every fragment would
probably break us), but it should probably live next to the fragmentation
logic.
Change-Id: I54097d84ad8294bc6c42a84d6f22f496e63eb2a8
Reviewed-on: https://boringssl-review.googlesource.com/3763
Reviewed-by: Adam Langley <agl@google.com>
If the peer fragments Finished into multiple pieces, there is no need to
retransmit multiple times.
Change-Id: Ibf708ad079e1633afd420ff1c9be88a80020cba9
Reviewed-on: https://boringssl-review.googlesource.com/3762
Reviewed-by: Adam Langley <agl@google.com>
This transcription bug comes from the start of BoringSSL and, as you can
imagine, was a complete delight to track down.
Change-Id: I3051934195098a1d3bf893b154389ec7f14d3609
Reviewed-on: https://boringssl-review.googlesource.com/3740
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Some code, sadly, tests the error and the extra error is breaking it.
Change-Id: I89eabadf5d2c5f7dd761030da33dd4c3f2ac8382
Reviewed-on: https://boringssl-review.googlesource.com/3720
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
We don't support DTLS renego. Removing this separately from the rewrite to call
out intentionally dropping this logic.
Change-Id: Ie4428eea0d2dbbb8b4b8b6474df4821de62558cc
Reviewed-on: https://boringssl-review.googlesource.com/3761
Reviewed-by: Adam Langley <agl@google.com>
Instead, add a separate init_with_direction hook. Normal AEADs ignore the
direction, while legacy AEADs must be initialized with it. This avoids
maintaining extra state to support the delayed initialization.
Change-Id: I25271f0e56ee2783a2fd4d4026434154d58dc0a8
Reviewed-on: https://boringssl-review.googlesource.com/3731
Reviewed-by: Adam Langley <agl@google.com>
There's no good reason to do this, and it doesn't work; HMAC checks the length
of the key and runs it through the hash function if too long. The reuse occurs
after this check.
This allows us to shave 132 bytes off HMAC_CTX as this was the only reason it
ever stored the original key. It also slightly simplifies HMAC_Init_ex's
logic.
Change-Id: Ib56aabc3630b7178f1ee7c38ef6370c9638efbab
Reviewed-on: https://boringssl-review.googlesource.com/3733
Reviewed-by: Adam Langley <agl@google.com>
We've already initialized the context, HMAC_Init has questionable behavior
around NULL keys, and this avoids a size_t truncation.
Change-Id: Iab6bfc24fe22d46ca4c01be6129efe0630d553e6
Reviewed-on: https://boringssl-review.googlesource.com/3732
Reviewed-by: Adam Langley <agl@google.com>
These are upstream's prebuilt binaries of:
e9493171de0edd8879755aa7229a701010a19561 cmake-3.1.3-win32-x86.zip
ab6e7aee6a915c4d820b86f5227094763b649fce strawberry-perl-5.20.2.1-32bit-portable.zip
4c4d1951181a610923523cb10d83d9ae9952fbf3 yasm-1.2.0-win32.exe
This is intentionally using yasm 1.2.0 rather than the latest 1.3.0 to match
Chromium's current bundled version. Chromium has additional patches, but they
all seem to be either in 1.2.0 or not relevant for us.
Also update extract.py a little to account for these.
BUG=430237
Change-Id: Iad6687e493900b25390d99882c7ceea62fff8b9b
Reviewed-on: https://boringssl-review.googlesource.com/3710
Reviewed-by: Adam Langley <agl@google.com>
(There are times when I actually miss C++ templates.)
Change-Id: I3db56e4946ae4fb919105fa33e2cfce3c7542d37
Reviewed-on: https://boringssl-review.googlesource.com/3700
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I7b6acc9004beb7b7090de1837814ccdff2e9930e
Reviewed-on: https://boringssl-review.googlesource.com/3680
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Upstream decided to make the caller free the scratch space rather than the
callee. May as well match. (Existing code is pretty inconsistent. This API
pattern needs to go.)
See upstream's 9e442d485008046933cdc7da65080f436a4af089.
Change-Id: I7c9fcae5778a74d6ae8e9f546e03fb2cf6e48426
Reviewed-on: https://boringssl-review.googlesource.com/3671
Reviewed-by: Adam Langley <agl@google.com>
EC_GROUP_copy is an rather unfriendly function; it doesn't work if the groups
have different[*] underlying EC_METHODs, but this notion is not exposed through
the API. I found no callers of EC_GROUP_copy in external code.
This leaves the precompute_mult functions as the remaining mutable API exposed
through EC_GROUP.
[*] Though, of the two EC_METHODs right now, simple.c is entirely unused.
Change-Id: Iabb52518005250fb970e12b3b0ea78b4f6eff4a0
Reviewed-on: https://boringssl-review.googlesource.com/3631
Reviewed-by: Adam Langley <agl@google.com>
Built from:
45f4d3fa8a2f61cc092ae461aac4cac1bab4ac6706f98274ea7f314dd315c6d0 cmake-3.1.3.tar.gz
We're still waiting on infra before the buildbot master is up, but let's get
this ready for when we do; it should be fairly easy.
BUG=430237
Change-Id: I3a414743d44052e1aa48759fa5f125db4d4913b5
Reviewed-on: https://boringssl-review.googlesource.com/3670
Reviewed-by: Adam Langley <agl@google.com>
The old test just sent an empty ServerKeyExchange which is sufficient as we
reject the message early. But be more thorough and implement the actual
ephemeral key logic in the test server.
Change-Id: I016658762e4502c928c051e14d69eea67b5a495f
Reviewed-on: https://boringssl-review.googlesource.com/3650
Reviewed-by: Adam Langley <agl@google.com>
They do the same thing. This removes all callers of EC_GROUP_copy outside
EC_GROUP_dup.
Change-Id: I65433ee36040de79e56483dfece774e01e2e2743
Reviewed-on: https://boringssl-review.googlesource.com/3630
Reviewed-by: Adam Langley <agl@google.com>
This reverts the non-ARM portions of 97999919bb.
x86_64 perlasm already makes .globl imply .hidden. (Confusingly, ARM does not.)
Since we don't need it, revert those to minimize divergence with upstream.
Change-Id: I2d205cfb1183e65d4f18a62bde187d206b1a96de
Reviewed-on: https://boringssl-review.googlesource.com/3610
Reviewed-by: Adam Langley <agl@google.com>
That might be a reasonable check to make, maybe.
DTLS handshake message reading has a ton of other bugs and needs a complete
rewrite. But let's fix this and get a test in now.
Change-Id: I4981fc302feb9125908bb6161ed1a18288c39e2b
Reviewed-on: https://boringssl-review.googlesource.com/3600
Reviewed-by: Adam Langley <agl@google.com>
Test both asynchronous and synchronous versions. This callback is somewhat
different from others. It's NOT called a second time when the handshake is
resumed. This appears to be intentional and not a mismerge from the internal
patch. The caller is expected to set up any state before resuming the handshake
state machine.
Also test the early callback returning an error.
Change-Id: If5e6eddd7007ea5cdd7533b4238e456106b95cbd
Reviewed-on: https://boringssl-review.googlesource.com/3590
Reviewed-by: Adam Langley <agl@google.com>
(I got this wrong when reading the OpenSSL code.)
Change-Id: Ib289ef41d0ab5a3157ad8b9454d2de96d1f86c22
Reviewed-on: https://boringssl-review.googlesource.com/3620
Reviewed-by: Adam Langley <agl@google.com>
This gives a standard PERL_EXECUTABLE configuration knob which is useful for
specifying a perl to use without having it in PATH.
Change-Id: I4b196b77e0b4666081a3f291fee3654c47925844
Reviewed-on: https://boringssl-review.googlesource.com/3570
Reviewed-by: Adam Langley <agl@google.com>
This involves more synchronization with child exits as the kernel no longer
closes the pre-created pipes for free, but it works on Windows. As long as
TCP_NODELAY is set, the performance seems comparable. Though it does involve
dealing with graceful socket shutdown. I couldn't get that to work on Windows
without draining the socket; not even SO_LINGER worked. Current (untested)
theory is that Windows refuses to gracefully shutdown a socket if the peer
sends data after we've stopped reading.
cmd.ExtraFiles doesn't work on Windows; it doesn't use fds natively, so you
can't pass fds 4 and 5. (stdin/stdout/stderr are special slots in
CreateProcess.) We can instead use the syscall module directly and mark handles
as inheritable (and then pass the numerical values out-of-band), but that
requires synchronizing all of our shim.Start() calls and assuming no other
thread is spawning a process.
PROC_THREAD_ATTRIBUTE_HANDLE_LIST fixes threading problems, but requires
wrapping more syscalls. exec.Cmd also doesn't let us launch the process
ourselves. Plus it still requires every handle in the list be marked
inheritable, so it doesn't help if some other thread is launching a process
with bInheritHandles TRUE but NOT using PROC_THREAD_ATTRIBUTE_HANDLE_LIST.
(Like Go, though we can take syscall.ForkLock there.)
http://blogs.msdn.com/b/oldnewthing/archive/2011/12/16/10248328.aspx
The more natively Windows option seems to be named pipes, but that too requires
wrapping more system calls. (To be fair, that isn't too painful.) They also
involve a listening server, so we'd still have to synchronize with shim.Wait()
a la net.TCPListener.
Then there's DuplicateHandle, but then we need an out-of-band signal.
All in all, one cross-platform implementation with a TCP sockets seems
simplest.
Change-Id: I38233e309a0fa6814baf61e806732138902347c0
Reviewed-on: https://boringssl-review.googlesource.com/3563
Reviewed-by: Adam Langley <agl@google.com>
Though this doesn't mean that masm becomes supported, the script is
still provided on don't-ask-in-case-of-doubt-use-nasm basis.
See RT#3650 for background.
(Imported from upstream's 2f8d82d6418c4de8330e2870c1ca6386dc9e1b34)
The data_word changes were already fixed with our
3e700bb3e8, but best to avoid diverging there.
Change-Id: Iab5455534e8bd632fb2b247ff792d411b105f17a
Reviewed-on: https://boringssl-review.googlesource.com/3581
Reviewed-by: Adam Langley <agl@google.com>
There is exactly one implementation and it doesn't fail. Plus a cleanup
function that can fail is very bad manners; the caller has no choice but to
leak at that point.
Change-Id: I5b524617ef37bc7d92273472fa742416ea7dfd43
Reviewed-on: https://boringssl-review.googlesource.com/3564
Reviewed-by: Adam Langley <agl@google.com>
Align the DTLS and TLS implementations more. s3_pkt.c's version still has
remnants of fragmentable alerts and only one side marks some variables as
const. Also use warning/fatal constants rather than the numbers with comments.
Change-Id: Ie62d3af1747b6fe4336496c047dfccc9d71fde3f
Reviewed-on: https://boringssl-review.googlesource.com/3562
Reviewed-by: Adam Langley <agl@google.com>
