Commit Graph

2729 Commits

Author SHA1 Message Date
David Benjamin
45d45c1194 Trim the DTLS write code slightly.
Change-Id: I0fb4152ed656a60fae3aa7922652df766d4978d7
Reviewed-on: https://boringssl-review.googlesource.com/8178
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:33:20 +00:00
David Benjamin
936aada25a Move a bunch of public APIs from s3_lib.c to ssl_lib.c.
The separation is purely historical (what happened to use an SSL_ctrl hook), so
put them all in one place. Make a vague attempt to match the order of the
header file, though we're still very far from matching.

Change-Id: Iba003ff4a06684a6be342e438d34bc92cab1cd14
Reviewed-on: https://boringssl-review.googlesource.com/8189
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:27:44 +00:00
David Benjamin
01784b44b9 Rename -timeout to -idle-timeout.
-timeout collides with go test's flags.

Change-Id: Icfc954915a61f1bb4d0acc8f02ec8a482ea10158
Reviewed-on: https://boringssl-review.googlesource.com/8188
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:27:35 +00:00
David Benjamin
3dcec458f1 Rename SERVER_DONE to SERVER_HELLO_DONE.
Match the actual name of the type.

Change-Id: I0ad27196ee2876ce0690d13068fa95f68b05b0da
Reviewed-on: https://boringssl-review.googlesource.com/8187
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:26:59 +00:00
David Benjamin
cfec7c60b9 Rename s3_{clnt,srvr}.c
Give them much more reasonable names.

Change-Id: Id14d983ab3231da21a4f987e662c2e01af7a2cd6
Reviewed-on: https://boringssl-review.googlesource.com/8185
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:25:31 +00:00
David Benjamin
9f1dc8254e A bit of cleanup post state machine merging.
Reorder states and functions by where they appear in the handshake. Remove
unnecessary hooks on SSL_PROTOCOL_METHOD.

Change-Id: I78dae9cf70792170abed6f38510ce870707e82ff
Reviewed-on: https://boringssl-review.googlesource.com/8184
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:24:32 +00:00
David Benjamin
df50eecfbc Fold DTLS server state machine into TLS state machine.
Change-Id: I56d3d625dbe2e338f305bc1332fb0131a20e1c16
Reviewed-on: https://boringssl-review.googlesource.com/8183
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:21:14 +00:00
David Benjamin
aa7734b81b Fold the DTLS client handshake into the TLS one.
Change-Id: Ib8b1c646cf1652ee1481fe73589830be8263fc20
Reviewed-on: https://boringssl-review.googlesource.com/8182
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:20:02 +00:00
David Benjamin
0d21dcd9bb Remove unnecessary sectioning in ssl.h.
There's only one thing under "SNI Extension".

Change-Id: I8d8c54c286cb5775a20c4e2623896eb9be2f0009
Reviewed-on: https://boringssl-review.googlesource.com/8181
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:18:45 +00:00
David Benjamin
24fe4489d3 Consolidate dtls1_start_timer calls.
Rather than reset the timer on every message, start it up immediately after
flushing one of our flights.

Change-Id: I97f8b4f572ceff62c546c94933b2700975c50a02
Reviewed-on: https://boringssl-review.googlesource.com/8180
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:17:36 +00:00
David Benjamin
2a08c8d85d Remove ssl3_do_write's 0 case.
It's unreachable and wouldn't work anyway. We'd never bubble up to the caller
to retry. As a consequence, the TLS side doesn't actually need to pay attention
to init_off.

(For now anyway. We'll probably need state of this sort once the write half is
all reworked. All the craziness with wpend_buf ought to be limited to the
SSL_write bits.)

Change-Id: I951534f6bbeb547ce0492d5647aaf76be42108a3
Reviewed-on: https://boringssl-review.googlesource.com/8179
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:13:37 +00:00
David Benjamin
af62d61df3 Remove dtls1_read_bytes.
It can be folded into dtls1_read_app_data. This code, since it still takes an
output pointer, does not yet process records atomically. (Though, being DTLS,
it probably should...)

