Commit Graph

36 Commits

Author SHA1 Message Date
David Benjamin
f109f20873 Clear out a bunch of -Wextra-semi warnings.
Unfortunately, it's not enough to be able to turn it on thanks to the
PURE_VIRTUAL macro. But it gets us most of the way there.

Change-Id: Ie6ad5119fcfd420115fa49d7312f3586890244f4
Reviewed-on: https://boringssl-review.googlesource.com/c/34949
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2019-02-21 19:12:39 +00:00
David Benjamin
2e819d8be4 Unwind RDRAND functions correctly on Windows.
But for the ABI conversion bits, these are just leaf functions and don't
even need unwind tables. Just renumber the registers on Windows to only
used volatile ones.

In doing so, this switches to writing rdrand explicitly. perlasm already
knows how to manually encode it and our minimum assembler versions
surely cover rdrand by now anyway. Also add the .size directive. I'm not
sure what it's used for, but the other files have it.

(This isn't a generally reusable technique. The more complex functions
will need actual unwind codes.)

Bug: 259
Change-Id: I1d5669bcf8b6e34939885d78aea6f60597be1528
Reviewed-on: https://boringssl-review.googlesource.com/c/34867
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2019-02-12 20:24:27 +00:00
David Benjamin
33f456b8b0 Don't use bsaes over vpaes for CTR-DRBG.
RAND_bytes rarely uses large enough inputs for bsaes to be worth it.
https://boringssl-review.googlesource.com/c/boringssl/+/33589 includes some
rough benchmarks of various bits here. Some observations:

- 8 blocks of bsaes costs roughly 6.5 blocks of vpaes. Note the comparison
  isn't quite accurate because I'm measuring bsaes_ctr32_encrypt_blocks against
  vpaes_encrypt and vpaes in CTR mode today must make do with a C loop. Even
  assuming a cutoff of 6 rather than 7 blocks, it's rare to ask for 96 bytes
  of entropy at a time.

- CTR-DRBG performs some stray block operations (ctr_drbg_update), which bsaes
  is bad at without extra work to fold them into the CTR loop (not really worth
  it).

- CTR-DRBG calculates a couple new key schedules every RAND_bytes call. We
  don't currently have a constant-time bsaes key schedule. Unfortunately, even
  plain vpaes loses to the current aes_nohw used by bsaes, but it's not
  constant-time. Also taking CTR-DRBG out of the bsaes equation

- Machines without AES hardware (clients) are not going to be RNG-bound. It's
  mostly servers pushing way too many CBC IVs that care. This means bsaes's
  current side channel tradeoffs make even less sense here.

I'm not sure yet what we should do for the rest of the bsaes mess, but it seems
clear that we want to stick with vpaes for the RNG.

Bug: 256
Change-Id: Iec8f13af232794afd007cb1065913e8117eeee24
Reviewed-on: https://boringssl-review.googlesource.com/c/34744
Reviewed-by: Adam Langley <agl@google.com>
2019-02-01 18:03:39 +00:00
Adam Langley
e6bf9065af Remove pooling of PRNG state.
Prior to 82639e6f we used thread-local data for the PRNG state. That
change switched to using a mutex-protected pool instead in order to save
memory in heavily-threaded applications.

However, the pool mutex can get extremely hot in cases where the PRNG is
heavily used. 8e8f2504 was a short-term work around, but supporting both
modes is overly complex.

This change moves back to the state of the prior to 82639e6f. The best
way to review this is to diff the changed files against '82639e6f^' and
note that the only difference is a comment added in rand.c:
https://paste.googleplex.com/4997991748337664

Change-Id: I8febce089696fa6bc39f94f4a1e268127a8f78db
Reviewed-on: https://boringssl-review.googlesource.com/c/34024
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2019-01-03 20:19:44 +00:00
Adam Langley
8e8f250422 Use thread-local storage for PRNG states if fork-unsafe buffering is enabled.
We switched from thread-local storage to a mutex-pool in 82639e6f53
because, for highly-threaded processes, the memory used by all the
states could be quite large. I had judged that a mutex-pool should be
fine, but had underestimated the PRNG requirements of some of our jobs.

This change makes rand.c support using either thread-locals or a
mutex-pool. Thread-locals are used if fork-unsafe buffering is enabled.
While not strictly related to fork-safety, we already have the
fork-unsafe control, and it's already set by jobs that care a lot about
PRNG performance, so fits quite nicely here.

