Commit Graph

4902 Commits

Author SHA1 Message Date
Adam Langley
92b8ecdd0d Change from configuring a FAX scanner function to a FAX next-line function.
In order to process some NIST FAX files, we needed to implement a custom
scanner function to skip over lines that are effectively comments, but
not marked as such.

In the near future we'll need to process KAS FAX files, for which we
need not only to skip over unmarked comment lines, but also to skip some
lines of the response which the FAX doesn't include.

For this we need a more powerful callback function, which this change
provides.

Change-Id: Ibb12b97ac65b3e85317d2e97386ef1c2ea263d4b
Reviewed-on: https://boringssl-review.googlesource.com/24664
Reviewed-by: Adam Langley <agl@google.com>
2018-01-16 22:56:50 +00:00
David Benjamin
afd1cd959e Work around an NDK / Android bug.
The NDK r16 sometimes generates binaries with the DF_1_PIE, which the
runtime linker on Android N complains about. The next NDK revision
should work around this but, in the meantime, strip its error out.

https://github.com/android-ndk/ndk/issues/602
https://android-review.googlesource.com/c/platform/bionic/+/259790
https://android-review.googlesource.com/c/toolchain/binutils/+/571550

Change-Id: I99306d42f11179d5d19bd3f107a7386cc5c690db
Reviewed-on: https://boringssl-review.googlesource.com/24884
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-16 16:52:46 +00:00
Gabriel Redner
7c5e1400dd Fix reference to nonexistent function.
Change-Id: Ib02f1945117dfd7f7d46dbf0672091830c6f3481
Reviewed-on: https://boringssl-review.googlesource.com/24904
Reviewed-by: Adam Langley <agl@google.com>
2018-01-16 16:23:36 +00:00
David Benjamin
94cd196a80 Add files in third_party/fiat for Chromium to pick up.
Chromium's licenses.py is a little finicky.

Change-Id: I015a3565eb8f3cfecb357d142facc796a9c80888
Reviewed-on: https://boringssl-review.googlesource.com/24784
Reviewed-by: Adam Langley <agl@google.com>
2018-01-10 22:02:03 +00:00
David Benjamin
b6317b98ee Update googletest.
The latest MSVC 2017 complains about std::tr1::tuple, which was fixed in
upstream GTest.

Upstream have also merged all our patches, we now no longer are carrying
a diff. (Thanks, Gennadiy!)

Change-Id: I6932687b8e8c1eff8c2edf42da0a12080e7b61dd
Reviewed-on: https://boringssl-review.googlesource.com/24685
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-10 12:11:44 +00:00
Alessandro Ghedini
11a5726ee3 tool: update selection of draft22 TLS 1.3 variant
Change-Id: I7085a07dd2f3d802ada049a2f771ff0c74f4f902
Reviewed-on: https://boringssl-review.googlesource.com/24764
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-10 12:08:54 +00:00
Adam Langley
512a289a8a Add support for dummy PQ padding.
This extension will be used to measure the latency impact of potentially
sending a post-quantum key share by default. At this time it's purely
measuring the impact of the client sending the key share, not the server
replying with a ciphertext.

We could use the existing padding extension for this but that extension
doesn't allow the server to echo it, so we would need a different
extension in the future anyway. Thus we just create one now.

We can assume that modern clients will be using TLS 1.3 by the time that
PQ key-exchange is established and thus the key share will be sent in
all ClientHello messages. However, since TLS 1.3 isn't quite here yet,
this extension is also sent for TLS 1.0–1.2 ClientHellos. The latency
impact should be the same either way.

Change-Id: Ie4a17551f6589b28505797e8c54cddbe3338dfe5
Reviewed-on: https://boringssl-review.googlesource.com/24585
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-10 00:27:31 +00:00
David Benjamin
3c92e80d7a Revert "Update tools."
This reverts commit 9d1f96606c.

Reason for revert: aarch64 bots are breaking for some reason.

Original change's description:
> Update tools.
> 
> In particular, get the new NDK. Unfortunately, the new clang picks up
> an unfortunate change for clang-cl that we now must work around.
> 
> http://llvm.org/viewvc/llvm-project?view=revision&revision=319116
> 
> Bug: 109
> Change-Id: I091ca7160683e70cd79b5c2b7a4267fea258ec17
> Reviewed-on: https://boringssl-review.googlesource.com/24644
> Reviewed-by: Steven Valdez <svaldez@google.com>
> Commit-Queue: David Benjamin <davidben@google.com>
> CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>