Change-Id: I57d60785c9c1dd13b5b2ed158a08a8f5a518db4f
Reviewed-on: https://boringssl-review.googlesource.com/8177
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 19:10:35 +00:00
David Benjamin
c660417bd7 Don't use dtls1_read_bytes to read messages.
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-08 19:09:46 +00:00
David Benjamin
585320c9e9 Don't call read_bytes in read_change_cipher_spec.
Change-Id: If7d50e43c8ea28c5eed38209f31d481fb57bf225
Reviewed-on: https://boringssl-review.googlesource.com/8175
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:51:54 +00:00
David Benjamin
4aa4081e7f Don't use ssl3_read_bytes in ssl3_read_close_notify.
read_close_notify is a very straight-forward hook and doesn't need much.

Change-Id: I7407d842321ea1bcb47838424a0d8f7550ad71ca
Reviewed-on: https://boringssl-review.googlesource.com/8174
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:39:26 +00:00
David Benjamin
a7810c12e9 Make tls_open_record always in-place.
The business with ssl_record_prefix_len is rather a hassle. Instead, have
tls_open_record always decrypt in-place and give back a CBS to where the body
is.

This way the caller doesn't need to do an extra check all to avoid creating an
invalid pointer and underflow in subtraction.

Change-Id: I4e12b25a760870d8f8a503673ab00a2d774fc9ee
Reviewed-on: https://boringssl-review.googlesource.com/8173
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:39:07 +00:00
David Benjamin
728f354a2b Push alert handling down into the record functions.
Alert handling is more-or-less identical across all contexts. Push it down from
read_bytes into the low-level record functions. This also deduplicates the code
shared between TLS and DTLS.

Now the only type mismatch managed by read_bytes is if we get handshake data in
read_app_data.

Change-Id: Ia8331897b304566e66d901899cfbf31d2870194e
Reviewed-on: https://boringssl-review.googlesource.com/8124
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:35:58 +00:00
David Benjamin
ac2920200b Fix typo.
Change-Id: I70499c686b955152840987ffe65d2d3436bf6f6d
Reviewed-on: https://boringssl-review.googlesource.com/8194
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:16:14 +00:00
David Benjamin
4e9cc71a27 Add helper functions for info_callback and msg_callback.
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-08 18:13:53 +00:00
David Benjamin
15aa895a0b Tidy up the DTLS code's blocking-mode retransmits.
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-06-08 18:13:42 +00:00
David Benjamin
585d7a4987 Test both synchronous and asynchronous DTLS retransmit.
The two modes are quite different. One of them requires the BIO honor an
extra BIO_ctrl. Also add an explanation at the top of
addDTLSRetransmitTests for how these tests work. The description is
scattered across many different places.

BUG=63

