Division isn't constant-time on Intel chips so the code was adding a
large multiple of md_size to try and force the operation to always take
the maximum amount of time.
I'm less convinced, these days, that compilers aren't going to get smart
enough to optimise that away so use Barrett reduction instead.
Change-Id: Ib8c514192682a2fcb4b1fb7e7c6dd1301d9888d0
Reviewed-on: https://boringssl-review.googlesource.com/6906
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
MSVC doesn't have stdalign.h and so doesn't support |alignas| in C
code. Define |alignas(x)| as a synonym for |__decltype(align(x))|
instead for it.
This also fixes -Wcast-qual warnings in rsaz_exp.c.
Change-Id: Ifce9031724cb93f5a4aa1f567e7af61b272df9d5
Reviewed-on: https://boringssl-review.googlesource.com/6924
Reviewed-by: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
This removes 16k from a release-mode build of the bssl tool. Now that we've
finished the AEAD refactor, there's no use in keeping this around as a
prototype for "stateful AEADs".
Before:
Did 2264000 RC4-MD5 (16 bytes) seal operations in 1000430us (2263026.9 ops/sec): 36.2 MB/s
Did 266000 RC4-MD5 (1350 bytes) seal operations in 1000984us (265738.5 ops/sec): 358.7 MB/s
Did 50000 RC4-MD5 (8192 bytes) seal operations in 1014209us (49299.5 ops/sec): 403.9 MB/s
After:
Did 1895000 RC4-MD5 (16 bytes) seal operations in 1000239us (1894547.2 ops/sec): 30.3 MB/s
Did 199000 RC4-MD5 (1350 bytes) seal operations in 1001361us (198729.5 ops/sec): 268.3 MB/s
Did 39000 RC4-MD5 (8192 bytes) seal operations in 1014832us (38430.0 ops/sec): 314.8 MB/s
There is a non-trivial performance hit, but this cipher doesn't matter much and
the stitched mode code reaches into MD5_CTX and RC4_KEY in somewhat unfortunate
ways.
Change-Id: I9ecd28d6afb54e90ce61baecc641742af2ae6269
Reviewed-on: https://boringssl-review.googlesource.com/6752
Reviewed-by: Adam Langley <agl@google.com>
We can reuse the HMAC_CTX that stores the key. The API is kind of unfortunate
as, in principle, it should be possible to do an allocation-averse HMAC with a
shared key on multiple threads at once (EVP_AEAD_CTX is normally logically
const). At some point it may be worth rethinking those APIs somewhat. But
these "stateful AEADs" are already stateful in their EVP_CIPHER_CTX, so this is
fine.
Each cipher was run individually to minimize the effect of other ciphers doing
their mallocs. (Although the cost of a malloc is presumably going to depend a
lot on the malloc implementation and what's happened before in the process, so
take these numbers with a bucket of salt. They vary widely even with the same
arguments.)
Taking malloc out of seal/open also helps with the malloc tests. DTLS currently
cannot distinguish a malloc failure (should be fatal) from a decryption failure
(not fatal), so the malloc tests get stuck. But this doesn't completely get us
there since tls_cbc.c mallocs. This also assumes EVP_CIPHER_CTX, EVP_MD_CTX,
and HMAC_CTX are all clever about reusing their allocations when reset (which
they are).
Before:
Did 1315000 AES-128-CBC-SHA1 (16 bytes) seal operations in 1000087us (1314885.6 ops/sec): 21.0 MB/s
Did 181000 AES-128-CBC-SHA1 (1350 bytes) seal operations in 1004918us (180114.2 ops/sec): 243.2 MB/s
Did 34000 AES-128-CBC-SHA1 (8192 bytes) seal operations in 1024250us (33195.0 ops/sec): 271.9 MB/s
After:
Did 1766000 AES-128-CBC-SHA1 (16 bytes) seal operations in 1000319us (1765436.8 ops/sec): 28.2 MB/s
Did 187000 AES-128-CBC-SHA1 (1350 bytes) seal operations in 1004002us (186254.6 ops/sec): 251.4 MB/s
Did 35000 AES-128-CBC-SHA1 (8192 bytes) seal operations in 1014885us (34486.7 ops/sec): 282.5 MB/s
Before:
Did 391000 DES-EDE3-CBC-SHA1 (16 bytes) seal operations in 1000038us (390985.1 ops/sec): 6.3 MB/s
Did 16000 DES-EDE3-CBC-SHA1 (1350 bytes) seal operations in 1060226us (15091.1 ops/sec): 20.4 MB/s
Did 2827 DES-EDE3-CBC-SHA1 (8192 bytes) seal operations in 1035971us (2728.8 ops/sec): 22.4 MB/s
After:
Did 444000 DES-EDE3-CBC-SHA1 (16 bytes) seal operations in 1001814us (443196.0 ops/sec): 7.1 MB/s
Did 17000 DES-EDE3-CBC-SHA1 (1350 bytes) seal operations in 1042535us (16306.4 ops/sec): 22.0 MB/s
Did 2590 DES-EDE3-CBC-SHA1 (8192 bytes) seal operations in 1012378us (2558.3 ops/sec): 21.0 MB/s
Before:
Did 1316000 AES-256-CBC-SHA1 (16 bytes) seal operations in 1000510us (1315329.2 ops/sec): 21.0 MB/s
Did 157000 AES-256-CBC-SHA1 (1350 bytes) seal operations in 1002944us (156539.1 ops/sec): 211.3 MB/s
Did 29000 AES-256-CBC-SHA1 (8192 bytes) seal operations in 1030284us (28147.6 ops/sec): 230.6 MB/s
After:
Did 1645000 AES-256-CBC-SHA1 (16 bytes) seal operations in 1000313us (1644485.3 ops/sec): 26.3 MB/s
Did 162000 AES-256-CBC-SHA1 (1350 bytes) seal operations in 1003060us (161505.8 ops/sec): 218.0 MB/s
Did 36000 AES-256-CBC-SHA1 (8192 bytes) seal operations in 1014819us (35474.3 ops/sec): 290.6 MB/s
Before:
Did 1435000 RC4-SHA1 (16 bytes) seal operations in 1000245us (1434648.5 ops/sec): 23.0 MB/s
Did 207000 RC4-SHA1 (1350 bytes) seal operations in 1004675us (206036.8 ops/sec): 278.1 MB/s
Did 38000 RC4-SHA1 (8192 bytes) seal operations in 1022712us (37156.1 ops/sec): 304.4 MB/s
After:
Did 1853000 RC4-SHA1 (16 bytes) seal operations in 1000433us (1852198.0 ops/sec): 29.6 MB/s
Did 206000 RC4-SHA1 (1350 bytes) seal operations in 1002370us (205512.9 ops/sec): 277.4 MB/s
Did 42000 RC4-SHA1 (8192 bytes) seal operations in 1024209us (41007.3 ops/sec): 335.9 MB/s
Change-Id: I0edb89bddf146cf91a8e7a99c56b2278c8f38094
Reviewed-on: https://boringssl-review.googlesource.com/6751
Reviewed-by: Adam Langley <agl@google.com>
The consumers have all been updated, so we can move EVP_aead_chacha20_poly1305
to its final state. Unfortunately, the _rfc7539-suffixed version will need to
stick around for just a hair longer. Also the tls1.h macros, but the remaining
consumers are okay with that changing underneath them.
Change-Id: Ibbb70ec1860d6ac6a7e1d7b45e70fe692bf5ebe5
Reviewed-on: https://boringssl-review.googlesource.com/6600
Reviewed-by: Adam Langley <agl@google.com>
https://boringssl-review.googlesource.com/6101 was mismerged from *ring* and
lost some tests. Also add the corresponding tag truncation tests for the new
construction. So long as we have that feature, we should have tests for it.
(Although, do we actually need to support it?)
Change-Id: I70784cbac345e0ad11b496102856c53932b7362e
Reviewed-on: https://boringssl-review.googlesource.com/6682
Reviewed-by: Adam Langley <agl@google.com>
Some strange toolchains can have an implicit (or explicit) fcntl.h include,
so let's avoid using the name 'open' for local functions. This should not
cause any trouble.
Change-Id: Ie131b5920ac23938013c2c03302b97a7418c7180
Reviewed-on: https://boringssl-review.googlesource.com/6540
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
stdint.h already has macros for this. The spec says that, in C++,
__STDC_CONSTANT_MACROS is needed, so define it for bytestring_test.cc.
Chromium seems to use these macros without trouble, so I'm assuming we
can rely on them.
Change-Id: I56d178689b44d22c6379911bbb93d3b01dd832a3
Reviewed-on: https://boringssl-review.googlesource.com/6510
Reviewed-by: Adam Langley <agl@google.com>
Chromium's toolchains may now assume C++11 library support, so we may freely
use C++11 features. (Chromium's still in the process of deciding what to allow,
but we use Google's style guide directly, toolchain limitations aside.)
Change-Id: I1c7feb92b7f5f51d9091a4c686649fb574ac138d
Reviewed-on: https://boringssl-review.googlesource.com/6465
Reviewed-by: Adam Langley <agl@google.com>
This function allows one to extract the current IVs from an SSL
connection. This is needed for the CBC cipher suites with implicit IVs
because, for those, the IV can't be extracted from the handshake key
material.
Change-Id: I247a1d0813b7a434b3cfc88db86d2fe8754344b6
Reviewed-on: https://boringssl-review.googlesource.com/6433
Reviewed-by: Adam Langley <agl@google.com>
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.
Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations took a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.
The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.
The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.
This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.
Change-Id: I6e9dd09ff057c67941021d324a4fa1d39f58b0db
Reviewed-on: https://boringssl-review.googlesource.com/6405
Reviewed-by: Adam Langley <agl@google.com>
The documentation in md32_common.h is now (more) correct with respect
to the most important details of the layout of |HASH_CTX|. The
documentation explaining why sha512.c doesn't use md32_common.h is now
more accurate as well.
Before, the C implementations of HASH_BLOCK_DATA_ORDER took a pointer
to the |HASH_CTX| and the assembly language implementations tool a
pointer to the hash state |h| member of |HASH_CTX|. (This worked
because |h| is always the first member of |HASH_CTX|.) Now, the C
implementations take a pointer directly to |h| too.
The definitions of |MD4_CTX|, |MD5_CTX|, and |SHA1_CTX| were changed to
be consistent with |SHA256_CTX| and |SHA512_CTX| in storing the hash
state in an array. This will break source compatibility with any
external code that accesses the hash state directly, but will not
affect binary compatibility.
The second parameter of |HASH_BLOCK_DATA_ORDER| is now of type
|const uint8_t *|; previously it was |void *| and all implementations
had a |uint8_t *data| variable to access it as an array of bytes.
This change paves the way for future refactorings such as automatically
generating the |*_Init| functions and/or sharing one I-U-F
implementation across all digest algorithms.
Change-Id: I30513bb40b5f1d2c8932551d54073c35484b3f8b
Reviewed-on: https://boringssl-review.googlesource.com/6401
Reviewed-by: Adam Langley <agl@google.com>
This reverts most of commit 271777f5ac. The old
ChaCha20-Poly1305, though being transitioned to the old name, should not change
in behavior. This also avoids adding a special-case to SSL_AEAD_CTX.
Also revert the name change to SSL_CIPHER_is_CHACHA20POLY1305. The one consumer
for that function doesn't need to distinguish the old and new variants, so
avoid unnecessary turbulence.
Change-Id: I5a6f97fccc5839d4d25e74e304dc002329d21b4b
Reviewed-on: https://boringssl-review.googlesource.com/6385
Reviewed-by: Adam Langley <agl@google.com>
QUIC has a complex relationship with BoringSSL owing to it living both
in Chromium and the Google-internal repository. In order for it to
handle the ChaCha20-Poly1305 AEAD switch more easily this change gives
the unsuffixed name to the old AEAD, for now.
Once QUIC has moved to the “_old” version the unsuffixed name can be
given to the new version.
Change-Id: Id8a77be6e3fe2358d78e022413fe088e5a274dca
Reviewed-on: https://boringssl-review.googlesource.com/6361
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <alangley@gmail.com>
The tests in crypto/cipher/test/chacha20_poly1305_deprecated_tests.txt
were adapted to the RFC 7539 AEAD construction by recalculating the tags.
Also a few additional vectors were added. These vectors were verified
against nettle. See
feb7292bf1.
Change-Id: Ib3f2797d5825bc1e32c55f845b5070b6993e4aff
Reviewed-on: https://boringssl-review.googlesource.com/6144
Reviewed-by: Adam Langley <alangley@gmail.com>
This change reduces unnecessary copying and makes the pre-RFC-7539
nonces 96 bits just like the AES-GCM, AES-CCM, and RFC 7539
ChaCha20-Poly1305 cipher suites. Also, all the symbols related to
the pre-RFC-7539 cipher suites now have "_OLD" appended, in
preparation for adding the RFC 7539 variants.
Change-Id: I1f85bd825b383c3134df0b6214266069ded029ae
Reviewed-on: https://boringssl-review.googlesource.com/6103
Reviewed-by: Adam Langley <alangley@gmail.com>
The new function |CRYPTO_chacha_96_bit_nonce_from_64_bit_nonce| can be
used to adapt code from that uses 64 bit nonces, in a way that is
compatible with the old semantics.
Change-Id: I83d5b2d482e006e82982f58c9f981e8078c3e1b0
Reviewed-on: https://boringssl-review.googlesource.com/6100
Reviewed-by: Adam Langley <alangley@gmail.com>
gcm_test.cc needs to access the internal GCM symbols. This is
unfortunate because it means that they have to be marked OPENSSL_EXPORT
just for this.
To compensate, modes.h is removed and its contents copied into
crypto/modes/internal.h.
Change-Id: I1777b2ef8afd154c43417137673a28598a7ec30e
Reviewed-on: https://boringssl-review.googlesource.com/6360
Reviewed-by: Adam Langley <alangley@gmail.com>
This removes the confusion about whether |gcm128_context| copies the
key (it didn't) or whether the caller is responsible for keeping the
key alive for the lifetime of the |gcm128_context| (it was).
Change-Id: Ia0ad0a8223e664381fbbfb56570b2545f51cad9f
Reviewed-on: https://boringssl-review.googlesource.com/6053
Reviewed-by: Adam Langley <alangley@gmail.com>
It's very annoying having to remember the right incant every time I want
to switch around between my build, build-release, build-asan, etc.,
output directories.
Unfortunately, this target is pretty unfriendly without CMake 3.2+ (and
Ninja 1.5+). This combination gives a USES_TERMINAL flag to
add_custom_target which uses Ninja's "console" pool, otherwise the
output buffering gets in the way. Ubuntu LTS is still on an older CMake,
so do a version check in the meantime.
CMake also has its own test mechanism (CTest), but this doesn't use it.
It seems to prefer knowing what all the tests are and then tries to do
its own output management and parallelizing and such. We already have
our own runners. all_tests.go could actually be converted tidily, but
generate_build_files.py also needs to read it, and runner.go has very
specific needs.
Naming the target ninja -C build test would be nice, but CTest squats
that name and CMake grumps when you use a reserved name, so I've gone
with run_tests.
Change-Id: Ibd20ebd50febe1b4e91bb19921f3bbbd9fbcf66c
Reviewed-on: https://boringssl-review.googlesource.com/6270
Reviewed-by: Adam Langley <alangley@gmail.com>
Some ARM environments don't support |getauxval| or signals and need to
configure the capabilities of the chip at compile time. This change adds
defines that allow them to do so.
Change-Id: I4e6987f69dd13444029bc7ac7ed4dbf8fb1faa76
Reviewed-on: https://boringssl-review.googlesource.com/6280
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I36b2bb0e10c627ae6efa9d133df53b814922e652
Reviewed-on: https://boringssl-review.googlesource.com/6051
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
To avoid too much #if soup, e_aes.c uses a lot of dummy functions that
just call |abort|. This change makes them all static, which they should
have been all along.
Change-Id: I696f8a0560cf99631ed7adb42d1af10003db4a63
Reviewed-on: https://boringssl-review.googlesource.com/6004
Reviewed-by: Adam Langley <agl@google.com>
arm_arch.h is included from ARM asm files, but lives in crypto/, not
openssl/include/. Since the asm files are often built from a different
location than their position in the source tree, relative include paths
are unlikely to work so, rather than having crypto/ be a de-facto,
second global include path, this change moves arm_arch.h to
include/openssl/.
It also removes entries from many include paths because they should be
needed as relative includes are always based on the locations of the
source file.
Change-Id: I638ff43d641ca043a4fc06c0d901b11c6ff73542
Reviewed-on: https://boringssl-review.googlesource.com/5746
Reviewed-by: Adam Langley <agl@google.com>
Match the other stack-allocated types in that we expose a wrapper function to
get them into the zero state. Makes it more amenable to templates like
ScopedOpenSSLContext.
Change-Id: Ibc7b2b1bc0421ce5ccc84760c78c0b143441ab0f
Reviewed-on: https://boringssl-review.googlesource.com/5753
Reviewed-by: Adam Langley <agl@google.com>
(I couldn't find an authoritative source of test data, including in
OpenSSL's source, so I used OpenSSL's implementation to produce the
test ciphertext.)
This benefits globalplatform.
Change-Id: Ifb79e77afb7efed1c329126a1a459bbf7ce6ca00
Reviewed-on: https://boringssl-review.googlesource.com/5725
Reviewed-by: Adam Langley <agl@google.com>
Note that while |DES_ede2_cbc_encrypt| exists, I didn't use it: I
think it's easier to see what's happening this way.
(I couldn't find an authoritative source of test data, including in
OpenSSL's source, so I used OpenSSL's implementation to produce the
test ciphertext.)
This benefits globalplatform.
Change-Id: I7e17ca0b69067d7b3f4bc213b4616eb269882ae0
Reviewed-on: https://boringssl-review.googlesource.com/5724
Reviewed-by: Adam Langley <agl@google.com>
|DES_ecb_encrypt| was already present.
This benefits globalplatform.
Change-Id: I2ab41eb1936b3026439b5981fb27e29a12672b66
Reviewed-on: https://boringssl-review.googlesource.com/5723
Reviewed-by: Adam Langley <agl@google.com>
“!= 0” is implicit in if statements and it looks very weird here.
Change-Id: I7f4e71c479b8ff9821a040f1c542b15af19b8aed
Reviewed-on: https://boringssl-review.googlesource.com/5720
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Add it to |EVP_get_cipherbynid|, along with |EVP_rc2_40_cbc| and
|EVP_aes_192_cbc|.
Change-Id: Iee7621a91262359d1650684652995884a6cef37a
Reviewed-on: https://boringssl-review.googlesource.com/5590
Reviewed-by: Adam Langley <agl@google.com>
Some compilers warn about this duplicate declaration in some
configurations.
Change-Id: I3ff038a5feb0b2632b140e6632ea48acb0dcc118
Reviewed-on: https://boringssl-review.googlesource.com/5551
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Otherwise we leave stuff in the error queue for the next test.
Change-Id: I167b7420b9d3fada69d1d35ac8132dd21a04797c
Reviewed-on: https://boringssl-review.googlesource.com/5310
Reviewed-by: Adam Langley <agl@google.com>
Run a variant of every test which feeds the input in one byte at a time.
Change-Id: I2a05372ea0fbb20484493fd14e9f3c23fbb8d875
Reviewed-on: https://boringssl-review.googlesource.com/5301
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
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>
This isn't exhaustive. There are still failures in some tests which probably
ought to get C++'d first.
Change-Id: Iac58df9d98cdfd94603d54374a531b2559df64c3
Reviewed-on: https://boringssl-review.googlesource.com/4795
Reviewed-by: Adam Langley <agl@google.com>
Derived from upstream's new evp_test. The tests were taken from upstream
but tweaked so the diff from the old cipher_test.txt is more obvious.
Change-Id: Ic82593a8bb6aaee9b69fdc42a8b75516b03c1c5a
Reviewed-on: https://boringssl-review.googlesource.com/4707
Reviewed-by: Adam Langley <agl@google.com>
Change-Id: I3002d95d2b9db4dd05e1c56ef6ae410315b97ab9
Reviewed-on: https://boringssl-review.googlesource.com/4710
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This change makes it safe to call EVP_AEAD_CTX_cleanup after a failed
EVP_AEAD_CTX_init.
Change-Id: I608ed550e08d638cd7e941f5067edd3da4c850ab
Reviewed-on: https://boringssl-review.googlesource.com/4692
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
(The huge if-else was hard to visually parse.)
Change-Id: Ic2c94120f345085b619304181e861f662a931a29
Reviewed-on: https://boringssl-review.googlesource.com/4691
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
