Chromium uses a zygote process and a sandbox on Linux. In order for RAND_bytes
to be functional and guaranteed fork-safe inside the renderers, /dev/urandom
must be prewarmed. Calling RAND_bytes initializes a thread-local ChaCha20 key
when rdrand is available. So that key is fork-safe and to avoid tempting any
dragons by touching pthreads APIs before a non-exec fork, add a
RAND_set_urandom_fd API. It allows the consumer to supply the /dev/urandom fd
and promises to be fork-safe, both in initializing key material and use of
pthreads.
This doesn't affect any current shipping versions of Chrome.
BUG=462040
Change-Id: I1037e21e525918971380e4ea1371703c8237a0b0
Reviewed-on: https://boringssl-review.googlesource.com/5302
Reviewed-by: Adam Langley <agl@google.com>
This removes EVP_PKEY_HMAC and all the support code around it. EVP_MD requires
a lot of extra glue to support HMAC. This lets us prune it all away.
As a bonus, it removes a (minor) dependency from EVP to the legacy ASN.1 stack.
Change-Id: I5a9e3e39f518429828dbf13d14647fb37d9dc35a
Reviewed-on: https://boringssl-review.googlesource.com/5120
Reviewed-by: Adam Langley <agl@google.com>
This change causes the generated assembly files for ARM and AArch64 to
have #if guards for __arm__ and __aarch64__, respectively. Since
building on ARM is only supported for Linux, we only have to worry about
GCC/Clang's predefines.
Change-Id: I7198eab6230bcfc26257f0fb6a0cc3166df0bb29
Reviewed-on: https://boringssl-review.googlesource.com/5173
Reviewed-by: Adam Langley <agl@google.com>
I mistakenly believed that only RDSEED could fail. However, the Intel
manuals state that RDRAND can fail too.
I can't actually observe it failing, even with all cores running RDRAND in a
tight loop. In any case, the ChaCha20 masking means that it wouldn't be
a big deal anyway.
Still, this change tests the carry flag after RDRAND and the code falls
back to |CRYPTO_sysrand| if RDRAND has a hiccup. (The Intel manuals
suggest[1] calling RDRAND in a loop, ten times, before considering it to
have failed. But a single failure appears to be such a rare event that
the complexity in the asm code doesn't seem worth it.)
This change also adds an asm function to fill a buffer with random data.
Otherwise the overhead of calling |CRYPTO_rdrand|, and bouncing the data
in and out of memory starts to add up.
Thanks to W. Mark Kubacki, who may have reported this. (There's some
confusion in the bug report.)
Before:
Did 6148000 RNG (16 bytes) operations in 1000080us: 98.4 MB/s
Did 649000 RNG (256 bytes) operations in 1000281us: 166.1 MB/s
Did 22000 RNG (8192 bytes) operations in 1033538us: 174.4 MB/s
After:
Did 6573000 RNG (16 bytes) operations in 1000002us: 105.2 MB/s
Did 693000 RNG (256 bytes) operations in 1000127us: 177.4 MB/s
Did 24000 RNG (8192 bytes) operations in 1028466us: 191.2 MB/s
[1] Intel Reference Manual, section 7.3.17.1.
Change-Id: Iba7f82e844ebacef535472a31f2dd749aad1190a
Reviewed-on: https://boringssl-review.googlesource.com/5180
Reviewed-by: Adam Langley <agl@google.com>
AES-GCM should have a 12-byte nonce. However, non-standard nonce sizes
are defined by NIST and, although they are a bad idea, people have used
them because they've confused an IV with an nonce and passed in a
16-byte nonce.
This change adds a test for this.
Change-Id: If1efa1aaa19f0119ad4cab9a02a6417c040f45b2
Also implement it without reference to crypto/asn1 or fake ASN1_INTEGERs and
add a test. Some platform crypto APIs only give back the key size, and not the
encoded signature length. No sense in implementing it twice.
BUG=347404,499653
Change-Id: I9aa27d52674375f8b036e57bb5850f091c9b25dd
Reviewed-on: https://boringssl-review.googlesource.com/5080
Reviewed-by: Adam Langley <agl@google.com>
Platform crypto APIs for PKCS#1 RSA signatures vary between expecting the
caller to prepend the DigestInfo prefix (RSA_sign_raw) and prepending it
internally (RSA_sign). Currently, Chromium implements sign or sign_raw as
appropriate. To avoid needing both variants, the new asynchronous methods will
only expose the higher-level one, sign.
To satisfy ports which previously implemented sign_raw, expose the DigestInfo
prefix as a utility function.
BUG=347404
Change-Id: I04c397b5e9502b2942f6698ecf81662a3c9282e6
Reviewed-on: https://boringssl-review.googlesource.com/4940
Reviewed-by: Adam Langley <agl@google.com>
Also tighten X509_cmp_time to reject more than three fractional
seconds in the time; and to reject trailing garbage after the offset.
CVE-2015-1789
(Imported from upstream's 9bc3665ac9)
Change-Id: I2091b2d1b691c177d58dc7960e2e7eb4c97b1f69
Reviewed-on: https://boringssl-review.googlesource.com/5124
Reviewed-by: Adam Langley <agl@google.com>
If gdb is attached, it's convenient to be able to continue running.
Change-Id: I3bbb2634d05a08f6bad5425f71da2210dbb80cfe
Reviewed-on: https://boringssl-review.googlesource.com/5125
Reviewed-by: Adam Langley <agl@google.com>
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.
Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.
Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
The binaries for these already have underscores in, best to have the
source files match.
Change-Id: I32a419f32ec7786fe2537d061eb0706a7bc73f4a
Reviewed-on: https://boringssl-review.googlesource.com/5101
Reviewed-by: Adam Langley <agl@google.com>
Per malloc(3):
The UNIX 98 standard requires malloc(), calloc(), and realloc() to set
errno to ENOMEM upon failure. Glibc assumes that this is done (and the
glibc versions of these routines do this); if you use a private malloc
implementation that does not set errno, then certain library routines may
fail without having a reason in errno.
Notably, thread_test otherwise fails an assertion deep in glibc.
Change-Id: Ia2c0ab306987476e7d6570d4bbf04a2641398925
Reviewed-on: https://boringssl-review.googlesource.com/5111
Reviewed-by: Adam Langley <agl@google.com>
BN_copy can fail on malloc failure. The case in crypto/rsa was causing the
malloc tests in all_tests.go to infinite loop.
Change-Id: Id5900512013fba9960444d78a8c056aa4314fb2d
Reviewed-on: https://boringssl-review.googlesource.com/5110
Reviewed-by: Adam Langley <agl@google.com>
When cross-compiling for Windows on a Linux system, the filesystem is
case sensitive and so #includes with uppercase characters cause errors.
Change-Id: I6fe6d677887ce84ba8b2c3136cf2c70998d96e81
Reviewed-on: https://boringssl-review.googlesource.com/5060
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
In doing so, check for malloc failures and use scopers as appropriate.
This should clear rsa_test for use with malloc tests. Also replace the
SetKey macro and exploded RSA keys with a DER RSAPrivateKey structure.
Much less tedious.
Change-Id: I3ce092ef67e7ac2af74f509abbdf84b7f2b6d45d
Reviewed-on: https://boringssl-review.googlesource.com/5043
Reviewed-by: Adam Langley <agl@google.com>
We have need of it internally.
Change-Id: I564af468728b22245e8eab384ea7018b7e88cc86
Reviewed-on: https://boringssl-review.googlesource.com/5022
Reviewed-by: Matt Braithwaite <mab@google.com>
Reviewed-by: Adam Langley <agl@google.com>
Mac wants a stdlib.h. Windows wants a void and doesn't like static const in
array declarations.
Change-Id: If1e8fb141e41200cf8a7348c6918c2f22465f5fe
Reviewed-on: https://boringssl-review.googlesource.com/5030
Reviewed-by: Adam Langley <agl@google.com>
This change makes |CBS_get_any_asn1_element| only handle DER elements.
Another function, |CBS_get_any_ber_asn1_element| is exposed internally
for the cases where we need to process BER data.
Change-Id: I544141a1a3d7913986352a8fd9a6d00b9f282652
Reviewed-on: https://boringssl-review.googlesource.com/4994
Reviewed-by: Adam Langley <agl@google.com>
The documentation for |CBS_get_any_asn1_element| says that
|out_header_len| may be NULL, but in the case of an indefinite-length
element it would be written unconditionally.
Thanks to Doug Hogan for noticing this.
Change-Id: I17609b3465df73d42dd9efd75e783159aa99a59b
Reviewed-on: https://boringssl-review.googlesource.com/4993
Reviewed-by: Adam Langley <agl@google.com>
The client and server both have to decide on behaviour when resuming a
session where the EMS state of the session doesn't match the EMS state
as exchanged in the handshake.
Original handshake
| No Yes
------+--------------------------------------------------------------
|
R | Server: ok [1] Server: abort [3]
e No | Client: ok [2] Client: abort [4]
s |
u |
m |
e |
Yes | Server: don't resume No problem
| Client: abort; server
| shouldn't have resumed
[1] Servers want to accept legacy clients. The draft[5] says that
resumptions SHOULD be rejected so that Triple-Handshake can't be done,
but we'll rather enforce that EMS was used when using tls-unique etc.
[2] The draft[5] says that even the initial handshake should be aborted
if the server doesn't support EMS, but we need to be able to talk to the
world.
[3] This is a very weird case where a client has regressed without
flushing the session cache. Hopefully we can be strict and reject these.
[4] This can happen when a server-farm shares a session cache but
frontends are not all updated at once. If Chrome is strict here then
hopefully we can prevent any servers from existing that will try to
resume an EMS session that they don't understand. OpenSSL appears to be
ok here: https://www.ietf.org/mail-archive/web/tls/current/msg16570.html
[5] https://tools.ietf.org/html/draft-ietf-tls-session-hash-05#section-5.2
BUG=492200
Change-Id: Ie1225a3960d49117b05eefa5a36263d8e556e467
Reviewed-on: https://boringssl-review.googlesource.com/4981
Reviewed-by: Adam Langley <agl@google.com>
This implementation does not prompt for a password. It's just enough
to ensure that the many functions that take a tuple of
|pem_password_cb| and a |void *| to a password work in a reasonable
way when the latter is non-NULL.
Change-Id: Ic6bfc484630c67b5ede25277e14eb3b00c2024f0
Reviewed-on: https://boringssl-review.googlesource.com/4990
Reviewed-by: Adam Langley <agl@google.com>
It would be nice if assert(x) reduced to ((void) x) when NDEBUG was
defined, but it doesn't. Because of this, locally define CHECK, which
does. This avoids warnings with Clang.
Change-Id: I70882741da4984a025bcfaac1969032387f369de
Reviewed-on: https://boringssl-review.googlesource.com/4991
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
bsaes-armv7.S implements bsaes_cbc_encrypt if #if __ARM_MAX_ARCH__ >= 7
but e_aes.c instead used #if __ARM_ARCH >= 7 causing duplicate symbols
for linkers that care about that
Change-Id: I10ad8e24be75fdc03b0670869a53078b0477950b
Reviewed-on: https://boringssl-review.googlesource.com/4943
Reviewed-by: Adam Langley <agl@google.com>
The only flag is EVP_MD_CTX_FLAG_NO_INIT and no good can possibly come of
anyone outside EVP_PKEY_HMAC calling it. (And indeed no one calls it.
EVP_MD_CTX_set_flags has a caller in wpa_supplicant, but it uses
EVP_MD_CTX_FLAG_NON_FIPS_ALLOW which we don't define. The call is guarded by a
pair of ifdefs for some FIPS mode wpa_supplicant.)
Change-Id: I70ab8ffa646f3f75dfa4d37c96b9e82448ff1e40
Reviewed-on: https://boringssl-review.googlesource.com/4971
Reviewed-by: Adam Langley <agl@google.com>
It's never called externally and for good reason; the only flag to set is
EVP_MD_CTX_FLAG_NO_INIT which is an implementation detail of EVP_PKEY_HMAC
(hopefully to be removed eventually). Indeed, only EVP_PKEY_HMAC ever calls
this function. Except there's no need to because the HMAC_CTX has already been
initialized at that point. (And were it not initialized, that call would not
bode well for the poor HMAC_CTX.)
The legacy EVP_PKEY_HMAC API has test coverage and still works after this
change.
Change-Id: I2fb0bede3c24ad1519f9433f957606de15ba86c7
Reviewed-on: https://boringssl-review.googlesource.com/4970
Reviewed-by: Adam Langley <agl@google.com>
Not terribly important given that we already have NIST vectors, but may as
well. These tests come from upstream's
2cfbdd71dd.
Change-Id: I4f8dadc7d5d1599d0b75ecdef06f2fc6a5cd8003
Reviewed-on: https://boringssl-review.googlesource.com/4962
Reviewed-by: Adam Langley <agl@google.com>
The SSL_PROTOCOL_METHOD table needs work, but this makes it clearer
exactly what the shared interface between the upper later and TLS/DTLS
is.
BUG=468889
Change-Id: I38931c484aa4ab3f77964d708d38bfd349fac293
Reviewed-on: https://boringssl-review.googlesource.com/4955
Reviewed-by: Adam Langley <agl@google.com>
Enough code fails to check their return codes anyway. We ought to make
it official.
Change-Id: Ie646360fd7073ea943036f5e21bed13df7e1b77a
Reviewed-on: https://boringssl-review.googlesource.com/4954
Reviewed-by: Adam Langley <agl@google.com>
The SHA-2 family has some exceptions, but they're all programmer errors
and should be documented as such. (Are the failure cases even
necessary?)
Change-Id: I00bd0a9450cff78d8caac479817fbd8d3de872b8
Reviewed-on: https://boringssl-review.googlesource.com/4953
Reviewed-by: Adam Langley <agl@google.com>
Use sized integer types rather than unsigned char/int/long. The latter
two are especially a mess as they're both used in lieu of uint32_t.
Sometimes the code just blindly uses unsigned long and sometimes it uses
unsigned int when an LP64 architecture would notice.
Change-Id: I4c5c6aaf82cfe9fe523435588d286726a7c43056
Reviewed-on: https://boringssl-review.googlesource.com/4952
Reviewed-by: Adam Langley <agl@google.com>
At some point we might need to make this defined by the consumer.
BUG=495146
Change-Id: Iedac305f234cb383799a5afc14046cd10fb3256a
Reviewed-on: https://boringssl-review.googlesource.com/4963
Reviewed-by: Adam Langley <agl@google.com>
bn_test's output is meant to be piped to bc, but this got broken somewhat:
- OpenSSL uses uppercase hex rather than BoringSSL's lowercase. bc only accepts
uppercase. Document that this needs some shell pipeline until we replace
them with better tests because this is all ridiculous.
- Some stderr outputs moved to stdout to avoid cluttering stdout. Just remove
them. The operations are fast enough to not need progress.
- To cut down on noise, only write the bc transcript given a command-line flag.
Also remove the -results flag since it's pointless. (It writes only the
results and not the inputs.)
Change-Id: I08f87cac1e03fab461f0dc40b9d4285bd877807d
Reviewed-on: https://boringssl-review.googlesource.com/4896
Reviewed-by: Adam Langley <agl@google.com>
While this isn't really an issue, don't use the a - b comparator pattern since
it doesn't account for overflows. (They'll also break silently if that field
ever becomes unsigned as it should be.)
Change-Id: I613d19df6e4a785efd4cffd46e8b03dbc95b98e2
Reviewed-on: https://boringssl-review.googlesource.com/4890
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Adam Langley
Matt Braithwaite
Adam Langley
Joel Klinghed