TBR=davidben@google.com,svaldez@google.com

Change-Id: I98960f295987857c4e42c312059b6d5934bb5e43
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 109
Reviewed-on: https://boringssl-review.googlesource.com/24747
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>
2018-01-09 21:45:11 +00:00
David Benjamin
9d1f96606c Update tools.
In particular, get the new NDK. Unfortunately, the new clang picks up
an unfortunate change for clang-cl that we now must work around.

http://llvm.org/viewvc/llvm-project?view=revision&revision=319116

Bug: 109
Change-Id: I091ca7160683e70cd79b5c2b7a4267fea258ec17
Reviewed-on: https://boringssl-review.googlesource.com/24644
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-09 21:28:00 +00:00
David Benjamin
53ff70f68c Tidy up some warnings.
Updating clang seems to have upset the clang-cl build. I think because
they decided -Wall now matches MSVC's semantics, which is a little nuts.
Two of the warnings, however, weren't wrong, so fix those.

http://llvm.org/viewvc/llvm-project?view=revision&revision=319116

Change-Id: I168e52e4e70ca7b1069e0b0db241fb5305c12b1e
Reviewed-on: https://boringssl-review.googlesource.com/24684
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-09 16:01:32 +00:00
David Benjamin
e2b8466fa7 Update CMake on Windows bots to 3.10.1.
The 3.10 update had to be rolled back due to a bug with clang-cl that
has since been fixed.

Change-Id: I31c28aedb533f20ab01f105f6f3f7b3ee9c91784
Reviewed-on: https://boringssl-review.googlesource.com/24324
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-09 15:56:42 +00:00
Steven Valdez
74666da5b3 Update key share extension number for draft23.
Change-Id: I7561fc7e04d726ea9e26f645da10e45b62a20627
Reviewed-on: https://boringssl-review.googlesource.com/24704
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-09 15:22:02 +00:00
David Benjamin
0c9b7b5de2 Align various point_get_affine_coordinates implementations.
The P-224 implementation was missing the optimization to avoid doing
extra work when asking for only one coordinate (ECDH and ECDSA both
involve an x-coordinate query). The P-256 implementation was missing the
optimization to do one less Montgomery reduction.

TODO - Benchmarks

Change-Id: I268d9c24737c6da9efaf1c73395b73dd97355de7
Reviewed-on: https://boringssl-review.googlesource.com/24690
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>
2018-01-08 20:03:42 +00:00
David Benjamin
9112631c1f Remove ftmp* comments from P-256 addition code.
These are remnants of the old code which had a bunch of ftmp variables.

Change-Id: Id14cf414cb67ff08e240970767f7a5a58e883ce4
Reviewed-on: https://boringssl-review.googlesource.com/24689
Reviewed-by: Adam Langley <agl@google.com>
2018-01-08 19:51:03 +00:00
David Benjamin
3ab6ad6abd Simplify EC_KEY_set_public_key_affine_coordinates.
EC_POINT_set_affine_coordinates_GFp already rejects coordinates which
are out of range. There's no need to double-check.

Change-Id: Id1685355c555dda66d2a14125cb0083342f37e53
Reviewed-on: https://boringssl-review.googlesource.com/24688
Reviewed-by: Adam Langley <agl@google.com>
2018-01-08 19:50:42 +00:00
David Benjamin
99084cdd76 Fold away ec_point_set_Jprojective_coordinates_GFp.
p224-64.c can just write straight into the EC_POINT, as the other files
do, which saves the mess around BN_CTX. It's also more correct.
ec_point_set_Jprojective_coordinates_GFp abstracts out field_encode, but
then we would want to abstract out field_decode too when reading.

That then allows us to inline ec_point_set_Jprojective_coordinates_GFp
into ec_GFp_simple_point_set_affine_coordinates and get rid of an
unnecessary tower of helper functions. Also we can use the precomputed
value of one rather than recompute it each time.