Change-Id: Iff4cdd1fbf4f4439ae0c293f565eb6780c7c84f9
Reviewed-on: https://boringssl-review.googlesource.com/8121
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 18:11:41 +00:00
David Benjamin
ed9c8fcb23 Honor exit codes in run_android_tests.go.
adb kindly doesn't forward exit codes until N
(https://code.google.com/p/android/issues/detail?id=3254), so we need to work
around it. Otherwise all our test failures have been silently ignored (oops!).

Change-Id: I03440db7dd77e6b9af5445b309b67dc719cea054
Reviewed-on: https://boringssl-review.googlesource.com/8190
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-08 15:45:12 +00:00
David Benjamin
0a45822afe Fix some missing inits
(Imported from upstream's f792c663048f19347a1bb72125e535e4fb2ecf39.)

Change-Id: If9bbb10de3ea858076bd9587d21ec331e837dd53
Reviewed-on: https://boringssl-review.googlesource.com/8171
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-07 22:05:10 +00:00
David Benjamin
26b7c35d8c Fix DSA, preserve BN_FLG_CONSTTIME
Operations in the DSA signing algorithm should run in constant time in
order to avoid side channel attacks. A flaw in the OpenSSL DSA
implementation means that a non-constant time codepath is followed for
certain operations. This has been demonstrated through a cache-timing
attack to be sufficient for an attacker to recover the private DSA key.

CVE-2016-2178

(Imported from upstream's 621eaf49a289bfac26d4cbcdb7396e796784c534 and
b7d0f2834e139a20560d64c73e2565e93715ce2b.)

We should eventually not depend on BN_FLG_CONSTTIME since it's a mess (seeing
as the original fix was wrong until we reported b7d0f2834e to them), but, for
now, go with the simplest fix.

Change-Id: I9ea15c1d1cc3a7e21ef5b591e1879ec97a179718
Reviewed-on: https://boringssl-review.googlesource.com/8172
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-07 19:29:18 +00:00
David Benjamin
0d275bdb32 Don't call ERR_clear_system_error in so many places.
We've got it in entry points. That should be sufficient. (Do we even need it
there?)

Change-Id: I39b245a08fcde7b57e61b0bfc595c6ff4ce2a07a
Reviewed-on: https://boringssl-review.googlesource.com/8127
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-07 15:53:44 +00:00
David Benjamin
4bea8509da Lift an impossible check to an assert.
This cannot happen.

Change-Id: Ib1b473aa91d6479eeff43f7eaf94906d0b2c2a8f
Reviewed-on: https://boringssl-review.googlesource.com/8123
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 20:47:07 +00:00
David Benjamin
e90d004e00 Remove impossible condition.
ssl->cert is never NULL. It gets created in SSL_new unconditionally.

Change-Id: I5c54c9c73e281e61a554820d61421226d763d33a
Reviewed-on: https://boringssl-review.googlesource.com/8125
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 20:33:32 +00:00
David Benjamin
83042a8292 Add a no-op OpenSSL_add_all_algorithms_conf.
More spring-cleaning of unnecessary incompatibilities. Since
OpenSSL_add_all_algorithms_conf doesn't specify a configuration file, it's
perfectly sound to have such a function.

Dear BoringSSL, please add all algorithms.

  Uh, sure. They were already all there, but I have added them!

PS: Could you also load all your configuration files while you're at it.

  ...I don't have any. Fine. I have loaded all configuration files which I
  recognize. *mutters under breath* why does everyone ask all these strange
  questions...

Change-Id: I57f956933d9e519445bf22f89853bd5f56904172
Reviewed-on: https://boringssl-review.googlesource.com/8160
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 15:58:02 +00:00
Adam Langley
adf27430ef Be consistent about 𝑥_tests.txt
Some files were named 𝑥_test.txt and some 𝑥_tests.txt. This change
unifies around the latter.

Change-Id: Id6f29bad8b998f3c3466655097ef593f7f18f82f
Reviewed-on: https://boringssl-review.googlesource.com/8150
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 15:57:46 +00:00
David Benjamin
bbc7859817 Match OpenSSL's values for BIO_CTRL_*.
The fake numbers collide with other numbers defined below. Also PUSH and POP
are actually used. DUP legitimately isn't though.

Change-Id: Iaa15a065d846b89b9b7958b78068393cfee2bd6f
Reviewed-on: https://boringssl-review.googlesource.com/8143
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 14:51:50 +00:00
David Benjamin
f4978b78a0 Add some getters for the old lock callbacks.
Some OpenSSL consumers use them, so provide no-op versions to make porting code
easier.

Change-Id: I4348568c1cb08d2b2c0a9ec9a17e2c0449260965
Reviewed-on: https://boringssl-review.googlesource.com/8142
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 14:51:36 +00:00
David Benjamin
e7b3ce58ad Add BIO_set_conn_int_port.
Make building against software that expects OpenSSL easier.

