Commit Graph

1103 Commits

Author SHA1 Message Date
Adam Langley
d09175ffe3 Replace base64 decoding.
This code has caused a long history of problems. This change rewrites it
completely with something that is, hopefully, much simplier and robust
and adds more testing.

Change-Id: Ibeef51f9386afd95d5b73316e451eb3a2d7ec4e0
Reviewed-on: https://boringssl-review.googlesource.com/8033
Reviewed-by: Adam Langley <agl@google.com>
2016-05-26 17:59:10 +00:00
Steven Valdez
4f94b1c19f Adding TLS 1.3 constants.
Constants representing TLS 1.3 are added to allow for future work to be
flagged on TLS1_3_VERSION. To prevent BoringSSL from negotiating the
non-existent TLS 1.3 version, it is explicitly disabled using
SSL_OP_NO_TLSv1_3.

Change-Id: Ie5258a916f4c19ef21646c4073d5b4a7974d6f3f
Reviewed-on: https://boringssl-review.googlesource.com/8041
Reviewed-by: Adam Langley <agl@google.com>
2016-05-25 17:41:36 +00:00
Steven Valdez
1eca1d3816 Renaming Channel ID Encrypted Extensions.
This renames the Channel ID EncryptedExtensions message to allow for
compatibility with TLS 1.3 EncryptedExtensions.

Change-Id: I5b67d00d548518045554becb1b7213fba86731f2
Reviewed-on: https://boringssl-review.googlesource.com/8040
Reviewed-by: Adam Langley <agl@google.com>
2016-05-23 20:37:04 +00:00
David Benjamin
2f87112b96 Never expose ssl->bbio in the public API.
OpenSSL's bbio logic is kind of crazy. It would be good to eventually do the
buffering in a better way (notably, bbio is fragile, if not outright broken,
for DTLS). In the meantime, this fixes a number of bugs where the existence of
bbio was leaked in the public API and broke things.

- SSL_get_wbio returned the bbio during the handshake. It must always return
  the BIO the consumer configured. In doing so, internal accesses of
  SSL_get_wbio should be switched to ssl->wbio since those want to see bbio.
  For consistency, do the same with rbio.

- The logic in SSL_set_rfd, etc. (which I doubt is quite right since
  SSL_set_bio's lifetime is unclear) would get confused once wbio got wrapped.
  Those want to compare to SSL_get_wbio.

- If SSL_set_bio was called mid-handshake, bbio would get disconnected and lose
  state. It forgets to reattach the bbio afterwards. Unfortunately, Conscrypt
  does this a lot. It just never ended up calling it at a point where the bbio
  would cause problems.

- Make more explicit the invariant that any bbio's which exist are always
  attached. Simplify a few things as part of that.

Change-Id: Ia02d6bdfb9aeb1e3021a8f82dcbd0629f5c7fb8d
Reviewed-on: https://boringssl-review.googlesource.com/8023
Reviewed-by: Kenny Root <kroot@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2016-05-23 18:15:03 +00:00
Steven Valdez
ce902a9bcd Generalizing curves to groups in preparation for TLS 1.3.
The 'elliptic_curves' extension is being renamed to 'supported_groups'
in the TLS 1.3 draft, and most of the curve-specific methods are
generalized to groups/group IDs.

Change-Id: Icd1a1cf7365c8a4a64ae601993dc4273802610fb
Reviewed-on: https://boringssl-review.googlesource.com/7955
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-20 17:43:11 +00:00
Adam Langley
1aa03f0745 Add |EVP_dss1| as an alias for |EVP_sha1| in decrepit.
Change-Id: I51fa744c367d1f0c7044050f99c4992778e649bd
Reviewed-on: https://boringssl-review.googlesource.com/8030
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-20 15:31:52 +00:00
Adam Langley
7cb920b6ac Include crypto.h from pem.h.
open_iscsi assumes that it can get |OPENSSL_malloc| after including only
pem.h and err.h. Since pem.h already includes quite a lot, this change
adds crypto.h to that set so that open_iscsi is happy.

Change-Id: I6dc06c27088ce3ca46c1ab53bb29650033cba267
Reviewed-on: https://boringssl-review.googlesource.com/8031
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-20 15:31:26 +00:00
Steven Valdez
3686584d16 Separating HKDF into HKDFExtract and HKDFExpand.
The key schedule in TLS 1.3 requires a separate Extract and Expand phase
for the cryptographic computations.

Change-Id: Ifdac1237bda5212de5d4f7e8db54e202151d45ec
Reviewed-on: https://boringssl-review.googlesource.com/7983
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-20 15:17:17 +00:00
Matt Braithwaite
e25775bcac Elliptic curve + post-quantum key exchange
CECPQ1 is a new key exchange that concatenates the results of an X25519
key agreement and a NEWHOPE key agreement.

Change-Id: Ib919bdc2e1f30f28bf80c4c18f6558017ea386bb
Reviewed-on: https://boringssl-review.googlesource.com/7962
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-19 22:19:14 +00:00
Matt Braithwaite
e09e579603 Rename NEWHOPE functions to offer/accept/finish.
This is consistent with the new convention in ssl_ecdh.c.

Along the way, change newhope_test.c to not iterate 1000 times over each
test.

Change-Id: I7a500f45b838eba8f6df96957891aa8e880ba089
Reviewed-on: https://boringssl-review.googlesource.com/8012
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-19 18:17:48 +00:00
David Benjamin
ea77107e9a Remove references to non-existent BIO functions.
We don't have any of these.

Change-Id: I8d12284fbbab0ff35ac32d35a5f2eba326ab79f8
Reviewed-on: https://boringssl-review.googlesource.com/7981
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-18 23:41:08 +00:00
David Benjamin
1e6d6df943 Remove state parameters to ssl3_get_message.
They're completely unused now. The handshake message reassembly logic should
not depend on the state machine. This should partially free it up (ugly as it
is) to be shared with a future TLS 1.3 implementation while, in parallel, it
and the layers below, get reworked. This also cuts down on the number of states
significantly.

Partially because I expect we'd want to get ssl_hash_message_t out of there
too. Having it in common code is fine, but it needs to be in the (supposed to
be) protocol-agnostic handshake state machine, not the protocol-specific
handshake message layer.

Change-Id: I12f9dc57bf433ceead0591106ab165d352ef6ee4
Reviewed-on: https://boringssl-review.googlesource.com/7949
Reviewed-by: Adam Langley <agl@google.com>
2016-05-18 20:51:48 +00:00
David Benjamin
a6338be3fa Simplify ssl3_get_message.
Rather than this confusing coordination with the handshake state machine and
init_num changing meaning partway through, use the length field already in
BUF_MEM. Like the new record layer parsing, is no need to keep track of whether
we are reading the header or the body. Simply keep extending the handshake
message until it's far enough along.

ssl3_get_message still needs tons of work, but this allows us to disentangle it
from the handshake state.

Change-Id: Ic2b3e7cfe6152a7e28a04980317d3c7c396d9b08
Reviewed-on: https://boringssl-review.googlesource.com/7948
Reviewed-by: Adam Langley <agl@google.com>
2016-05-18 20:50:57 +00:00
David Benjamin
1f9329aaf5 Add BUF_MEM_reserve.
BUF_MEM is actually a rather silly API for the SSL stack. There's separate
length and max fields, but init_buf effectively treats length as max and max as
nothing.

We possibly don't want to be using it long-term anyway (if nothing else, the
char*/uint8_t* thing is irritating), but in the meantime, it'll be easier to
separately fix up get_message's book-keeping and state tracking from where the
handshake gets its messages from.

Change-Id: I9e56ea008173991edc8312ec707505ead410a9ee
Reviewed-on: https://boringssl-review.googlesource.com/7947
Reviewed-by: Adam Langley <agl@google.com>
2016-05-18 19:09:06 +00:00
David Benjamin
7f6706ce64 MSVC doesn't like C bitfields.
Change-Id: I88a415e3dd7ac9ea2fa83ca3e4d835efefa7fcc6
Reviewed-on: https://boringssl-review.googlesource.com/7970
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-17 21:50:45 +00:00
David Benjamin
fa214e4a18 Tidy up shutdown state.
The existing logic gets confused in a number of cases around close_notify vs.
fatal alert. SSL_shutdown, while still pushing to the error queue, will fail to
notice alerts. We also get confused if we try to send a fatal alert when we've
already sent something else.

Change-Id: I9b1d217fbf1ee8a9c59efbebba60165b7de9689e
Reviewed-on: https://boringssl-review.googlesource.com/7952
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-17 21:27:12 +00:00
Adam Langley
4fac8d0eae Add CRYPTO_has_asm.
This function will return whether BoringSSL was built with
OPENSSL_NO_ASM. This will allow us to write a test in our internal
codebase which asserts that normal builds should always have assembly
code included.

Change-Id: Ib226bf63199022f0039d590edd50c0cc823927b9
Reviewed-on: https://boringssl-review.googlesource.com/7960
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-17 19:03:31 +00:00
Brian Smith
448fa42779 Deprecate |BN_mod_exp2_mont| and simplify its implementation.
This function is only really useful for DSA signature verification,
which is something that isn't performance-sensitive. Replace its
optimized implementation with a naïve implementation that's much
simpler.

Note that it would be simpler to use |BN_mod_mul| in the new
implementation; |BN_mod_mul_montgomery| is used instead only to be
consistent with other work being done to replace uses of non-Montgomery
modular reduction with Montgomery modular reduction.

Change-Id: If587d463b73dd997acfc5b7ada955398c99cc342
Reviewed-on: https://boringssl-review.googlesource.com/7732
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-13 19:10:18 +00:00
David Benjamin
ada97998f2 Fix stack macro const-ness.
sk_FOO_num may be called on const stacks. Given that was wrong, I suspect no
one ever uses a const STACK_OF(T)...

Other macros were correctly const, but were casting the constness a way (only
to have it come back again).

Also remove the extra newline after a group. It seems depending on which
version of clang-format was being used, we'd either lose or keep the extra
newline. The current file doesn't have them, so settle on that.

Change-Id: I19de6bc85b0a043d39c05ee3490321e9f0adec60
Reviewed-on: https://boringssl-review.googlesource.com/7946
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-13 18:24:57 +00:00
David Benjamin
ef2dc61edd Remove invalid comment for alert_dispatch.
The array is of size two for the level and description, not because we allow
two alerts outstanding; we don't.

Change-Id: I25e42c059ce977a947397a3dc83e9684bc8f0595
Reviewed-on: https://boringssl-review.googlesource.com/7940
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-13 13:56:38 +00:00
Brian Smith
e31d103a0a Deprecate |BN_mod_exp_mont_word| and simplify its implementation.
|BN_mod_exp_mont_word| is only useful when the base is a single word
in length and timing side channel protection of the exponent is not
needed. That's never the case in real life.

Keep the function in the API, but removes its single-word-base
optimized implementation with a call to |BN_mod_exp_mont|.

Change-Id: Ic25f6d4f187210b681c6ee6b87038b64a5744958
Reviewed-on: https://boringssl-review.googlesource.com/7731
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-11 23:22:32 +00:00
Taylor Brandstetter
376a0fed24 Adding a method to change the initial DTLS retransmission timer value.
This allows an application to override the default of 1 second, which
is what's instructed in RFC 6347 but is not an absolute requirement.

Change-Id: I0bbb16e31990fbcab44a29325b6ec7757d5789e5
Reviewed-on: https://boringssl-review.googlesource.com/7930
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-11 22:36:26 +00:00
David Benjamin
3473315415 Reimplement PKCS #3 DH parameter parsing with crypto/bytestring.
Also add a test.

This is the last of the openssl/asn1.h includes from the directories that are
to be kept in the core libcrypto library. (What remains is to finish sorting
out the crypto/obj stuff. We'll also want to retain a decoupled version of the
PKCS#12 stuff.)

Functions that need to be audited for reuse:
i2d_DHparams

BUG=54

Change-Id: Ibef030a98d3a93ae26e8e56869f14858ec75601b
Reviewed-on: https://boringssl-review.googlesource.com/7900
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-09 19:36:41 +00:00
David Benjamin
9d908ba519 Add BORINGSSL_API_VERSION.
The BORINGSSL_YYYYMM #defines have served well to coordinate short-term skews
in BoringSSL's public API, but some consumers (notably wpa_supplicant in
Android) wish to build against multiple versions for an extended period of
time. Consumers should not do this unless there is no alternative, but to
accommodate this, start a BORINGSSL_API_VERSION counter. In future, instead of
BORINGSSL_YYYYMM #defines, we'll simply increment the number.

This is specifically called an "API version" rather than a plain "version" as
this number does not denote any particular point in development or stability.
It purely counts how many times we found it convenient to let the preprocessor
observe a public API change up to now.

Change-Id: I39f9740ae8e793cef4c2b5fb5707b9763b3e55ce
Reviewed-on: https://boringssl-review.googlesource.com/7870
Reviewed-by: Adam Langley <agl@google.com>
2016-05-05 23:06:09 +00:00
Brian Smith
598e55a795 Do RSA blinding unless |e| is NULL and specifically requested not to.
Change-Id: I189db990df2a3cbf68f820a8f9f16142ccd7070f
Reviewed-on: https://boringssl-review.googlesource.com/7595
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-04 23:14:08 +00:00
Brian Smith
86080c336f Verify RSA private key operation regardless of whether CRT is used.
Previously, the verification was only done when using the CRT method,
as the CRT method has been shown to be extremely sensitive to fault
attacks. However, there's no reason to avoid doing the verification
when the non-CRT method is used (performance-sensitive applications
should always be using the CRT-capable keys).

Previously, when we detected a fault (attack) through this verification,
libcrypto would fall back to the non-CRT method and assume that the
non-CRT method would give a correct result, despite having just
detecting corruption that is likely from an attack. Instead, just give
up, like NSS does.

Previously, the code tried to handle the case where the input was not
reduced mod rsa->n. This is (was) not possible, so avoid trying to
handle that. This simplifies the equality check and lets us use
|CRYPTO_memcmp|.

Change-Id: I78d1e55520a1c8c280cae2b7256e12ff6290507d
Reviewed-on: https://boringssl-review.googlesource.com/7582
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-04 22:58:17 +00:00
David Benjamin
52a3bf2835 Add checks to X509_NAME_oneline()
Sanity check field lengths and sums to avoid potential overflows and reject
excessively large X509_NAME structures.

Issue reported by Guido Vranken.

