Commit Graph

1674 Commits

Author SHA1 Message Date
David Benjamin
4b968339e3 Add a compatibility EVP_CIPH_OCB_MODE value.
Node references it these days. Also replace the no-op modes with negative
numbers rather than zero. Stream ciphers like RC4 report a "mode" of zero, so
code comparing the mode to a dummy value will get confused.

(I came across https://github.com/nodejs/node/pull/23635, though we'd have run
into it sooner or later anyway. Better to just define the value and avoid ifdef
proliferation.)

Change-Id: I223f25663e138480ad83f35aa16f5218f1425563
Reviewed-on: https://boringssl-review.googlesource.com/c/32464
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-10-16 19:41:50 +00:00
David Benjamin
b68b832238 Fix undefined function pointer casts in LHASH.
Bug: chromium:785442
Change-Id: I516e42684b913dc0de778dd9134f1ca108c04dfc
Reviewed-on: https://boringssl-review.googlesource.com/c/32120
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-15 23:53:24 +00:00
David Benjamin
1eff9482ca Use proper functions for lh_*.
As with sk_*, this. This doesn't fix the function pointer casts. Those
will be done in a follow-up change. Also add a test for lh_*_doall so we
cover both function pointer shapes.

Update-Note: This reworks how LHASH_OF(T) is implemented and also only
pulls in the definitions where used, but LHASH_OF(T) is never used
externally, so I wouldn't expect this to affect things.

Change-Id: I7970ce8c41b8589d6672b71dd03658d0e3bd89a7
Reviewed-on: https://boringssl-review.googlesource.com/c/32119
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-15 23:37:04 +00:00
David Benjamin
2d98d49cf7 Add a per-SSL TLS 1.3 downgrade enforcement option and improve tests.
Due to non-compliant middleboxes, it is possible we'll need to do some
surgery to this mechanism. Making it per-SSL is a little more flexible
and also eases some tests in Chromium until we get its SSL_CTX usage
fixed up.

Also fix up BoringSSL tests. We forgot to test it at TLS 1.0 and use the
-expect-tls13-downgrade flag.

Bug: 226
Change-Id: Ib39227e74e2d6f5e1fbc1ebcc091e751471b3cdc
Reviewed-on: https://boringssl-review.googlesource.com/c/32424
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-10-10 19:50:19 +00:00
David Benjamin
62a4dcd256 Fix section header capitalization.
We only capitalize the first word. I've left Token Binding alone because
that appears to be the full name. But "QUIC Transport Parameters" just
describe's QUIC's transport parameters.

Change-Id: I7e0f69e24ff4080c0470c87825dffa1a9aa6df97
Reviewed-on: https://boringssl-review.googlesource.com/c/32344
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-10-03 16:23:08 +00:00
David Benjamin
e1ee0f5b47 Fix build in consumers that flag unused parameters.
Change-Id: I4ec8a21264c2c73ebf8ca6a93b96eba29bd2d29e
Reviewed-on: https://boringssl-review.googlesource.com/c/32345
Reviewed-by: Robert Sloan <varomodt@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-10-02 22:49:32 +00:00
David Benjamin
fac6fb99da Opaquify CONF.
This removes the last mention of LHASH in public headers. This can only
break people who stack-allocate CONF or access the data field. The
latter does not happen (external code never calls lh_CONF_VALUE_*
functions). The former could not work as there would be no way to clean
it up.

Update-Note: CONF is now opaque.
Change-Id: Iad3796c4e75874530d7a70fde2f84a390def2d49
Reviewed-on: https://boringssl-review.googlesource.com/32118
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 23:56:19 +00:00
David Benjamin
e17e14dfe1 Remove LHASH_OF mention in X509V3_EXT_conf_nid.
Everyone calls this with NULL anyway. People never actually use
lh_CONF_VALUE_* functions (or any other lh_* functions for that matter).

Also remove unused X509V3_EXT_CRL_add_conf prototype.

This removes one of the last mentions of LHASH_OF in public headers.

Update-Note: X509V3_EXT_conf_nid calls that pass a non-NULL first
    parameter will fail to compile.

Change-Id: Ia6302ef7b494efeb9b63ab75a18bc340909dcba3
Reviewed-on: https://boringssl-review.googlesource.com/32117
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 23:26:40 +00:00
David Benjamin
a943613e40 Inline functions are apparently really complicated.
C and C++ handle inline functions differently. In C++, an inline function is
defined in just the header file, potentially emitted in multiple compilation
units (in cases the compiler did not inline), but each copy must be identical
to satsify ODR. In C, a non-static inline must be manually emitted in exactly
one compilation unit with a separate extern inline declaration.

In both languages, exported inline functions referencing file-local symbols are
problematic. C forbids this altogether (though GCC and Clang seem not to
enforce it). It works in C++, but ODR requires the definitions be identical,
including all names in the definitions resolving to the "same entity". In
practice, this is unlikely to be a problem, but an inline function that returns
a pointer to a file-local symbol could compile oddly.

Historically, we used static inline in headers. However, to satisfy ODR, use
plain inline in C++, to allow inline consumer functions to call our header
functions. Plain inline would also work better with C99 inline, but that is not
used much in practice, extern inline is tedious, and there are conflicts with
the old gnu89 model: https://stackoverflow.com/questions/216510/extern-inline

For dual C/C++ code, use a macro to dispatch between these. For C++-only
code, stop using static inline and just use plain inline.

Update-Note: If you see weird C++ compile or link failures in header
    functions, this change is probably to blame. Though this change
    doesn't affect C and non-static inline is extremely common in C++,
    so I would expect this to be fine.

Change-Id: Ibb0bf8ff57143fc14e10342854e467f85a5e4a82
Reviewed-on: https://boringssl-review.googlesource.com/32116
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 22:57:00 +00:00
David Benjamin
7c3ce519e8 Actually disable RandTest.Fork on iOS.
TARGET_OS_IPHONE isn't defined without including <TargetConditionals.h>. Oops.
Confirmed now that OPENSSL_IOS gets defined where we expect.

Update-Note: There is some chance this will fail to build on some macOS host
builds of Android? https://codereview.chromium.org/538563002 suggests something
weird happens. However those Android builds of BoringSSL would already be
problematic because they'd set OPENSSL_STATIC_ARMCAP thinking they were iOS.
Thus I've intentionally kept the assumption that __APPLE__ implies a Darwin
target. If it goes through, all is well. If not, we'll learn more about that
configuration and that we likely need to revise our OPENSSL_APPLE definition.

Bug: chromium:890115
Change-Id: I1df73ac2321391d2449edbeb9cfa295fd607f935
Reviewed-on: https://boringssl-review.googlesource.com/32204
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 20:34:39 +00:00
David Benjamin
52483994c8 Mostly fix undefined casts around STACK_OF's comparator.
The calls to qsort and bsearch are still invalid, but not avoidable
without reimplementing them. Fortunately, they cross libraries, so CFI
does not object.

With that, all that's left is LHASH!