Change-Id: Iaf1e0171c70d4c8dbe1e42283ea13df5b613cb2d
Reviewed-on: https://boringssl-review.googlesource.com/c/31564
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
2018-12-28 18:05:18 +00:00
David Benjamin
5ecfb10d54 Modernize OPENSSL_COMPILE_ASSERT, part 2.
The change seems to have stuck, so bring us closer to C/++11 static asserts.

(If we later find we need to support worse toolchains, we can always use
__LINE__ or __COUNTER__ to avoid duplicate typedef names and just punt on
embedding the message into the type name.)

Change-Id: I0e5bb1106405066f07740728e19ebe13cae3e0ee
Reviewed-on: https://boringssl-review.googlesource.com/c/33145
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-11-14 16:06:37 +00:00
David Benjamin
e62bf02a13 Don't overflow state->calls on 16TiB RAND_bytes calls.
This is an extremely important and practical use case. The comment that
state->calls is bounded by the reseed interval isn't quite true. We only
check on entry to the function, which means that it may exceed it by one
call's worth. Switch it to a size_t (which doesn't actually increase
memory because the struct was already padded).

Change-Id: Ia7646fd5b4142789c1d613280223baa4cd1a4a9b
Reviewed-on: https://boringssl-review.googlesource.com/c/32804
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-11-02 18:33:43 +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
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
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
5ede28c8a4 Tighten up getrandom handling.
While I don't believe EINTR can occur with a non-blocking getrandom call
when talking to the kernel directly, that may not be true when certain
sandboxing systems are being used.

Additionally, with this change we will no longer silently ignore errors
other than ENOSYS.

Update-Note: update internal bug 115344138.

Change-Id: I952c132cf325dcc17dc38e68f054abc41de1f8b0
Reviewed-on: https://boringssl-review.googlesource.com/32006
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-20 16:15:51 +00:00
Adam Langley
6410e18e91 Update several assembly files from upstream.
This change syncs several assembly files from upstream. The only meanful
additions are more CFI directives.

Change-Id: I6aec50b6fddbea297b79bae22cfd68d5c115220f
Reviewed-on: https://boringssl-review.googlesource.com/30364
Reviewed-by: Adam Langley <agl@google.com>
2018-08-07 18:57:17 +00:00
Adam Langley
82639e6f53 Use a pool of |rand_state| objects.
Previously we used thread-local state objects in rand.c. However, for
applications with large numbers of threads, this can lead to excessive
memory usage.

This change causes us to maintain a mutex-protected pool of state
objects where the size of the pool equals the maximum concurrency of
|RAND_bytes|. This might lead to state objects bouncing between CPUs
more often, but should help the memory usage problem.

Change-Id: Ie83763d3bc139e64ac17bf7e015ad082b2f8a81a
Reviewed-on: https://boringssl-review.googlesource.com/29565
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-07-06 21:25:37 +00:00
Adam Langley
03de6813d8 Write error messages in the FIPS module to stderr.
Previously, delocate.go couldn't handle GOT references and so |stderr|
was a problematic symbol. We can cope with them now, so write FIPS
power-on test and urandom errors to stderr rather than stdout.

Change-Id: If6d7c19ee5f22dcbd74fb01c231500c2e130e6f7
Update-note: resolves internal bug 110102292.
Reviewed-on: https://boringssl-review.googlesource.com/29244
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-06-25 10:30:42 +00:00
Adam Langley
7d1f35985b Show an error before we abort the process for an entropy failure.
Change-Id: I8d8483d38de15dcde18141bb9cc9e79d585d24ad
Reviewed-on: https://boringssl-review.googlesource.com/27045
Reviewed-by: Adam Langley <agl@google.com>
2018-05-11 22:30:24 +00:00
David Benjamin
a1bc1ba47c Fix up CTR_DRBG_update comment.
The original comment was a little confusing. Also lowercase
CTR_DRBG_update to make our usual naming for static functions.

Bug: 227
Change-Id: I381c7ba12b788452d54520b7bc3b13bba8a59f2d
Reviewed-on: https://boringssl-review.googlesource.com/25204
Reviewed-by: Adam Langley <agl@google.com>
2018-01-23 22:19:03 +00:00
David Benjamin
f231d6bfa6 Remove CTR_DRBG_STATE alignment marker.
We don't get up to 16-byte alignment without additional work like
https://boringssl-review.googlesource.com/20204. This just makes UBSan
unhappy at us.

Change-Id: I55d9cb5b40e5177c3c7aac7828c1d22f2bfda9a6
Reviewed-on: https://boringssl-review.googlesource.com/20464
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>
2017-09-18 19:17:52 +00:00
Peter Wu
2c46c10631 Fix build when linux-headers are not installed.
linux/random.h is not really needed if FIPS mode is not enabled. Note
that use of the getrandom syscall is unaffected by this header.