Change-Id: I8282dc66a4a437f5a3b6a1a59cc39be4cb71ccf9
Reviewed-on: https://boringssl-review.googlesource.com/24687
Reviewed-by: Adam Langley <agl@google.com>
2018-01-08 19:48:37 +00:00
David Benjamin
1eddb4be29 Make EC_POINT_set_compressed_coordinates_GFp use BIGNUM directly.
All the messing around with field_mul and field_sqr does the same thing
as calling EC_GROUP_get_curve_GFp. This is in preparation for ultimately
moving the field elements to an EC_FELEM type.

Where we draw the BIGNUM / EC_FELEM line determines what EC_FELEM
operations we need. Since we don't care much about the performance of
this function, leave it in BIGNUM so we don't need an EC_FELEM
BN_mod_sqrt just yet. We can push it down later if we feel so inclined.

Change-Id: Iec07240d40828df6b7a29fd1f430e3b390d5f506
Reviewed-on: https://boringssl-review.googlesource.com/24686
Reviewed-by: Adam Langley <agl@google.com>
2018-01-08 19:40:21 +00:00
Matthew Braithwaite
9770532afa Map NOT_YET_VALID errors to |certificate_expired|.
The language of RFC 5246 is "A certificate has expired or is not
currently valid", which sounds to me like |certificate_expired| should
pertain to any case where the current time is outside the
certificate's validity period.

Along the way, group the |unknown_ca| errors together.

Change-Id: I92c1fe3fc898283d0c7207625de36662cd0f784e
Reviewed-on: https://boringssl-review.googlesource.com/24624
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>
2018-01-05 23:40:40 +00:00
David Benjamin
92e332501a Add a function for encoding SET OF.
The Chromium certificate verifier ends up encoding a SET OF when
canonicalizing X.509 names. Requiring the caller canonicalize a SET OF
is complicated enough that we should probably sort it for folks. (We
really need to get this name canonicalization insanity out of X.509...)

This would remove the extra level of indirection in Chromium
net/cert/internal/verify_name_match.cc CBB usage.

Note this is not quite the same order as SET, but SET is kind of
useless. Since it's encoding heterogeneous values, it is reasonable to
require the caller just encode them in the correct order. In fact, a DER
SET is just SEQUENCE with a post-processing step on the definition to
fix the ordering of the fields. (Unless the SET contains an untagged
CHOICE, in which case the ordering is weird, but SETs are not really
used in the real world, much less SETs with untagged CHOICEs.)

Bug: 11
Change-Id: I51e7938a81529243e7514360f867330359ae4f2c
Reviewed-on: https://boringssl-review.googlesource.com/24444
Reviewed-by: Adam Langley <agl@google.com>
2018-01-05 23:39:02 +00:00
David Benjamin
00208b443c Use fiat-crypto's freeze function for fe_tobytes.
It requires a handful of additional intrinsics for now.

Fiat's freeze function only works on the tight bounds, so fe_isnonzero
gains an extra fe_carry. But all other calls of fe_tobytes are of tight
bounds anyway.

Change-Id: I834858cee7863c7344e456d7a7dbf4f414f04ae5
Reviewed-on: https://boringssl-review.googlesource.com/24545
Reviewed-by: Adam Langley <agl@google.com>
2018-01-05 23:38:26 +00:00
Adam Langley
2f9b47fb19 Better pack structs in ssl/internal.h
Change-Id: I632a5c9067860216f9252907b104ba605c33a50d
Reviewed-on: https://boringssl-review.googlesource.com/24584
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-04 21:08:36 +00:00
Marek Gilbert
11850d5f61 Rename all googletest CMake targets
CMake targets are visible globally but gtest_main has boringssl-specific
behavior that isn't appropriate for general use.

This change makes it possible to use boringssl and abseil-cpp in the
same project (since abseil-cpp expects gtest_main to exist and be useful
for its own tests).

Change-Id: Icc81c11b8bb4b1e21cea7c9fa725b6c082bd5369
Reviewed-on: https://boringssl-review.googlesource.com/24604
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>
2018-01-04 16:30:54 +00:00
David Benjamin
915c121bb5 Remove some outdated preconditions and postconditions.
These date to the old code and have been replaced by the fe and fe_loose
bounds in the header file. Also fix up a comment that the comment
converter didn't manage to convert.