Bug: chromium:785442
Change-Id: I6d29f60fac5cde1f7870d7cc515346e55b98315b
Reviewed-on: https://boringssl-review.googlesource.com/32114
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 20:25:15 +00:00
David Benjamin
fb4e2e0f0c Fix undefined casts in sk_*_pop_free and sk_*_deep_copy.
Unfortunately, some projects are calling into sk_pop_free directly, so
we must leave a compatibility version around for now.

Bug: chromium:785442
Change-Id: I1577fce6f23af02114f7e9f7bf2b14e9d22fa9ae
Reviewed-on: https://boringssl-review.googlesource.com/32113
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 20:04:07 +00:00
David Benjamin
792c1dc43e Rewrite PEM_X509_INFO_read_bio.
This fixes:

- Undefined function pointer casts.
- Missing X509_INFO_new malloc failure checks.
- Pointless (int) cast on strlen.
- Missing ERR_GET_LIB in PEM_R_NO_START_LINE check.
- Broken error-handling if passing in an existing stack and we hit a
  syntax error.

Bug: chromium:785442
Change-Id: I8be3523b0f13bdb3745938af9740d491486f8bf1
Reviewed-on: https://boringssl-review.googlesource.com/32109
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 17:35:10 +00:00
David Benjamin
419144adce Fix undefined function pointer casts in {d2i,i2d}_Foo_{bio,fp}
Lacking C++, this instead adds a mess of macros. With this done, all the
function-pointer-munging "_of" macros in asn1.h can also be removed.

Update-Note: A number of *really* old and unused ASN.1 macros were
removed.

Bug: chromium:785442
Change-Id: Iab260d114c7d8cdf0429759e714d91ce3f3c04b2
Reviewed-on: https://boringssl-review.googlesource.com/32106
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-10-01 17:34:53 +00:00
David Benjamin
217bfd3c96 Fix undefined function pointer casts in IMPLEMENT_PEM_*.
While it is okay to cast function pointers into different types for
generic storage, the pointer must be cast back to the exact same type
when calling. In particular, although C libraries do this sort of thing
all the time, calling a T* d2i function as a void* d2i function is
undefined:

  If the function is defined with a type that is not compatible with the
  type (of the expression) pointed to by the expression that denotes the
  called function, the behavior is undefined

Fix some instances in the PEM/ASN1 wrapper functions. Synthesize helper
functions instead.

This CL just addresses the function pointer issues. The inherited legacy
OpenSSL ASN.1 code is still full other questionable data pointer dances
that will be much more difficult to excise. Continuing to exise that
code altogether (it is already unshipped from Cronet and unshipped from
Chrome but for WebRTC) is probably a better tack there.

This removes one (of many many) places where we require
-fsanitize-cfi-icall-generalize-pointers.

Bug: chromium:785442
Change-Id: Id8056ead6ef471f0fdf263bb50dc659da500e8ce
Reviewed-on: https://boringssl-review.googlesource.com/32105
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-10-01 17:34:44 +00:00
David Benjamin
13fd627449 Disable RandTest.Fork on iOS.
iOS doesn't support fork.

Bug: chromium:890115
Change-Id: Idac6c0e180bbc1088ca5c562b8c1e646bff00b25
Reviewed-on: https://boringssl-review.googlesource.com/32164
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-09-28 15:42:18 +00:00
David Benjamin
8d2f4b993f Const-correct sk_find and sk_delete_ptr.
Change-Id: I7ddc2c4827602ddac2a4aec5f9ccfa21d6c0bc40
Reviewed-on: https://boringssl-review.googlesource.com/32112
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
2018-09-27 16:18:18 +00:00
David Benjamin
5b33effa72 Rename OPENSSL_NO_THREADS, part 1.
BoringSSL depends on the platform's locking APIs to make internal global
state thread-safe, including the PRNG. On some single-threaded embedded
platforms, locking APIs may not exist, so this dependency may be disabled
with a build flag.

Doing so means the consumer promises the library will never be used in any
multi-threaded address space. It causes BoringSSL to be globally thread-unsafe.
Setting it inappropriately will subtly and unpredictably corrupt memory and
leak secret keys.

Unfortunately, folks sometimes misinterpreted OPENSSL_NO_THREADS as skipping an
internal thread pool or disabling an optionally extra-thread-safe mode. This is
not and has never been the case. Rename it to
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED to clarify what
this option does.

Update-Note: As a first step, this CL makes both OPENSSL_NO_THREADS and
OPENSSL_NO_THREADS_CORRUPT_MEMORY_AND_LEAK_SECRETS_IF_THREADED work. A later CL
will remove the old name, so migrate callers after or at the same time as
picking up this CL.

Change-Id: Ibe4964ae43eb7a52f08fd966fccb330c0cc11a8c
Reviewed-on: https://boringssl-review.googlesource.com/32084
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-09-26 19:10:02 +00:00
David Benjamin
4902598935 Remove SHA384_Transform from sha.h.
This function doesn't actually exist. (If it did, it would be the same as
SHA512_Transform. We currently omit SHA224 and SHA384's low-level transform
functions.)

Change-Id: Ia9d3d7c86e8f70fd5e4f13b8de4f08440dccbdcb
Reviewed-on: https://boringssl-review.googlesource.com/32064
Reviewed-by: Adam Langley <agl@google.com>
2018-09-20 14:57:36 +00:00
David Benjamin
371305f58a Push an error on sigalg mismatch in X509_verify.
It was failing but not pushing an error. See
https://github.com/google/conscrypt/pull/537

Change-Id: Iafba1a5c0c7ef8e0535b335aa93df6f520c3803e
Reviewed-on: https://boringssl-review.googlesource.com/32044
Reviewed-by: Adam Langley <agl@google.com>
2018-09-19 03:44:50 +00:00
David Benjamin
5cf05ad21c Add missing bssl::UpRef overloads.
Change-Id: I840307526238219385ea6144739f5df78c596a4f
Reviewed-on: https://boringssl-review.googlesource.com/31946
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-09-14 16:45:54 +00:00
David Benjamin
689019fe40 Fix BORINGSSL_NO_CXX.
BSSL_NAMESPACE_BEGIN needs to be defined unconditionally.

Change-Id: I1770ca6b6c19f9c732ef00ba8c89b112b421929d
Reviewed-on: https://boringssl-review.googlesource.com/31824
Reviewed-by: Adam Langley <agl@google.com>
2018-09-11 16:39:42 +00:00
Adam Langley
e77c27d734 Automatically disable assembly with MSAN.
MSAN is incompatible with hand-written assembly code. Previously we
required that OPENSSL_NO_ASM be set when building with MSAN, and the
CMake build would take care of this. However, with other build systems
it wasn't always so easy.

This change automatically disables assembly when the compiler is
configured for MSAN.