Change-Id: I1af090ae8208218d6e226ee0baf51053699d85cc
Reviewed-on: https://boringssl-review.googlesource.com/8141
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-06 14:49:09 +00:00
David Benjamin
dbec90b623 Sort out signedness issues.
Windows is, not unreasonably, complaining that taking abs() of an unsigned is
ridiculous. But these values actually are signed and fit very easily in an int
anyway.

Change-Id: I34fecaaa3616732112e3eea105a7c84bd9cd0bae
Reviewed-on: https://boringssl-review.googlesource.com/8144
Reviewed-by: Adam Langley <agl@google.com>
2016-06-03 22:13:30 +00:00
David Benjamin
d206dfa91f Add missing newline in newhope.h.
doc.go is still a little unhappy.

Change-Id: I5a8f3da91dabb45d29d0e08f13b7dabdcd521c38
Reviewed-on: https://boringssl-review.googlesource.com/8145
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-03 22:01:13 +00:00
Adam Langley
aa80ff50bf crypto/newhope: add OPENSSL_EXPORT to functions used by tests.
Change-Id: Ie6701d6ea809f5c590f0773cb4b733a208553879
2016-06-03 14:45:18 -07:00
Adam Langley
a34bd8e38c crypto/newhope: fix comment typo.
Change-Id: Ic7dc57680e8cc8306fb1541249fb356eece30999
2016-06-03 14:37:03 -07:00
Adam Langley
77fe71101b crypto/newhope: print values as unsigneds.
Otherwise builds fail with:
  crypto/newhope/newhope_statistical_test.cc:136:27: error: format specifies type 'long' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]

Change-Id: I85d5816c1d7ee71eef362bffe983b2781ce310a4
2016-06-03 14:32:59 -07:00
Matt Braithwaite
6b7436b0d2 newhope: restore statistical tests.
One of these tests the distribution of noise polynomials; the other
tests that that agreed-upon keys (prior to whitening) have roughly equal
numbers of 0s and 1s.

Along the way, expose a few more API bits.

Change-Id: I6b04708d41590de45d82ea95bae1033cfccd5d67
Reviewed-on: https://boringssl-review.googlesource.com/8130
Reviewed-by: Adam Langley <agl@google.com>
2016-06-03 21:26:18 +00:00
David Benjamin
0fc7df55c0 Add SSL_CIPHER_is_DHE.
Change-Id: I158d1fa1a6b70a278054862326562988c97911b5
Reviewed-on: https://boringssl-review.googlesource.com/8140
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-03 17:57:05 +00:00
Steven Valdez
66af3b0ebc Adding TLS 1.3 Record Layer.
In TLS 1.3, the actual record type is hidden within the encrypted data
and the record layer defaults to using a TLS 1.0 {3, 1} record version
for compatibility. Additionally the record layer no longer checks the
minor version of the record layer to maintain compatibility with the
TLS 1.3 spec.

Change-Id: If2c08e48baab170c1658e0715c33929d36c9be3a
Reviewed-on: https://boringssl-review.googlesource.com/8091
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-02 22:18:48 +00:00
David Benjamin
1a88df13e5 Update style guide note on files which match OpenSSL.
They match the new style not the old EAY style now. They're also not
likely to be reformatted. It's just the legacy ASN.1 stuff now and we're
intentionally not doing much with those. (The old text was written back
before the SSL stack had been reformatted.)

Change-Id: I4852761b013e8c2688ebc7eaf4970afbdc69e858
Reviewed-on: https://boringssl-review.googlesource.com/8129
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-02 21:37:21 +00:00
Steven Valdez
3084e7b87d Adding ECDHE-PSK GCM Ciphersuites.
Change-Id: Iecf534ca0ebdcf34dbf4f922f5000c096a266862
Reviewed-on: https://boringssl-review.googlesource.com/8101
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-02 21:27:16 +00:00
Piotr Sikora
8ca0b4127d Fix X25519 on OS X when using build systems other than CMake.
Assembly code for X25519 wasn't included on OS X when built with
build systems other than CMake, which lead to a SIGTRAP due to a
missing x25519_x86_64.

