Commit Graph

5500 Commits

Author SHA1 Message Date
David Benjamin
32345ce6f2 Minor fixes to bytestring.h header.
Ryan noticed that CBS_ASN1_{SEQUENCE,SET} used CBS_ASN1_CONSTRUCTED
before it was defined. The C preprocessor expands late, so this works,
but it is weird. Flip the order.

There was also some question about the constructed bit, which is
different from how ASN.1 formally specifies it. (ASN.1 believes the
constructed bit is a property of the element, not the tag. We fold it in
because it's entirely computable[*] from the type in DER, so it's easier
to fold it in.) Move existing text to the section header and expand on
it.

[*] DER forbids constructed strings so string types are always
primitive. ASN.1 forbids implicitly tagging CHOICE or ANY, so the
inherited constructed bit cannot vary by value.

Change-Id: Ieb91f9d6898d582dda19fec8b042051b67f217a8
Reviewed-on: https://boringssl-review.googlesource.com/c/32725
Reviewed-by: Ryan Sleevi <rsleevi@chromium.org>
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-30 21:00:45 +00:00
David Benjamin
42d723f634 Test CBC padding more aggressively.
tls_cbc.c is concerned with the variation in where the padding+mac may
end, counted in blocks. Hash blocks are larger than block cipher blocks,
and the hash itself appends some padding. Thus maximal padding off a
64-hash.Size() bytes may not fully stress things.

Just run all inputs modulo the hash block size, so we don't have to
think very hard about the "most difficult" input.

Change-Id: I8da1427dfff855931c14a9135c22afbff4f367c0
Reviewed-on: https://boringssl-review.googlesource.com/c/32724
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-10-30 20:53:00 +00:00
David Benjamin
ad898b1fb9 Restore CHECKED_CAST.
Although this macro is not public API and is unused in BoringSSL,
wpa_supplicant uses it to define its own stacks. Remove this once
wpa_supplicant has been fixed.

Change-Id: I1f85e06efe4057b6490bf93bf4dea773dcb491c5
Reviewed-on: https://boringssl-review.googlesource.com/c/32764
Reviewed-by: Robert Sloan <varomodt@google.com>
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-30 20:47:00 +00:00
armfazh
144d924e0b Fix EVP_tls_cbc_digest_record is slow using SHA-384 and short messages
Symptom: When using larger hash functions and short messages,
these six blocks take too much time to be conditionally copied.

Observations:
 - SHA-384 consumes more data per iteration, unlike SHA-256.
 - The value of `kVarianceBlocks` must depend on the parameters
   of the selected hash algorithm.
 - Avoid magic constants.

Changes:
 - A new formula for the kVarianceBlocks value.
 - Stronger test vectors were created in change: 32724.
 - The new formula passes these tests.

Discussion:
 OpenSSL team: https://github.com/openssl/openssl/pull/7342
 Quoting mattcaswell:
> The "real" data that needs to be hashed has to be padded for the
> hashing algorithm. For SHA1 the smallest amount of padding that
> can be added is the "0x80" byte plus 8 bytes containing the message
> length, i.e. 9 bytes. If the data length is within 9 bytes of the
> end of the hash block boundary then the padding will push it into
> an extra block to be hashed.

Change-Id: Id1ad2389927014316eed2b453aac6e4c2a585c5c
Reviewed-on: https://boringssl-review.googlesource.com/c/32624
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-10-29 18:26:27 +00:00
David Benjamin
aa8d29dbd1 Tidy up dsa_sign_setup.
This function is not exported, so we don't need the optional BN_CTX
logic. Additionally, the cleanup code can be made a bit simpler and more
idiomatic.

Change-Id: Ib326eab4813fd9de9ac1df8fdc9e470c26aff092
Reviewed-on: https://boringssl-review.googlesource.com/c/32704
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-10-25 21:51:57 +00:00
David Benjamin
53d9fdd548 Fix the build on glibc 2.15.
glibc didn't add getauxval or sys/auxv.h until 2.16. glib 2.16.0 is six
years old and thus glibc 2.15 is past our support horizon, however
Android is using an outdated sysroot. Temporarily allow this until they
fix their toolchain.

Change-Id: I24e231cf40829e446969f67bf15c32e0b007de4c
Reviewed-on: https://boringssl-review.googlesource.com/c/32686
Reviewed-by: Robert Sloan <varomodt@google.com>
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-10-24 17:39:48 +00:00
David Benjamin
749d187063 Modernize OPENSSL_COMPILE_ASSERT.
MSVC 2015 supports the static_assert keyword in C mode (not quite what C11
specifies: _Static_assert is the keyword and static_assert is a macro in
assert.h, but close enough). GCC and Clang both support _Static_assert at all C
versions. GCC has supported it in GCC 4.6.