Change-Id: I2e3ea867a8cea2b347d09c304a17e532b2e36545
Reviewed-on: https://boringssl-review.googlesource.com/24525
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 23:03:32 +00:00
David Benjamin
3144d92ab8 Add some missing array parameter length annotations.
Not that anything checks them...

Change-Id: Iae1b5dbdb3c20a9ebd841bcd32cc5c725c68eb01
Reviewed-on: https://boringssl-review.googlesource.com/24524
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:34:22 +00:00
David Benjamin
d9f49974e3 Support high tag numbers in CBS/CBB.
This is a reland of https://boringssl-review.googlesource.com/2330. I
believe I've now cleared the fallout.

Android's attestion format uses some ludicrously large tag numbers:
https://developer.android.com/training/articles/security-key-attestation.html#certificate_schema

Add support for these in CBS/CBB. The public API does not change for
callers who were using the CBS_ASN1_* constants, but it is no longer the
case that tag representations match their DER encodings for small tag
numbers. When passing tags into CBS/CBB, use CBS_ASN1_* constants. When
working with DER byte arrays (most commonly test vectors), use the
numbers themselves.

Bug: 214
Update-Note: The in-memory representation of CBS/CBB tags changes.
   Additionally, we now support tag numbers above 30. I believe I've now
   actually cleared the fallout of the former. There is one test in
   Chromium and the same test in the internal repository that needs
   fixing.

Change-Id: I49b9d30df01f023c646d31156360ff69c91626a3
Reviewed-on: https://boringssl-review.googlesource.com/24404
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:28:32 +00:00
David Benjamin
5bcaa113e2 Tighten EC_KEY's association with its group.
This is to simplify
https://boringssl-review.googlesource.com/c/boringssl/+/24445/.

Setting or changing an EC_KEY's group after the public or private keys
have been configured is quite awkward w.r.t. consistency checks. It
becomes additionally messy if we mean to store private keys as
EC_SCALARs (and avoid the BIGNUM timing leak), whose size is
curve-dependent.

Instead, require that callers configure the group before setting either
half of the keypair. Additionally, reject EC_KEY_set_group calls that
change the group. This will simplify clearing one more BIGNUM timing
leak.

Update-Note: This will break code which sets the group and key in a
    weird order. I checked calls of EC_KEY_new and confirmed they all
    set the group first. If I missed any, let me know.

Change-Id: Ie89f90a318b31b6b98f71138e5ff3de5323bc9a6
Reviewed-on: https://boringssl-review.googlesource.com/24425
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:15:11 +00:00
Matthew Braithwaite
e15019572b SSL_alert_from_verify_result: expose.
This function maps |X509_V_ERR_*| to SSL alarm codes.  It's used
internally when certs are verified with X509_verify_cert(), and is
helpful to callers who want to call that function, but who also want
to report its errors in a less implementation-dependent way.

Change-Id: I2900cce2eb631489f0947c317beafafd3ea57a75
Reviewed-on: https://boringssl-review.googlesource.com/24564
Commit-Queue: Matt Braithwaite <mab@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-03 22:02:42 +00:00
Adam Langley
ef16f19ef2 Support delocating vpbroadcastq.
(This can be generated with -mavx2.)

Change-Id: I6d92d9e93eb448357342ef86d050321f0ef40f9e
Reviewed-on: https://boringssl-review.googlesource.com/24504
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-01-02 21:06:52 +00:00
Adam Langley
380bc30f0c Fix |ASN1_INTEGER_set| when setting zero.
|ASN1_INTEGER_set| and |BN_to_ASN1_INTEGER| disagree about how to encode
zero. OpenSSL master has aligned around the behaviour of the latter
(i.e. a single zero byte) so fix |ASN1_INTEGER_set| to do that. (This is
also the form that DER requires.)

At the same time, fix undefined behaviour when negative a |long| whose
value is |LONG_MIN|.

