Commit Graph

16 Commits

Author SHA1 Message Date
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
David Benjamin
41c10e2b5f Disable AES-GCM-SIV assembly on Windows.
I'm working on a test harness to check our assembly correctly restores
callee-saved registers. It caught this.

While perlasm tries to smooth over the differences between Windows and SysV
ABIs, it does not capture the difference in xmm registers. All xmm registers
are volatile in SysV, while Windows makes xmm6 through xmm15 callee-saved.

Change-Id: Ia549b0f126885768f7fb330271a590174c483a3d
Reviewed-on: https://boringssl-review.googlesource.com/c/33685
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2018-12-17 17:54:07 +00:00
David Benjamin
e1b2a65e7f Fix typo in AES-GCM-SIV comments.
Change-Id: I73bd495cf99bbc8a993a726b009d68e74c893420
Reviewed-on: https://boringssl-review.googlesource.com/c/33684
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>
2018-12-17 17:39:46 +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
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
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
Martin Kreichgauer
6dc892fcdf Remove redundant calls to |OPENSSL_cleanse| and |OPENSSL_realloc_clean|.
Change-Id: I5c85c4d072ec157b37ed95b284a26ab32c0c42d9
Reviewed-on: https://boringssl-review.googlesource.com/19824
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-18 19:16:51 +00:00
Adam Langley
a16e86ced5 Don't depend on 16-byte alignment from malloc.
Windows provides _aligned_malloc, so we could provide an
|OPENSSL_aligned_malloc| in the future. However, since we're still
trying to get the zeroisation change landed everywhere, a self-contained
change seems easier until that has settled down.

Change-Id: I47bbd811a7fa1758f3c0a8a766a1058523949b7f
Reviewed-on: https://boringssl-review.googlesource.com/20204
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-09-12 19:12:20 +00:00
Martin Kreichgauer
c0e15d1d9d Zero memory in |OPENSSL_free|.
Allocations by |OPENSSL_malloc| are prefixed with their length.
|OPENSSL_free| zeros the allocation before calling free(), eliminating
the need for a separate call to |OPENSSL_cleanse| for sensitive data.

This change will be followed up by the cleanup in
https://boringssl-review.googlesource.com/c/boringssl/+/19824.

Change-Id: Ie272f07e9248d7d78af9aea81dacec0fdb7484c4
Reviewed-on: https://boringssl-review.googlesource.com/19544
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-06 19:22:46 +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
Martin Kreichgauer
abbf365b6d Make the bssl::SealRecord out_suffix arg fixed length.
Similarly, add EVP_AEAD_CTX_tag_len which computes the exact tag length
for required by EVP_AEAD_CTX_seal_scatter.

Change-Id: I069b0ad16fab314fd42f6048a3c1dc45e8376f7f
Reviewed-on: https://boringssl-review.googlesource.com/18324
Reviewed-by: Adam Langley <agl@google.com>
2017-07-28 21:42:25 +00:00
David Benjamin
b0651775c2 Reduce the alignment tag on aead_aes_gcm_siv_asm_ctx.
This tag doesn't actually do anything, except cause UBSan to point out
that malloc doesn't align that tightly. malloc does, however, appear to
align up to 16-bytes, which is the actual alignment requirement of that
code. So just replace 64 with 16.

When we're juggling less things, it'd be nice to see what toolchain
support for the various aligned allocators looks like. Or maybe someday
we can use C++ new which one hopes is smart enough to deal with all
this.

Change-Id: Idbdde66852d5dad25a044d4c68ffa3b3f213025a
Reviewed-on: https://boringssl-review.googlesource.com/17706
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-07-10 23:54:41 +00:00
Martin Kreichgauer
74bce29965 Change EVP_AEAD_CTX_seal_scatter to support an additional plaintext input.
Change-Id: I7e2fc8588d799d01d94cb5d94e49b53b367380ab
Reviewed-on: https://boringssl-review.googlesource.com/17344
Reviewed-by: Adam Langley <agl@google.com>
2017-06-27 23:09:31 +00:00
Martin Kreichgauer
18d9f28f0d Add EVP_AEAD_CTX_{seal_scatter,open_gather}.
These behave like EVP_AEAD_CTX_{seal,open} respectively, but receive
ciphertext and authentication tag as separate arguments, rather than one
contiguous out or in buffer.

Change-Id: Ia4f1b83424bc7067c55dd9e5a68f18061dab4d07
Reviewed-on: https://boringssl-review.googlesource.com/16924
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: Adam Langley <agl@google.com>
2017-06-09 23:10:49 +00:00
Adam Langley
c5e9ac1cac Move AES-GCM-SIV out from SMALL and handle unaligned keys.
In order to use AES-GCM-SIV in the open-source QUIC boxer, it needs to
be moved out from OPENSSL_SMALL. (Hopefully the linker can still discard
it in the vast majority of cases.)

Additionally, the input to the key schedule function comes from outside
and may not be aligned, thus we need to use unaligned instructions to
read it.

Change-Id: I02c261fe0663d13a96c428174943c7e5ac8415a7
Reviewed-on: https://boringssl-review.googlesource.com/16824
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-06-01 18:45:06 +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