Fixes commit bc7daec4d8

Change-Id: Ia367aeffb3f2802ba97fd1507de0b718d9ac2c55
Reviewed-on: https://boringssl-review.googlesource.com/19644
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>
2017-08-24 00:35:05 +00:00
David Benjamin
808f832917 Run the comment converter on libcrypto.
crypto/{asn1,x509,x509v3,pem} were skipped as they are still OpenSSL
style.

Change-Id: I3cd9a60e1cb483a981aca325041f3fbce294247c
Reviewed-on: https://boringssl-review.googlesource.com/19504
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>
2017-08-18 21:49:04 +00:00
David Benjamin
27e377ec65 Fix miscellaneous clang-tidy warnings.
There are still a ton of them, almost exclusively complaints that
function declaration and definitions have different parameter names. I
just fixed a few randomly.

Change-Id: I1072f3dba8f63372cda92425aa94f4aa9e3911fa
Reviewed-on: https://boringssl-review.googlesource.com/18706
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-08-01 20:39:46 +00:00
David Benjamin
7d53638872 Use __NR_getrandom rather than SYS_getrandom.
The former is defined by the kernel and is a straightforward number. The
latter is defined by glibc as:

  #define SYS_getrandom __NR_getrandom

which does not work when kernel headers are older than glibc headers.
Instead, use the kernel values.

Bug: chromium:742260
Change-Id: Id162f125db660643269e0b1329633437048575c4
Reviewed-on: https://boringssl-review.googlesource.com/17864
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>
2017-07-18 16:28:41 +00:00
David Benjamin
21882c5c75 Clarify rand locking comment.
This was specific to some old software on the test machine. Shrinking
the critical section to not cover getrandom is probably worthwhile
anyway though, so keep it around but make the comment less scary.

Change-Id: I8c17b6688ae93f6aef5d89c252900985d9e7bb52
Reviewed-on: https://boringssl-review.googlesource.com/16992
Reviewed-by: Adam Langley <agl@google.com>
2017-06-08 22:26:40 +00:00
David Benjamin
17ce286e07 Work around an apparent Linux or glibc bug on ppc64le in FIPS mode.
POWER8 has hardware transactional memory, which glibc uses to implement
locks. In some cases, taking a lock begins a transaction, wrapping
arbitrary user code (!) until the lock is released. If the transaction
is aborted, everything rewinds and glibc tries again with some other
implementation.

The kernel will abort the transaction in a variety of cases. Notably, on
a syscall, the transaction aborts and the syscall *does not happen*.
https://www.kernel.org/doc/Documentation/powerpc/transactional_memory.txt

Yet, for some reason, although the relevant change does appear to be in
the kernel, the transaction is being rewound with getrandom happening
anyway. This does not work very well.

Instead, only guard the DRBG access with the lock, not CRYPTO_sysrand.
This lock is only used to protect the DRBG from the destructor that
zeros everything.

Change-Id: Ied8350f1e808a09300651de4200c7b0d07b3a158
Reviewed-on: https://boringssl-review.googlesource.com/16985
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>
2017-06-07 19:59:24 +00:00
David Benjamin
6757fbf8e3 Convert a number of tests to GTest.
BUG=129

Change-Id: Ifcdacb2f5f59fd03b757f88778ceb1e672208fd9
Reviewed-on: https://boringssl-review.googlesource.com/16744
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>
2017-06-01 17:02:13 +00:00
Adam Langley
c655cb7bf9 Break hwrand as well as urandom when FIPS_BREAK_TEST=CRNG is set.
Without this, trying to trigger the CRNGT on a system with RDRAND won't
work.

Change-Id: I0658a1f045620a2800df36277f67305bc0efff8b
Reviewed-on: https://boringssl-review.googlesource.com/16766
Reviewed-by: Adam Langley <agl@google.com>
2017-06-01 00:06:31 +00:00
Adam Langley
0ffc795efb Clear PRNG states in FIPS mode.
FIPS requires that the CTR-DRBG state be zeroed on process exit, however
destructors for thread-local data aren't called when the process exits.

This change maintains a linked-list of thread-local state which is
walked on exit to zero each thread's PRNG state. Any concurrently
running threads block until the process finishes exiting.

Change-Id: Ie5dc18e1bb2941a569d8b309411cf20c9bdf52ef
Reviewed-on: https://boringssl-review.googlesource.com/16764
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>
2017-05-31 23:39:53 +00:00
Adam Langley
f64a6eeaf0 Switch to new delocate tool.
Most importantly, this version of delocate works for ppc64le. It should
also work for x86-64, but will need significant testing to make sure
that it covers all the cases that the previous delocate.go covered.