glibc supports the assert.h macro since glibc 2.16, but does condition it on
the version, so we likely can't rely on that yet. Still, this means we should
be able to rely on proper static assertions at this point. In particular, this
means we'd no longer worry about emitting multiple typedefs of the same name.

Though at some point, it'd be nice to rely on being built in C11 mode. Then we
can just pull in assert.h and use bare static_assert, and the atomics business
needn't be a build flag.

Update-Note: If static asserts break the build, it's this CL's fault.
Change-Id: I1b09043aae41242f6d40386c063e381d00b028d8
Reviewed-on: https://boringssl-review.googlesource.com/c/32604
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-24 00:55:44 +00:00
Robert Sloan
127a1ec080 Fix redefinition of AEAD asserts in e_aes.c.
Following https://boringssl-review.googlesource.com/c/32506. Many parts
of android don't have c11 support, and so they complain when these
asserts implicitly redefine, e.g. AEAD_state_too_small.

Failure reference: https://android-build.googleplex.com/builds/pending/P6876320/aosp_cf_x86_phone-userdebug/latest/view/logs/build_error.log

Change-Id: Icbdd9aec6bf3b3d87e15d7f4f37505a1639b59c0
Reviewed-on: https://boringssl-review.googlesource.com/c/32684
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-24 00:29:29 +00:00
Robert Sloan
b64c53fcfd Guard sys/auxv.h include on !BORINGSSL_ANDROID.
Some versions of Android libc don't even include the header.

Change-Id: Ib1033d2b8a10ba69d834ac1ed2564870e0e35d61
Reviewed-on: https://boringssl-review.googlesource.com/c/32664
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-23 18:20:59 +00:00
Adam Langley
35fb591f24 Flatten EVP_AEAD_CTX
An EVP_AEAD_CTX used to be a small struct that contained a pointer to
an AEAD-specific context. That involved heap allocating the
AEAD-specific context, which was a problem for users who wanted to setup
and discard these objects quickly.

Instead this change makes EVP_AEAD_CTX large enough to contain the
AEAD-specific context inside itself. The dominant AEAD is AES-GCM, and
that's also the largest. So, in practice, this shouldn't waste too much
memory.

Change-Id: I795cb37afae9df1424f882adaf514a222e040c80
Reviewed-on: https://boringssl-review.googlesource.com/c/32506
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-10-22 23:12:57 +00:00
Jeremy Apthorp
c0c9001440 Implement SSL_get_tlsext_status_type
It's used by Node.js[1], and is simple to implement.

[1]: e2f58c71dd/src/node_crypto.cc (L2390)

Change-Id: Ie5c76b848623d00f7478aeae0214c25472de523c
Reviewed-on: https://boringssl-review.googlesource.com/c/32525
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-19 00:30:32 +00:00
David Benjamin
6f579c0e9e Fix documentation sectioning.
Sections are separated by two blank lines.

Change-Id: If4f94a3b8f96044e83ab116e7603f1654130a551
Reviewed-on: https://boringssl-review.googlesource.com/c/32584
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-18 19:53:35 +00:00
David Benjamin
cbfe4f5a8e Remove support for GCC 4.7.
This reverts https://boringssl-review.googlesource.com/24924. As noted
there, GCC 4.7 support ends 2018-03-23, which has passed. GCC 4.8.0 was
released 2013-03-22, so we are now past the five year mark, matching
Abseil's guidelines.

Abseil also now explicitly lists supported compilers and explicitly
requires GCC 4.8+. https://abseil.io/docs/cpp/platforms/platforms

gRPC also now requires 4.8 per
https://github.com/grpc/grpc/issues/10036#issuecomment-290248204

Update-Note: On the off chance someone was using GCC 4.7, which only
started working in January, that'll no longer work.

Change-Id: Ie017822e903f98293e7b5e9bda10f104f17be7b3
Reviewed-on: https://boringssl-review.googlesource.com/c/32564
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-10-18 19:01:20 +00:00
Adam Langley
dd412c428a Print the name of the binary when blocking in getrandom.
If a startup process blocks, it's very useful to know which it was.

Change-Id: I04dd541695a61cfceb8142ea45d4bd5e3492c6ec
Update-note: updates internal bug 117227663.
Reviewed-on: https://boringssl-review.googlesource.com/c/32544
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-10-18 18:13:12 +00:00
Adam Langley
f8a8946841 Undo recent changes to |X509V3_EXT_conf_nid|.
cryptography.io wraps this function and so we have to keep the LHASH_OF
argument for now.