Change-Id: I1198de35e61a286ac6472e99152f3d22fda59044
Reviewed-on: https://boringssl-review.googlesource.com/24485
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-02 16:11:31 +00:00
Adam Langley
f8d05579b4 Add ASN1_INTEGET_set_uint64.
Change-Id: I3298875a376c98cbb60deb8c99b9548c84b014df
Reviewed-on: https://boringssl-review.googlesource.com/24484
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-01-02 16:01:31 +00:00
Andres Erbsen
0a54e99848 Add links to proofs of elliptic curve formulas.
Change-Id: I166f740185f26770b51759714efd5d634fbcc173
Reviewed-on: https://boringssl-review.googlesource.com/24424
Reviewed-by: David Benjamin <davidben@google.com>
2017-12-22 19:52:44 +00:00
David Benjamin
80ede1df8e Fix early_mac_len computation.
We would set it to block_size rather than zero. This doesn't cause
problems (the code behaves correctly with either value), but it is a
tiny missed optimization.

Change-Id: Ic751352750cc7ef74aa25a6cc96da82007199941
Reviewed-on: https://boringssl-review.googlesource.com/24364
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-21 21:41:39 +00:00
Andres Erbsen
36fce983b6 add fiat-crypto code generation readme
Change-Id: Ie4060121f6bc8da07d87db8ec8133ea17e99e1fe
Reviewed-on: https://boringssl-review.googlesource.com/24344
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>
2017-12-21 18:35:39 +00:00
David Benjamin
6df6540766 Add a draft TLS 1.3 anti-downgrade signal.
TLS 1.3 includes a server-random-based anti-downgrade signal, as a
workaround for TLS 1.2's ServerKeyExchange signature failing to cover
the entire handshake. However, because TLS 1.3 draft versions are each
doomed to die, we cannot deploy it until the final RFC. (Suppose a
draft-TLS-1.3 client checked the signal and spoke to a final-TLS-1.3
server. The server would correctly negotiate TLS 1.2 and send the
signal. But the client would then break. An anologous situation exists
with reversed roles.)

However, it appears that Cisco devices have non-compliant TLS 1.2
implementations[1] and copy over another server's server-random when
acting as a TLS terminator (client and server back-to-back).

Hopefully they are the only ones doing this. Implement a
measurement-only version with a different value. This sentinel must not
be enforced, but it will tell us whether enforcing it will cause
problems.

[1] https://www.ietf.org/mail-archive/web/tls/current/msg25168.html

Bug: 226
Change-Id: I976880bdb2ef26f51592b2f6b3b97664342679c8
Reviewed-on: https://boringssl-review.googlesource.com/24284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2017-12-21 01:50:33 +00:00
David Benjamin
02e6256b16 Move early_data_accepted to ssl->s3.
This is connection state, not configuration, so it must live on
ssl->s3, otherwise SSL_clear will be confused.

Change-Id: Id7c87ced5248d3953e37946e2d0673d66bfedb08
Reviewed-on: https://boringssl-review.googlesource.com/24264
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-19 15:44:38 +00:00
David Benjamin
a0c87adbf0 Add RSA_flags and RSA_METHOD_FLAG_NO_CHECK.
RSA_METHOD_FLAG_NO_CHECK is the same as our RSA_FLAG_OPAQUE. cURL uses
this to determine if it should call SSL_CTX_check_private_key.

Change-Id: Ie2953632346a31de346a4452f4eaad8435cf76e8
Reviewed-on: https://boringssl-review.googlesource.com/24245
Reviewed-by: Adam Langley <agl@google.com>
2017-12-18 23:56:15 +00:00
David Benjamin
0551feb3a1 Trim some unused RSA flags.
Update-Note: Some RSA_FLAG_* constants are gone. Code search says they
   were unused, but they can be easily restored if this breaks anything.
Change-Id: I47f642af5af9f8d80972ca8da0a0c2bd271c20eb
Reviewed-on: https://boringssl-review.googlesource.com/24244
Reviewed-by: Adam Langley <agl@google.com>
2017-12-18 23:55:27 +00:00
David Benjamin
d90b8033d7 Clear the error queue in fuzzer-mode Channel ID hooks.
Otherwise it leaves something on the error queue and confuses
SSL_get_error, should the handshake state machine fail immediately
afterwards because of a BIO-level error.

Change-Id: I2c7b5e31368b9c5b2efa324166f52972430d6074
Reviewed-on: https://boringssl-review.googlesource.com/24247
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-18 21:56:32 +00:00
David Benjamin
287ac180ee Refresh fuzzer corpora.
The TLS 1.3 variants got renumbered (and many dropped).