It's less stringtastic than the old code, however the parser isn't as
nice as I would have liked. I thought that the reason we put up with
AT&T syntax with Intel is so that assembly syntax could be somewhat
consistent across platforms. At least for ppc64le, that does not appear
to be the case.

Change-Id: Ic7e3c6acc3803d19f2c3ff5620c5e39703d74212
Reviewed-on: https://boringssl-review.googlesource.com/16464
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>
2017-05-30 18:00:16 +00:00
Adam Langley
429e85b516 Have a single function for FIPS test failures.
Change-Id: Iab7a738a8981de7c56d1585050e78699cb876dab
Reviewed-on: https://boringssl-review.googlesource.com/16467
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>
2017-05-18 20:33:55 +00:00
Adam Langley
5f107ce4d8 Prefer RDRAND in FIPS mode.
This change causes FIPS mode to use RDRAND in preference to the kernel's
entropy pool. This prevents issues where the ioctl that we have to do
when getrandom isn't supported transiently reports that the pool is
“empty” and causes us to block.

Change-Id: Iad50e443d88b168bf0b85fe1e91e153d79ab3703
Reviewed-on: https://boringssl-review.googlesource.com/16466
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>
2017-05-18 20:32:45 +00:00
Martin Kreichgauer
866c219432 crypto/fipsmodule: Allow breaking CRNG self-test.
Change-Id: I3d1ddc8cca9fb1da5d0b6a68ba2125c89e5bc0ce
Reviewed-on: https://boringssl-review.googlesource.com/16304
Commit-Queue: Martin Kreichgauer <martinkr@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>
2017-05-12 18:43:09 +00:00
Matthew Braithwaite
20d202bb0e unrandom: #define _GNU_SOURCE, for syscall().
This is needed when unrandom.c is compiled on its own.

Change-Id: Ia46e06d267c097e5fa0296092a7270a4cd0b2044
Reviewed-on: https://boringssl-review.googlesource.com/16085
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>
2017-05-09 17:41:17 +00:00
Adam Langley
2e2a226ac9 Move cipher/ into crypto/fipsmodule/
Change-Id: Id65e0988534056a72d9b40cc9ba5194e2d9b8a7c
Reviewed-on: https://boringssl-review.googlesource.com/15904
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>
2017-05-05 22:39:40 +00:00
Michael Ryleev
a90044a463 Bypass building fipsmodule/rand/urandom.c when builing for Trusty
Change-Id: Icf1d6ec9d3fb33a124a9f61c75d29248a2582680
Reviewed-on: https://boringssl-review.googlesource.com/15964
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>
2017-05-05 17:21:23 +00:00
Adam Langley
c88f24596c Don't print message when waiting for urandom entropy.
This doesn't actually measure what we need(*) and, because of that, it's
way more noisy than expected.

(*) We want to know whether the pool has been initialised, not whether
it currently thinks it has a lot of bits, but we can't get what we want
without getrandom() support in the kernel.

Change-Id: I20accb99a592739c786a25c1656aeea050ae81a3
Reviewed-on: https://boringssl-review.googlesource.com/15624
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>
2017-04-27 21:38:21 +00:00
David Benjamin
91871018a4 Add an OPENSSL_ia32cap_get() function for C code.
OPENSSL_ia32cap_addr avoids any relocations within the module, at the
cost of a runtime TEXTREL, which causes problems in some cases.
(Notably, if someone links us into a binary which uses the GCC "ifunc"
attribute, the loader crashes.)

Fix C references of OPENSSL_ia32cap_addr with a function. This is
analogous to the BSS getters. A follow-up commit will fix perlasm with a
different scheme which avoids calling into a function (clobbering
registers and complicating unwind directives.)

Change-Id: I09d6cda4cec35b693e16b5387611167da8c7a6de
Reviewed-on: https://boringssl-review.googlesource.com/15525
Reviewed-by: Adam Langley <agl@google.com>
2017-04-27 20:34:23 +00:00
Adam Langley
7784104dd8 Move much of rand/ into the FIPS module.
Support for platforms that we don't support FIPS on doesn't need to be
in the module. Also, functions for dealing with whether fork-unsafe
buffering is enabled are left out because they aren't implementing any
cryptography and they use global r/w state, making their inclusion
painful.

Change-Id: I71a0123db6f5449e9dfc7ec7dea0944428e661aa
Reviewed-on: https://boringssl-review.googlesource.com/15084
Reviewed-by: Adam Langley <agl@google.com>
2017-04-21 22:03:18 +00:00