Change-Id: I4e071dee973c3931a4005678ce4135161a5861bd
Reviewed-on: https://boringssl-review.googlesource.com/c/32524
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-10-17 21:05:45 +00:00
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
Aaron Green
0e150027f9 [util] Mark srtp.h as an SSL header file
This CL adds srtp.h to the list of SSLHeaderFiles, in order to move it
from ssl_h_files to crypto_h_files. The header file only includes an
inclusion of ssl.h. ssl_h_files can depend on crypt_h_files but not the
other way around.

Change-Id: If7410624a8b2bbbd5afb7f66ec6f491968faf24e
Reviewed-on: https://boringssl-review.googlesource.com/c/32505
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-16 19:26:06 +00:00
Aaron Green
8c659c1fce [rand] Disable RandTest.Fork on Fuchsia
This CL omits the RandTest.Fork unit test on Fuchsia, which does not
have fork().  Fuchsia has a bug (SEC-140) to create a suitable
replacement test.

Change-Id: Ic42f9149c24dc7321bfac1c718e9ecbb4a18b5d0
Reviewed-on: https://boringssl-review.googlesource.com/c/32504
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-16 18:57:29 +00:00
David Benjamin
6650898e09 Remove -fsanitize-cfi-icall-generalize-pointers.
Bug: chromium:785442
Change-Id: Ia073fcae716541bc9d008e3e2148e9f0ac30e637
Reviewed-on: https://boringssl-review.googlesource.com/c/32121
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:54:44 +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
Adam Langley
b018908475 Better handle AVX-512 assembly syntax.
https://boringssl-review.googlesource.com/c/boringssl/+/24945 was mistaken in
that it thought that these AVX-512 assembly extensions were an
instruction-level thing, whereas they actually appear to be an argument-level
modifier.

This change parses them as such and unbreaks some AVX-512 instructions that can
be emitted by compilers with certain combinations of flags.

Change-Id: I9af5a4fec21f55d3198a248c9175252e229c355a
Reviewed-on: https://boringssl-review.googlesource.com/c/32484
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-10-15 23:31:23 +00:00
David Benjamin
80aa694975 Always push errors on BIO_read_asn1 failure.
This is consistent with the old behavior of d2i_*_fp and avoids tripping
Conscrypt's unnecessarily fragile error-handling (see
https://github.com/google/conscrypt/pull/552).

Additionally, by source inspection, CPython expects
ASN1_R_HEADER_TOO_LONG on EOF, analogously to PEM_R_NO_START_LINE. Fix
that. The other errors are a bit haphazard in the old implementation
(that code is really hard to follow), so I didn't match it too
carefully. In particular, OpenSSL would report ASN1_R_HEADER_TOO_LONG on
some generic tag parsing, but that is inconsistent with
ASN1_R_HEADER_TOO_LONG being an EOF signal.

Update-Note: https://boringssl-review.googlesource.com/32106 may have
caused some compatibility issues. This should fix it.

Change-Id: Idfe2746ffd7733de4338e14c58a40753e98a791e
Reviewed-on: https://boringssl-review.googlesource.com/c/32444
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-11 19:53:15 +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
Yoshisato Yanagisawa
e341802802 Fix div.c to divide BN_ULLONG only if BN_CAN_DIVIDE_ULLONG defined.
Since clang-cl uses __udivti3 for __uint128_t division, linking div.obj
fails.  Let me make div.c use BN_CAN_DIVIDE_ULLONG to decide using
__uint128_t division instead of BN_ULLONG.

Bug: https://bugs.chromium.org/p/chromium/issues/detail?id=787617
Change-Id: I3ebe245f6b8917d59409591992efbabddea08187
Reviewed-on: https://boringssl-review.googlesource.com/c/32404
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-10-10 15:33:35 +00:00
Aaron Green
28babde159 Include aes.h in mode/internal.h
block128_f was recently changed to take an AES_KEY instead of a void*,
but AES_KEY is not defined in base.h.  internal.h should not depend on
other sources to include aes.h for it.

Change-Id: I81aab5124ce4397eb76a83ff09779bfaea66d3c1
Reviewed-on: https://boringssl-review.googlesource.com/32364
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-10-03 17:36:04 +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
Aaron Green
c1eef7f795 [perlasm] Hide OPENSSL_armcap_P in assembly
This CL changes adds a ".hidden OPENSSL_armcap_P" statement to the
".comm OPENSSL_armcap_P" statements for the sha*-armv8.pl files,
similar to what was doen for the sha*-armv4.pl files in CL 3471.