Change-Id: I6c219120f62d16b99bafc2efb02948ecbecaf87f
Reviewed-on: https://boringssl-review.googlesource.com/31724
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-09-07 21:12:37 +00:00
David Benjamin
19ac2666b9 Make symbol-prefixing work on ARM.
The assembly files need some includes. Also evp.h has some conflicting
macros. Finally, md5.c's pattern of checking if a function name is
defined needs to switch to checking MD5_ASM.

Change-Id: Ib1987ba6f279144f0505f6951dead53968e05f20
Reviewed-on: https://boringssl-review.googlesource.com/31704
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-09-07 17:43:05 +00:00
David Benjamin
4b85a94542 Document alternative functions to BIO_f_base64.
Also point out that we're a cryptography library, not a text encoding library.
Not that that'll dissuade anyone.

Change-Id: Ia324e08c5cdd108fa182d2610f80447262e0bd5c
Reviewed-on: https://boringssl-review.googlesource.com/31664
Reviewed-by: Adam Langley <agl@google.com>
2018-09-07 15:58:12 +00:00
Joshua Liebow-Feeser
8c7c6356e6 Support symbol prefixes
- In base.h, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
  defined with appropriate values depending on whether
  BORINGSSL_PREFIX is defined; these macros are used in place
  of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
  and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
  are defined, run util/make_prefix_headers.go to generate header
  files
- In various CMakeLists.txt files, add "global_target" that all
  targets depend on to give us a place to hook logic that must run
  before all other targets (in particular, the header file generation
  logic)
- Document this in BUILDING.md, including the fact that it is
  the caller's responsibility to provide the symbol list and keep it
  up to date
- Note that this scheme has not been tested on Windows, and likely
  does not work on it; Windows support will need to be added in a
  future commit

Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-06 20:07:52 +00:00
David Benjamin
92812cb73c Tidy up docs for #defines.
This removes the special-case for #defines in doc.go.