Reported by Gurgen Hrachyan.

Change-Id: Ib6026f31cce0405ec3e75d8a52bf0940e57c62c8
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/8111
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-02 19:48:27 +00:00
Matt Braithwaite
27e863e711 newhope: improve test vectors.
This commit adds coverage of the "offer" (first) step, as well as
testing all outputs of the "accept" (second) step, not just the shared
key.

Change-Id: Id11fe24029abc302442484a6c01fa496a1578b3a
Reviewed-on: https://boringssl-review.googlesource.com/8100
Reviewed-by: Adam Langley <agl@google.com>
2016-06-02 19:28:00 +00:00
David Benjamin
686bb19ba1 Add a unit test for one-sided shutdown.
OpenSSL was actually super-buggy here (though known bugs on our end have been
fixed), but pyOpenSSL was confused and incorrectly documented that callers call
SSL_read after SSL_shutdown to do bidi shutdown, so we should probably support
this. Add a test that it works.

Change-Id: I2b6d012161330aeb4cf894bae3a0b6a55d53c70d
Reviewed-on: https://boringssl-review.googlesource.com/8093
Reviewed-by: Adam Langley <agl@google.com>
2016-06-02 19:24:05 +00:00
Steven Valdez
bbd43b5e90 Renaming SSL3_MT_NEWSESSION_TICKET to SSL3_MT_NEW_SESSION_TICKET.
This keeps the naming convention in line with the actual spec.

Change-Id: I34673f78dbc29c1659b4da0e49677ebe9b79636b
Reviewed-on: https://boringssl-review.googlesource.com/8090
Reviewed-by: David Benjamin <davidben@google.com>
2016-06-01 15:58:53 +00:00
Matt Braithwaite
db207264ad newhope: refactor and add test vectors.
The test vectors are taken from the reference implementation, modified
to output the results of its random-number generator, and the results of
key generation prior to SHA3.  This allows the interoperability of the
two implementations to be tested somewhat.

To accomplish the testing, this commit creates a new, lower-level API
that leaves the generation of random numbers and all wire encoding and
decoding up to the caller.

Change-Id: Ifae3517696dde4be4a0b7c1998bdefb789bac599
Reviewed-on: https://boringssl-review.googlesource.com/8070
Reviewed-by: Adam Langley <agl@google.com>
2016-05-31 21:57:45 +00:00
David Benjamin
3995a38f3b Print out whether EMS was supported.
Change-Id: I3c5aa418fe767bce883fcdd0a926f922f9f8bbd3
Reviewed-on: https://boringssl-review.googlesource.com/8082
Reviewed-by: Adam Langley <agl@google.com>
2016-05-31 21:11:55 +00:00
David Benjamin
156edfe536 Switch Windows CRYPTO_MUTEX implementation to SRWLOCK.
Now that we no longer support Windows XP, this is available.
Unfortunately, the public header version of CRYPTO_MUTEX means we
still can't easily merge CRYPTO_MUTEX and CRYPTO_STATIC_MUTEX.

BUG=37

Change-Id: If309de3f06e0854c505083b72fd64d1dbb3f4563
Reviewed-on: https://boringssl-review.googlesource.com/8081
Reviewed-by: Adam Langley <agl@google.com>
2016-05-31 21:11:36 +00:00
David Benjamin
29270dea85 Split unlock functions into read/write variants.
Windows SRWLOCK requires you call different functions here. Split
them up in preparation for switching Windows from CRITICAL_SECTION.

BUG=37

Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35
Reviewed-on: https://boringssl-review.googlesource.com/8080
Reviewed-by: Adam Langley <agl@google.com>
2016-05-31 21:09:29 +00:00