Caught by clang scan-build. (The allocation was larger than it should have
been.)
Change-Id: Ideb800118f65aaba1ee85b7611c8a705671025a8
Reviewed-on: https://boringssl-review.googlesource.com/1340
Reviewed-by: Adam Langley <agl@google.com>
Where possible, functions should return one for success and zero for
error. The use of additional negative values to indicate an error is,
itself, error prone.
This change fixes many EVP functions to remove the possibility of
negative return values. Existing code that is testing for <= 0 will
continue to function, although there is the possibility that some code
was differentiating between negative values (error) and zero (invalid
signature) for the verify functions and will now show the wrong error
message.
Change-Id: I982512596bb18a82df65861394dbd7487783bd3d
Reviewed-on: https://boringssl-review.googlesource.com/1333
Reviewed-by: Adam Langley <agl@google.com>
Use same logic when determining when to expect a client certificate for
both TLS and DTLS.
PR#3452
(Imported from upstream's 666a597ffb9bcf3ba2d49e711fcca28df91eff9d)
Change-Id: Ia267255a32c0b3b9a7da1c53f13ef6f620ff5ec1
PR#3272
(Imported from upstream's 14183e50e75f54c44df6be69670180860ac19550 and
802fdcda1ebc4241a8e02af0046ba2f5264f71f6)
Change-Id: Ied6183d938e320f953a18f6616890d88b74def3f
(Imported from upstream's 912f08dd5ed4f68fb275f3b2db828349fcffba14,
52f856526c46ee80ef4c8c37844f084423a3eff7 and
377551b9c4e12aa7846f4d80cf3604f2e396c964)
Change-Id: Ic2bf93371f6d246818729810e7a45b3f0021845a
OIDs with one component don't have an encoding.
PR#2556 (Bug#1)
(Imported from upstream's ff4cfc4c588c41d5e8d2d530231bc36cbc525add and
65e4dca40cb15f3acc878e26d734ec93bd367dca)
Change-Id: I55b54f23e891abc2c1e0b2976531fba1f16070bb
This ensures high performance is situations when assembler supports
AVX2, but not AD*X.
(Imported from upstream's 82a9dafe32e1e39b5adff18f9061e43d8df3d3c5)
Change-Id: Ie67f49a1c5467807139b6a8a0d4e62162d8a974f
One of the state transitions wasn't rewritten to CR_CHANGE. Add a test to
exercise this codepath. Also SSL_cutthrough_complete references the state.
Change-Id: Ib2f7ac5ac3f0348864efa93cf13cfd87454572f0
Reviewed-on: https://boringssl-review.googlesource.com/1337
Reviewed-by: Adam Langley <agl@google.com>
A lot of applications use ERR_TXT_STRING flag in order to determine
whether the error string is printable or not.
Change-Id: Ic79cdb55d43fdfac48da5de5e18c0c9c03c8f747
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Reviewed-on: https://boringssl-review.googlesource.com/1192
Reviewed-by: Adam Langley <agl@google.com>
The <opensslfeatures.h> header provides #defines which are normally
declared by OpenSSL when features are disabled.
This way applications are able to detect which features were removed
from BoringSSL and use it as drop-in replacement for OpenSSL.
Inspired by LibreSSL, which provides similar header.
Change-Id: If4352743fd938267e2640fb09ca851464b9240b9
Signed-off-by: Piotr Sikora <piotr@cloudflare.com>
Reviewed-on: https://boringssl-review.googlesource.com/1191
Reviewed-by: Adam Langley <agl@google.com>
The length is the number of elements now, not the size in bytes. Caught by
ASan.
Change-Id: I4c5ccee61711e8d2e272b9bacd292dbff04b5133
Reviewed-on: https://boringssl-review.googlesource.com/1336
Reviewed-by: Adam Langley <agl@google.com>
Although the PKCS#1 padding check is internally constant-time, it is not
constant time at the crypto/ ssl/ API boundary. Expose a constant-time
RSA_message_index_PKCS1_type_2 function and integrate it into the
timing-sensitive portion of the RSA key exchange logic.
Change-Id: I6fa64ddc9d65564d05529d9b2985da7650d058c3
Reviewed-on: https://boringssl-review.googlesource.com/1301
Reviewed-by: Adam Langley <agl@google.com>
Now that the flag is set accurately, use it to enforce that the handshake and
CCS synchronization. If EXPECT_CCS is set, enforce that:
(a) No handshake records may be received before ChangeCipherSpec.
(b) There is no pending handshake data at the point EXPECT_CCS is set.
Change-Id: I04b228fe6a7a771cf6600b7d38aa762b2d553f08
Reviewed-on: https://boringssl-review.googlesource.com/1299
Reviewed-by: Adam Langley <agl@google.com>
Introduce a CR_CHANGE state just before entering CR_FINISHED_A. This replaces
the CCS_OK in the CR_FINISHED_A/CR_FINISHED_B case which otherwise would get
applied after partial reads of Finished. The other CCS_OK settings are
redundant with this one.
The copy in tls_secret_session_cb codepath is made unnecessary with
9eaeef81fa.
The copy in the normal session resumption case is unnecessary with
6444287806. Before that commit, OpenSSL would
potentially read Finished a state early. Now that we are strict (and get the
book-keeping correct) for expecting the NewSessionTicket message it too is
redundant.
Of particular note is the one after ssl3_send_finished. That was added in
response to upstream's PR#3400. I've reproduced the bug and concluded it was
actually a bug around expecting a NewSessionTicket message. That has been fixed
properly in 6444287806 by resetting
tlsext_expect_ticket on renegotiations.
Change-Id: I6a928386994fcd5efff26a5f0efb12b65bf7f299
Reviewed-on: https://boringssl-review.googlesource.com/1298
Reviewed-by: Adam Langley <agl@google.com>
Rename SSL3_ST_SR_POST_CLIENT_CERT to SSL3_ST_SR_CHANGE and have this be the
point at which CCS_OK is set. The copy before ssl3_get_finished is redundant as
we never transition to SR_FINISHED directly.
Change-Id: I3eefeb821e7ae53d52dacc587fdc59de9ea9a667
Reviewed-on: https://boringssl-review.googlesource.com/1297
Reviewed-by: Adam Langley <agl@google.com>
Due to merging our patched 1.0.1 code with the 1.0.2 code, some parts of
upstream's 25f93585a70fb05bb9f911884ab95e560f662a5d didn't make it into
the code.
https://code.google.com/p/chromium/issues/detail?id=397333
Change-Id: Iceb13e63a7ac91474fd39e7faad11fa52c56185d
Reviewed-on: https://boringssl-review.googlesource.com/1310
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This removes the last case where the server generates an RSA key for the
ServerKeyExchange. Remove the code for this. Client support to accept them
still remains.
Leave the APIs for now, but they don't do anything anymore.
Change-Id: I84439e034cc575719f5bc9b3e501165e12b62107
Reviewed-on: https://boringssl-review.googlesource.com/1286
Reviewed-by: Adam Langley <agl@google.com>
Slightly cleaner; it means we can use CBS_stow.
Change-Id: I074aa2d73a79648013dea025ee531beeea2af4a2
Reviewed-on: https://boringssl-review.googlesource.com/1287
Reviewed-by: Adam Langley <agl@google.com>
Now the only case where temporary RSA keys are used on the server end is
non-signing keys.
Change-Id: I55f6c206e798dd28548c386fdffd555ccc395477
Reviewed-on: https://boringssl-review.googlesource.com/1285
Reviewed-by: Adam Langley <agl@google.com>
SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG and
SSL_OP_NETSCAPE_DEMO_CIPHER_CHANGE_BUG. Neither of them have code that's even
enabled.
Change-Id: I866aabe1aa37e8ee145aaeaecaff6704c3ad21bc
Reviewed-on: https://boringssl-review.googlesource.com/1284
Reviewed-by: Adam Langley <agl@google.com>
Also fix a place where fixes for the condition for sending ServerKeyExchange in
s3_srvr.c were never propogated to d1_srvr.c. Tidy up that logic to use
ssl_cipher_requires_server_key_exchange and simplify the PSK check.
Change-Id: Ie36d378f733e59a8df405bc869f2346af59bd574
Reviewed-on: https://boringssl-review.googlesource.com/1283
Reviewed-by: Adam Langley <agl@google.com>
This removes support code for a "stream_mac" mode only used by GOST. Also get
rid of this
/* I should fix this up TLS TLS TLS TLS TLS XXXXXXXX */
comment next to it. It's not actually related to GOST (dates to OpenSSL initial
commit), but isn't especially helpful at this point.
Change-Id: Ib13c6e27e16e0d1fb59ed0142ddf913b9abc20b7
Reviewed-on: https://boringssl-review.googlesource.com/1281
Reviewed-by: Adam Langley <agl@google.com>
Some ssl23 functions that can be folded into ssl3, declarations and macros that
don't exist anymore.
Change-Id: I8057fb0bab8b6fe7e4da7b90a4945f7f22e29cd9
Reviewed-on: https://boringssl-review.googlesource.com/1280
Reviewed-by: Adam Langley <agl@google.com>
Without SSLv2, all cipher suite values are 2 bytes. Represent them as a
uint16_t and make all functions pass those around rather than pointers.
This removes SSL_CIPHER_find as it's unused.
Change-Id: Iea0b75abee4352a8333a4b8e39a161430ae55ea6
Reviewed-on: https://boringssl-review.googlesource.com/1259
Reviewed-by: Adam Langley <agl@google.com>
Accepting them as a server is still necessary, but this code is unreachable.
Without SSLv2 support, none of the cipher suites are SSLv2, so
ssl23_no_ssl2_ciphers always returns true and we send a V3ClientHello.
Change-Id: I09030f2c6e375660453c74e4f094d95e9908c3e1
Reviewed-on: https://boringssl-review.googlesource.com/1258
Reviewed-by: Adam Langley <agl@google.com>
Test both when the peer doesn't support session tickets and when the server
promises a NewSessionTicket message but doesn't deliver.
Change-Id: I48f338094002beac2e6b80e41851c72822b3b9d5
Reviewed-on: https://boringssl-review.googlesource.com/1300
Reviewed-by: Adam Langley <agl@google.com>
Don't retain curve IDs in serialized form; serialization only happens when
writing and reading from the wire. The internal representation is a uint16_t
which matches the range of the value and avoids all the checks for the first
byte being 0.
This also fixes a bug in tls1_check_ec_tmp_key's suite B logic; the || should
have been &&, though now it's gone.
This doesn't relieve some of the other assumptions about curve IDs:
tls1_set_curves still assumes that all curve IDs are under 32, and
tls1_ec_curve_id2nid still assumes 0 is not a valid curve ID. Add a
compile-time assert and a comment to document this. We're up to 28 now, so this
may well need to be revised sooner or later.
Remove SSL_get_shared_curve as it's new and unused API, using it in a loop is
O(N^3), and lets us simplify a function.
Change-Id: I82778cb82648d82f7b5de8c5341e0e1febdf5611
Reviewed-on: https://boringssl-review.googlesource.com/1256
Reviewed-by: Adam Langley <agl@google.com>