Change-Id: I6bf750485a94ad28c3975644c74a17c550bb3224
Reviewed-on: https://boringssl-review.googlesource.com/31505
Reviewed-by: Adam Langley <agl@google.com>
2018-09-04 22:32:01 +00:00
Joshua Liebow-Feeser
67e64342c1 Document that ED25519_sign only fails on allocation failure
Change-Id: I45866c3a4aa98ebac51d4e554a22eb5add45002f
Reviewed-on: https://boringssl-review.googlesource.com/31404
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-29 18:35:12 +00:00
David Benjamin
2556f8ba60 Clarify thread-safety of key objects.
This often causes confusion since, for various silly reasons (intrinsic
ref-counting, FOO_METHOD, and RSA's cached Montgomery bits), the thread
safety of some functions don't match the usual const/non-const
distinction. Fix const-ness where easy and document it otherwise.

Change-Id: If2037a4874d7580cc79b18ee21f12ae0f47db7fd
Reviewed-on: https://boringssl-review.googlesource.com/31344
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-28 18:52:59 +00:00
David Benjamin
6855e0a470 Switch the default TLS 1.3 variant to tls13_rfc.
Update-Note: If not explicitly configured to use tls13_all, callers that enable
TLS 1.3 will now only enable the final standard version.

Change-Id: Ifcfc65a9d8782c983df6e002925e8f77f45b6e53
Reviewed-on: https://boringssl-review.googlesource.com/31384
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-28 13:58:28 +00:00
Steven Valdez
4ac9405eba Remove unused BORINGSSL_PREFIX.
Change-Id: Ifd7239106471bb59057b0a65c6e91837379c78bf
Reviewed-on: https://boringssl-review.googlesource.com/31164
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-21 14:53:16 +00:00
Joshua Liebow-Feeser
21558f43ce Document error behavior of PKCS5_PBKDF2_HMAC and EVP_PBE_scrypt
Change-Id: I5ce176538a53136aff3eea4af04b762ac9a5a994
Reviewed-on: https://boringssl-review.googlesource.com/31044
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-21 01:08:24 +00:00
David Benjamin
a130ce0b71 Update TLS 1.3 citations for the final RFC.
Change-Id: I2d1671a4f21a602191fd0c9b932244a376ac5713
Reviewed-on: https://boringssl-review.googlesource.com/31104
Reviewed-by: David Benjamin <davidben@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-16 18:37:50 +00:00
Steven Valdez
f1af129fb4 Implement TLS 1.3 anti-downgrade signal.
Change-Id: Ib4739350948ec339457d993daef582748ed8f100
Reviewed-on: https://boringssl-review.googlesource.com/30924
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-08-15 15:23:43 +00:00
Adam Langley
ae3223957f Remove dummy PQ padding extension.
Results written up at https://www.imperialviolet.org/2018/04/11/pqconftls.html

Change-Id: I4614fbda555323c67a7ee4683441b59b995f97fb
Reviewed-on: https://boringssl-review.googlesource.com/31064
Commit-Queue: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-15 00:28:52 +00:00
Joshua Liebow-Feeser
978169951f Document error behavior of various functions
- Document error behavior of:
  - EVP_PKEY_assign_XXX
  - EVP_PKEY_set1_XXX
  - EVP_PKEY_assign
  - EVP_PKEY_set_type
  - EC_GROUP_new_by_curve_name
  - EC_KEY_set_group
  - ECDSA_size
  - HMAC_Final
- Document that EVP_parse_public_key sets the curve for EC keys

Change-Id: I498ae19a8729680216fee518f97bd0cbaab94c40
Reviewed-on: https://boringssl-review.googlesource.com/30985
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-14 20:42:21 +00:00
Joshua Liebow-Feeser
dea6d90de2 Document failure conditions of some EVP, HMAC, and CBB functions
- Document failure conditions of EVP_MD_CTX_copy_ex,
  EVP_DigestInit_ex, HMAC_Init_ex, and CBB_init

Change-Id: I643d1b92e88e7f690fa555f7d908317a23e5cd95
Reviewed-on: https://boringssl-review.googlesource.com/30964
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-14 19:31:43 +00:00
David Benjamin
bc3286bb8c Add a pile of compatibility functions.
Change-Id: I75c8783139c717be34a9159a2291d2ae55ee71d5
Reviewed-on: https://boringssl-review.googlesource.com/30984
Reviewed-by: Adam Langley <agl@google.com>
2018-08-13 23:13:26 +00:00
Steven Valdez
d451453067 Implement final TLS 1.3 RFC!!!
The anti-downgrade signal is being implemented in a follow-up change.

Change-Id: I5ea3ff429ed1389a3577026588fef3660d2d0615
Reviewed-on: https://boringssl-review.googlesource.com/30904
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-08-13 20:34:23 +00:00
Jesse Selover
1c337e566d Option to reverify certs on resumption.
Works in the 1.3 and 1.2 client handshakes, not implemented on the
server for now.
Creates an SSL_CTX option to reverify the server certificate on session
resumption. Reverification only runs the client's certificate verify callback.
Adds new states to the client handshakes: state_reverify_server_certificate in
TLS 1.2, and state_server_certificate_reverify in TLS 1.3.
Adds a negative test to make sure that by default we don't verify the
certificate on resumption, and positive tests that make sure we do when the
new option is set.

Change-Id: I3a47ff3eacb3099df4db4c5bc57f7c801ceea8f1
Bug: chromium:347402
Reviewed-on: https://boringssl-review.googlesource.com/29984
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-10 20:06:22 +00:00
Adam Langley
7c1f21a1d1 Add XChaCha20-Poly1305 AEAD.
This is a version of ChaCha20-Poly1305 that takes a 24-byte nonce,
making the nonce suitable for random generation. It's compatible with
the AEAD of the same name in libsodium.

Change-Id: Ie8b20ba551e5a290b390d362e487f06377166f4c
Reviewed-on: https://boringssl-review.googlesource.com/30384
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-08-09 18:49:09 +00:00
David Benjamin
a3202d7bc1 Add EVP_CTRL_AEAD_* constants.
Upstream generalized most of the EVP_CTRL_GCM_* constants to be their general
AEAD API in 1.1.0. Define them for better compatibility with code that targets
OpenSSL 1.1.0.

Change-Id: Ieaed8379eebde3718e3048f6290c21cdeac01efd
Reviewed-on: https://boringssl-review.googlesource.com/30604
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-08-09 17:08:15 +00:00
Adam Langley
826ce15092 Support OpenSSL APIs SSL[_CTX]_set1_sigalgs[_list].
These functions can be used to configure the signature algorithms. One
of them is a string mini-languaging parsing function, which we generally
dislike because it defeats static analysis. However, some dependent
projects (in this case TensorFlow) need it and we also dislike making
people patch.

Change-Id: I13f990c896a7f7332d78b1c351357d418ade8d11
Reviewed-on: https://boringssl-review.googlesource.com/30304
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-08-09 16:57:09 +00:00
Adam Langley
e27793940e Don't accept “SSL client” as a substitute for S/MIME in the Netscape cert type extension.
I believe that case was the only way that X509_check_purpose could
return anything other than zero or one. Thus eliminate the last use of
X509_V_FLAG_X509_STRICT.

Change-Id: If2f071dfa934b924491db2b615ec17390564e7de
Reviewed-on: https://boringssl-review.googlesource.com/30344
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-08-06 21:52:28 +00:00
Adam Langley
8bd1d07535 Require basicConstraints cA flag in intermediate certs.
OpenSSL 1.0.2 (and thus BoringSSL) accepts keyUsage certSign or a
Netscape CA certificate-type in lieu of basicConstraints in an
intermediate certificate (unless X509_V_FLAG_X509_STRICT) is set.

Update-Note: This change tightens the code so that basicConstraints is required for intermediate certificates when verifying chains. This was previously only enabled if X509_V_FLAG_X509_STRICT was set, but that flag also has other effects.

Change-Id: I9e41f4c567084cf30ed08f015a744959982940af
Reviewed-on: https://boringssl-review.googlesource.com/30185
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-08-01 19:10:19 +00:00
Adam Langley
0224a3294a Add X509_V_FLAG_REQUIRE_CA_BASIC_CONSTRAINTS.
This change adds a new flag, X509_V_FLAG_REQUIRE_CA_BASIC_CONSTRAINTS,
which causes basicConstraints with isCA to be required for intermediate
CA certificates. Without this, intermediates are also acceptable if
they're missing basicConstraints, but include either a certSign
keyUsage, or a CA Netscape certificate type.

This is a short-term change for patching. I'll undo a lot of it and make
this the default in the next change.

Change-Id: I7f42ffd76c57de3037f054108951e230c1b4e415
Reviewed-on: https://boringssl-review.googlesource.com/30184
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-08-01 18:55:50 +00:00
Matthew Braithwaite
548c27646a shim: perform split handshakes in a separate binary.
The new binary, called |handshaker|, allows split-handshakes to be
tested using shim and handshaker binaries built at different
revisions.

The shim now proxies traffic to the handshaker during the split
handshake.  The handoff and handback steps serialize additional state
about the test being performed, and its results.

The proxy and handshaker make heavy use of Unix-isms, and so
split-handshake tests are now restricted to Linux.

Change-Id: I048f0540c3978a31b3e573e00da17caf41a8059e
Reviewed-on: https://boringssl-review.googlesource.com/29348
Reviewed-by: Adam Langley <agl@google.com>
2018-08-01 01:44:53 +00:00
Adam Langley
4732c544f7 Add ECDH_compute_key_fips inside the module.
This change adds a function so that an ECDH and the hashing of the
resulting 'x' coordinate can occur inside the FIPS boundary.

Change-Id: If93c20a70dc9dcbca49056f10915d3ce064f641f
Reviewed-on: https://boringssl-review.googlesource.com/30104
Reviewed-by: Adam Langley <agl@google.com>
2018-07-30 22:40:31 +00:00
Daniel Hirche
9af1edbe22 Don't build test/malloc.cc with TSAN.
Change-Id: I33c5259f066693c912ba751dff0205ae240f4a92
Reviewed-on: https://boringssl-review.googlesource.com/29964
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-24 15:38:09 +00:00
David Benjamin
c59b9aace6 Remove more remnants of SSLv3.
Mostly in comments, but there is one special-case around renegotiation_info
that can now be removed.

Change-Id: I2a9114cbff05e0cfff95fe93270fe42379728012
Reviewed-on: https://boringssl-review.googlesource.com/29824
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-17 20:02:35 +00:00
David Benjamin
35b4a1255c Namespace CertCompressionAlg and use more scopers.
Change-Id: I52ab2dbf92bbdbc8cb0dd811bf9eaafe0c903b66
Reviewed-on: https://boringssl-review.googlesource.com/29808
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-15 14:12:13 +00:00
David Benjamin
bc118ee64c Add SSL_get0_peer_verify_algorithms.
Callers who use SSL_get0_certificate_types today will find an empty list
in TLS 1.3, which removed it. To provide feature parity, add an accessor
for the signature algorithms list. SSL_get_signature_algorithm_key_type
can be used to map it to a key type.

"Peer signature algorithms" was already taken in the public API by
SSL_get_peer_signature_algorithm to refer to which the peer selected, so
I named this matching SSL_CTX_set_verify_algorithm_prefs.

Change-Id: I12d411d7350e744ed9f88c610df48e0d9fc13256
Reviewed-on: https://boringssl-review.googlesource.com/29684
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Vartanian <flooey@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-07-14 03:50:40 +00:00
David Benjamin
0a3e07ac1d Remove custom extensions support.
Update-Note: Custom extensions APIs are removed.
Change-Id: Ic5e0fb3c018bf15d35d9149623f6b29940041b59
Reviewed-on: https://boringssl-review.googlesource.com/29685
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-07-14 03:33:00 +00:00
Adam Langley
428fb3ad52 Make |BORINGSSL_MAKE_UP_REF| a no-op when C++ is disabled.
Change-Id: I436cc772eb975ad989035ee154a2e050c65e2961
Reviewed-on: https://boringssl-review.googlesource.com/29664
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-10 14:59:12 +00:00
Adam Langley
859679518d Drop C++ from certificate compression API.
It's 2018, but passing STL objects across the API boundary turns out to
still be more bother than it's worth. Since we're dropping UniquePtr in
the API anyway, go the whole way and make it a plain-C API.

Change-Id: Ic0202012e5d81afe62d71b3fb57e6a27a8f63c65
Update-note: this will need corresponding changes to the internal use of SSL_CTX_add_cert_compression_alg.
Reviewed-on: https://boringssl-review.googlesource.com/29564
Reviewed-by: David Benjamin <davidben@google.com>
2018-07-04 16:39:14 +00:00
David Benjamin
58150ed59b Add lh_FOO_retrieve_key to avoid stack-allocating SSL_SESSION.
lh_FOO_retrieve is often called with a dummy instance of FOO that has
only a few fields filled in. This works fine for C, but a C++
SSL_SESSION with destructors is a bit more of a nuisance here.

Instead, teach LHASH to allow queries by some external key type. This
avoids stack-allocating SSL_SESSION. Along the way, fix the
make_macros.sh script.

Change-Id: Ie0b482d4ffe1027049d49db63274c7c17f9398fa
Reviewed-on: https://boringssl-review.googlesource.com/29586
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-07-03 22:56:46 +00:00
David Benjamin
2908dd141f Add bssl::UpRef.
bssl::UniquePtr and FOO_up_ref do not play well together. Add a helper
to simplify this. This allows us to write things like:

   foo->cert = UpRef(bar->cert);

instead of:

   if (bar->cert) {
     X509_up_ref(bar->cert.get());
   }
   foo->cert.reset(bar->cert.get());

This also plays well with PushToStack. To append something to a stack
while taking a reference, it's just:

   PushToStack(certs, UpRef(cert))

Change-Id: I99ae8de22b837588a2d8ffb58f86edc1d03ed46a
Reviewed-on: https://boringssl-review.googlesource.com/29584
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-07-03 22:47:36 +00:00
Alessandro Ghedini
a0373182eb Update QUIC transport parameters extension codepoint
This was changed in draft-ietf-quic-tls-13 to use a codepoint from the
reserved range.

Change-Id: Ia3cda249a3f37bc244d5c8a7765ec34a5708c9ae
Reviewed-on: https://boringssl-review.googlesource.com/29464
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-28 17:41:58 +00:00
David Benjamin
9bb15f58f7 Remove SSL 3.0 implementation.
Update-Note: SSL_CTX_set_min_proto_version(SSL3_VERSION) now fails.
   SSL_OP_NO_SSLv3 is now zero. Internal SSL3-specific "AEAD"s are gone.

Change-Id: I34edb160be40a5eea3e2e0fdea562c6e2adda229
Reviewed-on: https://boringssl-review.googlesource.com/29444
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-06-28 16:54:58 +00:00
David Benjamin
3815720cf3 Add a bunch of compatibility functions for PKCS#7.
The full library is a bit much, but this is enough to appease most of
cryptography.io.

Change-Id: I1bb0d83744c4550d5fe23c5c98cfd7e36b17fcc9
Reviewed-on: https://boringssl-review.googlesource.com/29365
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-26 18:42:49 +00:00
David Benjamin
79c97bf37c Allow empty return values from PKCS7_get_*.
Right now we're inconsistent about it. If the OPTIONAL container is
missing, we report an error, but if the container is empty, we happily
return nothing. The latter behavior is more convenient for emulating
OpenSSL's PKCS#7 functions.

These are our own functions, so we have some leeway here. Looking
through callers, they appear to handle this fine.

Update-Note: This is a behavior change.
Change-Id: I1321025a64df3054d380003c90e57d9eb95e610f
Reviewed-on: https://boringssl-review.googlesource.com/29364
Reviewed-by: Adam Langley <agl@google.com>
2018-06-26 07:24:51 +00:00
David Benjamin
8803c0589d Properly advance the CBS when parsing BER structures.
CBS_asn1_ber_to_der was a little cumbersome to use. While it, in theory,
allowed callers to consistently advance past the element, no caller
actually did so consistently. Instead they would advance if conversion
happened, and not if it was already DER. For the PKCS7_* functions, this
was even caller-exposed.

Change-Id: I658d265df899bace9ba6616cb465f19c9e6c3534
Reviewed-on: https://boringssl-review.googlesource.com/29304
Reviewed-by: Adam Langley <agl@google.com>
2018-06-26 07:23:10 +00:00
Adam Langley
bcfb49914b Add special AES-GCM AEAD for TLS 1.3.
This change adds an AES-GCM AEAD that enforces nonce uniqueness inside
the FIPS module, like we have for TLS 1.2. While TLS 1.3 has not yet
been mentioned in the FIPS 140 IG, we expect it to be in the next ~12
months and so are preparing for that.

Change-Id: I65a7d8196b08dc0033bdde5c844a73059da13d9e
Reviewed-on: https://boringssl-review.googlesource.com/29224
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-25 10:23:22 +00:00
Adam Langley
0080d83b9f Implement the client side of certificate compression.
Change-Id: I0aced480af98276ebfe0970b4afb9aa957ee07cb
Reviewed-on: https://boringssl-review.googlesource.com/29024
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 22:16:11 +00:00
David Benjamin
f6e5d0d5a1 Add AES-192-OFB.
cryptography.io gets offended if the library supports some OFB sizes but
not others.

Change-Id: I7fc7b12e7820547a82aae84d9418457389a482fe
Reviewed-on: https://boringssl-review.googlesource.com/29204
Reviewed-by: Adam Langley <agl@google.com>
2018-06-18 21:58:46 +00:00
David Benjamin
7139f755b6 Fix some timing leaks in the DSA code.
The DSA code is deprecated and will, hopefully, be removed in the future.
Nonetheless, this is easy enough to fix. It's the analog of the work we'd
already done for ECDSA.

- Document more clearly that we don't care about the DSA code.

- Use the existing constant-time modular addition function rather than
  the ad-hoc code.

- Reduce the digest to satisfy modular operations' invariants. (The
  underlying algorithms could accept looser bounds, but we reduce for
  simplicity.) There's no particular reason to do this in constant time,
  but we have the code for it, so we may as well.

