crypto/bn/bn_test.cc:404:44: error: ‘n’ may be used uninitialized in this
function [-Werror=maybe-uninitialized]
Change-Id: Id590dfee4b9ae1a4fbd0965e133310dac0d06ed3
Two of these were even regression tests for a past bug. These are also
moved to the file, now with the amazing innovation that we *actually
check the regression test gave the right answer*.
BUG=31
Change-Id: I8097336ad39a2bb5c0af07dd8e1e34723b68d182
Reviewed-on: https://boringssl-review.googlesource.com/8525
Reviewed-by: Adam Langley <agl@google.com>
This adds tests for:
for i = 0 to 199:
Sum: 2^i
A: 2^i - 1
B: 1
for i = 0 to 199:
Sum: 2^200
A: 2^200 - 2^i
B: 2^i
I don't believe any of the existing tests actually stressed this,
amazingly enough.
Change-Id: I5edab6327bad45fc21c62bd47f4169f8bb745ff7
Reviewed-on: https://boringssl-review.googlesource.com/8523
Reviewed-by: Adam Langley <agl@google.com>
This took some finesse. I merged the lshift1 and rshift1 test vectors as
one counted down and the other up. The rshift1 vectors were all rounded
to even numbers, with the test handling the odd case. Finally, each run
only tested positive or negative (it wasn't re-randomized), so I added
both positive and negative versions of each test vector.
BUG=31
Change-Id: Ic7de45ab797074547c44c2e4ff8089b1feec5d57
Reviewed-on: https://boringssl-review.googlesource.com/8522
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
Test vectors taken from one run of bc_test with the -bc flag, along with
a handful of manual test vectors around numbers close to zero. (The
output was compared against bc to make sure it was correct.)
BUG=31
Change-Id: I9e9263ece64a877c8497716cd4713b4c3e44248c
Reviewed-on: https://boringssl-review.googlesource.com/8521
Reviewed-by: Adam Langley <agl@google.com>
Upstream added new instructions in
f4d456408d9d7bca31f34765d1a05fbd9fa55826 and
4e3d2866b6e8e7a700ea22e05840a093bfd7a4b1.
Change-Id: I835650426a0dffca2d8686d64aef99097a4bd186
Reviewed-on: https://boringssl-review.googlesource.com/8520
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 67b8bf4d849a7c40d0226de4ebe2590c4cc7c1f7.)
Verified a no-op in generate_build_files.py.
Change-Id: I09648893ab5c795f3934da0b2ecbc5fd7eb068d5
Reviewed-on: https://boringssl-review.googlesource.com/8519
Reviewed-by: Adam Langley <agl@google.com>
Depending on architecture, perlasm differed on which one or both of:
perl foo.pl flavor output.S
perl foo.pl flavor > output.S
Upstream has now unified on the first form after making a number of
changes to their files (the second does not even work for their x86
files anymore). Sync those portions of our perlasm scripts with upstream
and update CMakeLists.txt and generate_build_files.py per the new
convention.
This imports various commits like this one:
184bc45f683c76531d7e065b6553ca9086564576 (this was done by taking a
diff, so I don't have the full list)
Confirmed that generate_build_files.py sees no change.
BUG=14
Change-Id: Id2fb5b8bc2a7369d077221b5df9a6947d41f50d2
Reviewed-on: https://boringssl-review.googlesource.com/8518
Reviewed-by: Adam Langley <agl@google.com>
We're not using the masm output (and upstream does not even support it).
Reduce unnecessary diff from upstream.
Change-Id: Ic0b0f804bd7ec1429b3b1f40746297b57dcfcef6
Reviewed-on: https://boringssl-review.googlesource.com/8517
Reviewed-by: Adam Langley <agl@google.com>
OBJ_NAME in OpenSSL has an 'alias' field which some code consumes. We never
report anything OpenSSL considers an alias, so just leave it zero. It also has
a 'data' field which, confusingly, is a pointer to the EVP_CIPHER or EVP_MD
despite being a char pointer.
See calls to and implementation of OBJ_NAME_add in OpenSSL for comparison.
Change-Id: Ifc5c70424569db8783deb2fda7736c1954b5dd3a
Reviewed-on: https://boringssl-review.googlesource.com/8515
Reviewed-by: Adam Langley <agl@google.com>
It was missing. Writing NewSessionTicket will need it.
Change-Id: I39de237894f2e8356bd6861da2b8a4d805dcd2d6
Reviewed-on: https://boringssl-review.googlesource.com/8439
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
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>
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>
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>
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>
The only thing we've written before the signature is the hash. We can just
choose it anew. This is along the way to getting init_buf out of the handshake
output side. (init_buf is kind of a mess since it doesn't integrate nicely with
a top-level CBB. Some of the logic hasn't been converted to CBB because they're
interspersed with a BUF_MEM_grow.)
Change-Id: I693e834b5a03849bebb04f3f6b81f81fb04e2530
Reviewed-on: https://boringssl-review.googlesource.com/8431
Reviewed-by: Adam Langley <agl@google.com>
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>
Saves us some mess if they're never zero. This also fixes a bug in
ssl3_get_max_client_version where it didn't account for all versions being
disabled properly.
Change-Id: I4c95ff57cf8953cb4a528263b252379f252f3e01
Reviewed-on: https://boringssl-review.googlesource.com/8512
Reviewed-by: David Benjamin <davidben@google.com>
Otherwise if the client's ClientHello logic is messed up and ServerHello is
fine, we won't notice.
Change-Id: I7f983cca45f7da1113ad4a72de1f991115e1b29a
Reviewed-on: https://boringssl-review.googlesource.com/8511
Reviewed-by: David Benjamin <davidben@google.com>
This also adds a missing check to the C half to ensure fake record types are
always correct, to keep implementations honest.
Change-Id: I1d65272e647ffa67018c721d52c639f8ba47d647
Reviewed-on: https://boringssl-review.googlesource.com/8510
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
As of 67cb49d045 and the corresponding upstream
change, BN_mod_word may fail, like BN_div_word. Handle this properly and
document in bn.h. Thanks to Brian Smith for pointing this out.
Change-Id: I6d4f32dc37bcabf70847c9a8b417d55d31b3a380
Reviewed-on: https://boringssl-review.googlesource.com/8491
Reviewed-by: Adam Langley <agl@google.com>
This function returns a tri-state -1 on error. We should check this.
Change-Id: I6fe130c11d10690923aac5ac7a6dfe3e3ff3f5e9
Reviewed-on: https://boringssl-review.googlesource.com/8490
Reviewed-by: Adam Langley <agl@google.com>
It was already nearly clean. Just one undeclared variable.
(Imported from upstream's abeae4d3251181f1cedd15e4433e79406b766155.)
Change-Id: I3b8f20034f914fc44faabf165d1553d4084c87cc
Reviewed-on: https://boringssl-review.googlesource.com/8393
Reviewed-by: Adam Langley <agl@google.com>
We were missing this case. It is possible to receive an early unencrypted
ChangeCipherSpec alert in DTLS because they aren't ordered relative to the
handshake. Test this case. (ChangeCipherSpec in DTLS is kind of pointless.)
Change-Id: I84268bc1821734f606fb20bfbeda91abf372f32c
Reviewed-on: https://boringssl-review.googlesource.com/8460
Reviewed-by: Adam Langley <agl@google.com>
With IPv6, splitting a colon-separated host/port becomes more complicated.
Change-Id: I5073a5cbaa0714f2f8b9c837bb0809dd20304a3c
Reviewed-on: https://boringssl-review.googlesource.com/8441
Reviewed-by: Adam Langley <agl@google.com>
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>
That both exist with nearly the same name is unfortunate. This also does away
with cert_req being unnecessarily tri-state.
Change-Id: Id83e13d0249b80700d9258b363d43b15d22898d8
Reviewed-on: https://boringssl-review.googlesource.com/8247
Reviewed-by: Adam Langley <agl@google.com>
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>
This functionally pulls in a number of changes from upstream, including:
4e3d2866b6e8e7a700ea22e05840a093bfd7a4b1
1eb12c437bbeb2c748291bcd23733d4a59d5d1ca
6a4ea0022c475bbc2c7ad98a6f05f6e2e850575b
c25278db8e4c21772a0cd81f7873e767cbc6d219
e0a651945cb5a70a2abd9902c0fd3e9759d35867
d405aa2ff265965c71ce7331cf0e49d634a06924
ce3d25d3e5a7e82fd59fd30dff7acc39baed8b5e
9ba96fbb2523cb12747c559c704c58bd8f9e7982
Notably, c25278db8e4c21772a0cd81f7873e767cbc6d219 makes it enable 'use strict'.
To avoid having to deal with complex conflicts, this was done by taking a diff
of our copy of the file with the point just before
c25278db8e4c21772a0cd81f7873e767cbc6d219, and reapplying the non-reverting
parts of our diff on top of upstream's current version.
Confirmed with generate_build_files.py that this makes no changes *except*
d405aa2ff265965c71ce7331cf0e49d634a06924 causes this sort of change throughout
chacha-x86_64.pl's nasm output:
@@ -1179,7 +1179,7 @@ $L$oop8x:
vpslld ymm14,ymm0,12
vpsrld ymm0,ymm0,20
vpor ymm0,ymm14,ymm0
- vbroadcasti128 ymm14,YMMWORD[r11]
+ vbroadcasti128 ymm14,XMMWORD[r11]
vpaddd ymm13,ymm13,ymm5
vpxor ymm1,ymm13,ymm1
vpslld ymm15,ymm1,12
This appears to be correct. vbroadcasti128 takes a 128-bit-wide second
argument, so it wants XMMWORD, not YMMWORD. I suppose nasm just didn't care.
(Looking at a diff-diff may be a more useful way to review this CL.)
Change-Id: I61be0d225ddf13b5f05d1369ddda84b2f322ef9d
Reviewed-on: https://boringssl-review.googlesource.com/8392
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
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>
This implements the cipher suite constraints in "fake TLS 1.3". It also makes
bssl_shim and runner enable it by default so we can start adding MaxVersion:
VersionTLS12 markers to tests as 1.2 vs. 1.3 differences begin to take effect.
Change-Id: If1caf6e43938c8d15b0a0f39f40963b8199dcef5
Reviewed-on: https://boringssl-review.googlesource.com/8340
Reviewed-by: David Benjamin <davidben@google.com>
They were defined with the wrong MAC.
Change-Id: I531678dccd53850221d271c79338cfe37d4bb298
Reviewed-on: https://boringssl-review.googlesource.com/8422
Reviewed-by: Nick Harper <nharper@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
In order to ensure that we don't randomly interoperate with
implementations that don't mask scalars correctly, always generate
scalars with the wrong fixed bits.
Change-Id: I82536a856f034cfe4464fc545a99c21b3cff1691
Reviewed-on: https://boringssl-review.googlesource.com/8391
Reviewed-by: David Benjamin <davidben@google.com>
It's always one. We don't support other kinds of curves with this framework.
(Curve25519 uses a much simpler API.) This also allows us to remove the
check_pub_key_order logic.
Change-Id: Ic15e1ecd68662b838c76b1e0aa15c3a93200d744
Reviewed-on: https://boringssl-review.googlesource.com/8350
Reviewed-by: Adam Langley <agl@google.com>
This unifies a bunch of tests and also adds a few missing ones.
Change-Id: I91652bd010da6cdb62168ce0a3415737127e1577
Reviewed-on: https://boringssl-review.googlesource.com/8360
Reviewed-by: Nick Harper <nharper@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
DSA is deprecated, but get this aligned with some of the BN_FLG_CONSTTIME work
going on elsewhere.
Change-Id: I676ceab298a69362bef1b61d6f597c5c90da2ff0
Reviewed-on: https://boringssl-review.googlesource.com/8309
Reviewed-by: Adam Langley <agl@google.com>
It's a prime, so computing a constant-time mod inverse is straight-forward.
Change-Id: Ie09b84363c3d5da827989300a844c470437fd8f2
Reviewed-on: https://boringssl-review.googlesource.com/8308
Reviewed-by: Adam Langley <agl@google.com>
The case where ec_group_get_mont_data is NULL is only for arbitrary groups
which we now require to be prime order. BN_mod_exp_mont is fine with a NULL
BN_MONT_CTX. It will just compute it. Saves a bit of special-casing.
Also don't mark p-2 as BN_FLG_CONSTTIME as the exponent is public anyway.
Change-Id: Ie868576d52fc9ae5f5c9f2a4039a729151bf84c7
Reviewed-on: https://boringssl-review.googlesource.com/8307
Reviewed-by: Adam Langley <agl@google.com>
Check against the write encryption state, not the read state.
Change-Id: Ib3d8e02800e37bd089ef02c67a0b7e5dc009b1a5
Reviewed-on: https://boringssl-review.googlesource.com/8330
Reviewed-by: Nick Harper <nharper@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