Change-Id: I75f63e7188bb22eb115e7f4393e67dc696c013c5
Reviewed-on: https://boringssl-review.googlesource.com/24246
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
2017-12-18 21:54:26 +00:00
Steven Valdez
64cc121f41 Remove deprecated TLS 1.3 variants.
Upgrade-Note: SSL_CTX_set_tls13_variant(tls13_experiment) on the server
should switch to SSL_CTX_set_tls13_variant(tls13_experiment2).
(Configuring any TLS 1.3 variants on the server enables all variants,
so this is a no-op. We're just retiring some old experiments.)
Change-Id: I60f0ca3f96ff84bdf59e1a282a46e51d99047462
Reviewed-on: https://boringssl-review.googlesource.com/23784
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2017-12-18 21:20:32 +00:00
David Benjamin
ea52ec98a5 Perform the RSA CRT reductions with Montgomery reduction.
The first step of RSA with the CRT optimization is to reduce our input
modulo p and q. We can do this in constant-time[*] with Montgomery
reduction. When p and q are the same size, Montgomery reduction's bounds
hold. We need two rounds of it because the first round gives us an
unwanted R^-1.

This does not appear to have a measurable impact on performance. Also
add a long TODO describing how to make the rest of the function
constant-time[*] which hopefully we'll get to later. RSA blinding should
protect us from it all, but make this constant-time anyway.

Since this and the follow-up work will special-case weird keys, add a
test that we don't break those unintentionally. (Though I am not above
breaking them intentionally someday...)

Thanks to Andres Erbsen for discussions on how to do this bit properly.

[*] Ignoring the pervasive bn_correct_top problem for the moment.

Change-Id: Ide099a9db8249cb6549be99c5f8791a39692ea81
Reviewed-on: https://boringssl-review.googlesource.com/24204
Reviewed-by: Adam Langley <agl@google.com>
2017-12-18 18:59:18 +00:00
David Benjamin
f88242d1c1 SSL_export_keying_material should work in half-RTT.
QUIC will need to derive keys at this point. This also smooths over a
part of the server 0-RTT abstraction. Like with False Start, the SSL
object is largely in a functional state at this point.

Bug: 221
Change-Id: I4207d8cb1273a1156e728a7bff3943cc2c69e288
Reviewed-on: https://boringssl-review.googlesource.com/24224
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-18 16:53:13 +00:00
David Benjamin
ebd87230ac Bring ERR_ERROR_STRING_BUF_LEN down to 120.
Originally, the only OpenSSL API to stringify errors was:

  char *ERR_error_string(unsigned long e, char *buf);

This API leaves callers a choice to either be thread unsafe (buf = NULL)
or pass in a buffer with unknown size. Indeed the original
implementation was just a bunch of unchecked sprintfs with, in the buf =
NULL case, a static 256-byte buffer.

388f2f56f2/crypto/err/err.c (L374)

Then ERR_error_string was documented that the buffer must be size 120.
Nowhere in the code was 120 significant. I expect OpenSSL just made up a
number.

388f2f56f2

Then upstream added the ERR_error_string_n API. Although the
documentation stated 120 bytes, the internal buffer was 256, so the code
actually translates ERR_error_string to ERR_error_string_n(e, buf, 256),
not ERR_error_string_n(e, buf, 120)!

e5c84d5152

So the documentation was wrong all this time! OpenSSL 1.1.0 corrected
the documentation to 256, but, alas, a lot of code used the
documentation and sized the buffer at 120. We should fix all
ERR_error_string callers to ERR_error_string_n but, in the meantime,
using 120 is probably less effort.

Note this also affects ERR_print_errors_cb right now. We don't have
function codes, so 120 bytes leaves 60 bytes for the reason code. Our
longest one, TLS_PEER_DID_NOT_RESPOND_WITH_CERTIFICATE_LIST is 46 bytes,
so it's a little tight, but, if needed, we can recover 20-ish bytes by
shrinking the library names. We can also always make ERR_print_errors_cb
use a larger buffer.