- This additionally adds a missing check that num_bits(q) is a multiple
  of 8. We otherwise don't compute the right answer. Verification
  already rejected all 160-, 224-, and 256-bit keys, and we only
  generate DSA parameters where the length of q matches some hash
  function's length, so this is unlikely to cause anyone trouble.

- Use Montgomery reduction to perform the modular multiplication. This
  could be optimized to save a couple Montgomery reductions as in ECDSA,
  but DSA is deprecated, so I haven't bothered optimizing this.

- The reduction from g^k (mod p) to r = g^k (mod p) (mod q) is left
  in variable time, but reversing it would require a discrete log
  anyway. (The corresponding ECDSA operation is much easier to make
  constant-time due to Hasse's theorem, though that's actually still a
  TODO. I need to finish lifting EC_FELEM up the stack.)

Thanks to Keegan Ryan from NCC Group for reporting the modular addition issue
(CVE-2018-0495). The remainder is stuff I noticed along the way.

Update-Note: See the num_bits(q) change.

Change-Id: I4f032b041e2aeb09f9737a39f178c24e6a7fa1cb
Reviewed-on: https://boringssl-review.googlesource.com/29145
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-15 02:37:45 +00:00
David Benjamin
3b2ff028c4 Add SSL_SESSION_get0_id_context.
This matches OpenSSL 1.1.0. Someone requested it.

