Commit Graph

1164 Commits

Author SHA1 Message Date
Adam Langley
d3459fb2f9 Don't randomly print stuff to stderr.
Change-Id: I821c546612bdd7fca2c3d6a043a4f888f928ee61
Reviewed-on: https://boringssl-review.googlesource.com/3470
Reviewed-by: Adam Langley <agl@google.com>
2015-02-19 19:58:10 +00:00
David Benjamin
b180ee98a6 Style guide tweaks.
I think this is better wording for function documentation. Also note that C++
code uses Google C++ naming rather than OpenSSL, per
c273d2c537.

Change-Id: I7334296bf1490395b2ba02e8b6ce245635826df2
Reviewed-on: https://boringssl-review.googlesource.com/3500
Reviewed-by: Adam Langley <agl@google.com>
2015-02-19 18:33:20 +00:00
David Benjamin
195dc78c6e Allow False Start only for >= TLS 1.2 && AEAD && forward-secure && ALPN/NPN.
Tighten up the requirements for False Start. At this point, neither
AES-CBC or RC4 are something that we want to use unless we're sure that
the server wants to speak them.

Rebase of original CL at: https://boringssl-review.googlesource.com/#/c/1980/

BUG=427721

Change-Id: I9ef7a596edeb8df1ed070aac67c315b94f3cc77f
Reviewed-on: https://boringssl-review.googlesource.com/3501
Reviewed-by: Adam Langley <agl@google.com>
2015-02-19 18:32:39 +00:00
David Benjamin
5f237bc843 Add support for Chromium's JSON test result format.
Also adds a flag to runner.go to make it more suitable for printing to a pipe.

Change-Id: I26fae21f3e4910028f6b8bfc4821c8c595525504
Reviewed-on: https://boringssl-review.googlesource.com/3490
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 23:37:12 +00:00
David Benjamin
1b5cfb5ba3 Pull in a pre-built tarball of cmake 3.1.3 for the bots.
Built from:
45f4d3fa8a2f61cc092ae461aac4cac1bab4ac6706f98274ea7f314dd315c6d0  cmake-3.1.3.tar.gz

Also drop in an extraction script.

Change-Id: I3487e9d432290a7dbabf854b927412c58c35d12b
Reviewed-on: https://boringssl-review.googlesource.com/3492
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 23:27:14 +00:00
David Benjamin
e079927ceb Drop in Go toolchain bootstrap scripts for the bots.
Severely trimmed version of Chrome infra's scripts.

Change-Id: I378b68be670b74fe0518de5d66e0aa8b2d709f26
Reviewed-on: https://boringssl-review.googlesource.com/3491
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 23:25:12 +00:00
David Benjamin
dd1ca99da4 Remove X509_get_pubkey_parameters.
It's never called in outside code. This too seems to be a remnant of the DSA
PKIX optional parameter stuff. This is confirmed both by a removed comment and
by the brief documentation at http://www.umich.edu/~x509/ssleay/x509_pkey.html

RFC 5480 does not allow ECDSA keys to be missing parameters, so this logic is
incorrect for ECDSA anyway.  It was also failing to check
EVP_PKEY_copy_parameters' return value. And that logic looks pretty suspect if
you have a chain made up multiple certificate types.

Change-Id: Id6c60659a0162356c7f3eae5c797047366baae1c
Reviewed-on: https://boringssl-review.googlesource.com/3485
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 23:15:47 +00:00
David Benjamin
310db06b79 Don't EVP_PKEY_copy_parameters when configuring cert and key.
I believe this is a remnant of DSA. The logic strangely fails to check for
failure and then goes out of its way to ERR_clear_error. I believe this is so
that keys that are missing parameters silently move on. This dates to
upstream's dfeab0689f69c0b4bd3480ffd37a9cacc2f17d9c, which is SSLeay 0.9.1b. At
that time, EVP_PKEY_copy_parameters only did anything for DSA. (Now it only
does anything for ECDSA.)

My read is that this comes from DSA in PKIX's "optional domain parameters"
craziness. RFC 3279 says:

   If the DSA domain parameters are omitted from the SubjectPublicKeyInfo
   AlgorithmIdentifier and the CA signed the subject certificate using a
   signature algorithm other than DSA, then the subject's DSA domain parameters
   are distributed by other means.

This was probably part of some weird thing where, if your certificate is
missing parameters, the server would know what to use based on the private key.

(Also this was making the malloc tests unhappy.)