Change-Id: I524b3dce7e5cfe017498847fbf9b8a5df4b98fce
Reviewed-on: https://boringssl-review.googlesource.com/c/32324
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-10-02 20:36:43 +00:00
David Benjamin
ce00828c89 Test the binary search more aggressively.
https://boringssl-review.googlesource.com/c/boringssl/+/32115/ wasn't
worth it, but we may as well keep the test.  Also add a comment about
the asymptotics in case it ever comes up.

Change-Id: Ic4773106f1003adc56b4ce36520a18d3ac2d6f13
Reviewed-on: https://boringssl-review.googlesource.com/32284
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-02 00:02:19 +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
9e97c022e6 Bring Mac and iOS builders back to the CQ.
The vpython issue appears to have gone away and hermetic Xcode sorted
out the other problem.

Bug: chromium:888687, chromium:890351
Change-Id: I9da893b7f21f0bc7c03e1e70c0e3e86f9720cec1
Reviewed-on: https://boringssl-review.googlesource.com/32304
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-10-01 23:31:45 +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
cbc3e076fc Take iOS builders out of the CQ rotation too.
Bug: chromium:890351
Change-Id: Ia11b2b97f25d0c37e491641db6c48aa37c03de30
Reviewed-on: https://boringssl-review.googlesource.com/32224
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 17:41:54 +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
73535ab252 Fix undefined block128_f, etc., casts.
This one is a little thorny. All the various block cipher modes
functions and callbacks take a void *key. This allows them to be used
with multiple kinds of block ciphers.

However, the implementations of those callbacks are the normal typed
functions, like AES_encrypt. Those take AES_KEY *key. While, at the ABI
level, this is perfectly fine, C considers this undefined behavior.

If we wish to preserve this genericness, we could either instantiate
multiple versions of these mode functions or create wrappers of
AES_encrypt, etc., that take void *key.

The former means more code and is tedious without C++ templates (maybe
someday...). The latter would not be difficult for a compiler to
optimize out. C mistakenly allowed comparing function pointers for
equality, which means a compiler cannot replace pointers to wrapper
functions with the real thing. (That said, the performance-sensitive
bits already act in chunks, e.g. ctr128_f, so the function call overhead
shouldn't matter.)

But our only 128-bit block cipher is AES anyway, so I just switched
things to use AES_KEY throughout. AES is doing fine, and hopefully we
would have the sense not to pair a hypothetical future block cipher with
so many modes!

Change-Id: Ied3e843f0e3042a439f09e655b29847ade9d4c7d
Reviewed-on: https://boringssl-review.googlesource.com/32107
Reviewed-by: Adam Langley <agl@google.com>
2018-10-01 17:35:02 +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
Adam Langley
3474270abd Always print some diagnostic information when POST fails.
Debugging a POST failure when it prints nothing is painful. The
|check_test| helper already prints out information when it fails, but
some other paths were not handled. This change adds printfs for those
cases.

Change-Id: Ife71bb292a4f69679d0fa56686863aae9423e451
Updating-Note: updates internal bug 116469121
Reviewed-on: https://boringssl-review.googlesource.com/32145
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-28 19:33:38 +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
892a31b5fb Add a test for STACK_OF(T).
Amazingly, this module didn't have a unit test yet.

Change-Id: I021bb83cc747174196958db14c97154f0574c2e8
Reviewed-on: https://boringssl-review.googlesource.com/32111
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-09-26 23:28:50 +00:00
Matthew Braithwaite
7039f40368 Rename inject-hash: Bazel does not like hyphens.
(Only in package names.  Hyphens in file names are file.)

Change-Id: I80b705a780ffbad056abe7a7868d5682b30d2d44
Reviewed-on: https://boringssl-review.googlesource.com/32144
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-09-26 21:50:36 +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
1764d7a3ea Fix ERR_GET_REASON checks.
Reason codes across libraries may collide. One must never check
ERR_GET_REASON without also checking ERR_GET_LIB.

Change-Id: I0b58ce27a5571ab173d231c1a673bce1cf0427aa
Reviewed-on: https://boringssl-review.googlesource.com/32110
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-26 19:02:42 +00:00
David Benjamin
e7692f5598 Add a basic test for PEM_X509_INFO_read_bio.
This format is kind of silly, but it seems not completely unused? Add a
basic test for it before I rewrite it to fix the function pointer casts.

Change-Id: Ib2d1563419b72cf468180b9cda4d13e216b7eb3a
Reviewed-on: https://boringssl-review.googlesource.com/32108
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-26 17:42:58 +00:00