Change-Id: I230bb9ec646cd32e71413a68e93058818c8f2aad
Reviewed-on: https://boringssl-review.googlesource.com/29004
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-11 14:25:23 +00:00
David Benjamin
1c68fa2350 Hide SSL_SESSION.
The last libssl struct is now opaque! (Promote the SSL_MAX_* constants
as folks use them pretty frequently.)

Update-Note: SSL_SESSION is now opaque. I believe everything handles
this now.

Bug: 6
Change-Id: I8cd29d16173e4370f3341c0e6f0a56e00ea188e9
Reviewed-on: https://boringssl-review.googlesource.com/28964
Reviewed-by: Adam Langley <agl@google.com>
2018-06-07 02:58:27 +00:00
David Benjamin
5267ef7b4a Reject unexpected application data in bidirectional shutdown.
Update-Note: This tweaks the SSL_shutdown behavior. OpenSSL's original
SSL_shutdown behavior was an incoherent mix of discarding the record and
rejecting it (it would return SSL_ERROR_SYSCALL but retrying the
operation would discard it). SSLeay appears to have intended to discard
it, so we previously "fixed" it actually discard.

However, this behavior is somewhat bizarre and means we skip over
unbounded data, which we typically try to avoid. If you are trying to
cleanly shutdown the TLS portion of your protocol, surely it is at a
point where additional data is a syntax error. I suspect I originally
did not realize that, because the discarded record did not properly
continue the loop, SSL_shutdown would appear as if it rejected the data,
and so it's unlikely anyone was relying on that behavior.

Discussion in https://github.com/openssl/openssl/pull/6340 suggests
(some of) upstream also prefers rejecting.

Change-Id: Icde419049306ed17eb06ce1a7e1ff587901166f3
Reviewed-on: https://boringssl-review.googlesource.com/28864
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-06-04 21:39:58 +00:00
Adam Langley
a307cb7d58 Preliminary support for compressed certificates.
This change adds server-side support for compressed certificates.