(Imported from upstream's 9b08619cb45e75541809b1154c90e1a00450e537.)

Change-Id: Ib2e1e7cd086f9c3f0d689d61947f8ec3e9220049
Reviewed-on: https://boringssl-review.googlesource.com/7842
Reviewed-by: Adam Langley <agl@google.com>
2016-05-03 16:34:59 +00:00
Adam Langley
b83c680d03 Add |CRYPTO_is_confidential_build|.
In the past we have needed the ability to deploy security fixes to our
frontend systems without leaking them in source code or in published
binaries.

This change adds a function that provides some infrastructure for
supporting this in BoringSSL while meeting our internal build needs. We
do not currently have any specific patch that requires this—this is
purely preparation.

Change-Id: I5c64839e86db4e5ea7419a38106d8f88b8e5987e
Reviewed-on: https://boringssl-review.googlesource.com/7849
Reviewed-by: David Benjamin <davidben@google.com>
2016-05-03 16:24:50 +00:00
David Benjamin
0a63b96535 Make CRYPTO_is_NEON_capable aware of the buggy CPU.
If we're to allow the buggy CPU workaround to fire when __ARM_NEON__ is set,
CRYPTO_is_NEON_capable also needs to be aware of it. Also add an API to export
this value out of BoringSSL, so we can get some metrics on how prevalent this
chip is.

BUG=chromium:606629

Change-Id: I97d65a47a6130689098b32ce45a8c57c468aa405
Reviewed-on: https://boringssl-review.googlesource.com/7796
Reviewed-by: Adam Langley <agl@google.com>
2016-04-28 16:42:21 +00:00
David Benjamin
862c0aa880 Revert md_len removal from SHA256_CTX and SHA512_CTX.
This reverts commits:
- 9158637142
- a90aa64302
- c0d8b83b44

It turns out code outside of BoringSSL also mismatches Init and Update/Final
functions. Since this is largely cosmetic, it's probably not worth the cost to
do this.

Change-Id: I14e7b299172939f69ced2114be45ccba1dbbb704
Reviewed-on: https://boringssl-review.googlesource.com/7793
Reviewed-by: Adam Langley <agl@google.com>
2016-04-27 19:01:23 +00:00
David Benjamin
9158637142 Make SHA256_Final actually only return one.
As with SHA512_Final, use the different APIs rather than store md_len.

Change-Id: Ie1150de6fefa96f283d47aa03de0f18de38c93eb
Reviewed-on: https://boringssl-review.googlesource.com/7722
Reviewed-by: Adam Langley <agl@google.com>
2016-04-27 18:46:17 +00:00
David Benjamin
c0d8b83b44 Make SHA512_Final actually only return one.
Rather than store md_len, factor out the common parts of SHA384_Final and
SHA512_Final and then extract the right state. Also add a missing
SHA384_Transform and be consistent about "1" vs "one" in comments.

This also removes the NULL output special-case which no other hash function
had.

Change-Id: If60008bae7d7d5b123046a46d8fd64139156a7c5
Reviewed-on: https://boringssl-review.googlesource.com/7720
Reviewed-by: Adam Langley <agl@google.com>
2016-04-27 18:42:37 +00:00
David Benjamin
3baee2a495 Banish SSL_add_dir_cert_subjects_to_stack and OPENSSL_DIR_CTX to decrepit.
There was only one function that required BoringSSL to know how to read
directories. Unfortunately, it does have some callers and it's not immediately
obvious whether the code is unreachable. Rather than worry about that, just
toss it all into decrepit.

In doing so, do away with the Windows and PNaCl codepaths. Only implement
OPENSSL_DIR_CTX on Linux.

Change-Id: Ie64d20254f2f632fadc3f248bbf5a8293ab2b451
Reviewed-on: https://boringssl-review.googlesource.com/7661
Reviewed-by: Adam Langley <agl@google.com>
2016-04-27 18:40:25 +00:00
Matt Braithwaite
045a0ffe35 Import `newhope' (post-quantum key exchange).
This derives from the reference implementation:

Source: https://github.com/tpoeppelmann/newhope/tree/master/ref at bc06c1ac
Paper: https://eprint.iacr.org/2015/1092

However, it does not interoperate, due to the replacement of SHAKE-128
with AES-CTR (for polynomial generation) and the replacement of SHA-3
with SHA-256 (for key whitening).

Change-Id: I6a55507aea85331245e2fbd41bae5cc049fdca3c
Reviewed-on: https://boringssl-review.googlesource.com/7690
Reviewed-by: Adam Langley <agl@google.com>
2016-04-26 22:53:59 +00:00
David Benjamin
c25d2e6379 Resolve -Wextern-c-compat warnings with OPENSSL_NO_THREADS.
C and C++ disagree on the sizes of empty structs, which can be rather bad for
structs embedded in public headers. Stick a char in them to avoid issues. (It
doesn't really matter for CRYPTO_STATIC_MUTEX, but it's easier to add a char in
there too.)

Thanks to Andrew Chi for reporting this issue.

Change-Id: Ic54fff710b688decaa94848e9c7e1e73f0c58fd3
Reviewed-on: https://boringssl-review.googlesource.com/7760
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-26 22:19:51 +00:00
David Benjamin
6e96d2be3d Remove stale wpa_supplicant hacks.
aosp-master has been updated past the point that this is necessary. Sadly, all
the other hacks still are. I'll try to get things rolling so we can ditch the
others in time.

Change-Id: If7b3aad271141fb26108a53972d2d3273f956e8d
Reviewed-on: https://boringssl-review.googlesource.com/7751
Reviewed-by: Adam Langley <agl@google.com>
2016-04-25 21:19:12 +00:00
David Benjamin
1be6a7e442 Add another temporary hack for wpa_supplicant.
Due to Android's complex branching scheme, we have to keep building against a
snapshotted version of wpa_supplicant. wpa_supplicant, in preparation for
OpenSSL 1.1.0, added compatibility versions of some accessors that we, in
working towards opaquification, have imported. This causes a conflict (C does
not like having static and non-static functions share a name).

Add a hack in the headers to suppress the conflicting accessors when
BORINGSSL_SUPPRESS_ACCESSORS is defined. Android releases which include an
updated BoringSSL will also locally carry this #define in wpa_supplicant build
files. Once we can be sure releases of BoringSSL will only see a new enough
wpa_supplicant (one which includes a to-be-submitted patch), we can ditch this.

Change-Id: I3e27fde86bac1e59077498ee5cbd916cd880821e
Reviewed-on: https://boringssl-review.googlesource.com/7750
Reviewed-by: Adam Langley <agl@google.com>
2016-04-25 21:18:37 +00:00
David Benjamin
818aff01fb Add SSL_SESSION_get_master_key.
Opaquifying SSL_SESSION is less important than the other structs, but this will
cause less turbulence in wpa_supplicant if we add this API too. Semantics and
name taken from OpenSSL 1.1.0 to match.

BUG=6

Change-Id: Ic39f58d74640fa19a60aafb434dd2c4cb43cdea9
Reviewed-on: https://boringssl-review.googlesource.com/7725
Reviewed-by: Adam Langley <agl@google.com>
2016-04-21 21:14:36 +00:00
David Benjamin
7fadfc6135 Move TLS-specific "AEAD" functions to the bottom of aead.h.
Probably better to keep it out of the way for someone just trying to figure out
how to use the library. Notably, we don't really want people to think they need
to use the directioned init function.

Change-Id: Icacc2061071581abf46e38eb1d7a52e7b1f8361b
Reviewed-on: https://boringssl-review.googlesource.com/7724
Reviewed-by: Adam Langley <agl@google.com>
2016-04-21 20:34:41 +00:00
David Benjamin
325664eec4 Add hkdf.h to doc.config.
It has all of one function in there.

Change-Id: I86f0fbb76d267389c62b63ac01df685acb70535e
Reviewed-on: https://boringssl-review.googlesource.com/7723
Reviewed-by: Adam Langley <agl@google.com>
2016-04-21 20:25:15 +00:00
David Benjamin
582d2847ed Reimplement PKCS#12 key derivation.
This is avoids pulling in BIGNUM for doing a straight-forward addition on a
block-sized value, and avoids a ton of mallocs. It's also -Wconversion-clean,
unlike the old one.

In doing so, this replaces the HMAC_MAX_MD_CBLOCK with EVP_MAX_MD_BLOCK_SIZE.
By having the maximum block size available, most of the temporary values in the
key derivation don't need to be malloc'd.

BUG=22

Change-Id: I940a62bba4ea32bf82b1190098f3bf185d4cc7fe
Reviewed-on: https://boringssl-review.googlesource.com/7688
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-19 18:16:38 +00:00
David Benjamin
0e21f41fe8 Switch all 'num' parameters in crypto/modes to unsigned.
Also switch the EVP_CIPHER copy to cut down on how frequently we need to cast
back and forth.

BUG=22

Change-Id: I9af1e586ca27793a4ee6193bbb348cf2b28a126e
Reviewed-on: https://boringssl-review.googlesource.com/7689
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-19 17:56:25 +00:00
David Benjamin
1a0a8b6760 Make EVP_MD_CTX size functions return size_t.
The EVP_MD versions do, so the types should bubble up.

BUG=22

Change-Id: Ibccbc9ff35bbfd3d164fc28bcdd53ed97c0ab338
Reviewed-on: https://boringssl-review.googlesource.com/7687
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-19 15:12:41 +00:00
Brian Smith
86361a3910 Require the public exponent to be available in RSA blinding.
Require the public exponent to be available unless
|RSA_FLAG_NO_BLINDING| is set on the key. Also, document this.

If the public exponent |e| is not available, then we could compute it
from |p|, |q|, and |d|. However, there's no reasonable situation in
which we'd have |p| or |q| but not |e|; either we have all the CRT
parameters, or we have (e, d, n), or we have only (d, n). The
calculation to compute |e| exposes the private key to risk of side
channel attacks.

Also, it was particularly wasteful to compute |e| for each
|BN_BLINDING| created, instead of just once before the first
|BN_BLINDING| was created.

|BN_BLINDING| now no longer needs to contain a duplicate copy of |e|,
so it is now more space-efficient.

Note that the condition |b->e != NULL| in |bn_blinding_update| was
always true since commit cbf56a5683.

Change-Id: Ic2fd6980e0d359dcd53772a7c31bdd0267e316b4
Reviewed-on: https://boringssl-review.googlesource.com/7594
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-18 23:34:46 +00:00
Brian Smith
d035730ac7 Make return value of |BN_MONT_CTX_set_locked| int.
This reduces the chance of double-frees.

BUG=10

Change-Id: I11a240e2ea5572effeddc05acb94db08c54a2e0b
Reviewed-on: https://boringssl-review.googlesource.com/7583
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-18 23:19:08 +00:00
Brian Smith
c0b196d4eb Drop support for engines-provided signature verification.
We do not need to support engine-provided verification methods.

Change-Id: Iaad8369d403082b728c831167cc386fdcabfb067
Reviewed-on: https://boringssl-review.googlesource.com/7311
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-18 20:40:17 +00:00
David Benjamin
26993ad55e Only use recv/send for socket BIOs on Windows.
In OpenSSL, socket BIOs only used recv/send on Windows and read/write on POSIX.
Align our socket BIOs with that behavior. This should be a no-op, but avoids
frustrating consumers overly sensitive to the syscalls used now that SSL_set_fd
has switched to socket BIOs to align with OpenSSL. b/28138582.

Change-Id: Id4870ef8e668e587d6ef51c5b5f21e03af66a288
Reviewed-on: https://boringssl-review.googlesource.com/7686
Reviewed-by: Adam Langley <agl@google.com>
2016-04-15 20:31:05 +00:00
Steven Valdez
9980ce1dcd Add BORINGSSL_PREFIX for prefixing symbols
This currently doesn't prefix assembly symbols since they don't pull in
openssl/base.h

BUG=5

Change-Id: Ie0fdc79ae73099f84ecbf3f17604a1e615569b3b
Reviewed-on: https://boringssl-review.googlesource.com/7681
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-14 20:49:31 +00:00
David Benjamin
a82e8dd9d2 Tweak X25519 documentation.
Both the header-level and section-level documentation define curve25519 which
is a little odd.

Change-Id: I81aa2b74e8028d3cfd5635e1d3cfda402ba1ae38
Reviewed-on: https://boringssl-review.googlesource.com/7680
Reviewed-by: Adam Langley <agl@google.com>
2016-04-14 14:20:56 +00:00
Adam Langley
aaccbfec04 Export RSA_padding_add_PKCS1_OAEP[_mgf1]
This is needed by trousers. As with the PSS function, the version that
assumes SHA-1 is put into decrepit.

Change-Id: I153e8ea0150e48061b978384b600a7b990d21d03
Reviewed-on: https://boringssl-review.googlesource.com/7670
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-13 21:15:20 +00:00
David Benjamin
e4c678adda Revert "Banish SSL_add_dir_cert_subjects_to_stack and OPENSSL_DIR_CTX to decrepit."
This reverts commit 112c4dd1ff. Accidentally used
the wrong push line.
2016-04-11 18:04:18 -04:00
David Benjamin
112c4dd1ff Banish SSL_add_dir_cert_subjects_to_stack and OPENSSL_DIR_CTX to decrepit.
There was only one function that required BoringSSL to know how to read
directories. Unfortunately, it does have some callers and it's not immediately
obvious whether the code is unreachable. Rather than worry about that, just
toss it all into decrepit.

In doing so, do away with the Windows and PNaCl codepaths. Only implement
OPENSSL_DIR_CTX on Linux.

Change-Id: I3eb55b098e3aa042b422bb7da115c0812685553e
2016-04-11 18:01:54 -04:00
David Benjamin
14420e91e0 Remove EVP_aead_chacha20_poly1305_rfc7539 alias.
This slipped through, but all the callers are now using
EVP_aead_chacha20_poly1305, so we can remove this version.

Change-Id: I76eb3a4481aae4d18487ca96ebe3776e60d6abe8
Reviewed-on: https://boringssl-review.googlesource.com/7650
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-04-11 19:32:55 +00:00
David Benjamin
d44a943111 Fix docs typo.
Change-Id: Idb786ee2ca6354dcf2f665e9229aef4a43e05dd4
Reviewed-on: https://boringssl-review.googlesource.com/7614
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 22:15:51 +00:00
David Benjamin
0d76c402b8 Decouple crypto/ec from the OID table.
Instead, embed the (very short) encoding of the OID into built_in_curve.

BUG=chromium:499653

Change-Id: I0db36f83c71fbd3321831f54fa5022f8304b30cd
Reviewed-on: https://boringssl-review.googlesource.com/7564
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 22:12:09 +00:00
David Benjamin
981936791e Remove some easy obj.h dependencies.
A lot of consumers of obj.h only want the NID values. Others didn't need
it at all. This also removes some OBJ_nid2sn and OBJ_nid2ln calls in EVP
error paths which isn't worth pulling a large table in for.

BUG=chromium:499653

Change-Id: Id6dff578f993012e35b740a13b8e4f9c2edc0744
Reviewed-on: https://boringssl-review.googlesource.com/7563
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 20:50:33 +00:00
David Benjamin
5d38f78e29 Rename obj_mac.h to nid.h and make it a multiply-includable header.
obj_mac.h is missing #include guards, so one cannot use NIDs without
pulling in the OBJ_* functions which depend on the giant OID table. Give
it #include guards, tidy up the style slightly, and also rename it to
nid.h which is a much more reasonable name.

obj_mac.h is kept as a forwarding header as, despite it being a little
screwy, some code #includes it anyway.

BUG=chromium:499653

Change-Id: Iec0b3f186c02e208ff1f7437bf27ee3a5ad004b7
Reviewed-on: https://boringssl-review.googlesource.com/7562
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 20:45:35 +00:00
David Benjamin
9e5eb63d01 Document that CRYPTO_library_init may be called concurrently.
This was fixed in 93a5b44296, but it wasn't
documented. Now that there are no pre-init functions to call like
CRYPTO_set_neon_capable, one instance of BoringSSL may be safely shared between
multiple consumers. As part of that, multiple consumers need to be able to call
CRYPTO_library_init possibly redundantlyand possibly on different threads
without synchronization.

(Though there is still that static initializer nuisance. It would be nice to
replace this with internal CRYPTO_once_t's and then CRYPTO_library_init need
only be called to prime armcap for a sandbox. But one thing at a time.)

Change-Id: I48430182d3649c8cf19082e34da24dee48e6119e
Reviewed-on: https://boringssl-review.googlesource.com/7571
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-31 19:35:53 +00:00
David Benjamin
4b7b048417 Spell __attribute__((format(printf, ...))) with more underscores.
They may be spelled with or without underscores. Alas, a lot of C code (adb,
cURL) seems to find it a popular pasttime to #define printf *before* including
external headers. This is completely nonsense and invalid, but working around
it is easy and is what we (and OpenSSL) were doing before
061332f216.

I'll be sending a patch to cURL tomorrow to make them at least do their macro
trickery after external #includes for sanity. adb's sysdeps.h is a lot longer
and consistently #included first so I'll probably leave that be for lack of
time.

Change-Id: I03a0a253f2c902eb45f45faace1e5c5df4335ebf
Reviewed-on: https://boringssl-review.googlesource.com/7605
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-30 14:46:42 +00:00
David Benjamin
3c4a5cbb71 Revert "Enable upstream's Poly1305 code."
This reverts commit 6f0c4db90e except for the
imported assembly files, which are left as-is but unused. Until upstream fixes
https://rt.openssl.org/Ticket/Display.html?id=4483, we shouldn't ship this
code. Once that bug has been fixed, we'll restore it.

Change-Id: I74aea18ce31a4b79657d04f8589c18d6b17f1578
Reviewed-on: https://boringssl-review.googlesource.com/7602
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 22:47:11 +00:00
Brian Smith
f08c1c6895 Drop support for custom |mod_exp| hooks in |RSA_METHOD|.
The documentation in |RSA_METHOD| says that the |ctx| parameter to
|mod_exp| can be NULL, however the default implementation doesn't
handle that case. That wouldn't matter since internally it is always
called with a non-NULL |ctx| and it is static, but an external
application could get a pointer to |mod_exp| by extracting it from
the default |RSA_METHOD|. That's unlikely, but making that impossible
reduces the chances that future refactorings will cause unexpected
trouble.

Change-Id: Ie0e35e9f107551a16b49c1eb91d0d3386604e594
Reviewed-on: https://boringssl-review.googlesource.com/7580
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-29 22:20:48 +00:00
David Benjamin
baca950e8e Remove in_handshake.
The removes the last of OpenSSL's variables that count occurrences of a
function on the stack.

Change-Id: I1722c6d47bedb47b1613c4a5da01375b5c4cc220
Reviewed-on: https://boringssl-review.googlesource.com/7450
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:24:28 +00:00
David Benjamin
282511d7eb Consolidate shutdown state.
fatal_alert isn't read at all right now, and warn_alert is only checked
for close_notify. We only need three states:

 - Not shutdown.
 - Got a fatal alert (don't care which).
 - Got a warning close_notify.

Leave ssl->shutdown alone for now as it's tied up with SSL_set_shutdown
and friends. To distinguish the remaining two, we only need a boolean.

Change-Id: I5877723af82b76965c75cefd67ec1f981242281b
Reviewed-on: https://boringssl-review.googlesource.com/7434
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 20:04:34 +00:00
David Benjamin
054e151b16 Rewrite ARM feature detection.
This removes the thread-unsafe SIGILL-based detection and the
multi-consumer-hostile CRYPTO_set_NEON_capable API. (Changing
OPENSSL_armcap_P after initialization is likely to cause problems.)

The right way to detect ARM features on Linux is getauxval. On aarch64,
we should be able to rely on this, so use it straight. Split this out
into its own file. The #ifdefs in the old cpu-arm.c meant it shared all
but no code with its arm counterpart anyway.

Unfortunately, various versions of Android have different missing APIs, so, on
arm, we need a series of workarounds. Previously, we used a SIGILL fallback
based on OpenSSL's logic, but this is inherently not thread-safe. (SIGILL also
does not tell us if the OS knows how to save and restore NEON state.) Instead,
base the behavior on Android NDK's cpu-features library, what Chromium
currently uses with CRYPTO_set_NEON_capable:

- Android before API level 20 does not provide getauxval. Where missing,
  we can read from /proc/self/auxv.

- On some versions of Android, /proc/self/auxv is also not readable, so
  use /proc/cpuinfo's Features line.

- Linux only advertises optional features in /proc/cpuinfo. ARMv8 makes NEON
  mandatory, so /proc/cpuinfo can't be used without additional effort.

Finally, we must blacklist a particular chip because the NEON unit is broken
(https://crbug.com/341598).

Unfortunately, this means CRYPTO_library_init now depends on /proc being
available, which will require some care with Chromium's sandbox. The
simplest solution is to just call CRYPTO_library_init before entering
the sandbox.

It's worth noting that Chromium's current EnsureOpenSSLInit function already
depends on /proc/cpuinfo to detect the broken CPU, by way of base::CPU.
android_getCpuFeatures also interally depends on it. We were already relying on
both of those being stateful and primed prior to entering the sandbox.

BUG=chromium:589200

Change-Id: Ic5d1c341aab5a614eb129d8aa5ada2809edd6af8
Reviewed-on: https://boringssl-review.googlesource.com/7506
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-26 04:54:44 +00:00
Brian Smith
24493a4ff4 Always cache Montgomery contexts in RSA.
Simplify the code by always caching Montgomery contexts in the RSA
structure, regardless of the |RSA_FLAG_CACHE_PUBLIC| and
|RSA_FLAG_CACHE_PRIVATE| flags. Deprecate those flags.

Now that we do this no more than once per key per RSA exponent, the
private key exponents better because the initialization of the
Montgomery contexts isn't perfectly side-channel protected.

Change-Id: I4fbcfec0f2f628930bfeb811285b0ae3d103ac5e
Reviewed-on: https://boringssl-review.googlesource.com/7521
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-25 20:04:24 +00:00
Matt Mueller
897be6afe3 Add CBS_ASN1_UTF8STRING define.
Change-Id: I34384feb46c15c4f443f506d724ad500a4cf0f36
Reviewed-on: https://boringssl-review.googlesource.com/7525
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-23 19:29:49 +00:00
Piotr Sikora
8b0fe8c0ac Add missing prototypes.
Partially fixes build with -Wmissing-prototypes.

Change-Id: If04d8fe7cbf068883485e95bd5ea6cdab6743e46
Signed-off-by: Piotr Sikora <piotrsikora@google.com>
Reviewed-on: https://boringssl-review.googlesource.com/7513
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-20 16:43:50 +00:00
David Benjamin
b371f1b9dd Include time.h in time_support.h.
For time_t and struct tm.

BUG=595118

Change-Id: I6c7f05998887ed2bd3fb56c83ac543894ef27fe6
Reviewed-on: https://boringssl-review.googlesource.com/7462
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: Nico Weber <thakis@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-17 17:27:27 +00:00
David Benjamin
6f7374b0ed Restore EC_GROUP_new_by_curve_name and EC_GROUP_set_generator.
Having a different API for this case than upstream is more trouble than is
worth it. This is sad since the new API avoids incomplete EC_GROUPs at least,
but I don't believe supporting this pair of functions will be significantly
more complex than supporting EC_GROUP_new_arbitrary even when we have static
EC_GROUPs.

For now, keep both sets of APIs around, but we'll be able to remove the scar
tissue once Conscrypt's complex dependencies are resolved.

Make the restored EC_GROUP_set_generator somewhat simpler than before by
removing the ability to call it multiple times and with some parameters set to
NULL. Keep the test.

Change-Id: I64e3f6a742678411904cb15c0ad15d56cdae4a73
Reviewed-on: https://boringssl-review.googlesource.com/7432
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-15 18:53:12 +00:00
Emily Stark
5c05648b8d Tiny documentation fix for EC_POINT_set_affine_coordinates_GFp
Change-Id: Icfd9986272f6e1adba54aa7521c28901fa02dfb7
Reviewed-on: https://boringssl-review.googlesource.com/7470
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-15 18:42:08 +00:00
David Benjamin
a2f2bc3a40 Align with upstream's error strings, take two.
I messed up a few of these.

ASN1_R_UNSUPPORTED_ALGORITHM doesn't exist. X509_R_UNSUPPORTED_ALGORITHM does
exist as part of X509_PUBKEY_set, but the SPKI parser doesn't emit this. (I
don't mind the legacy code having really weird errors, but since EVP is now
limited to things we like, let's try to keep that clean.) To avoid churn in
Conscrypt, we'll keep defining X509_R_UNSUPPORTED_ALGORITHM, but not actually
do anything with it anymore.  Conscrypt was already aware of
EVP_R_UNSUPPORTED_ALGORITHM, so this should be fine. (I don't expect
EVP_R_UNSUPPORTED_ALGORITHM to go away. The SPKI parsers we like live in EVP
now.)

A few other ASN1_R_* values didn't quite match upstream, so make those match
again. Finally, I got some of the rsa_pss.c values wrong. Each of those
corresponds to an (overly specific) RSA_R_* value in upstream. However, those
were gone in BoringSSL since even the initial commit. We placed the RSA <-> EVP
glue in crypto/evp (so crypto/rsa wouldn't depend on crypto/evp) while upstream
placed them in crypto/rsa.

Since no one seemed to notice the loss of RSA_R_INVALID_SALT_LENGTH, let's undo
all the cross-module errors inserted in crypto/rsa. Instead, since that kind of
specificity is not useful, funnel it all into X509_R_INVALID_PSS_PARAMETERS
(formerly EVP_R_INVALID_PSS_PARAMETERS, formerly RSA_R_INVALID_PSS_PARAMETERS).

Reset the error codes for all affected modules.

(That our error code story means error codes are not stable across this kind of
refactoring is kind of a problem. Hopefully this will be the last of it.)

Change-Id: Ibfb3a0ac340bfc777bc7de6980ef3ddf0a8c84bc
Reviewed-on: https://boringssl-review.googlesource.com/7458
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-15 16:02:12 +00:00
David Benjamin
fb8e678897 Match upstream's error codes for the old sigalg code.
People seem to condition on these a lot. Since this code has now been moved
twice, just make them all cross-module errors rather than leave a trail of
renamed error codes in our wake.

Change-Id: Iea18ab3d320f03cf29a64a27acca119768c4115c
Reviewed-on: https://boringssl-review.googlesource.com/7431
Reviewed-by: Emily Stark (Dunn) <estark@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-11 21:15:47 +00:00
Adam Langley
b8b28a64ff Add CRYPTO_[malloc|free|realloc] as aliases for the OPENSSL_𝑥 names.
This makes building OpenLDAP easier.

Change-Id: Ic1c5bcb2ec35c61c048e780ebc56db033d8382d8
Reviewed-on: https://boringssl-review.googlesource.com/7406
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-10 17:44:23 +00:00
David Benjamin
23afa68937 Fix the shared library build.
libdecrepit wants some symbols visible. Also a build file typo.

Change-Id: I670d2324ab9048f84e7f80afdefc98cbab80335d
Reviewed-on: https://boringssl-review.googlesource.com/7411
Reviewed-by: Adam Langley <agl@google.com>
2016-03-09 20:13:41 +00:00
Adam Langley
a7a226add9 Add |OBJ_NAME_do_all_sorted|.
This another of those functions that tries to turn C into Python. In
this case, implement it in terms of the similar functions in EVP so that
at least we only have one list of things.

This makes life with nmap easier.

Change-Id: I6d01c43f062748d4ba7d7020587c286322e610bb
Reviewed-on: https://boringssl-review.googlesource.com/7403
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-09 19:38:06 +00:00
Adam Langley
ff452c1d0e Add RIPEMD160 support in decrepit.
This version is taken from OpenSSL 1.0.2 with tweaks to support the
changes that we have made to md32_common.h. None of the assembly
implementations have been imported.

This makes supporting nmap easier.

Change-Id: Iae9241abdbc9021cc6bc35a65b40c3d739011ccc
Reviewed-on: https://boringssl-review.googlesource.com/7402
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-09 19:37:14 +00:00
Adam Langley
bfb38b1a3c Add |RC4_options| to decrepit.
I've no idea who thought that this function was a good idea in the first
place, but including it in decrepit makes supporting nmap easier.

Change-Id: I7433cda6a6ddf1cc545126edf779625e9fc70ada
Reviewed-on: https://boringssl-review.googlesource.com/7401
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-09 01:14:30 +00:00
Adam Langley
a34a5aacff Add one-shot |MD4| function.
This could live in decrepit, but it's tiny and having it makes the
interface more uniform that what we have for MD5 so I put it in the main
code. This is to more easily support nmap.

Change-Id: Ia098cc7ef6e00a90d2f3f56ee7deba8329c9a82e
Reviewed-on: https://boringssl-review.googlesource.com/7400
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-09 01:13:55 +00:00
Brian Smith
617804adc5 Always use |BN_mod_exp_mont|/|BN_mod_exp_mont_consttime| in RSA.
This removes a hard dependency on |BN_mod_exp|, which will allow the
linker to drop it in programs that don't use other features that
require it.

Also, remove the |mont| member of |bn_blinding_st| in favor of having
callers pass it when necssaary. The |mont| member was a weak reference,
and weak references tend to be error-prone.

Finally, reduce the scope of some parts of the blinding code to
|static|.

Change-Id: I16d8ccc2d6d950c1bb40377988daf1a377a21fe6
Reviewed-on: https://boringssl-review.googlesource.com/7111
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-08 22:30:19 +00:00
Adam Langley
6e96eaebe0 Add |X509_EXT_conf_nid| to decrepit.
This function is a deprecated version of |X509_EXT_nconf_nid| that takes
a hash of |CONF_VALUE|s directly rather than a |CONF|.

Change-Id: I5fd1025b31d73b988d9298b2624453017dd34ff4
Reviewed-on: https://boringssl-review.googlesource.com/7363
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-08 01:50:10 +00:00
Adam Langley
8ba4b2d5bf Add |RSA_[padding_add|verify]_PKCS1_PSS to decrepit.
These functions are just like the _mgf1 versions but omit one of the
parameters. It's easier to add them than to patch the callers in some
cases.

Change-Id: Idee5b81374bf15f2ea89b7e0c06400c2badbb275
Reviewed-on: https://boringssl-review.googlesource.com/7362
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-08 01:45:40 +00:00
Adam Langley
4435e96b08 Include buffer.h from bio.h.
We shouldn't really have to do this, but there's a lot of code that
doesn't always include what it uses. In this case, since bio.h
references |BUF_MEM| in function signatures, it seems a little less
distasteful.

Change-Id: Ifb50f8bce40639f977b4447404597168a68c8388
Reviewed-on: https://boringssl-review.googlesource.com/7361
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-08 01:44:37 +00:00
Adam Langley
99a24ba0f1 Add DSA_generate_parameters to decrepit.
This function was deprecated by OpenSSL in 0.9.8 but code that uses it
still exists. This change adds an implementation of this function to
decreipt/ to support these programs.

Change-Id: Ie99cd00ff8b0ab2675f2b1c821c3d664b9811f16
Reviewed-on: https://boringssl-review.googlesource.com/7360
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-08 01:44:27 +00:00
David Benjamin
22ce9b2d08 SSL_set_fd should create socket BIOs, not fd BIOs.
In OpenSSL, they create socket BIOs. The distinction isn't important on UNIX.
On Windows, file descriptors are provided by the C runtime, while sockets must
use separate recv and send APIs. Document how these APIs are intended to work.

Also add a TODO to resolve the SOCKET vs int thing. This code assumes that
Windows HANDLEs only use the bottom 32 bits of precision. (Which is currently
true and probably will continue to be true for the foreseeable future[*], but
it'd be nice to do this right.)

Thanks to Gisle Vanem and Daniel Stenberg for reporting the bug.

[*] Both so Windows can continue to run 32-bit programs and because of all the
random UNIX software, like OpenSSL and ourselves, out there which happily
assumes sockets are ints.

Change-Id: I67408c218572228cb1a7d269892513cda4261c82
Reviewed-on: https://boringssl-review.googlesource.com/7333
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-07 18:19:12 +00:00
Tom Thorogood
66b2fe8e02 Add |SSL_CTX_set_private_key_method| to parallel |SSL_set_private_key_method|
This change adds a |SSL_CTX_set_private_key_method| method that sets key_method on a SSL_CTX's cert.

It allows the private key method to be set once and inherited.

A copy of key_method (from SSL_CTX's cert to SSL's cert) is added in |ssl_cert_dup|.

Change-Id: Icb62e9055e689cfe2d5caa3a638797120634b63f
Reviewed-on: https://boringssl-review.googlesource.com/7340
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-07 18:16:58 +00:00
David Benjamin
ad004af661 Rename NID_x25519 to NID_X25519.
I went with NID_x25519 to match NID_sha1 and friends in being lowercase.
However, upstream seems to have since chosen NID_X25519. Match their
name.

Change-Id: Icc7b183a2e2dfbe42c88e08e538fcbd242478ac3
Reviewed-on: https://boringssl-review.googlesource.com/7331
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-07 15:48:51 +00:00
David Benjamin
bc5b2a2e22 Add a deterministic PRNG for fuzzing.
If running the stack through a fuzzer, we would like execution to be
completely deterministic. This is gated on a
BORINGSSL_UNSAFE_FUZZER_MODE #ifdef.

For now, this just uses the zero ChaCha20 key and a global counter. As
needed, we can extend this to a thread-local counter and a separate
ChaCha20 stream and counter per input length.

Change-Id: Ic6c9d8a25e70d68e5dc6804e2c234faf48e51395
Reviewed-on: https://boringssl-review.googlesource.com/7286
Reviewed-by: Adam Langley <agl@google.com>
2016-03-03 01:36:19 +00:00
Adam Langley
29ec5d1fda Add dummy |SSL_get_server_tmp_key|.
Node.js calls it but handles it failing. Since we have abstracted this
in the state machine, we mightn't even be using a cipher suite where the
server's key can be expressed as an EVP_PKEY.

Change-Id: Ic3f013dc9bcd7170a9eb2c7535378d478b985849
Reviewed-on: https://boringssl-review.googlesource.com/7272
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-02 15:57:47 +00:00
Adam Langley
d323f4b1e1 Bring back |verify_store|.
This was dropped in d27441a9cb due to lack
of use, but node.js now needs it.

Change-Id: I1e207d4b46fc746cfae309a0ea7bbbc04ea785e8
Reviewed-on: https://boringssl-review.googlesource.com/7270
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-02 15:57:27 +00:00
Adam Langley
7a17ba2e3a Add |FIPS_mode|, which returns zero.
(node.js calls it.)

Change-Id: I7401f4cb4dfc61d500331821784ae717ad9f7adf
Reviewed-on: https://boringssl-review.googlesource.com/7271
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-02 00:15:37 +00:00
Arnar Birgisson
f27459e412 Add SPAKE2 over Ed25519.
SPAKE2 is a password-authenticated key exchange. This implementation is
over the twisted Edwards curve Ed25519, and uses SHA-512 as the hash
primitive.

See https://tools.ietf.org/html/draft-irtf-cfrg-spake2-03

Change-Id: I2cd3c3ebdc3d55ac3aea3a9eb0d06275509597ac
Reviewed-on: https://boringssl-review.googlesource.com/7114
Reviewed-by: Adam Langley <agl@google.com>
2016-03-01 19:34:10 +00:00
Adam Langley
e4f3f4df6e Add test that A+A = 2×A on elliptic curves.
Change-Id: I914efab9a15c903f79a1b83388b577b14c534269
Reviewed-on: https://boringssl-review.googlesource.com/7247
Reviewed-by: David Benjamin <davidben@google.com>
2016-03-01 18:08:51 +00:00
David Benjamin
fb2c6f8c85 ASN1_get_object should not accept large universal tags.
The high bits of the type get used for the V_ASN1_NEG bit, so when used with
ASN1_ANY/ASN1_TYPE, universal tags become ambiguous. This allows one to create
a negative zero, which should be impossible. Impose an upper bound on universal
tags accepted by crypto/asn1 and add a test.

BUG=590615

Change-Id: I363e01ebfde621c8865101f5bcbd5f323fb59e79
Reviewed-on: https://boringssl-review.googlesource.com/7238
Reviewed-by: Adam Langley <agl@google.com>
2016-02-29 21:17:19 +00:00
David Benjamin
f4e447c16d Move ASN1_bn_print to a static function in evp/print.c.
It's not used anywhere else, in the library or consumers (Google ones or
ones I could find on Debian codesearch). This is a sufficiently
specialized function that the risk of a third-party library newly
depending on it is low. This removes the last include of asn1.h or
x509.h in crypto/evp.

(This is almost entirely cosmetic because it wasn't keeping the static linker
from doing the right thing anyway. But if we were want to separate the legacy
ASN.1 stack into its own decrepit-like target, we'll need to be pickier about
separation.)

Change-Id: I9be97c9321572e3a2ed093e1d50036b7654cff41
Reviewed-on: https://boringssl-review.googlesource.com/7080
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 23:35:10 +00:00
David Benjamin
63d9246812 Reset crypto/evp error codes.
A number of values have fallen off now that code's been shuffled
around.

Change-Id: I5eac1d3fa4a9335c6aa72b9876d37bb9a9a029ac
Reviewed-on: https://boringssl-review.googlesource.com/7029
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 23:34:04 +00:00
David Benjamin
921d906bb6 Reimplement d2i_PrivateKey.
Functions which lose object reuse and need auditing:
- d2i_PrivateKey

This removes evp_asn1.c's dependency on the old stack. (Aside from
obj/.) It also takes old_priv_decode out of EVP_ASN1_METHOD in favor of
calling out to the new-style function. EVP_ASN1_METHOD no longer has any
old-style type-specific serialization hooks, only the PKCS#8 and SPKI
ones.

BUG=499653

Change-Id: Ic142dc05a5505b50e4717c260d3893b20e680194
Reviewed-on: https://boringssl-review.googlesource.com/7027
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 23:33:50 +00:00
David Benjamin
8ebc0f55a0 Decouple the EVP and PEM code.
EVP_PKEY_asn1_find can already be private. EVP_PKEY_asn1_find_str is used
only so the PEM code can get at legacy encoders. Since this is all
legacy non-PKCS8 stuff, we can just explicitly list out the three cases
in the two places that need it. If this changes, we can later add a
table in crypto/pem mapping string to EVP_PKEY type.

With this, EVP_PKEY_ASN1_METHOD is no longer exposed in the public API
and nothing outside of EVP_PKEY reaches into it. Unexport all of that.

Change-Id: Iab661014247dbdbc31e5e9887364176ec5ad2a6d
Reviewed-on: https://boringssl-review.googlesource.com/6871
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 22:50:21 +00:00
David Benjamin
17727c6843 Move all signature algorithm code to crypto/x509.
All the signature algorithm logic depends on X509_ALGOR. This also
removes the X509_ALGOR-based EVP functions which are no longer used
externally. I think those APIs were a mistake on my part. The use in
Chromium was unnecessary (and has since been removed anyway). The new
X.509 stack will want to process the signatureAlgorithm itself to be
able to enforce policies on it.

This also moves the RSA_PSS_PARAMS bits to crypto/x509 from crypto/rsa.
That struct is also tied to crypto/x509. Any new RSA-PSS code would
have to use something else anyway.

BUG=499653

Change-Id: I6c4b4573b2800a2e0f863d35df94d048864b7c41
Reviewed-on: https://boringssl-review.googlesource.com/7025
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 22:39:02 +00:00
Emily Stark
95a79eec40 Add a stub for SSL_get_shared_ciphers().
This stub returns an empty string rather than NULL (since some callers
might assume that NULL means there are no shared ciphers).

Change-Id: I9537fa0a80c76559b293d8518599b68fd9977dd8
Reviewed-on: https://boringssl-review.googlesource.com/7196
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-26 21:10:13 +00:00
David Benjamin
6f0c4db90e Enable upstream's Poly1305 code.
The C implementation is still our existing C implementation, but slightly
tweaked to fit with upstream's init/block/emits convention.

I've tested this by looking at code coverage in kcachegrind and

  valgrind --tool=callgrind --dump-instr=yes --collect-jumps=yes

(NB: valgrind 3.11.0 is needed for AVX2. And even that only does 64-bit AVX2,
so we can't get coverage for the 32-bit code yet. But I had to disable that
anyway.)

This was paired with a hacked up version of poly1305_test that would repeat
tests with different ia32cap and armcap values. This isn't checked in, but we
badly need a story for testing all the different variants.

I'm not happy with upstream's code in either the C/asm boundary or how it
dispatches between different versions, but just debugging the code has been a
significant time investment. I'd hoped to extract the SIMD parts and do the
rest in C, but I think we need to focus on testing first (and use that to
guide what modifications would help). For now, this version seems to work at
least.

The x86 (not x86_64) AVX2 code needs to be disabled because it's broken. It
also seems pretty unnecessary.
https://rt.openssl.org/Ticket/Display.html?id=4346

Otherwise it seems to work and buys us a decent performance improvement.
Notably, my Nexus 6P is finally faster at ChaCha20-Poly1305 than my Nexus 4!

bssl speed numbers follow:

x86
---
Old:
Did 1554000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000536us (1553167.5 ops/sec): 24.9 MB/s
Did 136000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1003947us (135465.3 ops/sec): 182.9 MB/s
Did 30000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1022990us (29325.8 ops/sec): 240.2 MB/s
Did 1888000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000206us (1887611.2 ops/sec): 30.2 MB/s
Did 173000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1003036us (172476.4 ops/sec): 232.8 MB/s
Did 30000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1027759us (29189.7 ops/sec): 239.1 MB/s
New:
Did 2030000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000507us (2028971.3 ops/sec): 32.5 MB/s
Did 404000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000287us (403884.1 ops/sec): 545.2 MB/s
Did 83000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1001258us (82895.7 ops/sec): 679.1 MB/s
Did 2018000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000006us (2017987.9 ops/sec): 32.3 MB/s
Did 360000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001962us (359295.1 ops/sec): 485.0 MB/s
Did 85000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1002479us (84789.8 ops/sec): 694.6 MB/s

x86_64, no AVX2
---
Old:
Did 2023000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000258us (2022478.2 ops/sec): 32.4 MB/s
Did 466000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1002619us (464782.7 ops/sec): 627.5 MB/s
Did 90000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1001133us (89898.1 ops/sec): 736.4 MB/s
Did 2238000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000175us (2237608.4 ops/sec): 35.8 MB/s
Did 483000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001348us (482349.8 ops/sec): 651.2 MB/s
Did 90000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1003141us (89718.2 ops/sec): 735.0 MB/s
New:
Did 2558000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000275us (2557296.7 ops/sec): 40.9 MB/s
Did 510000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1001810us (509078.6 ops/sec): 687.3 MB/s
Did 115000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1006457us (114262.2 ops/sec): 936.0 MB/s
Did 2818000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000187us (2817473.1 ops/sec): 45.1 MB/s
Did 418000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001140us (417524.0 ops/sec): 563.7 MB/s
Did 91000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1002539us (90769.5 ops/sec): 743.6 MB/s

x86_64, AVX2
---
Old:
Did 2516000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000115us (2515710.7 ops/sec): 40.3 MB/s
Did 774000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000300us (773767.9 ops/sec): 1044.6 MB/s
Did 171000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1004373us (170255.5 ops/sec): 1394.7 MB/s
Did 2580000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000144us (2579628.5 ops/sec): 41.3 MB/s
Did 769000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000472us (768637.2 ops/sec): 1037.7 MB/s
Did 169000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1000320us (168945.9 ops/sec): 1384.0 MB/s
New:
Did 3240000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000114us (3239630.7 ops/sec): 51.8 MB/s
Did 932000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000059us (931945.0 ops/sec): 1258.1 MB/s
Did 217000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1003282us (216290.1 ops/sec): 1771.8 MB/s
Did 3187000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000100us (3186681.3 ops/sec): 51.0 MB/s
Did 926000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000071us (925934.3 ops/sec): 1250.0 MB/s
Did 215000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1000479us (214897.1 ops/sec): 1760.4 MB/s

arm, Nexus 4
---
Old:
Did 430248 ChaCha20-Poly1305 (16 bytes) seal operations in 1000153us (430182.2 ops/sec): 6.9 MB/s
Did 115250 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000549us (115186.8 ops/sec): 155.5 MB/s
Did 27000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1030124us (26210.4 ops/sec): 214.7 MB/s
Did 451750 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000549us (451502.1 ops/sec): 7.2 MB/s
Did 118000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1001557us (117816.6 ops/sec): 159.1 MB/s
Did 27000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1024263us (26360.4 ops/sec): 215.9 MB/s
New:
Did 553644 ChaCha20-Poly1305 (16 bytes) seal operations in 1000183us (553542.7 ops/sec): 8.9 MB/s
Did 126000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1000396us (125950.1 ops/sec): 170.0 MB/s
Did 27000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1000336us (26990.9 ops/sec): 221.1 MB/s
Did 559000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1001465us (558182.3 ops/sec): 8.9 MB/s
Did 124000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1000824us (123897.9 ops/sec): 167.3 MB/s
Did 28000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1034854us (27057.0 ops/sec): 221.7 MB/s

aarch64, Nexus 6P
---
Old:
Did 358000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000358us (357871.9 ops/sec): 5.7 MB/s
Did 45000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1022386us (44014.7 ops/sec): 59.4 MB/s
Did 8657 ChaCha20-Poly1305 (8192 bytes) seal operations in 1063722us (8138.4 ops/sec): 66.7 MB/s
Did 350000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000074us (349974.1 ops/sec): 5.6 MB/s
Did 44000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1007907us (43654.8 ops/sec): 58.9 MB/s
Did 8525 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1042644us (8176.3 ops/sec): 67.0 MB/s
New:
Did 713000 ChaCha20-Poly1305 (16 bytes) seal operations in 1000190us (712864.6 ops/sec): 11.4 MB/s
Did 180000 ChaCha20-Poly1305 (1350 bytes) seal operations in 1004249us (179238.4 ops/sec): 242.0 MB/s
Did 41000 ChaCha20-Poly1305 (8192 bytes) seal operations in 1005811us (40763.1 ops/sec): 333.9 MB/s
Did 775000 ChaCha20-Poly1305-Old (16 bytes) seal operations in 1000719us (774443.2 ops/sec): 12.4 MB/s
Did 182000 ChaCha20-Poly1305-Old (1350 bytes) seal operations in 1003529us (181360.0 ops/sec): 244.8 MB/s
Did 41000 ChaCha20-Poly1305-Old (8192 bytes) seal operations in 1010576us (40570.9 ops/sec): 332.4 MB/s

Change-Id: Iaa4ab86ac1174b79833077963cc3616cfb08e686
Reviewed-on: https://boringssl-review.googlesource.com/7226
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 16:05:14 +00:00
David Benjamin
a211aee545 Add SSL_CIPHER_has_SHA256_HMAC.
Change-Id: I05a8f5d1778aba1813fe4d34b4baa21849158218
Reviewed-on: https://boringssl-review.googlesource.com/7215
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 01:33:11 +00:00
David Benjamin
e593fed378 Rename opensslfeatures.h to opensslconf.h.
Some software #includes opensslconf.h which typically contains settings that we
put in opensslfeatures.h (a header name not in OpenSSL). Rename it to
opensslconf.h.

Change-Id: Icd21dde172e5e489ce90dd5c16ae4d2696909fb6
Reviewed-on: https://boringssl-review.googlesource.com/7216
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 01:32:50 +00:00
David Benjamin
a473e554b4 Add BIO_do_connect.
Some consumers of connect BIOs connect them explicitly, and we already have the
BIO_ctrl hooked up.

Change-Id: Ie6b14f8ceb272b560e2b534e0b6c32fae050475b
Reviewed-on: https://boringssl-review.googlesource.com/7217
Reviewed-by: Adam Langley <agl@google.com>
2016-02-26 01:32:21 +00:00
Brian Smith
76c6381c21 Return 0 on error in |EC_POINT_is_on_curve| instead of -1.
Callers of this function are not checking for the -1 result. Change
the semantics to match their expectations and to match the common
semantics of most other parts of BoringSSL.

Change-Id: I4ec537d7619e20e8ddfee80c72125e4c02cfaac1
Reviewed-on: https://boringssl-review.googlesource.com/7125
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24 22:00:58 +00:00
Steven Valdez
d8eea14443 BIO_new_mem_buf should take const void *
BIO_FLAGS_MEM_RDONLY keeps the invariant.

(Imported from upstream's a38a159bfcbc94214dda00e0e6b1fc6454a23b78)

Change-Id: I4cb35615d76b77929915e370dbb7fec1455da069
Reviewed-on: https://boringssl-review.googlesource.com/7214
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24 19:14:19 +00:00
Steven Valdez
b9824e2417 Handle SSL_shutdown while in init more appropriately
Calling SSL_shutdown while in init previously gave a "1" response,
meaning everything was successfully closed down (even though it
wasn't). Better is to send our close_notify, but fail when trying to
receive one.

The problem with doing a shutdown while in the middle of a handshake
is that once our close_notify is sent we shouldn't really do anything
else (including process handshake/CCS messages) until we've received a
close_notify back from the peer. However the peer might send a CCS
before acting on our close_notify - so we won't be able to read it
because we're not acting on CCS messages!

(Imported from upstream's f73c737c7ac908c5d6407c419769123392a3b0a9)
Change-Id: Iaad5c5e38983456d3697c955522a89919628024b
Reviewed-on: https://boringssl-review.googlesource.com/7207
Reviewed-by: David Benjamin <davidben@google.com>
2016-02-24 15:57:09 +00:00
David Benjamin
85003903fc Remove CRYPTO_set_NEON_functional.
This depends on https://codereview.chromium.org/1730823002/. The bit was only
ever targetted to one (rather old) CPU. Disable NEON on it uniformly, so we
don't have to worry about whether any new NEON code breaks it.

BUG=589200

Change-Id: Icc7d17d634735aca5425fe0a765ec2fba3329326
Reviewed-on: https://boringssl-review.googlesource.com/7211
Reviewed-by: Adam Langley <agl@google.com>
2016-02-23 23:19:46 +00:00
David Benjamin
de94238217 Fix SSL_get_{read,write}_sequence.
I switched up the endianness. Add some tests to make sure those work right.

Also tweak the DTLS semantics. SSL_get_read_sequence should return the highest
sequence number received so far. Include the epoch number in both so we don't
need a second API for it.

Change-Id: I9901a1665b41224c46fadb7ce0b0881dcb466bcc
Reviewed-on: https://boringssl-review.googlesource.com/7141
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 22:05:29 +00:00
David Benjamin
e30a09e604 Implement new PKCS#8 parsers.
As with SPKI parsers, the intent is make EVP_PKEY capture the key's
constraints in full fidelity, so we'd have to add new types or store the
information in the underlying key object if people introduce variant key
types with weird constraints on them.

Note that because PKCS#8 has a space for arbitrary attributes, this
parser must admit a hole. I'm assuming for now that we don't need an API
that enforces no attributes and just ignore trailing data in the
structure for simplicity.

BUG=499653

Change-Id: I6fc641355e87136c7220f5d7693566d1144a68e8
Reviewed-on: https://boringssl-review.googlesource.com/6866
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 17:24:10 +00:00
David Benjamin
239a0abfd5 Slightly simplify and deprecate i2d_{Public,Private}Key.
There are all the type-specific serializations rather than something
tagged with a type. i2d_PrivateKey's PKCS#8 codepath was unreachable
because every EVP_PKEY type has an old_priv_encode function.

To prune EVP_PKEY_ASN1_METHOD further, replace i2d_PrivateKey into a
switch case so we don't need to keep old_priv_encode around. This cuts
down on a case of outside modules reaching into crypto/evp method
tables.

Change-Id: I30db2eed836d560056ba9d1425b960d0602c3cf2
Reviewed-on: https://boringssl-review.googlesource.com/6865
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:31:26 +00:00
David Benjamin
32fdc512ca Remove param_decode and param_encode EVP_PKEY hooks.
They're only used by a pair of PEM functions, which are never used.

BUG=499653

Change-Id: I89731485c66ca328c634efbdb7e182a917f2a963
Reviewed-on: https://boringssl-review.googlesource.com/6863
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:30:29 +00:00
David Benjamin
68772b31b0 Implement new SPKI parsers.
Many consumers need SPKI support (X.509, TLS, QUIC, WebCrypto), each
with different ways to set signature parameters. SPKIs themselves can
get complex with id-RSASSA-PSS keys which come with various constraints
in the key parameters. This suggests we want a common in-library
representation of an SPKI.

This adds two new functions EVP_parse_public_key and
EVP_marshal_public_key which converts EVP_PKEY to and from SPKI and
implements X509_PUBKEY functions with them. EVP_PKEY seems to have been
intended to be able to express the supported SPKI types with
full-fidelity, so these APIs will continue this.

This means future support for id-RSASSA-PSS would *not* repurpose
EVP_PKEY_RSA. I'm worried about code assuming EVP_PKEY_RSA implies
acting on the RSA* is legal. Instead, it'd add an EVP_PKEY_RSA_PSS and
the data pointer would be some (exposed, so the caller may still check
key size, etc.) RSA_PSS_KEY struct. Internally, the EVP_PKEY_CTX
implementation would enforce the key constraints. If RSA_PSS_KEY would
later need its own API, that code would move there, but that seems
unlikely.

Ideally we'd have a 1:1 correspondence with key OID, although we may
have to fudge things if mistakes happen in standardization. (Whether or
not X.509 reuses id-ecPublicKey for Ed25519, we'll give it a separate
EVP_PKEY type.)

DSA parsing hooks are still implemented, missing parameters and all for
now. This isn't any worse than before.

Decoupling from the giant crypto/obj OID table will be a later task.

BUG=522228

Change-Id: I0e3964edf20cb795a18b0991d17e5ca8bce3e28c
Reviewed-on: https://boringssl-review.googlesource.com/6861
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 16:28:07 +00:00
David Benjamin
fda22a7573 Reimplement DSA parsing logic with crypto/asn1.
Functions which lose object reuse and need auditing:
- d2i_DSA_SIG
- d2i_DSAPublicKey
- d2i_DSAPrivateKey
- d2i_DSAparams

BUG=499653

Change-Id: I1cc2ae10e1e77eb57da3a858ac8734a95715ce4b
Reviewed-on: https://boringssl-review.googlesource.com/7022
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 00:26:01 +00:00
David Benjamin
3cadf63c68 Remove DSA write_params.
This imports upstream's ea6b07b54c1f8fc2275a121cdda071e2df7bd6c1 along
with a bugfix in 987157f6f63fa70dbeffca3c8bc62f26e9767ff2.

In an SPKI, a DSA key is only an INTEGER, with the group information in
the AlgorithmIdentifier. But a standalone DSAPublicKey is more complex
(and apparently made up by OpenSSL). OpenSSL implemented this with a
write_params boolean and making DSAPublicKey a CHOICE.

Instead, have p_dsa_asn1.c encode an INTEGER directly. d2i_DSAPublicKey
only parses the standalone form. (That code will be replaced later, but
first do this in preparation for rewriting the DSA ASN.1 code.)

Change-Id: I6fbe298d2723b9816806e9c196c724359b9ffd63
Reviewed-on: https://boringssl-review.googlesource.com/7021
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 23:54:38 +00:00
David Benjamin
2f6410ba4e Rewrite ECPrivateKey serialization.
Functions which lose object reuse and need auditing:
- d2i_ECParameters
- d2i_ECPrivateKey

This adds a handful of bytestring-based APIs to handle EC key
serialization. Deprecate all the old serialization APIs. Notes:

- An EC_KEY has additional state that controls its encoding, enc_flags
  and conv_form. conv_form is left alone, but enc_flags in the new API
  is an explicit parameter.

- d2i_ECPrivateKey interpreted its T** argument unlike nearly every
  other d2i function. This is an explicit EC_GROUP parameter in the new
  function.

- The new specified curve code is much stricter and should parse enough
  to uniquely identify the curve.

- I've not bothered with a new version of i2d_ECParameters. It just
  writes an OID. This may change later when decoupling from the giant
  OID table.

- Likewise, I've not bothered with new APIs for the public key since the
  EC_POINT APIs should suffice.

- Previously, d2i_ECPrivateKey would not call EC_KEY_check_key and it
  was possible for the imported public and private key to mismatch. It
  now calls it.

BUG=499653

Change-Id: I30b4dd2841ae76c56ab0e1808360b2628dee0615
Reviewed-on: https://boringssl-review.googlesource.com/6859
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 23:51:09 +00:00
David Benjamin
375124b162 Parse BER for PKCS#12 more accurately.
CBS_asn1_ber_to_der currently uses heuristics because implicitly-tagged
constructed strings in BER are ambiguous with implicitly-tagged sequences. It's
not possible to convert BER to DER without knowing the schema.

Fortunately, implicitly tagged strings don't appear often so instead split the
job up: CBS_asn1_ber_to_der fixes indefinite-length elements and constructed
strings it can see. Implicitly-tagged strings it leaves uncoverted, but they
will only nest one level down (because BER kindly allows one to nest
constructed strings arbitrarily!).

CBS_get_asn1_implicit_string then performs the final concatenation at parse
time. This isn't much more complex and lets us parse BER more accurately and
also reject a number of mis-encoded values (e.g. constructed INTEGERs are not a
thing) we'd previously let through. The downside is the post-conversion parsing
code must be aware of this limitation of CBS_asn1_ber_to_der. Fortunately,
there's only one implicitly-tagged string in our PKCS#12 code.

(In the category of things that really really don't matter, but I had spare
cycles and the old BER converter is weird.)

Change-Id: Iebdd13b08559fa158b308ef83a5bb07bfdf80ae8
Reviewed-on: https://boringssl-review.googlesource.com/7052
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 19:32:50 +00:00
Adam Langley
d057454f90 Changes to support node.js's use of PKCS#12.
node.js uses a memory BIO in the wrong mode which, for now, we work
around. It also passes in NULL (rather than empty) strings and a
non-NULL out-arg for |d2i_PKCS12_bio|.

Change-Id: Ib565b4a202775bb32fdcb76db8a4e8c54268c052
Reviewed-on: https://boringssl-review.googlesource.com/7012
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 19:21:59 +00:00
David Benjamin
6014ea6248 Add EC_POINT_point2cbb.
This slightly simplifies the SSL_ECDH code and will be useful later on
in reimplementing the key parsing logic.

Change-Id: Ie41ea5fd3a9a734b3879b715fbf57bd991e23799
Reviewed-on: https://boringssl-review.googlesource.com/6858
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 19:04:33 +00:00
David Benjamin
47ebec1210 Validate DH public keys for RFC 5114 groups.
This is CVE-2016-0701 for OpenSSL, reported by Antonio Sanso. It is a no-op for
us as we'd long removed SSL_OP_DH_SINGLE_USE and static DH cipher suites. (We
also do not parse or generate X9.42 DH parameters.)

However, we do still have the APIs which return RFC 5114 groups, so we should
perform the necessary checks in case later consumers reuse keys.

Unlike groups we generate, RFC 5114 groups do not use "safe primes" and have
many small subgroups. In those cases, the subprime q is available. Before using
a public key, ensure its order is q by checking y^q = 1 (mod p). (q is assumed
to be prime and the existing range checks ensure y is not 1.)

(Imported from upstream's 878e2c5b13010329c203f309ed0c8f2113f85648 and
75374adf8a6ff69d6718952121875a491ed2cd29, but with some bugs fixed. See
RT4278.)

Change-Id: Ib18c3e84819002fa36a127ac12ca00ee33ea018a
Reviewed-on: https://boringssl-review.googlesource.com/7001
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 16:44:38 +00:00
David Benjamin
43946d44ae Update references to the extended master secret draft.
It's now an RFC too.

Change-Id: I2aa7a862bf51ff01215455e87b16f259fc468490
Reviewed-on: https://boringssl-review.googlesource.com/7028
Reviewed-by: Adam Langley <agl@google.com>
2016-02-02 16:37:55 +00:00
David Benjamin
415564fe2c Update draft-irtf-cfrg-curves-11 references to RFC 7748.
Change-Id: I6148df93a1748754ee6be9e2b98cc8afd38746cb
Reviewed-on: https://boringssl-review.googlesource.com/6960
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:53:26 +00:00
David Benjamin
f6094e05ef Don't allow EVP_PKEY_RSA2.
OpenSSL accepts both OID 2.5.8.1.1 and OID 1.2.840.113549.1.1.1 for RSA
public keys. The latter comes from RFC 3279 and is widely implemented.
The former comes from the ITU-T version of X.509. Interestingly,
2.5.8.1.1 actually has a parameter, which OpenSSL ignores:

  rsa ALGORITHM ::= {
     KeySize
     IDENTIFIED BY id-ea-rsa
  }
  KeySize ::= INTEGER

Remove support for 2.5.8.1.1 completely. In tests with a self-signed
certificate and code inspection:

- IE11 on Win8 does not accept the certificate in a TLS handshake at
  all. Such a certificate is fatal and unbypassable. However Microsoft's
  libraries do seem to parse it, so Chrome on Windows allows one to
  click through the error. I'm guessing either the X.509 stack accepts
  it while the TLS stack doesn't recognize it as RSA or the X.509 stack
  is able to lightly parse it but not actually understand the key. (The
  system certificate UI didn't display it as an RSA key, so probably the
  latter?)

- Apple's certificate library on 10.11.2 does not parse the certificate
  at all. Both Safari and Chrome on Mac treat it as a fatal and
  unbypassable error.

- mozilla::pkix, from code inspection, does not accept such
  certificates. However, Firefox does allow clicking through the error.
  This is likely a consequence of mozilla::pkix and NSS having different
  ASN.1 stacks. I did not test this, but I expect this means Chrome on
  Linux also accepts it.

Given IE and Safari's results, it should be safe to simply remove this.
Firefox's data point is weak (perhaps someone is relying on being able
to click-through a self-signed 2.5.8.1.1 certificate), but it does
further ensure no valid certificate could be doing this.

The following is the 2.5.8.1.1 certificate I constructed to test with.
The private key is key.pem from ssl/test/runner:

-----BEGIN CERTIFICATE-----
MIICVTCCAb6gAwIBAgIJAPuwTC6rEJsMMA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNV
BAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBX
aWRnaXRzIFB0eSBMdGQwHhcNMTQwNDIzMjA1MDQwWhcNMTcwNDIyMjA1MDQwWjBF
MQswCQYDVQQGEwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50
ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIGcMAoGBFUIAQECAgQAA4GNADCBiQKBgQDY
K8imMuRi/03z0K1Zi0WnvfFHvwlYeyK9Na6XJYaUoIDAtB92kWdGMdAQhLciHnAj
kXLI6W15OoV3gA/ElRZ1xUpxTMhjP6PyY5wqT5r6y8FxbiiFKKAnHmUcrgfVW28t
Q+0rkLGMryRtrukXOgXBv7gcrmU7G1jC2a7WqmeI8QIDAQABo1AwTjAdBgNVHQ4E
FgQUi3XVrMsIvg4fZbf6Vr5sp3Xaha8wHwYDVR0jBBgwFoAUi3XVrMsIvg4fZbf6
Vr5sp3Xaha8wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0BAQUFAAOBgQAIZuUICtYv
w3cbpCGX6HNCtyI0guOfbytcdwzRkQaCsYNSDrTxrSSWxHwqg3Dl/RlvS+T3Yaua
Xkioadstwt7GDP6MwpIpdbjchh0XZd3kjdJWqXSvihUDpRePNjNS2LmJW8GWfB3c
F6UVyNK+wcApRY+goREIhyYupAHUexR7FQ==
-----END CERTIFICATE-----

BUG=522228

Change-Id: I031d03c0f53a16cbc749c4a5d8be6efca50dc863
Reviewed-on: https://boringssl-review.googlesource.com/6852
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:43:37 +00:00
David Benjamin
719594e512 Un-const EVP_PKEY_CTX_set0_rsa_oaep_label and fix overflow check.
It takes ownership of the buffer, so it's not actually const. The
const-ness gets dropped once it transits through EVP_PKEY_CTX_ctrl.

Also compare against INT_MAX explicitly for the overflow check. I'm not sure
whether the casting version is undefined, but comparing against INT_MAX matches
the rest of the codebase when transiting in and out of signed ints.

Change-Id: I131165a4b5f0ebe02c6db3e7e3e0d1af5b771710
Reviewed-on: https://boringssl-review.googlesource.com/6850
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:34:38 +00:00
David Benjamin
b6155e60f3 Remove app_data from EVP_PKEY_CTX.
It's never used. It's not clear why one would want such a thing.
EVP_PKEY_CTX has no way for callers to register callbacks, which means
there shouldn't be a way for the library to present you an EVP_PKEY_CTX
out-of-context. (Whereas app_data/ex_data makes sense on SSL because of
its numerous callbacks or RSA because of RSA_METHOD.)

Change-Id: I55af537ab101682677af34f6ac1f2c27b5899a89
Reviewed-on: https://boringssl-review.googlesource.com/6849
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:29:34 +00:00
David Benjamin
acb2451807 Rename the BIGNUM ASN.1 functions.
There's many ways to serialize a BIGNUM, so not including asn1 in the name is
confusing (and collides with BN_bn2cbb_padded). Since BN_asn12bn looks
ridiculous, match the parse/marshal naming scheme of other modules instead.

Change-Id: I53d22ae0537a98e223ed943e943c48cb0743cf51
Reviewed-on: https://boringssl-review.googlesource.com/6822
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:37:44 +00:00
David Benjamin
8f2d4e344c Fix documentation string.
SSL_CTX_set_retain_only_sha256_of_client_certs's comment wasn't quite right.

Change-Id: I40527eebf2988d117cd8bd544bb5f16936c2cbfb
Reviewed-on: https://boringssl-review.googlesource.com/6982
Reviewed-by: Adam Langley <agl@google.com>
2016-01-27 22:20:32 +00:00
David Benjamin
91b2501f02 Add functions for accessing read_sequence and write_sequence.
OpenSSL 1.1.0 doesn't seem to have these two, so this isn't based on anything.
Have them return uint64_t in preparation for switching the internal
representation to uint64_t so ssl_record_sequence_update can go away.

Change-Id: I21d55e9a29861c992f409ed293e0930a7aaef7a3
Reviewed-on: https://boringssl-review.googlesource.com/6941
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:03:57 +00:00
David Benjamin
cdd0b7e775 Add SSL_CTX_set_retain_only_sha256_of_client_certs.
We have the hook on the SSL_CTX, but it should be possible to set it without
reaching into SSL_CTX.

Change-Id: I93db070c7c944be374543442a8de3ce655a28928
Reviewed-on: https://boringssl-review.googlesource.com/6880
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 22:02:53 +00:00
David Benjamin
b83003ebc6 Don't initialize enc_method before version negotiation.
Move it into ssl->s3 so it automatically behaves correctly on SSL_clear.
ssl->version is still a mess though.

Change-Id: I17a692a04a845886ec4f8de229fa6cf99fa7e24a
Reviewed-on: https://boringssl-review.googlesource.com/6844
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-27 21:38:12 +00:00
Adam Langley
ce9d85eedd Tweaks for node.js
node.js is, effectively, another bindings library. However, it's better
written than most and, with these changes, only a couple of tiny fixes
are needed in node.js. Some of these changes are a little depressing
however so we'll need to push node.js to use APIs where possible.

Changes:
  ∙ Support verify_recover. This is very obscure and the motivation
    appears to be https://github.com/nodejs/node/issues/477 – where it's
    not clear that anyone understands what it means :(
  ∙ Add a few, no-op #defines
  ∙ Add some members to |SSL_CTX| and |SSL| – node.js needs to not
    reach into these structs in the future.
  ∙ Add EC_get_builtin_curves.
  ∙ Add EVP_[CIPHER|MD]_do_all_sorted – these functions are limited to
    decrepit.

Change-Id: I9a3566054260d6c4db9d430beb7c46cc970a9d46
Reviewed-on: https://boringssl-review.googlesource.com/6952
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:42 +00:00
Adam Langley
eac0ce09d8 Have doc.go parse struct comments.
In code, structs that happened to have a '(' somewhere in their body
would cause the parser to go wrong. This change fixes that and updates
the comments on a number of structs.

Change-Id: Ia76ead266615a3d5875b64a0857a0177fec2bd00
Reviewed-on: https://boringssl-review.googlesource.com/6970
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 23:23:23 +00:00
David Benjamin
f6494f4928 Add a SSL_get_pending_cipher API.
Conscrypt needs to, in the certificate verification callback, know the key
exchange + auth method of the current cipher suite to pass into
X509TrustManager.checkServerTrusted. Currently it reaches into the struct to
get it. Add an API for this.

Change-Id: Ib4e0a1fbf1d9ea24e0114f760b7524e1f7bafe33
Reviewed-on: https://boringssl-review.googlesource.com/6881
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-26 21:51:02 +00:00
David Benjamin
7027d25c6b Also add a no-op stub for OPENSSL_config.
Apparently OpenSSL's API is made entirely of initialization functions.
Some external libraries like to initialize with OPENSSL_config instead.

Change-Id: I28efe97fc5eb21309f560c84112b80e947f8bb17
Reviewed-on: https://boringssl-review.googlesource.com/6981
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 15:48:51 +00:00
David Benjamin
e5aa791a1c Add a few more no-op stubs for cURL compatibility.
With these stubs, cURL should not need any BoringSSL #ifdefs at all,
except for their OCSP #ifdefs (which can switch to the more generally
useful OPENSSL_NO_OCSP) and the workaround for wincrypt.h macro
collisions. That we intentionally leave to the consumer rather than add
a partial hack that makes the build sensitive to include order.

(I'll send them a patch upstream once this cycles in.)

Change-Id: I815fe67e51e80e9aafa9b91ae68867ca1ff1d623
Reviewed-on: https://boringssl-review.googlesource.com/6980
Reviewed-by: Adam Langley <agl@google.com>
2016-01-26 15:48:41 +00:00
David Benjamin
232127d245 Fold EC_GROUP_new_curve_GFp and EC_GROUP_set_generator into a EC_GROUP_new_arbitrary.
This is only for Conscrypt which always calls the pair in succession. (Indeed
it wouldn't make any sense to not call it.) Remove those two APIs and replace
with a single merged API. This way incomplete EC_GROUPs never escape outside
our API boundary and EC_GROUPs may *finally* be made immutable.

Also add a test for this to make sure I didn't mess it up.

Add a temporary BORINGSSL_201512 define to ease the transition for Conscrypt.
Conscrypt requires https://android-review.googlesource.com/#/c/187801/ before
picking up this change.

Change-Id: I3706c2ceac31ed2313175ba5ee724bd5c74ef6e1
Reviewed-on: https://boringssl-review.googlesource.com/6550
Reviewed-by: Adam Langley <agl@google.com>
2016-01-21 22:35:46 +00:00
David Benjamin
95219feafd Fix some documentation comments.
The new OPENSSL_PRINTF_FORMAT_FUNC macro let doc.go catch a few problems. It
also confuses doc.go, but this CL doesn't address that. At some point we
probably need to give it a real C parser.

Change-Id: I39f945df04520d1e0a0ba390cac7b308baae0622
Reviewed-on: https://boringssl-review.googlesource.com/6940
Reviewed-by: Adam Langley <agl@google.com>
2016-01-21 22:12:08 +00:00
Brian Smith
8d3c43e4b1 Annotate |ERR_add_error_dataf| as |OPENSSL_PRINTF_FORMAT_FUNC|.
Besides being a good idea anyway, this avoids clang warning about using
a non-literal format string when |ERR_add_error_dataf| calls
|BIO_vsnprintf|.

Change-Id: Iebc84d9c9d85e08e93010267d473387b661717a5
Reviewed-on: https://boringssl-review.googlesource.com/6920
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:59:18 +00:00
Brian Smith
061332f216 Define |OPENSSL_PRINTF_FORMAT_FUNC| for format string annotations.
This centralizes the conditional logic into openssl/base.h so that it
doesn't have to be repeated. The name |OPENSSL_PRINTF_FORMAT_FUNC| was
chosen in anticipation of eventually defining an
|OPENSSL_PRINTF_FORMAT_ARG| for MSVC-style parameter annotations.

Change-Id: I273e6eddd209e696dc9f82099008c35b6d477cdb
Reviewed-on: https://boringssl-review.googlesource.com/6909
Reviewed-by: David Benjamin <davidben@google.com>
2016-01-21 20:58:51 +00:00
Adam Langley
3a39b06011 Import “altchains” support.
This change imports the following changes from upstream:

6281abc79623419eae6a64768c478272d5d3a426
dfd3322d72a2d49f597b86dab6f37a8cf0f26dbf
f34b095fab1569d093b639bfcc9a77d6020148ff
21376d8ae310cf0455ca2b73c8e9f77cafeb28dd
25efcb44ac88ab34f60047e16a96c9462fad39c1
56353962e7da7e385c3d577581ccc3015ed6d1dc
39c76ceb2d3e51eaff95e04d6e4448f685718f8d
a3d74afcae435c549de8dbaa219fcb30491c1bfb

These contain the “altchains” functionality which allows OpenSSL to
backtrack when chain building.

Change-Id: I8d4bc2ac67b90091f9d46e7355cae878b4ccf37d
Reviewed-on: https://boringssl-review.googlesource.com/6905
Reviewed-by: Adam Langley <agl@google.com>
2016-01-19 17:02:31 +00:00
Adam Langley
62882187c9 Update comments to better document in-place semantics.
(Comment-only change; no functional difference.)

Some code was broken by the |d2i_ECDSA_SIG| change in 87897a8c. It was
passing in a pointer to an existing |ECDSA_SIG| as the first argument
and then simply assuming that the structure would be updated in place.
The comments on the function suggested that this was reasonable.

This change updates the comments that use similar wording to either note
that the function will never update in-place, or else to note that
depending on that is a bad idea for the future.

I've also audited all the uses of these functions that I can find and,
in addition to the one case with |d2i_ECDSA_SIG|, there are several
users of |d2i_PrivateKey| that could become a problem in the future.
I'll try to fix them before it does become an issue.

Change-Id: I769f7b2e0b5308d09ea07dd447e02fc161795071
Reviewed-on: https://boringssl-review.googlesource.com/6902
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2016-01-19 17:01:37 +00:00
David Benjamin
928f32a492 Add APIs to extract the SSL key block.
This is a companion to SSL_get_rc4_state and SSL_get_ivs which doesn't
require poking at internal state. Partly since it aligns with the
current code and partly the off chance we ever need to get
wpa_supplicant's EAP-FAST code working, the API allows one to generate
more key material than is actually in the key block.

Change-Id: I58bc3f2b017482dbb8567dcd0cd754947a95397f
Reviewed-on: https://boringssl-review.googlesource.com/6839
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-15 22:09:24 +00:00
David Benjamin
79978df4ec Move aead_{read,write}_ctx and next_proto_negotiated into ssl->s3.
Both are connection state rather than configuration state. Notably this
cuts down more of SSL_clear that can't just use ssl_free + ssl_new.

Change-Id: I3c05b3ae86d4db8bd75f1cd21656f57fc5b55ca9
Reviewed-on: https://boringssl-review.googlesource.com/6835
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-15 21:40:25 +00:00
David Benjamin
4119d42e7c Tidy up keyblock and CCS logic slightly.
Move the actual SSL_AEAD_CTX swap into the record layer. Also revise the
intermediate state we store between setup_key_block and
change_cipher_state. With SSL_AEAD_CTX_new abstracted out, keeping the
EVP_AEAD around doesn't make much sense. Just store enough to partition
the key block.

Change-Id: I773fb46a2cb78fa570f00c0a89339c15bbb1d719
Reviewed-on: https://boringssl-review.googlesource.com/6832
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-15 20:40:44 +00:00
David Benjamin
96ba15fc69 Add SSL_get_client_random and SSL_get_server_random.
wpa_supplicant needs to get at the client and server random. OpenSSL
1.1.0 added these APIs, so match their semantics.

Change-Id: I2b71ba850ac63e574c9ea79012d1d0efec5a979a
Reviewed-on: https://boringssl-review.googlesource.com/6830
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-15 20:15:29 +00:00
David Benjamin
fe5f7c7b56 Only reserve EVP_MAX_MD_SIZE for the Finished, not twice of it.
EVP_MAX_MD_SIZE is large enough to fit a MD5/SHA1 concatenation, and
necessarily is because EVP_md5_sha1 exists. This shaves 128 bytes of
per-connection state.

Change-Id: I848a8563dfcbac14735bb7b302263a638528f98e
Reviewed-on: https://boringssl-review.googlesource.com/6804
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 23:29:21 +00:00
David Benjamin
974c7ba4ef Route DHE through the SSL_ECDH abstraction as well.
This unifies the ClientKeyExchange code rather nicely. ServerKeyExchange
is still pretty specialized. For simplicity, I've extended the yaSSL bug
workaround for clients as well as servers rather than route in a
boolean.

Chrome's already banished DHE to a fallback with intention to remove
altogether later, and the spec doesn't say anything useful about
ClientDiffieHellmanPublic encoding, so this is unlikely to cause
problems.

Change-Id: I0355cd1fd0fab5729e8812e4427dd689124f53a2
Reviewed-on: https://boringssl-review.googlesource.com/6784
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 23:17:32 +00:00
David Benjamin
4cc36adf5a Make it possible to tell what curve was used on the server.
We don't actually have an API to let you know if the value is legal to
interpret as a curve ID. (This was kind of a poor API. Oh well.) Also add tests
for key_exchange_info. I've intentionally left server-side plain RSA missing
for now because the SSL_PRIVATE_KEY_METHOD abstraction only gives you bytes and
it's probably better to tweak this API instead.

(key_exchange_info also wasn't populated on the server, though due to a
rebasing error, that fix ended up in the parent CL. Oh well.)

Change-Id: I74a322c8ad03f25b02059da7568c9e1a78419069
Reviewed-on: https://boringssl-review.googlesource.com/6783
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 23:12:25 +00:00
David Benjamin
4298d77379 Implement draft-ietf-tls-curve25519-01 in C.
The new curve is not enabled by default.

As EC_GROUP/EC_POINT is a bit too complex for X25519, this introduces an
SSL_ECDH_METHOD abstraction which wraps just the raw ECDH operation. It
also tidies up some of the curve code which kept converting back and
force between NIDs and curve IDs. Now everything transits as curve IDs
except for API entry points (SSL_set1_curves) which take NIDs. Those
convert immediately and act on curve IDs from then on.

Note that, like the Go implementation, this slightly tweaks the order of
operations. The client sees the server public key before sending its
own. To keep the abstraction simple, SSL_ECDH_METHOD expects to
generate a keypair before consuming the peer's public key. Instead, the
client handshake stashes the serialized peer public value and defers
parsing it until it comes time to send ClientKeyExchange. (This is
analogous to what it was doing before where it stashed the parsed peer
public value instead.)

It still uses TLS 1.2 terminology everywhere, but this abstraction should also
be compatible with TLS 1.3 which unifies (EC)DH-style key exchanges.
(Accordingly, this abstraction intentionally does not handle parsing the
ClientKeyExchange/ServerKeyExchange framing or attempt to handle asynchronous
plain RSA or the authentication bits.)

BUG=571231

Change-Id: Iba09dddee5bcdfeb2b70185308e8ab0632717932
Reviewed-on: https://boringssl-review.googlesource.com/6780
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 21:51:30 +00:00
David Benjamin
c18ef750ee Allocate a NID for X25519.
No corresponding OID, but SSL_CTX_set1_curves assumes NIDs exist.

BUG=571231

Change-Id: Id5221cdc59132e26a89ae5f8978b946de690b4e0
Reviewed-on: https://boringssl-review.googlesource.com/6779
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 18:56:53 +00:00
David Benjamin
2a0b391ac9 Rewrite ssl3_send_server_key_exchange to use CBB.
There is some messiness around saving and restoring the CBB, but this is
still significantly clearer.

Note that the BUF_MEM_grow line is gone in favor of a fixed CBB like the
other functions ported thus far. This line was never necessary as
init_buf is initialized to 16k and none of our key exchanges get that
large. (The largest one can get is DHE_RSA. Even so, it'd take a roughly
30k-bit DH group with a 30k-bit RSA key.)

Having such limits and tight assumptions on init_buf's initial size is
poor (but on par for the old code which usually just blindly assumed the
message would not get too large) and the size of the certificate chain
is much less obviously bounded, so those BUF_MEM_grows can't easily go.

My current plan is convert everything but those which legitimately need
BUF_MEM_grow to CBB, then atomically convert the rest, remove init_buf,
and switch everything to non-fixed CBBs. This will hopefully also
simplify async resumption. In the meantime, having a story for
resumption means the future atomic change is smaller and, more
importantly, relieves some complexity budget in the ServerKeyExchange
code for adding Curve25519.

Change-Id: I1de6af9856caaed353453d92a502ba461a938fbd
Reviewed-on: https://boringssl-review.googlesource.com/6770
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 17:23:58 +00:00
David Benjamin
ae0eaaa397 Convert ssl3_send_client_key_exchange to CBB.
This relieves some complexity budget for adding Curve25519 to this
code.

This also adds a BN_bn2cbb_padded helper function since this seems to be a
fairly common need.

Change-Id: Ied0066fdaec9d02659abd6eb1a13f33502c9e198
Reviewed-on: https://boringssl-review.googlesource.com/6767
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 17:00:56 +00:00
Matt Braithwaite
c75c0ae088 Add #defines for ED25519 key and signature lengths.
Change-Id: I3299f8fc4602c24688339dbffb1d7939282ad6b3
Reviewed-on: https://boringssl-review.googlesource.com/6723
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 16:06:07 +00:00
David Benjamin
4cc671cbf4 Add CBB_reserve and CBB_did_write.
These will be needed when we start writing variable-length things to a
CBB.

Change-Id: Ie7b9b140f5f875b43adedc8203ce9d3f4068dfea
Reviewed-on: https://boringssl-review.googlesource.com/6764
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:23:52 +00:00
David Benjamin
e13263d5e4 Resolve a few old TODOs.
A lot of commented-out code we haven't had to put them back, so these
can go now. Also remove the TODO about OAEP having a weird API. The API
is wrong, but upstream's shipped it with the wrong API, so that's what
it is now.

Change-Id: I7da607cf2d877cbede41ccdada31380f812f6dfa
Reviewed-on: https://boringssl-review.googlesource.com/6763
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:14:35 +00:00
David Benjamin
841934f079 Remove stack macros for nonexistent types.
There's a few that can't work since the types don't even exist.

Change-Id: Idf860b146439c95d33814d25bbc9b8f61774b569
Reviewed-on: https://boringssl-review.googlesource.com/6762
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:12:38 +00:00
David Benjamin
70ab223490 Remove ASN1_R_MALLOC_FAILURE.
There was a TODO to remove it once asn1_mac.h was trimmed. This has now
happened. Remove it and reset error codes for crypto/asn1.

Change-Id: Iaf2f3e75741914415372939471b135618910f95d
Reviewed-on: https://boringssl-review.googlesource.com/6761
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:12:24 +00:00
David Benjamin
3f5b43df07 Simplify RSA key exchange padding check.
This check was fixed a while ago, but it could have been much simpler.

In the RSA key exchange, the expected size of the output is known, making the
padding check much simpler. There isn't any use in exporting the more general
RSA_message_index_PKCS1_type_2. (Without knowing the expected size, any
integrity check or swap to randomness or other mitigation is basically doomed
to fail.)

Verified with the valgrind uninitialized memory trick that we're still
constant-time.

Also update rsa.h to recommend against using the PKCS#1 v1.5 schemes.

Thanks to Ryan Sleevi for the suggestion.

Change-Id: I4328076b1d2e5e06617dd8907cdaa702635c2651
Reviewed-on: https://boringssl-review.googlesource.com/6613
Reviewed-by: Adam Langley <agl@google.com>
2015-12-22 00:10:14 +00:00
David Benjamin
13414b3a04 Implement draft-ietf-tls-chacha20-poly1305-04.
Only ECDHE-based ciphers are implemented. To ease the transition, the
pre-standard cipher shares a name with the standard one. The cipher rule parser
is hacked up to match the name to both ciphers. From the perspective of the
cipher suite configuration language, there is only one cipher.

This does mean it is impossible to disable the old variant without a code
change, but this situation will be very short-lived, so this is fine.

Also take this opportunity to make the CK and TXT names align with convention.

Change-Id: Ie819819c55bce8ff58e533f1dbc8bef5af955c21
Reviewed-on: https://boringssl-review.googlesource.com/6686
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 23:34:56 +00:00
David Benjamin
8ffab72683 Point EVP_aead_chacha20_poly1305 at the standardized version.
The consumers have all been updated, so we can move EVP_aead_chacha20_poly1305
to its final state. Unfortunately, the _rfc7539-suffixed version will need to
stick around for just a hair longer. Also the tls1.h macros, but the remaining
consumers are okay with that changing underneath them.

Change-Id: Ibbb70ec1860d6ac6a7e1d7b45e70fe692bf5ebe5
Reviewed-on: https://boringssl-review.googlesource.com/6600
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 21:22:11 +00:00
David Benjamin
a01deee96b Make CBB_len relative to its argument.
Rather than the length of the top-level CBB, which is kind of odd when ASN.1
length prefixes are not yet determined, return the number of bytes written to
the CBB so far. This can be computed without increasing the size of CBB at all.
Have offset and pending_*.

This means functions which take in a CBB as argument will not be sensitive to
whether the CBB is a top-level or child CBB. The extensions logic had to be
careful to only ever compare differences of lengths, which was awkward.

The reversal will also allow for the following pattern in the future, once
CBB_add_space is split into, say, CBB_reserve and CBB_did_write and we add a
CBB_data:

  uint8_t *signature;
  size_t signature_len = 0;
  if (!CBB_add_asn1(out, &cert, CBB_ASN1_SEQUENCE) ||
      /* Emit the TBSCertificate. */
      !CBB_add_asn1(&cert, &tbs_cert, CBS_ASN1_SEQUENCE) ||
      !CBB_add_tbs_cert_stuff(&tbs_cert, stuff) ||
      !CBB_flush(&cert) ||
      /* Feed it into md_ctx. */
      !EVP_DigestSignInit(&md_ctx, NULL, EVP_sha256(), NULL, pkey) ||
      !EVP_DigestSignUpdate(&md_ctx, CBB_data(&cert), CBB_len(&cert)) ||
      /* Emit the signature algorithm. */
      !CBB_add_asn1(&cert, &sig_alg, CBS_ASN1_SEQUENCE) ||
      !CBB_add_sigalg_stuff(&sig_alg, other_stuff) ||
      /* Emit the signature. */
      !EVP_DigestSignFinal(&md_ctx, NULL, &signature_len) ||
      !CBB_reserve(&cert, &signature, signature_len) ||
      !EVP_DigestSignFinal(&md_ctx, signature, &signature_len) ||
      !CBB_did_write(&cert, signature_len)) {
    goto err;
  }

(Were TBSCertificate not the first field, we'd still have to sample
CBB_len(&cert), but at least that's reasonable straight-forward. The
alternative would be if CBB_data and CBB_len somehow worked on
recently-invalidated CBBs, but that would go wrong once the invalidated CBB's
parent flushed and possibly shifts everything.)

And similar for signing ServerKeyExchange.

Change-Id: I7761e492ae472d7632875b5666b6088970261b14
Reviewed-on: https://boringssl-review.googlesource.com/6681
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 21:16:12 +00:00
David Benjamin
0aff3ffb88 Store the partial block as uint8_t, not uint32_t.
The uint32_t likely dates to them using HASH_LONG everywhere. Nothing ever
touches c->data as a uint32_t, only bytes. (Which makes sense seeing as it
stores the partial block.)

Change-Id: I634cb7f2b6306523aa663f8697b7dc92aa491320
Reviewed-on: https://boringssl-review.googlesource.com/6651
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 19:59:29 +00:00
David Benjamin
78fefbf3bb Reformat md32_common.h, part 2.
Manual tweaks and then clang-formatted again.

Change-Id: I809fdb71b2135343e5c1264dd659b464780fc54a
Reviewed-on: https://boringssl-review.googlesource.com/6649
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 19:52:06 +00:00
David Benjamin
d9f0671bbe Remove |need_record_splitting| from |SSL3_STATE|.
It is redundant given the other state in the connection.

Change-Id: I5dc71627132659ab4316a5ea360c9ca480fb7c6c
Reviewed-on: https://boringssl-review.googlesource.com/6646
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:45:48 +00:00
David Benjamin
cd480380fa Remove unused fields from SSL3_STATE.
These have been unused since we unified everything on EVP_AEAD. I must
have missed them when clearing out dead state. This shaves 136 bytes of
per-connection state.

Change-Id: I705f8de389fd34ab4524554ee9e4b1d6be198994
Reviewed-on: https://boringssl-review.googlesource.com/6645
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:42:56 +00:00
David Benjamin
7fc010014c Slightly simplify SSL3_RECORD.
There's no need to track consumed bytes, so rr->data and rr->off may be
merged together.

Change-Id: I8842d005665ea8b4d4a0cced941f3373872cdac4
Reviewed-on: https://boringssl-review.googlesource.com/6644
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:41:59 +00:00
David Benjamin
ece5ba2797 Reset ssl error codes.
38 error codes have fallen off the list since the last time we did this.

Change-Id: Id7ee30889a5da2f6ab66957fd8e49e97640c8489
Reviewed-on: https://boringssl-review.googlesource.com/6643
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:38:20 +00:00
David Benjamin
a41280d8cb Pull ChangeCipherSpec into the handshake state machine.
This uses ssl3_read_bytes for now. We still need to dismantle that
function and then invert the handshake state machine, but this gets
things closer to the right shape as an intermediate step and is a large
chunk in itself. It simplifies a lot of the CCS/handshake
synchronization as a lot of the invariants much more clearly follow from
the handshake itself.

Tests need to be adjusted since this changes some error codes. Now all
the CCS/Handshake checks fall through to the usual
SSL_R_UNEXPECTED_RECORD codepath. Most of what used to be a special-case
falls out naturally. (If half of Finished was in the same record as the
pre-CCS message, that part of the handshake record would have been left
unconsumed, so read_change_cipher_spec would have noticed, just like
read_app_data would have noticed.)

Change-Id: I15c7501afe523d5062f0e24a3b65f053008d87be
Reviewed-on: https://boringssl-review.googlesource.com/6642
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 18:36:57 +00:00
David Benjamin
8fd5c23218 Simplify fragmented HelloRequest state.
With server-side renegotiation gone, handshake_fragment's only purpose
in life is to handle a fragmented HelloRequest (we probably do need to
support those if some server does 1/n-1 record-splitting on handshake
records). The logic to route the data into
ssl3_read_bytes(SSL3_RT_HANDSHAKE) never happens, and the contents are
always a HelloRequest prefix.

This also trims a tiny bit of per-connection state.

Change-Id: Ia1b0dda5b7e79d817c28da1478640977891ebc97
Reviewed-on: https://boringssl-review.googlesource.com/6641
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 17:45:31 +00:00
David Benjamin
c3ae38b4f8 Remove DH EVP_PKEY hooks.
They would never work. Better notice when callers depend on it than fail at
runtime.

This depends on https://android-review.googlesource.com/#/c/183610/ in
Conscrypt.

Change-Id: I3411f291416df834cf85850890617625a2e76939
Reviewed-on: https://boringssl-review.googlesource.com/6552
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 17:38:06 +00:00
David Benjamin
45dab251f3 Skip free callbacks on empty CRYPTO_EX_DATAs.
Avoids bouncing on the lock, but it doesn't really matter since it's all
taking read locks. If we're declaring that callbacks don't get to see
every object being created, they shouldn't see every object being
destroyed.

CRYPTO_dup_ex_data also already had this optimization, though it wasn't
documented.

BUG=391192

Change-Id: I5b8282335112bca3850a7c0168f8bd7f7d4a2d57
Reviewed-on: https://boringssl-review.googlesource.com/6626
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 21:32:14 +00:00
David Benjamin
8a58933db0 Remove the CRYPTO_EX_new callback.
This callback is never used. The one caller I've ever seen is in Android
code which isn't built with BoringSSL and it was a no-op.

It also doesn't actually make much sense. A callback cannot reasonably
assume that it sees every, say, SSL_CTX created because the index may be
registered after the first SSL_CTX is created. Nor is there any point in
an EX_DATA consumer in one file knowing about an SSL_CTX created in
completely unrelated code.

Replace all the pointers with a typedef to int*. This will ensure code
which passes NULL or 0 continues to compile while breaking code which
passes an actual function.

This simplifies some object creation functions which now needn't worry
about CRYPTO_new_ex_data failing. (Also avoids bouncing on the lock, but
it's taking a read lock, so this doesn't really matter.)

BUG=391192

Change-Id: I02893883c6fa8693682075b7b130aa538a0a1437
Reviewed-on: https://boringssl-review.googlesource.com/6625
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 21:29:46 +00:00
David Benjamin
5ddffbb8bc Make SSL_(CTX_)?set_tmp_ecdh call SSL_(CTX_)?set1_curves.
Then deprecate the old functions. Thanks to upstream's
6977e8ee4a718a76351ba5275a9f0be4e530eab5 for the idea.

Change-Id: I916abd6fca2a3b2a439ec9902d9779707f7e41eb
Reviewed-on: https://boringssl-review.googlesource.com/6622
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 20:28:47 +00:00
David Benjamin
53e5c2c225 Remove SSL_(CTX_)?set_ecdh_callback.
It has no callers. I prepped for its removal earlier with
c05697c2c5
and then completely forgot.

Thanks to upstream's 6f78b9e824c053d062188578635c575017b587c5 for
the reminder. Quoth them:

> This only gets used to set a specific curve without actually checking
> that the peer supports it or not and can therefor result in handshake
> failures that can be avoided by selecting a different cipher.

It's also a very confusing API since it does NOT pass ownership of the
EC_KEY to the caller.

Change-Id: I6a00643b3a2d6746e9e0e228b47c2bc9694b0084
Reviewed-on: https://boringssl-review.googlesource.com/6621
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 20:07:37 +00:00
David Benjamin
e9cddb8879 Remove SSL_OP_LEGACY_SERVER_CONNECT.
I don't think we're ever going to manage to enforce this, and it doesn't
seem worth the trouble. We don't support application protocols which use
renegotiation outside of the HTTP/1.1 mid-stream client auth hack.
There, it's on the server to reject legacy renegotiations.

This removes the last of SSL_OP_ALL.

Change-Id: I996fdeaabf175b6facb4f687436549c0d3bb0042
Reviewed-on: https://boringssl-review.googlesource.com/6580
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 19:22:53 +00:00
David Benjamin
03f000577f Remove SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER.
This dates to SSLeay 0.8.0 (or earlier). The use counter sees virtually
no hits.

Change-Id: Iff4c8899d5cb0ba4afca113c66d15f1d980ffe41
Reviewed-on: https://boringssl-review.googlesource.com/6558
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 19:14:00 +00:00
David Benjamin
ef5e515819 Remove SSL_OP_TLS_D5_BUG.
This dates to SSLeay 0.9.0. The Internet seems to have completely
forgotten what "D5" is. (I can't find reference to it beyond
documentation of this quirk.) The use counter we added sees virtually no
hits.

Change-Id: I9781d401acb98ce3790b1b165fc257a6f5e9b155
Reviewed-on: https://boringssl-review.googlesource.com/6557
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 19:11:41 +00:00
Brian Smith
a3d9de05fb Add |EC_GROUP_get0_order| to replace |EC_GROUP_get_order|.
|EC_GROUP_get0_order| doesn't require any heap allocations and never
fails, so it is much more convenient and more efficient for callers to
call.

Change-Id: Ic60f768875e7bc8e74362dacdb5cbbc6957b05a6
Reviewed-on: https://boringssl-review.googlesource.com/6532
Reviewed-by: Adam Langley <agl@google.com>
2015-12-15 18:18:13 +00:00
Joachim Bauch
afd565ff9c Add defines for SRTP profiles using GCM ciphers from RFC 7714.
BUG=webrtc:5222

Change-Id: I8399bd595564dedbe5492b8ea6eb915f41367cbf
Reviewed-on: https://boringssl-review.googlesource.com/6690
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2015-12-10 23:18:16 +00:00
Adam Langley
902870e3b5 Gate SHA_CTX compatibility on !WINDOWS.
Windows does support anonymous unions but warns about it. Since I'm not
sure what warnings we have enabled in Chromium, this change just drops
the union for Windows.

Change-Id: I914f8cd5855eb07153105250c0f026eaedb35365
Reviewed-on: https://boringssl-review.googlesource.com/6631
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-12-04 22:13:19 +00:00
Adam Langley
34aa55c05e Support the SHA_CTX hack without ANDROID.
wpa_supplicant needs access to the internals of SHA_CTX. We supported
this only for builds with ANDROID defined previously but that's a pain
for wpa_supplicant to deal with. Thus this change enables it
unconditionally.

Perhaps in the future we'll be able to get a function to do this into
OpenSSL and BoringSSL.

Change-Id: Ib5d088c586fe69249c87404adb45aab5a7d5cf80
Reviewed-on: https://boringssl-review.googlesource.com/6630
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-12-04 20:23:46 +00:00
David Benjamin
758d12732a Add get0 getters for EVP_PKEY.
Right now your options are:
- Bounce on a reference and deal with cleanup needlessly.
- Manually check the type tag and peek into the union.

We probably have no hope of opaquifying this struct, but for new code, let's
recommend using this function rather than the more error-prone thing.

Change-Id: I9b39ff95fe4264a3f7d1e0d2894db337aa968f6c
Reviewed-on: https://boringssl-review.googlesource.com/6551
Reviewed-by: Adam Langley <agl@google.com>
2015-11-20 23:34:12 +00:00
Brian Smith
bf762186c6 Remove the |ri| field of |BN_MONT_CTX|.
The |ri| field was only used in |BN_MONT_CTX_set|, so make it a local
variable of that function.

Change-Id: Id8c3d44ac2e30e3961311a7b1a6731fe2c33a0eb
Reviewed-on: https://boringssl-review.googlesource.com/6526
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:40:13 +00:00
David Benjamin
9f2e2770e1 Remove strength_bits.
Trim the cipher table further. Those values are entirely determined by
algorithm_enc.

Change-Id: I355c245b0663e41e54e62d15903a4a9a667b4ffe
Reviewed-on: https://boringssl-review.googlesource.com/6516
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:32:28 +00:00
David Benjamin
d6e9eec3f8 Remove algo_strength.
FIPS is the same as HIGH (but for CHACHA20), so those are redundant.
Likewise, MEDIUM vs HIGH was just RC4. Remove those in favor of
redefining those legacy rules to mean this.

One less field to keep track of in each cipher.

Change-Id: I2b2489cffb9e16efb0ac7d7290c173cac061432a
Reviewed-on: https://boringssl-review.googlesource.com/6515
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:30:44 +00:00
David Benjamin
dcb6ef0f0b Remove algorithm_ssl.
It's redundant with other cipher properties. We can express these in code.
Cipher rule matching gets a little bit complicated due to the confusing legacy
protocol version cipher rules, so add some tests for it. (It's really hard to
grep for uses of them, so I've kept them working to be safe.)

Change-Id: Ic6b3fcd55d76d4a51b31bf7ae629a2da50a7450e
Reviewed-on: https://boringssl-review.googlesource.com/6453
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:28:24 +00:00
David Benjamin
d28f59c27b Switch the keylog BIO to a callback.
The keylog BIO is internally synchronized by the SSL_CTX lock, but an
application may wish to log keys from multiple SSL_CTXs. This is in
preparation for switching Chromium to use a separate SSL_CTX per profile
to more naturally split up the session caches.

It will also be useful for routing up SSLKEYLOGFILE in WebRTC. There,
each log line must be converted to an IPC up from the renderer
processes.

This will require changes in Chromium when we roll BoringSSL.

BUG=458365,webrtc:4417

Change-Id: I2945bdb4def0a9c36e751eab3d5b06c330d66b54
Reviewed-on: https://boringssl-review.googlesource.com/6514
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:23:49 +00:00
Brian Smith
f3376ace43 Remove |EC_POINTs_mul| & simplify p256-x86_64.
Without |EC_POINTs_mul|, there's never more than one variable point
passed to a |EC_METHOD|'s |mul| method. This allows them to be
simplified considerably. In this commit, the p256-x86_64 implementation
has been simplified to eliminate the heap allocation and looping
related that was previously necessary to deal with the possibility of
there being multiple input points. The other implementations were left
mostly as-is; they should be similarly simplified in the future.

Change-Id: I70751d1d5296be2562af0730e7ccefdba7a1acae
Reviewed-on: https://boringssl-review.googlesource.com/6493
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:08:46 +00:00
Brian Smith
e2136d9c28 Remove |EC_GROUP_precompute_mult| and |EC_KEY_precompute_mult|.
Change-Id: I1663ec6046b8f1f67a62e4c6483af719d6f362ad
Reviewed-on: https://boringssl-review.googlesource.com/6486
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:35:59 +00:00
Brian Smith
9b26297608 Make |EC_GROUP_precompute_mult|/|EC_KEY_precompute_mult| no-ops.
This moves us closer to having |EC_GROUP| and |EC_KEY| being immutable.
The functions are left as no-ops for backward compatibility.

Change-Id: Ie23921ab0364f0771c03aede37b064804c9f69e0
Reviewed-on: https://boringssl-review.googlesource.com/6485
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:27:39 +00:00
Adam Langley
9e65d487b8 Allow |CRYPTO_is_NEON_capable| to be known at compile time, if possible.
If -mfpu=neon is passed then we don't need to worry about checking for
NEON support at run time. This change allows |CRYPTO_is_NEON_capable| to
statically return 1 in this case. This then allows the compiler to
discard generic code in several cases.

Change-Id: I3b229740ea3d5cb0a304f365c400a0996d0c66ef
Reviewed-on: https://boringssl-review.googlesource.com/6523
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 00:15:11 +00:00
Adam Langley
4fb0dc4b03 Add X25519 and Ed25519 support.
(Ed25519 support is disabled when |OPENSSL_SMALL| is defined.)

libcrypto.a sizes:

x86-64 -O3 -march=native: +78012 (1584902 → 1662914)
x86-64 -O3 -march=native -DOPENSSL_SMALL: +10596 (1356206 → 1366802)
Android armv7 Thumb -O2 -DOPENSSL_SMALL: +13132 (1258462 → 1271594)

Change-Id: I6a7e64d481e4ce4daa7d5057578081358746cfb9
Reviewed-on: https://boringssl-review.googlesource.com/6497
Reviewed-by: Adam Langley <agl@google.com>
2015-11-17 21:56:12 +00:00
Adam Langley
b00061cea7 Add SSL_CIPHER_is_AES[128|256]CBC.
Change-Id: I3072f884be77b8646e90d316154b96448f0cf2a1
Reviewed-on: https://boringssl-review.googlesource.com/6520
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-17 19:15:06 +00:00
David Benjamin
3a59611726 size_t SSL*_use_*_ASN1.
So long as we're not getting rid of them (the certificate variants may
be useful when we decouple from crypto/x509 anyway), get the types and
bounds checks right.

Also reject trailing data and require the input be a single element.
Note: this is a slight compatibility risk, but we did it for
SSL*_use_RSAPrivateKey_ASN1 previously and I think it's probably worth
seeing if anything breaks here.

Change-Id: I64fa3fc6249021ccf59584d68e56ff424a190082
Reviewed-on: https://boringssl-review.googlesource.com/6490
Reviewed-by: Adam Langley <agl@google.com>
2015-11-16 23:59:14 +00:00
Adam Langley
c5c85defb2 Make RAND_seed read a byte of random data.
OpenSSH calls |RAND_seed| before jailing in the expectation that that
will be sufficient to ensure that later RAND calls are successful.

See internal bug 25695426.

Change-Id: I9d3f5665249af6610328ac767cb83059bb2953dd
Reviewed-on: https://boringssl-review.googlesource.com/6494
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-16 21:58:46 +00:00
Brian Smith
8bde5d2e51 Remove the unused |Ni| member of |BN_MONT_CTX|.
Change-Id: I0a542c48c7adae28f05778d6c34c9b6836fc3449
Reviewed-on: https://boringssl-review.googlesource.com/6480
Reviewed-by: Adam Langley <agl@google.com>
2015-11-12 20:04:43 +00:00
David Benjamin
cd24a39f1b Limit DHE groups to 4096-bit.
dh.c had a 10k-bit limit but it wasn't quite correctly enforced. However,
that's still 1.12s of jank on the IO thread, which is too long. Since the SSL
code consumes DHE groups from the network, it should be responsible for
enforcing what sanity it needs on them.

Costs of various bit lengths on 2013 Macbook Air:
1024 - 1.4ms
2048 - 14ms
3072 - 24ms
4096 - 55ms
5000 - 160ms
10000 - 1.12s

UMA says that DHE groups are 0.2% 4096-bit and otherwise are 5.5% 2048-bit and
94% 1024-bit and some noise. Set the limit to 4096-bit to be conservative,
although that's already quite a lot of jank.

BUG=554295

Change-Id: I8e167748a67e4e1adfb62d73dfff094abfa7d215
Reviewed-on: https://boringssl-review.googlesource.com/6464
Reviewed-by: Adam Langley <agl@google.com>
2015-11-11 22:18:39 +00:00
David Benjamin
6e80765774 Add SSL_get_server_key_exchange_hash.
This exposes the ServerKeyExchange signature hash type used in the most recent
handshake, for histogramming on the client.

BUG=549662

Change-Id: I8a4e00ac735b1ecd2c2df824112c3a0bc62332a7
Reviewed-on: https://boringssl-review.googlesource.com/6413
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 22:35:28 +00:00
Adam Langley
59b0fccb51 Define BORINGSSL_201510.
I've used these defines to easy the update of BoringSSL in Android
because Android's external/boringssl is a different git repository from
the rest of Android and thus it's not possible to land changes the
atomically update several things at once.

For this I tended just to add this define in the Android copy of
BoringSSL, but we're starting to see that bleed into other situations
now so it's looking like this will be generally useful.

These defines may be added when useful but shouldn't build up: once the
change has been done, the #if'ed code elsewhere that uses it should be
cleaned up. So far, that's worked ok. (I.e. we've had a BORINGSSL_201509
that correctly disappeared.)

Change-Id: I8cbb4731efe840cc798c970d37bc040b16a4a755
Reviewed-on: https://boringssl-review.googlesource.com/6442
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 21:44:17 +00:00
David Benjamin
e6d1e5a871 Use typedef names, not struct names.
Not sure if we want to leave bio.h and bytestring.h's instance as-is, but the
evp.h ones are just baffling.

Change-Id: I485c2e355ba93764da0c4c72c48af48b055a8500
Reviewed-on: https://boringssl-review.googlesource.com/6454
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 21:44:06 +00:00
David Benjamin
e82e6f6696 Constify more BN_MONT_CTX parameters.
Most functions can take this in as const. Note this changes an
RSA_METHOD hook, though one I would not expect anyone to override.

Change-Id: Ib70ae65e5876b01169bdc594e465e3e3c4319a8b
Reviewed-on: https://boringssl-review.googlesource.com/6419
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 20:04:36 +00:00
David Benjamin
c7817d8ce2 Add SSL_CIPHER_get_min_version and tidy up SSL_TLSV1_2 logic.
Later when TLS 1.3 comes around, we'll need SSL_CIPHER_get_max_version too. In
the meantime, hide the SSL_TLSV1_2 messiness behind a reasonable API.

Change-Id: Ibcc17cccf48dd99e364d6defdfa5a87d031ecf0a
Reviewed-on: https://boringssl-review.googlesource.com/6452
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 19:56:29 +00:00
Adam Langley
38feb990a1 Require that EC points are on the curve.
This removes a sharp corner in the API where |ECDH_compute_key| assumed
that callers were either using ephemeral keys, or else had already
checked that the public key was on the curve.

A public key that's not on the curve can be in a small subgroup and thus
the result can leak information about the private key.

This change causes |EC_POINT_set_affine_coordinates_GFp| to require that
points are on the curve. |EC_POINT_oct2point| already does this.

Change-Id: I77d10ce117b6efd87ebb4a631be3a9630f5e6636
Reviewed-on: https://boringssl-review.googlesource.com/5861
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 19:35:42 +00:00
David Benjamin
ef793f4b6f Add various functions for SSL_CIPHER.
Change-Id: I21051a6d1594c2606e171449d377663f8eccc847
Reviewed-on: https://boringssl-review.googlesource.com/6450
Reviewed-by: Adam Langley <agl@google.com>
2015-11-06 19:26:22 +00:00
Adam Langley
5f88999a1e Fix up several comments and detect problems in the future.
This change fixes up several comments (many of which were spotted by
Kenny Root) and also changes doc.go to detect cases where comments don't
start with the correct word. (This is a common error.)

Since we have docs builders now, these errors will be found
automatically in the future.

Change-Id: I58c6dd4266bf3bd4ec748763c8762b1a67ae5ab3
Reviewed-on: https://boringssl-review.googlesource.com/6440
Reviewed-by: Adam Langley <agl@google.com>
2015-11-05 20:12:45 +00:00
Adam Langley
e57a19203f Add missing newline in aead.h.
c2d3280f was missing a newline before the trailer.

Change-Id: I0118259b7a8ab15aaaa55125a0f92f3a97794b81
2015-11-04 11:53:46 -08:00
Adam Langley
c2d3280f0f Add SSL_get_ivs.
This function allows one to extract the current IVs from an SSL
connection. This is needed for the CBC cipher suites with implicit IVs
because, for those, the IV can't be extracted from the handshake key
material.

Change-Id: I247a1d0813b7a434b3cfc88db86d2fe8754344b6
Reviewed-on: https://boringssl-review.googlesource.com/6433
Reviewed-by: Adam Langley <agl@google.com>
2015-11-04 19:45:28 +00:00
David Benjamin
a97b737fb0 Separate CCS and handshake writing in DTLS.
They run through completely different logic as only handshake is fragmented.
This'll make it easier to rewrite the handshake logic in a follow-up.

Change-Id: I9515feafc06bf069b261073873966e72fcbe13cb
Reviewed-on: https://boringssl-review.googlesource.com/6420
Reviewed-by: Adam Langley <agl@google.com>
2015-11-04 00:11:14 +00:00
Brian Smith
ac9404c3a8 Improve crypto/digest/md32_common.h mechanism.
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.

Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations took a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.

The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.

The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.

This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.

Change-Id: I6e9dd09ff057c67941021d324a4fa1d39f58b0db
Reviewed-on: https://boringssl-review.googlesource.com/6405
Reviewed-by: Adam Langley <agl@google.com>
2015-11-04 00:01:09 +00:00
David Benjamin
8fb0f525e1 Free BN_MONT_CTX in generic code.
Although those are only created by code owned by RSA_METHOD, custom RSA_METHODs
shouldn't be allowed to squat our internal fields and then change how you free
things.

Remove 'method' from their names now that they're not method-specific.

Change-Id: I9494ef9a7754ad59ac9fba7fd463b3336d826e0b
Reviewed-on: https://boringssl-review.googlesource.com/6423
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 23:39:41 +00:00
David Benjamin
e8f783ac0d Unwind DH_METHOD and DSA_METHOD.
This will allow a static linker (with -ffunction-sections since things aren't
split into files) to drop unused parts of DH and DSA. Notably, the parameter
generation bits pull in primality-checking code.

Change-Id: I25087e4cb91bc9d0ab43bcb267c2e2c164e56b59
Reviewed-on: https://boringssl-review.googlesource.com/6388
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 22:54:36 +00:00
David Benjamin
3fc138eccd Don't bother sampling __func__.
Removing the function codes continued to sample __func__ for compatibility with
ERR_print_errors_cb, but not ERR_error_string_n. We can just emit
OPENSSL_internal for both. ERR_print_errors_cb already has the file and line
number available which is strictly more information than the function name.
(ERR_error_string_n does not, but we'd already turned that to
OPENSSL_internal.)

This shaves 100kb from a release build of the bssl tool.

In doing so, put an unused function code parameter back into ERR_put_error to
align with OpenSSL. We don't need to pass an additional string in anymore, so
OpenSSL compatibility with anything which uses ERR_LIB_USER or
ERR_get_next_error_library costs nothing. (Not that we need it.)

Change-Id: If6af34628319ade4145190b6f30a0d820e00b20d
Reviewed-on: https://boringssl-review.googlesource.com/6387
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 22:50:59 +00:00
Adam Langley
27a0d086f7 Add ssl_renegotiate_ignore.
This option causes clients to ignore HelloRequest messages completely.
This can be suitable in cases where a server tries to perform concurrent
application data and handshake flow, e.g. because they are trying to
“renew” symmetric keys.

Change-Id: I2779f7eff30d82163f2c34a625ec91dc34fab548
Reviewed-on: https://boringssl-review.googlesource.com/6431
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 21:58:13 +00:00
Matt Braithwaite
fa9eb568b0 Correct the spelling of "primitive".
Spotted by Matt Smart.

Change-Id: Id9c61ba6a293ddc52b2e2c93c427860765848c6d
Reviewed-on: https://boringssl-review.googlesource.com/6430
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 21:47:19 +00:00
Adam Langley
f1c1cf8794 Revert "Improve crypto/digest/md32_common.h mechanism."
This reverts commit 00461cf201.

Sadly it broke wpa_supplicant.
2015-11-02 18:14:34 -08:00
Brian Smith
00461cf201 Improve crypto/digest/md32_common.h mechanism.
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.

Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations tool a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.

The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.

The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.

This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.

Change-Id: I30513bb40b5f1d2c8932551d54073c35484b3f8b
Reviewed-on: https://boringssl-review.googlesource.com/6401
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 02:04:38 +00:00
Adam Langley
efb42fbb60 Make BN_mod_exp_mont_consttime take a const context.
BN_mod_exp_mont_consttime does not modify its |BN_MONT_CTX| so that
value should be const.

Change-Id: Ie74e48eec8061899fd056fbd99dcca2a86b02cad
Reviewed-on: https://boringssl-review.googlesource.com/6403
Reviewed-by: Adam Langley <agl@google.com>
2015-11-03 01:58:12 +00:00
David Benjamin
13e81fc971 Fix DTLS asynchronous write handling.
Although the DTLS transport layer logic drops failed writes on the floor, it is
actually set up to work correctly. If an SSL_write fails at the transport,
dropping the buffer is fine. Arguably it works better than in TLS because we
don't have the weird "half-committed to data" behavior. Likewise, the handshake
keeps track of how far its gotten and resumes the message at the right point.

This broke when the buffering logic was rewritten because I didn't understand
what the DTLS code was doing. The one thing that doesn't work as one might
expect is non-fatal write errors during rexmit are not recoverable. The next
timeout must fire before we try again.

This code is quite badly sprinkled in here, so add tests to guard it against
future turbulence. Because of the rexmit issues, the tests need some hacks
around calls which may trigger them. It also changes the Go DTLS implementation
from being completely strict about sequence numbers to only requiring they be
monotonic.

The tests also revealed another bug. This one seems to be upstream's fault, not
mine. The logic to reset the handshake hash on the second ClientHello (in the
HelloVerifyRequest case) was a little overenthusiastic and breaks if the
ClientHello took multiple tries to send.

Change-Id: I9b38b93fff7ae62faf8e36c4beaf848850b3f4b9
Reviewed-on: https://boringssl-review.googlesource.com/6417
Reviewed-by: Adam Langley <agl@google.com>
2015-11-02 23:16:22 +00:00
Torbjörn Granlund
069bedfe0c Fix documentation typo.
Change-Id: Ia060a9f8c30ea7446ddffcc2221a8f5a37f14d8c
Reviewed-on: https://boringssl-review.googlesource.com/6415
Reviewed-by: Adam Langley <agl@google.com>
2015-11-02 22:53:27 +00:00
Adam Langley
bb85f3d655 Reorganise |SSL_SESSION| and |SSL| to save a little memory.
This is a fairly timid, first step at trying to pack common structures a
little better.

This change reorders a couple of structures a little and turns some
variables into bit-fields. Much more can still be done.

Change-Id: Idbe0f54d66559c0ad654bf7e8dea277a771a568f
Reviewed-on: https://boringssl-review.googlesource.com/6394
Reviewed-by: Adam Langley <agl@google.com>
2015-10-30 21:01:09 +00:00
David Benjamin
8e4db8b1c2 Fix even more ChaCha20 rename deadlocks.
QUIC code references the TXT macro. Also get rid of
TLS1_TXT_DHE_RSA_WITH_CHACHA20_POLY1305 which wasn't renamed for some reason.

Change-Id: I0308e07104b3cec394d748f3f1146bd786d2ace2
Reviewed-on: https://boringssl-review.googlesource.com/6384
Reviewed-by: Adam Langley <agl@google.com>
2015-10-28 21:09:46 +00:00
David Benjamin
87cfcdb6df Resolve another ChaCha20-Poly1305 rename deadlock.
WebRTC can't roll into Chromium without picking up the iOS build fix, but we
can't roll BoringSSL forwards because WebRTC also depends on the previously
exposed ChaCha20-Poly1305 cipher suite constants.

Define the old constants again.

Change-Id: If8434a0317e42b3aebe1bc1c5a58ed97a89a0230
Reviewed-on: https://boringssl-review.googlesource.com/6382
Reviewed-by: Adam Langley <agl@google.com>
2015-10-28 19:25:36 +00:00
David Benjamin
7dd3f77256 Fix aead.h header typo.
EVP_aead_chacha20_poly1305_old is listed twice instead of
EVP_aead_chacha20_poly1305.

Change-Id: I281eee7a8359cd2a2b04047c829ef351ea4a7b82
Reviewed-on: https://boringssl-review.googlesource.com/6381
Reviewed-by: Adam Langley <agl@google.com>
2015-10-28 17:50:23 +00:00
David Benjamin
f8e9dcaeea iOS builds use the static ARM CPU configuration.
The other codepath is Linux-specific. This should get tidied up a bit but, in
the meantime, fix the Chromium iOS (NO_ASM) build. Even when the assembly gets
working, it seems iOS prefers you make fat binaries rather than detect features
at runtime, so this is what we want anyway.

BUG=548539

Change-Id: If19b2e380a96918b07bacc300a3a27b885697b99
Reviewed-on: https://boringssl-review.googlesource.com/6380
Reviewed-by: Adam Langley <agl@google.com>
2015-10-28 17:25:25 +00:00
Brian Smith
0dc2a8aee2 Clean up |ECDH_compute_key|.
1. Check for the presence of the private key before allocating or
   computing anything.
2. Check the return value of |BN_CTX_get|.
3. Don't bother computing the Y coordinate since it is not used.
4. Remove conditional logic in cleanup section.

Change-Id: I4d8611603363c7e5d16a8e9f1d6c3a56809f27ae
Reviewed-on: https://boringssl-review.googlesource.com/6171
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 17:00:25 +00:00
Brian Smith
274341dd6e Change the type of |EC_GROUP_get_degree| and friends to |unsigned|.
These functions ultimately return the result of |BN_num_bits|, and that
function's return type is |unsigned|. Thus, these functions' return
type should also be |unsigned|.

Change-Id: I2cef63e6f75425857bac71f7c5517ef22ab2296b
Reviewed-on: https://boringssl-review.googlesource.com/6170
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:48:04 +00:00
David Benjamin
e93ffa5da7 Clarify that SSL_get_peer_cert_chain returns the unverified chain.
This came up and I wasn't sure which it was without source-diving.

Change-Id: Ie659096e0f42a7448f81dfb1006c125d292fd7fd
Reviewed-on: https://boringssl-review.googlesource.com/6354
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:42:30 +00:00
Adam Langley
05ee4fda1c Add no-op functions |CRYPTO_malloc_init| and |ENGINE_load_builtin_engines|.
This reduces the impact on Netty. See
904b84ce41 (commitcomment-12159877)

Change-Id: I22f9e1edaeb9e721326867ae2b4f3da2c5441437
Reviewed-on: https://boringssl-review.googlesource.com/5535
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:41:40 +00:00
Adam Langley
5dbdad9c33 For now, give the unsuffixed ChaCha20 AEAD name to the old version.
QUIC has a complex relationship with BoringSSL owing to it living both
in Chromium and the Google-internal repository. In order for it to
handle the ChaCha20-Poly1305 AEAD switch more easily this change gives
the unsuffixed name to the old AEAD, for now.

Once QUIC has moved to the “_old” version the unsuffixed name can be
given to the new version.

Change-Id: Id8a77be6e3fe2358d78e022413fe088e5a274dca
Reviewed-on: https://boringssl-review.googlesource.com/6361
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 16:39:12 +00:00
Brian Smith
271777f5ac Refactor ChaCha20-Poly1305 AEAD nonce handling.
This change reduces unnecessary copying and makes the pre-RFC-7539
nonces 96 bits just like the AES-GCM, AES-CCM, and RFC 7539
ChaCha20-Poly1305 cipher suites. Also, all the symbols related to
the pre-RFC-7539 cipher suites now have "_OLD" appended, in
preparation for adding the RFC 7539 variants.

Change-Id: I1f85bd825b383c3134df0b6214266069ded029ae
Reviewed-on: https://boringssl-review.googlesource.com/6103
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 01:01:42 +00:00
Brian Smith
3e23e4cb58 Add the RFC 7539 ChaCha20-Poly1305 AEAD.
Change-Id: I07dfde7cc304d903c2253600905cc3e6257716c5
Reviewed-on: https://boringssl-review.googlesource.com/6101
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-27 00:46:36 +00:00
Brian Smith
e80a2ecd0d Change |CRYPTO_chacha_20| to use 96-bit nonces, 32-bit counters.
The new function |CRYPTO_chacha_96_bit_nonce_from_64_bit_nonce| can be
used to adapt code from that uses 64 bit nonces, in a way that is
compatible with the old semantics.

Change-Id: I83d5b2d482e006e82982f58c9f981e8078c3e1b0
Reviewed-on: https://boringssl-review.googlesource.com/6100
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 23:58:46 +00:00
David Benjamin
da084a3ebd Fix shared library build on OS X.
It seems OS X actually cares about symbol resolution and dependencies
when you create a dylib. Probably because they do two-level name
resolution.

(Obligatory disclaimer: BoringSSL does not have a stable ABI and is thus
not suitable for a traditional system-wide library.)

BUG=539603

Change-Id: Ic26c4ad23840fe6c1f4825c44671e74dd2e33870
Reviewed-on: https://boringssl-review.googlesource.com/6131
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 23:39:47 +00:00
Adam Langley
0f9f0ead2e Fix the shared builders by exporting GCM symbols.
gcm_test.cc needs to access the internal GCM symbols. This is
unfortunate because it means that they have to be marked OPENSSL_EXPORT
just for this.

To compensate, modes.h is removed and its contents copied into
crypto/modes/internal.h.

Change-Id: I1777b2ef8afd154c43417137673a28598a7ec30e
Reviewed-on: https://boringssl-review.googlesource.com/6360
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 23:26:40 +00:00
Brian Smith
0f8bfdeb33 Make |gcm128_context| memcpy-safe.
This removes the confusion about whether |gcm128_context| copies the
key (it didn't) or whether the caller is responsible for keeping the
key alive for the lifetime of the |gcm128_context| (it was).

Change-Id: Ia0ad0a8223e664381fbbfb56570b2545f51cad9f
Reviewed-on: https://boringssl-review.googlesource.com/6053
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 22:05:33 +00:00
Brian Smith
3f3f25d8a2 Fix constness of |gcm128_context.key|.
The key is never modified through the key pointer member, and the
calling code relies on that fact for maintaining its own
const-correctness.

Change-Id: I63946451aa7c400cd127895a61c30d9a647b1b8c
Reviewed-on: https://boringssl-review.googlesource.com/6040
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 21:40:04 +00:00
Matt Braithwaite
29d8adbdc6 Better handle IPv6.
∙ host:port parsing, where unavoidable, is now IPv6-friendly.
  ∙ |BIO_C_GET_CONNECT| is simply removed.
  ∙ bssl -accept now listens on both IPv6 and IPv4.

Change-Id: I1cbd8a79c0199bab3ced4c4fd79d2cc5240f250c
Reviewed-on: https://boringssl-review.googlesource.com/6214
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 21:06:01 +00:00
nagendra modadugu
3398dbf279 Add server-side support for asynchronous RSA decryption.
Change-Id: I6df623f3e9bc88acc52043f16b34649b7af67663
Reviewed-on: https://boringssl-review.googlesource.com/5531
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 20:26:20 +00:00
David Benjamin
091c4b9869 Add an option to disable NPN on a per-SSL basis.
Right whether NPN is advertised can only be configured globally on the SSL_CTX.
Rather than adding two pointers to each SSL*, add an options bit to disable it
so we may plumb in a field trial to disable NPN.

Chromium wants to be able to route a bit in to disable NPN, but it uses SSL_CTX
incorrectly and has a global one, so it can't disconnect the callback. (That
really needs to get fixed. Although it's not clear this necessarily wants to be
lifted up to SSL_CTX as far as Chromium's SSLClientSocket is concerned since
NPN doesn't interact with the session cache.)

BUG=526713

Change-Id: I49c86828b963eb341c6ea6a442557b7dfa190ed3
Reviewed-on: https://boringssl-review.googlesource.com/6351
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:56:52 +00:00
David Benjamin
12f7737d32 Remove BN_MONT_CTX_init.
One less exported function. Nothing ever stack-allocates them, within BoringSSL
or in consumers. This avoids the slightly odd mechanism where BN_MONT_CTX_free
might or might not free the BN_MONT_CTX itself based on a flag.

(This is also consistent with OpenSSL 1.1.x which does away with the _init
variants of both this and BIGNUM so it shouldn't be a compatibility concern
long-term either.)

Change-Id: Id885ae35a26f75686cc68a8aa971e2ea6767ba88
Reviewed-on: https://boringssl-review.googlesource.com/6350
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:47:26 +00:00
David Benjamin
1269ddd377 Never use the internal session cache for a client.
The internal session cache is keyed on session ID, so this is completely
useless for clients (indeed we never look it up internally). Along the way,
tidy up ssl_update_cache to be more readable. The slight behavior change is
that SSL_CTX_add_session's return code no longer controls the external
callback. It's not clear to me what that could have accomplished. (It can only
fail on allocation error. We only call it for new sessions, so the duplicate
case is impossible.)

The one thing of value the internal cache might have provided is managing the
timeout. The SSL_CTX_flush_sessions logic would flip the not_resumable bit and
cause us not to offer expired sessions (modulo SSL_CTX_flush_sessions's delay
and any discrepancies between the two caches). Instead, just check expiration
when deciding whether or not to offer a session.

This way clients that set SSL_SESS_CACHE_CLIENT blindly don't accidentally
consume gobs of memory.

BUG=531194

Change-Id: If97485beab21874f37737edc44df24e61ce23705
Reviewed-on: https://boringssl-review.googlesource.com/6321
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:27:28 +00:00
David Benjamin
0f653957c1 Add tests for the internal session cache behavior.
In doing so, fix the documentation for SSL_CTX_add_session and
SSL_CTX_remove_session. I misread the code and documented the behavior
on session ID collision wrong.

Change-Id: I6f364305e1f092b9eb0b1402962fd04577269d30
Reviewed-on: https://boringssl-review.googlesource.com/6319
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:18:44 +00:00
David Benjamin
dc2aea2231 Remove all the logic around custom session IDs and retrying on collisions.
A random 32-byte (so 256-bit) session ID is never going to collide with
an existing one. (And, if it does, SSL_CTX_add_session does account for
this, so the server won't explode. Just attempting to resume some
session will fail.)

That logic didn't completely work anyway as it didn't account for
external session caches or multiple connections picking the same ID in
parallel (generation and insertion happen at different times) or
multiple servers sharing one cache. In theory one could fix this by
passing in a sufficiently clever generate_session_id, but no one does
that.

I found no callers of these functions, so just remove them altogether.

Change-Id: I8500c592cf4676de6d7194d611b99e9e76f150a7
Reviewed-on: https://boringssl-review.googlesource.com/6318
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:00:14 +00:00
David Benjamin
60be027625 Style: fix some header guards
Change-Id: I86c30c7fe489c720f83f744696df0a0a20268531
Reviewed-on: https://boringssl-review.googlesource.com/6317
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:47:51 +00:00
David Benjamin
f91fa5cfc6 Documentation typo.
Change-Id: Iedcba0ac15bc14def9c2dc2407ed29d130133c0c
Reviewed-on: https://boringssl-review.googlesource.com/6315
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:47:31 +00:00
David Benjamin
5ef619ef2a Hide some sections from the docs.
Private structs shouldn't be shown. Also there's a few sections that are
really more implementation details than anything else.

Change-Id: Ibc5a23ba818ab0531d9c68e7ce348f1eabbcd19a
Reviewed-on: https://boringssl-review.googlesource.com/6313
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:44:27 +00:00
David Benjamin
63006a913b Document the rest of ssl.h.
Although Chromium actually uses SSL_(get_)state as part of its fallback
reason heuristic, that function really should go in the deprecated
bucket. I kept SSL_state_string_long since having a human-readable
string is probably useful for logging.

SSL_set_SSL_CTX was only half-documented as the behavior of this
function is very weird. This warrants further investigation and
rethinking.

SSL_set_shutdown is absurd. I added an assert to trip up clearing bits
and set it to a bitwise OR since clearing bits may mess up the state
machine. Otherwise there's enough consumers and it's not quite the same
as SSL_CTX_set_quiet_shutdown that I've left it alone for now.

Change-Id: Ie35850529373a5a795f6eb04222668ff76d84aaa
Reviewed-on: https://boringssl-review.googlesource.com/6312
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:43:38 +00:00
David Benjamin
7a1eefd3cd Deprecate SSL_library_init.
It just calls CRYPTO_library_init and doesn't do anything else. If
anything, I'd like to make CRYPTO_library_init completely go away too.
We have CRYPTO_once now, so I think it's safe to assume that, if ssl/
ever grows initialization needs beyond that of crypto/, we can hide it
behind a CRYPTO_once and not burden callers.

Change-Id: I63dc362e0e9e98deec5516f4620d1672151a91b6
Reviewed-on: https://boringssl-review.googlesource.com/6311
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:36:23 +00:00
David Benjamin
5d8b128095 Document the (formerly) SSL_state wrapper macros.
SSL_in_connect_init and SSL_in_accept_init are removed as they're unused
both within the library and externally. They're also kind of silly.

Expand on how False Start works at the API level in doing so.

Change-Id: Id2a8e34b5bb8f28329e3b87b4c64d41be3f72410
Reviewed-on: https://boringssl-review.googlesource.com/6310
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:35:01 +00:00
David Benjamin
449c3c7b7b Take some definitions out of the Android compatibility layer.
They were since added to crypto.h and implemented in the library proper.

Change-Id: Idaa2fe2d9b213e67cf7ef61ff8bfc636dfa1ef1f
Reviewed-on: https://boringssl-review.googlesource.com/6309
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:33:10 +00:00
David Benjamin
6e0c17aa3a Private (and deprecated) types.
Change-Id: Ia66e485cb2de45c9fb0a1ecd9a703863ad24d9c9
Reviewed-on: https://boringssl-review.googlesource.com/6308
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:32:35 +00:00