Change-Id: I8d8122a9f50a19e2bbe067f311a8e2d30774935c
Reviewed-on: https://boringssl-review.googlesource.com/3484
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 21:03:29 +00:00
David Benjamin
fbdfefb76e Handle failures in ssl3_finish_mac.
It may fail because the BIO_write to the memory BIO can allocate.
Unfortunately, this bubbles up pretty far up now that we've moved the handshake
hash to ssl3_set_handshake_header.

Change-Id: I58884347a4456bb974ac4783078131522167e29d
Reviewed-on: https://boringssl-review.googlesource.com/3483
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 21:01:37 +00:00
David Benjamin
9d0847ae6d Add some missing error failure checks.
Found while diagnosing some crashes and hangs in the malloc tests. This (and
the follow-up) get us further but does not quite let the malloc tests pass
quietly, even without valgrind. DTLS silently ignores some malloc failures
(confusion with silently dropping bad packets) which then translate to hangs.

Change-Id: Ief06a671e0973d09d2883432b89a86259e346653
Reviewed-on: https://boringssl-review.googlesource.com/3482
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 20:55:56 +00:00
David Benjamin
ed7c475154 Rename cutthrough to False Start.
False Start is the name it's known by now. Deprecate the old API and expose new
ones with the new name.

Change-Id: I32d307027e178fd7d9c0069686cc046f75fdbf6f
Reviewed-on: https://boringssl-review.googlesource.com/3481
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 20:51:22 +00:00
David Benjamin
a54e2e85ee Remove server-side HelloVerifyRequest support.
I found no users of this. We can restore it if needbe, but I don't expect
anyone to find it useful in its current form. The API is suspect for the same
reasons DTLSv1_listen was. An SSL object is stateful and assumes you already
have the endpoint separated out.

If we ever need it, server-side HelloVerifyRequest and DTLSv1_listen should be
implemented by a separate stateless listener that statelessly handles
cookieless ClientHello + HelloVerifyRequest. Once a ClientHello with a valid
cookie comes in, it sets up a stateful SSL object and passes control along to
that.

Change-Id: I86adc1dfb6a81bebe987784c36ad6634a9a1b120
Reviewed-on: https://boringssl-review.googlesource.com/3480
Reviewed-by: Adam Langley <agl@google.com>
2015-02-17 20:50:08 +00:00
Adam Langley
26c2b929ba Switch nonce type in chacha_vec.c to uint32_t.
This was suggested in https://boringssl-review.googlesource.com/#/c/3460
but I forgot to upload the change before submitting in Gerrit.

Change-Id: I3a333fe2e8880603a9027638dd013f21d8270638
2015-02-13 13:16:59 -08:00
Adam Langley
d306f165a4 Don't require the ChaCha nonce to be aligned on ARM.
Change-Id: I34ee66fcc53d3371591beee3373c46598c31b5c5
Reviewed-on: https://boringssl-review.googlesource.com/3460
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-13 20:35:36 +00:00
Adam Langley
c64ccb51b0 Remove support for zero lengths from do_x509_check.
Change-Id: I9ea838850d4c7ea8280cacb1a275b2d6cee1cbbe
2015-02-13 11:00:48 -08:00
Adam Langley
6899b19464 Update API to use (char *) for email addresses and hostnames.
Reduces number of silly casts in OpenSSL code and likely most
applications.  Consistent with (char *) for "peername" value from
X509_check_host() and X509_VERIFY_PARAM_get0_peername().