(Although some definitions for client-side support are included in the
headers, there's no code behind them yet.)

Change-Id: I0f98abf0b782b7337ddd014c58e19e6b8cc5a3c2
Reviewed-on: https://boringssl-review.googlesource.com/27964
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-04 21:24:20 +00:00
David Benjamin
caf8ddd0ba Add SSL_SESSION_set1_id.
This matches the OpenSSL 1.1.0 spelling. I'd thought we could hide
SSL_SESSION this pass, but I missed one test that messed with session
IDs!

Bug: 6
Change-Id: I84ea113353eb0eaa2b06b68dec71cb9061c047ca
Reviewed-on: https://boringssl-review.googlesource.com/28866
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-06-04 14:25:28 +00:00
David Benjamin
fe7a17440f Fix typo.
Change-Id: Id7d8c8acf2f441dc34be7d363fb4dd2dfcb0e1c4
Reviewed-on: https://boringssl-review.googlesource.com/28804
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-30 15:42:35 +00:00
David Benjamin
a827d1809c Match OpenSSL's EVP_MD_CTX_reset return value.
In neither OpenSSL nor BoringSSL can this function actually fail, but
OpenSSL makes it return one anyway. Match them for compatibility.

Change-Id: I497437321ad9ccc5da738f06cd5b19c467167575
Reviewed-on: https://boringssl-review.googlesource.com/28784
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-29 17:07:16 +00:00
David Benjamin
f86693dff7 Document the correct nonce length for AES-GCM.
It would be nice to restrict these, limiting the incorrect sizes to a
separate EVP_AEAD, but start by documenting this.

Bug: 34
Change-Id: I09845882f76a53a010355ceefd168d4fc10a0681
Reviewed-on: https://boringssl-review.googlesource.com/28745
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-24 22:13:07 +00:00
David Benjamin
2f5100e629 More compatibility stuff.
cryptography.io wants things exposed out of EVP_get_cipherby* including,
sadly, ECB mode.

Change-Id: I9bac46f8ffad1a79d190cee3b0c0686bf540298e
Reviewed-on: https://boringssl-review.googlesource.com/28464
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-15 23:57:53 +00:00
David Benjamin
d12f2ba55e Tweak RSA errors for compatibility.
cryptography.io wants RSA_R_BLOCK_TYPE_IS_NOT_02, only used by the
ancient RSA_padding_check_SSLv23 function. Define it but never emit it.

Additionally, it's rather finicky about RSA_R_TOO_LARGE* errors. We
merged them in BoringSSL because having RSA_R_TOO_LARGE,
RSA_R_TOO_LARGE_FOR_MODULUS, and RSA_R_TOO_LARGE_FOR_KEY_SIZE is a
little silly. But since we don't expect well-behaved code to condition
on error codes anyway, perhaps that wasn't worth it.  Split them back
up.

Looking through OpenSSL, there is a vague semantic difference:

RSA_R_DIGEST_TOO_BIG_FOR_RSA_KEY - Specifically emitted if a digest is
too big for PKCS#1 signing with this key.

RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE - You asked me to sign or encrypt a
digest/plaintext, but it's too big for this key.

RSA_R_DATA_TOO_LARGE_FOR_MODULUS - You gave me an RSA ciphertext or
signature and it is not fully reduced modulo N.
-OR-
The padding functions produced something that isn't reduced, but I
believe this is unreachable outside of RSA_NO_PADDING.

RSA_R_DATA_TOO_LARGE - Some low-level padding function was told to copy
a digest/plaintext into some buffer, but the buffer was too small. I
think this is basically unreachable.
-OR-
You asked me to verify a PSS signature, but I didn't need to bother
because the digest/salt parameters you picked were too big.

Update-Note: This depends on cl/196566462.
Change-Id: I2e539e075eff8bfcd52ccde365e975ebcee72567
Reviewed-on: https://boringssl-review.googlesource.com/28547
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 23:02:49 +00:00
David Benjamin
d6e31f6a56 Return more placeholder version strings.
PyOpenSSL's tests expect all of the outputs to be distinct. OpenSSL also
tends to prefix the return values with strings like "compiler:", so do
something similar.

Change-Id: Ic411c95a276b477641ebad803ac309b3035c1b13
Reviewed-on: https://boringssl-review.googlesource.com/28544
Reviewed-by: Adam Langley <agl@google.com>
2018-05-15 22:57:30 +00:00
David Benjamin
5b220ee70d Add APIs to query authentication properties of SSL_SESSIONs.
This is so Chromium can verify the session before offering it, rather
than doing it after the handshake (at which point it's too late to punt
the session) as we do today. This should, in turn, allow us to finally
verify certificates off a callback and order it correctly relative to
CertificateRequest in TLS 1.3.

(It will also order "correctly" in TLS 1.2, but this is useless. TLS 1.2
does not bind the CertificateRequest to the certificate at the point the
client needs to act on it.)

Bug: chromium:347402
Change-Id: I0daac2868c97b820aead6c3a7e4dc30d8ba44dc4
Reviewed-on: https://boringssl-review.googlesource.com/28405
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-05-14 19:10:48 +00:00
David Benjamin
103ed08549 Implement legacy OCSP APIs for libssl.
Previously, we'd omitted OpenSSL's OCSP APIs because they depend on a
complex OCSP mechanism and encourage the the unreliable server behavior
that hampers using OCSP stapling to fix revocation today. (OCSP
responses should not be fetched on-demand on a callback. They should be
managed like other server credentials and refreshed eagerly, so
temporary CA outage does not translate to loss of OCSP.)

But most of the APIs are byte-oriented anyway, so they're easy to
support. Intentionally omit the one that takes a bunch of OCSP_RESPIDs.

The callback is benign on the client (an artifact of OpenSSL reading
OCSP and verifying certificates in the wrong order). On the server, it
encourages unreliability, but pyOpenSSL/cryptography.io depends on this.
Dcument that this is only for compatibility with legacy software.

Also tweak a few things for compatilibility. cryptography.io expects
SSL_CTX_set_read_ahead to return something, SSL_get_server_tmp_key's
signature was wrong, and cryptography.io tries to redefine
SSL_get_server_tmp_key if SSL_CTRL_GET_SERVER_TMP_KEY is missing.

Change-Id: I2f99711783456bfb7324e9ad972510be8a95e845
Reviewed-on: https://boringssl-review.googlesource.com/28404
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 22:21:26 +00:00
David Benjamin
f05e3eafbc Add a bunch of X509_STORE getters and setters.
These were added in OpenSSL 1.1.0.

Change-Id: I261e0e0ccf82544883c4a2ef5c5dc4a651c0c756
Reviewed-on: https://boringssl-review.googlesource.com/28329
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:59:58 +00:00
David Benjamin
2e67153de4 Add PKCS12_create.
PyOpenSSL calls this function these days. Tested by roundtripping with
ourselves and also manually confirming our output interoperates with
OpenSSL.  (For anyone repeating this experiment, the OpenSSL
command-line tool has a bug and does not correctly output friendlyName
attributes with non-ASCII characters. I'll send them a PR to fix this
shortly.)

Between this and the UTF-8 logic earlier, the theme of this patch series
seems to be "implement in C something I last implemented in
JavaScript"...

Change-Id: I258d563498d82998c6bffc6789efeaba36fe3a5e
Reviewed-on: https://boringssl-review.googlesource.com/28328
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:59:34 +00:00
David Benjamin
a3c2517bd9 Add i2d_PKCS12*.
This is not very useful without PKCS12_create, which a follow-up change
will implement.

Change-Id: I355ccd22a165830911ae189871ab90a6101f42ae
Reviewed-on: https://boringssl-review.googlesource.com/28327
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:59:20 +00:00
David Benjamin
bc2562e50e Treat PKCS#12 passwords as UTF-8.
This aligns with OpenSSL 1.1.0's behavior, which deviated from OpenSSL
1.0.2. OpenSSL 1.0.2 effectively assumed input passwords were always
Latin-1.

Update-Note: If anyone was using PKCS#12 passwords with non-ASCII
characters, this changes them from being encoding-confused to hopefully
interpretting "correctly". If this breaks anything, we can add a
fallback to PKCS12_get_key_and_certs/PKCS12_parse, but OpenSSL 1.1.0
does not have such behavior. It only implements a fallback in the
command-line tool, not the APIs.

Change-Id: I0aa92db26077b07a40f85b89f4d3e0f6b0d7be87
Reviewed-on: https://boringssl-review.googlesource.com/28326
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:58:56 +00:00
David Benjamin
ae153bb9a6 Use new encoding functions in ASN1_mbstring_ncopy.
Update-Note: This changes causes BoringSSL to be stricter about handling
Unicode strings:
  · Reject code points outside of Unicode
  · Reject surrogate values
  · Don't allow invalid UTF-8 to pass through when the source claims to
    be UTF-8 already.
  · Drop byte-order marks.

Previously, for example, a UniversalString could contain a large-valued
code point that would cause the UTF-8 encoder to emit invalid UTF-8.

Change-Id: I94d9db7796b70491b04494be84249907ff8fb46c
Reviewed-on: https://boringssl-review.googlesource.com/28325
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 21:58:47 +00:00
David Benjamin
5f001d1423 Const-correct some functions.
Callers should not mutate these.

Update-Note: I believe I've fixed up everything. If I missed one, the
fix should be straightforward.

Change-Id: Ifbce4961204822f57502a0de33aaa5a2a08b026d
Reviewed-on: https://boringssl-review.googlesource.com/28266
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-05-11 15:10:35 +00:00
Steven Valdez
56c4ed9ad7 Allow enabling all TLS 1.3 variants by setting |tls13_default|.
Update-Note: Enabling TLS 1.3 now enables both draft-23 and draft-28
by default, in preparation for cycling all to draft-28.
Change-Id: I9405f39081f2e5f7049aaae8a9c85399f21df047
Reviewed-on: https://boringssl-review.googlesource.com/28304
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-05-10 20:27:34 +00:00
Matthew Braithwaite
3babc86d0f Expand the documentation of |SSL_set_shed_handshake_config|.
Change-Id: I49a693ef8aef2a0d83bc5d1c71bd896e28bf1a98
Reviewed-on: https://boringssl-review.googlesource.com/28246
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-05-08 23:23:55 +00:00
David Benjamin
8094b54eb1 Add BIO versions of i2d_DHparams and d2i_DHparams.
Change-Id: Ie643aaaa44aef67932b107d31ef92c2649738051
Reviewed-on: https://boringssl-review.googlesource.com/28269
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-08 23:12:15 +00:00
David Benjamin
02de7bd3a0 Add some more accessors to SSL_SESSION.
Hopefully this is the last of it before we can hide the struct. We're
missing peer_sha256 accessors, and some test wants to mutate the ticket
in a test client.

Change-Id: I1a30fcc0a1e866d42acbc07a776014c9257f7c86
Reviewed-on: https://boringssl-review.googlesource.com/28268
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-08 22:50:45 +00:00
David Benjamin
5d626b223b Add some more compatibility functions.
Change-Id: I56afcd896cb9de1c69c788b4f6395f4e78140d81
Reviewed-on: https://boringssl-review.googlesource.com/28265
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-08 20:51:15 +00:00
David Benjamin
477a9262f2 Bump BORINGSSL_API_VERSION.
Update-Note: https://boringssl-review.googlesource.com/28224 added
i2d_re_X509_tbs which was a 1.0.2 API we'd missed. Adding it is
ultimately more compatible, but will break
https://github.com/google/certificate-transparency/blob/master/cpp/log/cert.cc#L34
due to its OPENSSL_IS_BORINGSSL ifdef.

Bump BORINGSSL_API_VERSION so that we can patch that file with a
BORINGSSL_API_VERSION version check.

Change-Id: I9c83f5138a0215b554351b67ed51714d04428bd2
Reviewed-on: https://boringssl-review.googlesource.com/28264
Reviewed-by: Adam Langley <agl@google.com>
2018-05-08 17:40:55 +00:00
David Benjamin
91374e0cd2 Add a stub e_os2.h header.
Some third-party projects include it for some inexplicable reason.

Change-Id: I57c406d77d82a4a9ba6b54519023f2b02f2eb5e2
Reviewed-on: https://boringssl-review.googlesource.com/28225
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-08 01:32:14 +00:00
David Benjamin
0318b051ee Add some OpenSSL compatibility functions and hacks.
Change-Id: Ie42e57441f5fd7d1557a7fc1c648cf3f28b9c4db
Reviewed-on: https://boringssl-review.googlesource.com/28224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-05-08 01:22:04 +00:00
David Benjamin
ed188fd8ef Enforce supported_versions in the second ServerHello.
We forgot to do this in our original implementation on general ecosystem
grounds. It's also mandated starting draft-26.

Just to avoid unnecessary turbulence, since draft-23 is doomed to die
anyway, condition this on our draft-28 implementation. (We don't support
24 through 27.)

We'd actually checked this already on the Go side, but the spec wants a
different alert.

Change-Id: I0014cda03d7129df0b48de077e45f8ae9fd16976
Reviewed-on: https://boringssl-review.googlesource.com/28124
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-05-07 19:05:20 +00:00
Matthew Braithwaite
e30fac6371 Fuzz SSL_serialize_handoff() and SSL_serialize_handback().
This is done by adding two new tagged data types to the shim's
transcript: one for the serialized handoff, and another for the
serialized handback.

Then, the handshake driver in |TLSFuzzer| is modified to be able to
drive a handoff+handback sequence in the same way as was done for
testing: by swapping |BIO|s into additional |SSL| objects.  (If a
particular transcript does not contain a serialized handoff, this is a
no-op.)

Change-Id: Iab23e4dc27959ffd3d444adc41d40a4274e83653
Reviewed-on: https://boringssl-review.googlesource.com/27204
Commit-Queue: Matt Braithwaite <mab@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-05 02:41:04 +00:00
David Benjamin
8e75ae4880 Add a Wycheproof driver for AES-CBC.
Change-Id: I782ea51e1db8d05f552832a7c6910954fa2dda5f
Reviewed-on: https://boringssl-review.googlesource.com/27924
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-05-02 19:41:48 +00:00
David Benjamin
6e678eeb6e Remove legacy SHA-2 CBC ciphers.
All CBC ciphers in TLS are broken and insecure. TLS 1.2 introduced
AEAD-based ciphers which avoid their many problems. It also introduced
new CBC ciphers based on HMAC-SHA256 and HMAC-SHA384 that share the same
flaws as the original HMAC-SHA1 ones. These serve no purpose. Old
clients don't support them, they have the highest overhead of all TLS
ciphers, and new clients can use AEADs anyway.

Remove them from libssl. This is the smaller, more easily reverted
portion of the removal. If it survives a week or so, we can unwind a lot
more code elsewhere in libcrypto. This removal will allow us to clear
some indirect calls from crypto/cipher_extra/tls_cbc.c, aligning with
the recommendations here:

https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#2-avoid-indirect-branches-in-constant-time-code

Update-Note: The following cipher suites are removed:
- TLS_RSA_WITH_AES_128_CBC_SHA256
- TLS_RSA_WITH_AES_256_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
- TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384

Change-Id: I7ade0fc1fa2464626560d156659893899aab6f77
Reviewed-on: https://boringssl-review.googlesource.com/27944
Reviewed-by: Adam Langley <agl@google.com>
2018-05-02 19:21:56 +00:00
David Benjamin
71666cb87c Allow renego and config shedding to coexist more smoothly.
Chrome needs to support renegotiation at TLS 1.2 + HTTP/1.1, but we're
free to shed the handshake configuration at TLS 1.3 or HTTP/2.

Rather than making config shedding implicitly disable renegotiation,
make the actual shedding dependent on a combination of the two settings.
If config shedding is enabled, but so is renegotiation (including
whether we are a client, etc.), leave the config around. If the
renegotiation setting gets disabled again after the handshake,
re-evaluate and shed the config then.

Bug: 123
Change-Id: Ie833f413b3f15b8f0ede617991e3fef239d4a323
Reviewed-on: https://boringssl-review.googlesource.com/27904
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-05-01 23:28:59 +00:00
Matthew Braithwaite
b7bc80a9a6 SSL_CONFIG: new struct for sheddable handshake configuration.
|SSL_CONFIG| is a container for bits of configuration that are
unneeded after the handshake completes.  By default it is retained for
the life of the |SSL|, but it may be shed at the caller's option by
calling SSL_set_shed_handshake_config().  This is incompatible with
renegotiation, and with SSL_clear().

|SSL_CONFIG| is reachable by |ssl->config| and by |hs->config|.  The
latter is always non-NULL.  To avoid null checks, I've changed the
signature of a number of functions from |SSL*| arguments to
|SSL_HANDSHAKE*| arguments.

When configuration has been shed, setters that touch |SSL_CONFIG|
return an error value if that is possible.  Setters that return |void|
do nothing.

Getters that request |SSL_CONFIG| values will fail with an |assert| if
the configuration has been shed.  When asserts are compiled out, they
will return an error value.

The aim of this commit is to simplify analysis of split-handshakes by
making it obvious that some bits of state have no effects beyond the
handshake.  It also cuts down on memory usage.

Of note: |SSL_CTX| is still reachable after the configuration has been
shed, and a couple things need to be retained only for the sake of
post-handshake hooks.  Perhaps these can be fixed in time.

Change-Id: Idf09642e0518945b81a1e9fcd7331cc9cf7cc2d6
Bug: 123
Reviewed-on: https://boringssl-review.googlesource.com/27644
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-05-01 20:40:16 +00:00
David Benjamin
855dabc9df Add an accessor for session->certs.
Chromium has some code which reaches into this field for memory
accounting.

This fixes a bug in doc.go where this line-wrapping confuses it. doc.go
needs a bit of a rewrite, but this is a bit better.

Change-Id: Ic9cc2c2fe9329d7bc366ccf91e0c9a92eae08ed2
Reviewed-on: https://boringssl-review.googlesource.com/27764
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-04-27 17:14:38 +00:00