Change-Id: I472a1a802f2e6281cc7515d2a452208d6bac1200
Reviewed-on: https://boringssl-review.googlesource.com/24184
Reviewed-by: Adam Langley <agl@google.com>
2017-12-14 19:47:23 +00:00
David Benjamin
875095aa7c Silence ARMv8 deprecated IT instruction warnings.
ARMv8 kindly deprecated most of its IT instructions in Thumb mode.
These files are taken from upstream and are used on both ARMv7 and ARMv8
processors. Accordingly, silence the warnings by marking the file as
targetting ARMv7. In other files, they were accidentally silenced anyway
by way of the existing .arch lines.

This can be reproduced by building with the new NDK and passing
-DCMAKE_ASM_FLAGS=-march=armv8-a. Some of our downstream code ends up
passing that to the assembly.

Note this change does not attempt to arrange for ARMv8-A/T32 to get
code which honors the constraints. It only silences the warnings and
continues to give it the same ARMv7-A/Thumb-2 code that backwards
compatibility dictates it continue to run.

Bug: chromium:575886, b/63131949
Change-Id: I24ce0b695942eaac799347922b243353b43ad7df
Reviewed-on: https://boringssl-review.googlesource.com/24166
Reviewed-by: Adam Langley <agl@google.com>
2017-12-14 01:56:22 +00:00
David Benjamin
9894ee9de2 Scope CMAKE_ASM_FLAGS workaround to the old NDK toolchain.
The one in the NDK works just fine. In particular, this means one can
pass -DCMAKE_ASM_FLAGS="-march=armv8-a" and test the ARMv8 assembler
warnings.

Additionally, make the workaround put the flags in the other order, so
-march is user-overridable.

Change-Id: I278ddd17ab688f83ee01f2aca4ff32307f5b0a2d
Reviewed-on: https://boringssl-review.googlesource.com/24164
Reviewed-by: Adam Langley <agl@google.com>
2017-12-14 01:55:26 +00:00
David Benjamin
528877962b Document the NDK's built-in toolchain file.
The third-party toolchain file doesn't actually work with newer NDKs,
and the one shipped with the NDK has fewer bugs.

Change-Id: I59e1db393f0d66b186fb71590fab14db7faa0756
Reviewed-on: https://boringssl-review.googlesource.com/24165
Reviewed-by: Adam Langley <agl@google.com>
2017-12-14 01:54:47 +00:00
David Benjamin
4358f104cf Remove clang assembler .arch workaround.
This makes it difficult to build against the NDK's toolchain file. The
problem is __clang__ just means Clang is the frontend and implies
nothing about which assembler. When using as, it is fine. When using
clang-as on Linux, one needs a clang-as from this year.

The only places where we case about clang's integrated assembler are iOS
(where perlasm strips out .arch anyway) and build environments like
Chromium which have a regularly-updated clang. Thus we can remove this
now.

Bug: 39
Update-Note: Holler if this breaks the build. If it doesn't break the
   build, you can probably remove any BORINGSSL_CLANG_SUPPORTS_DOT_ARCH
   or explicit -march armv8-a+crypto lines in your BoringSSL build.
Change-Id: I21ce54b14c659830520c2f1d51c7bd13e0980c68
Reviewed-on: https://boringssl-review.googlesource.com/24124
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-13 22:22:41 +00:00
David Benjamin
a9c5b7b3fb Roll back CMake update on Windows bots.
CMake screwed up. See
f969f1a9ce.

It looks like CMake 3.10.1 is in the process of being released. While we
wait for them to put together that build, I'll just revert this real
quick. It's nice to keep them all at the same version, but we really
just needed a new one for Android.

Change-Id: I01b5a54b65df2194d7b84c825dfdcf0fb87fd06b
Reviewed-on: https://boringssl-review.googlesource.com/24144
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>
2017-12-13 21:56:50 +00:00
David Benjamin
d870cbdd97 Update CMake to 3.10.0 on the bots.
The NDK toolchain file requires 3.6.0 or later. We were still using
3.5.0.

Change-Id: I216d33bed4187c7e62a2672eb4f92ce815b60b1c
Reviewed-on: https://boringssl-review.googlesource.com/24104
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-13 21:35:11 +00:00
David Benjamin
0c9c1aad35 Fix generate_build_files.py.
third_party/fiat/p256.c is weird. We need to switch everything to
sources.cmake.

Change-Id: I52e56e87a1ac5534b88a372ad68a1052fb019b67
Reviewed-on: https://boringssl-review.googlesource.com/24084
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-12-12 20:58:58 +00:00