(Imported from upstream's e83c913723fac7432a7706812f12394aaa00e8ce.)

Change-Id: Id0fc11773a0cee8933978cd4bdbd8251fd7cfb5f
2015-02-13 11:00:48 -08:00
Adam Langley
6f8c366989 Set optional peername when X509_check_host() succeeds.
Pass address of X509_VERIFY_PARAM_ID peername to X509_check_host().

(Imported from upstream's 55fe56837a65ff505b492aa6aee748bf5fa91fec.)

Change-Id: Ic21bfb361b8eb25677c4c2175882fa95ea44fc31
2015-02-13 11:00:48 -08:00
Adam Langley
d0f5df2d71 New peername element in X509_VERIFY_PARAM_ID.
Declaration, memory management, accessor and documentation.

(Imported from upstream's 1eb57ae2b78c119bfba7ab647951130e968d1664.)

Change-Id: Ifa9672e46445e44a78001b0f9430a93c138d73d7
2015-02-13 11:00:47 -08:00
Adam Langley
82fc3bd333 More complete input validation of X509_check_mumble.
(Imported from upstream's 3d15d58e55b97207188e87708a0e7f49b4bfd7fd.)

Change-Id: Iae9e3f839e03c22dc45ac2151884e7afcf31af7b
2015-02-13 10:59:10 -08:00
Adam Langley
589963f79e Multiple verifier reference identities.
(Imported from upstream's 8abffa4a73fcbf6536e0a42d736ed9211a8204ea,
9624b50d51de25bb2e3a72e81fe45032d80ea5c2 and
41e3ebd5abacfdf98461cdeb6fa97a4175b7aad3.)

Change-Id: Ic9099eb5704b19b4500229e89351371cc6184f9d
2015-02-13 10:59:10 -08:00
Adam Langley
a1048a772f Add sk_deep_copy and its macro.
The next change imported from upstream needs this function.

Change-Id: I547efa1f7f46f0558e88047837a26ede32b19275
2015-02-13 10:59:10 -08:00
Adam Langley
2d96a67218 Rerun make_macros.sh.
clang-format has changed a little. This is a semantic no-op but it makes
the diff in the next change smaller.

Change-Id: Ia492a81340a868b888d619a1c7740d1a86845e92
2015-02-13 10:59:10 -08:00
Adam Langley
c68f3e02b0 X509_check_mumble() failure is <= 0, not just 0.
(This change is for a future change that increases the range of the
return values.)

(Imported from upstream's 3fc0b1edad0c75d7beb51fa77f63ffe817295e2c.)

Change-Id: I221d4ee0e90586f89f731e01ff4d813058173211
2015-02-13 10:58:55 -08:00
Adam Langley
fcd34624a1 Drop hostlen from X509_VERIFY_PARAM_ID.
Just store NUL-terminated strings. This works better when we add
support for multiple hostnames.

(Imported from upstream's d93edc0aab98377f42dd19312248597a018a7889.)

Change-Id: Ib3bf8a8c654b829b4432782ba21ba55c3d4a0582
2015-02-13 10:51:02 -08:00
David Benjamin
c35fb014d9 Fix more memory leaks on malloc failure.
Caught by malloc valgrind tests on Basic-Client-Sync. Also one by inspection
and verified with valgrind. Those should pass now with the exception of
CRYPTO_free_ex_data being internally implemented with malloc.

(Clearly we next should make our malloc tests assert that the containing
function fails to catch when we fail to check for some error and things
silently move one.)

Change-Id: I56c51dc8a32a7d3c7ac907d54015dc241728c761
Reviewed-on: https://boringssl-review.googlesource.com/3440
Reviewed-by: Adam Langley <agl@google.com>
2015-02-13 18:43:01 +00:00
David Benjamin
776597dac7 Update BUILDING documentation regarding Go.
Go is not required for running all the tests and bash isn't.

Change-Id: I04d5981dbd2203e8bae27a1265a5db5e35ae5279
Reviewed-on: https://boringssl-review.googlesource.com/3450
Reviewed-by: Adam Langley <agl@google.com>
2015-02-13 00:21:28 +00:00
David Benjamin
3bb4178206 Fix memory leak in pqueue_test.
pqueue_free requires the queue be empty.

Change-Id: I633e18fe71ddec51d6005210fcb6570ef53b9808
Reviewed-on: https://boringssl-review.googlesource.com/3410
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:18:45 +00:00
David Benjamin
491b9219a9 Replace all_tests.sh with a test runner in Go.
This supports -valgrind as with runner.go. It also works on Windows and
provides a place for implementing Chrome infra's JSON test output format in the
future, as well as whatever magic may be needed for Android.

Change-Id: I26eb68053f95e825561a142dbcdc4fbd84e3687d
Reviewed-on: https://boringssl-review.googlesource.com/3411
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:17:50 +00:00
David Benjamin
6eb000dbee Add in missing curly braces part 3.
Everything else.

Change-Id: Iac02b144465b4e7b6d69ea22ff2aaf52695ae732
2015-02-11 15:14:46 -08:00
David Benjamin
9ab14e00d5 Add in missing curly braces part 2.
ECC code.

Change-Id: I1a960620edbb30e10dcbab0e8053a1deb9db3262
Reviewed-on: https://boringssl-review.googlesource.com/3402
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:14:04 +00:00
David Benjamin
3673be7cb6 Fix standalone build on Win64.
Win64 fires significantly more warnings than Win32. Also some recent
changes made it grumpy.

(We might want to reconsider enabling all of MSVC's warnings. Given the sorts
of warnings some of these are, I'm not sure MSVC's version of -Wall -Werror is
actually tenable. Plus, diverging from the Chromium build, especially before
the bots are ready, is going to break pretty readily.)

Change-Id: If3b8feccf910ceab4a233b0731e7624d7da46f87
Reviewed-on: https://boringssl-review.googlesource.com/3420
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:13:52 +00:00
David Benjamin
689be0f4b7 Reset all the error codes.
This saves about 6-7k of error data.

Change-Id: Ic28593d4a1f5454f00fb2399d281c351ee57fb14
Reviewed-on: https://boringssl-review.googlesource.com/3385
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:12:08 +00:00
David Benjamin
96396b3aaa Fix cross-module errors.
Some files in crypto/x509 were moved from crypto/asn1, so they emit errors from
another module. Fix make_errors.go to account for this: cross module errors
must use the foreign module as the first argument to OPENSSL_PUT_ERROR. Both
the function code and the error code should be declared in the foreign module.

Update make_errors.go to ignore cross-module error lines when deciding which
function tokens to emit.

Change-Id: Ic38377ddd56e22d033ef91318c30510762f6445d
Reviewed-on: https://boringssl-review.googlesource.com/3383
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 23:11:55 +00:00
David Benjamin
c9a202fee3 Add in missing curly braces part 1.
Everything before crypto/ec.

Change-Id: Icbfab8e4ffe5cc56bf465eb57d3fdad3959a085c
Reviewed-on: https://boringssl-review.googlesource.com/3401
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 19:31:01 +00:00
David Benjamin
efec193d27 Fix some function parameters in OPENSSL_PUT_ERROR.
We have got to get rid of that parameter...

Change-Id: I17f2d1282636f7d077f21dabdc135eecf9300998
Reviewed-on: https://boringssl-review.googlesource.com/3384
Reviewed-by: Adam Langley <agl@google.com>
2015-02-11 18:43:56 +00:00
David Benjamin
32f1650658 Split error string data across multiple lines.
Otherwise running git grep for a single function gives a ton of noise.

Change-Id: I18900d6269fd2be39ef9b579419aee1c7eca9143
Reviewed-on: https://boringssl-review.googlesource.com/3382
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 03:57:12 +00:00
David Benjamin
fc233962db Make make_errors.go -reset reproducible.
Change-Id: I71114e26149d66acc9f9c66464b8a2a64a59cadc
Reviewed-on: https://boringssl-review.googlesource.com/3381
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 03:56:04 +00:00
David Benjamin
e8fe46adf0 Make err_data_generate.go reproducible.
Sort all the files before processing them.

Change-Id: Id6b4519fa22f1770bb2ba2a792f5c27de9ea452d
Reviewed-on: https://boringssl-review.googlesource.com/3380
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 03:54:05 +00:00
Adam Langley
cf310a6197 Remove more bits of ERR_STRING_DATA.
Also, Clang doesn't like static asserts with the same message and
ERR_free_strings should still free the error queues, although it's badly
misnamed now.

Change-Id: Ibff8eb50f93c0b56c3eeb17a300e8501a31c3ab8
Reviewed-on: https://boringssl-review.googlesource.com/3370
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 02:10:46 +00:00
Adam Langley
29b186736c Precompute sorted array for error strings.
Previously, error strings were kept in arrays for each subdirectory and
err.c would iterate over them all and insert them at init time to a hash
table.

This means that, even if you have a shared library and lots of processes
using that, each process has ~30KB of private memory from building that
hash table.

This this change, all the error strings are built into a sorted list and
are thus static data. This means that processes can share the error
information and it actually saves binary space because of all the
pointer overhead in the old scheme. Also it saves the time taken
building the hash table at startup.

This removes support for externally-supplied error string data.

Change-Id: Ifca04f335c673a048e1a3e76ff2b69c7264635be
2015-02-09 17:35:31 -08:00
David Benjamin
65226257c1 Add SSL_CIPHER_get_rfc_name.
OpenSSL's internal names for the ciphers are not the standard ones and are not
easy to consistently map to the standard ones. Add an API to get the real names
out. (WebRTC wants an API to get the standard names out.)

Also change some incorrect flags on SHA-256 TLS 1.2 ciphers;
SSL_HANDSHAKE_MAC_DEFAULT and SSL_HANDSHAKE_MAC_SHA256 are the same after TLS
1.2. A TLS 1.2 cipher should be tagged explicitly with SHA-256. (This avoids
tripping a check in SSL_CIPHER_get_rfc_name which asserts that default-hash
ciphers only ever use SHA-1 or MD5 for the bulk cipher MAC.)

Change-Id: Iaec2fd4aa97df29883094d3c2ae60f0ba003bf07
2015-02-09 17:31:28 -08:00
David Benjamin
722696b39e Don't lock anything in SSL_set_generate_session_id.
Nothing else on SSL* is thread-safe. (Also SSL_set_generate_session_id is never
called.) This removes the last use of CRYPTO_LOCK_SSL.

Change-Id: I4cf8c05d7cef4ea27962ce29902649317c22f74d
Reviewed-on: https://boringssl-review.googlesource.com/3361
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:24:12 +00:00
David Benjamin
ed3d302190 Unrefcount SSL*.
Nothing ever increments the count.

Change-Id: I0b9396e0f5755fa7d9cfd522e17910c760c1aebd
Reviewed-on: https://boringssl-review.googlesource.com/3360
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:23:51 +00:00
David Benjamin
9e128b06a1 Fix memory leak on malloc failure.
Found by running malloc tests with -valgrind. Unfortunately, the next one is
deep in crypto/asn1 itself, so I'm going to stop here for now.

Change-Id: I7a33971ee07c6b7b7a98715f2f18e0f29380c0a1
Reviewed-on: https://boringssl-review.googlesource.com/3350
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:23:34 +00:00
David Benjamin
2d445c0921 Don't use a global for early_callback_called.
We have a stateful object hanging off the SSL* now. May as well use it and
avoid having to remember to reset that.

Change-Id: I5fc5269aa9b158517dd551036e658afaa2ef9acd
Reviewed-on: https://boringssl-review.googlesource.com/3349
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:20:19 +00:00
David Benjamin
c273d2c537 Use just one style for the shim.
It's currently a mix of GoogleCPlusPlusStyle and unix_hacker_style. Since it's
now been thoroughly C++-ified, let's go with the former. This also matches the
tool, our other bit of C++ code.

Change-Id: Ie90a166006aae3b8f41628dbb35fcd64e99205df
Reviewed-on: https://boringssl-review.googlesource.com/3348
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:18:24 +00:00
David Benjamin
1b8b691458 Test asynchronous session lookup.
Change-Id: I62c255590ba8e7352e3d6171615cfb369327a646
Reviewed-on: https://boringssl-review.googlesource.com/3347
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:18:22 +00:00
David Benjamin
2fff5bf4a8 Set rwstate inside ssl3_get_client_hello.
This is more consistent with other asynchronous hooks and gets it working in
DTLS.

Change-Id: Ia17d9d23910e8665b2756516ba729dffc79af8c0
Reviewed-on: https://boringssl-review.googlesource.com/3346
Reviewed-by: Adam Langley <agl@google.com>
2015-02-10 01:13:56 +00:00
David Benjamin
23a8ca1f10 Implement SSL_get1_session with SSL_SESSION_up_ref.
It doesn't appear that logic (added in upstream's
b7cfcfb7f8e17c17f457b3384010eb027f3aad72) is protecting against anything. On
the contrary, it prohibits implementing CRYPTO_add with real atomic operations!
There's no guarantee that those operations will interact with the locked
implementation.

https://www.mail-archive.com/openssl-users@openssl.org/msg63176.html

As long as ssl->session points to the same session, we know the session won't
be freed. There is no lock protecting, say, SSL_set_session, but a single SSL*
does not appear to be safe to use across threads. If this were to be supported,
both should be guarded by CRYPTO_LOCK_SSL (which is barely used).
CRYPTO_LOCK_SSL_SESSION isn't sufficient anyway; it could sample while
SSL_set_session is busy swapping out the now freed old session with the new.

Change-Id: I54623d0690c55c2c86720406ceff545e2e5f2f8f
Reviewed-on: https://boringssl-review.googlesource.com/3345
Reviewed-by: Adam Langley <agl@google.com>
2015-02-09 23:35:25 +00:00
David Benjamin
3363984d0d Add SSL_SESSION_up_ref.
The fact that an SSL_SESSION is reference-counted is already part of the API.
If an external application (like, say, the test code) wishes to participate, we
should let it.

Change-Id: If04d26a35141da14fd8d917de6cc1c10537ad11a
Reviewed-on: https://boringssl-review.googlesource.com/3344
Reviewed-by: Adam Langley <agl@google.com>
2015-02-09 23:34:41 +00:00