In OpenSSL, they create socket BIOs. The distinction isn't important on UNIX.
On Windows, file descriptors are provided by the C runtime, while sockets must
use separate recv and send APIs. Document how these APIs are intended to work.
Also add a TODO to resolve the SOCKET vs int thing. This code assumes that
Windows HANDLEs only use the bottom 32 bits of precision. (Which is currently
true and probably will continue to be true for the foreseeable future[*], but
it'd be nice to do this right.)
Thanks to Gisle Vanem and Daniel Stenberg for reporting the bug.
[*] Both so Windows can continue to run 32-bit programs and because of all the
random UNIX software, like OpenSSL and ourselves, out there which happily
assumes sockets are ints.
Change-Id: I67408c218572228cb1a7d269892513cda4261c82
Reviewed-on: https://boringssl-review.googlesource.com/7333
Reviewed-by: David Benjamin <davidben@google.com>
This change adds a |SSL_CTX_set_private_key_method| method that sets key_method on a SSL_CTX's cert.
It allows the private key method to be set once and inherited.
A copy of key_method (from SSL_CTX's cert to SSL's cert) is added in |ssl_cert_dup|.
Change-Id: Icb62e9055e689cfe2d5caa3a638797120634b63f
Reviewed-on: https://boringssl-review.googlesource.com/7340
Reviewed-by: David Benjamin <davidben@google.com>
Instead of crashing when an empty key is passed to
EVP_marshal_public_key(), return with an
EVP_R_UNSUPPORTED_ALGORITHM_ERROR. This brings e.g. X509_PUBKEY_set()
closer to how it behaved before 68772b31 (previously, it returned an
error on an empty public key rather than dereferencing pkey->ameth).
Change-Id: Ieac368725adb7f22329c035d9d0685b44b885888
Reviewed-on: https://boringssl-review.googlesource.com/7351
Reviewed-by: David Benjamin <davidben@google.com>
I went with NID_x25519 to match NID_sha1 and friends in being lowercase.
However, upstream seems to have since chosen NID_X25519. Match their
name.
Change-Id: Icc7b183a2e2dfbe42c88e08e538fcbd242478ac3
Reviewed-on: https://boringssl-review.googlesource.com/7331
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Change-Id: I9540c931b6cdd4d65fa9ebfc52e1770d2174abd2
Reviewed-on: https://boringssl-review.googlesource.com/7330
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
C is still kind of unsure about the whole two's complement thing and leaves
left-shifting of negative numbers undefined. Sadly, some sanitizers believe in
teaching the controversy and complain when code relies on the theory of two's
complement.
Shushing these sanitizers in this case is easier than fighting with build
configuration, so replace the shifts with masks. (This is equivalent as the
left-shift was of a value right-shifted by the same amount. Instead, we store
the unshifted value in carry0, etc., and mask off the bottom bits.) A few other
places get casts to unsigned types which, by some miracle, C compilers are
forbidden from miscompiling.
This is imported from upstream's b95779846dc876cf959ccf96c49d4c0a48ea3082 and
5b7af0dd6c9315ca76fba16813b66f5792c7fe6e.
Change-Id: I6bf8156ba692165940c0c4ea1edd5b3e88ca263e
Reviewed-on: https://boringssl-review.googlesource.com/7320
Reviewed-by: Adam Langley <agl@google.com>
Now that client.cc and server.cc run through application data, regenerate the
corpus.
Change-Id: I8278ebfe47fd2ba74f67db6f9b545aabf9fd1f84
Reviewed-on: https://boringssl-review.googlesource.com/7301
Reviewed-by: Adam Langley <agl@google.com>
As long as the HTTP/1.1 client auth hack forces use to support renego, having
it on seems much more useful than having it off for fuzzing purposes. Also read
app data to exercise that code and, on the client, trigger renegotiations as
needed.
Change-Id: I1941ded6ec9bd764abd199d1518420a1075ed1b2
Reviewed-on: https://boringssl-review.googlesource.com/7291
Reviewed-by: Adam Langley <agl@google.com>
Also pass set -e instead of chaining things with &&. (One line was missing the
&&.)
Change-Id: Ia04e7f40f46688c9664101efefef1d1ea069de71
Reviewed-on: https://boringssl-review.googlesource.com/7300
Reviewed-by: Adam Langley <agl@google.com>
The coverage tool revealed that we weren't testing all codepaths of the ChaCha
assembly. Add a standalone test as it's much easier to iterate over all lengths
when there isn't the entire AEAD in the way.
I wasn't able to find a really long test vector, so I generated a random one
with the Go implementation we have in runner.
This test gives us full coverage on the ChaCha20_ssse3 variant. (We'll see how
it fares on the other codepaths when the multi-variant test harnesses get in. I
certainly hope there isn't a more novel way to call ChaCha20 than this...)
Change-Id: I087e421c7351f46ea65dacdc7127e4fbf5f4c0aa
Reviewed-on: https://boringssl-review.googlesource.com/7299
Reviewed-by: Adam Langley <agl@google.com>
This is in preparation for adding AES_256_GCM in Chromium below AES_128_GCM.
For now, AES_128_GCM is preferable over AES_256_GCM for performance reasons.
While I'm here, swap the order of 3DES and RC4. Chromium has already disabled
RC4, but the default order should probably reflect that until we can delete it
altogether.
BUG=591516
Change-Id: I1b4df0c0b7897930be726fb8321cee59b5d93a6d
Reviewed-on: https://boringssl-review.googlesource.com/7296
Reviewed-by: Adam Langley <agl@google.com>
Only the 32-bit AVX2 code path needs this, but upstream choose to harmonize all
vector code paths.
RT#4346
(Imported from 1ea8ae5090f557fea2e5b4d5758b10566825d74b.)
Tested the new code manually on arm and aarch64, NEON and non-NEON. Steven
reports that all variants pass on x86 and x86-64 too.
I've left the 32-bit x86 AVX2 code disabled since valgrind can't measure the
code coverage, but this avoids diff with upstream. We can enable it if we ever
end up caring.
Change-Id: Id9becc2adfbe44b84764f8e9c1fb5e8349c4d5a8
Reviewed-on: https://boringssl-review.googlesource.com/7295
Reviewed-by: Adam Langley <agl@google.com>
Uses LCOV for C(++) line coverage and Valgrind's Callgrind tool to
generate assembly-level line coverage for the generated assembly
code.
BUG=590332
Change-Id: Ic70300a272c38f4fa6dd615747db568aa0853584
Reviewed-on: https://boringssl-review.googlesource.com/7251
Reviewed-by: David Benjamin <davidben@google.com>
This results from running the fuzzers for a little while with both the
8bit-counters change and after taking the transcripts from the runner
tests as seeds for the `client` and `server` fuzzers.
Change-Id: I545a89d8dccd7ef69dd97546ed61610eea4a27a3
Reviewed-on: https://boringssl-review.googlesource.com/7276
Reviewed-by: David Benjamin <davidben@google.com>
EC point format negotiation is dead and gone.
Change-Id: If13ed7c5f31b64df2bbe90c018b2683b6371a980
Reviewed-on: https://boringssl-review.googlesource.com/7293
Reviewed-by: Adam Langley <agl@google.com>
The |fprintf| dependency is quite heavyweight for small targets. Also,
using |fprintf| on a closed file dsecriptor is undefined behavior, and
there's no way that this code can know whether |stderr| has already
been closed. So, just don't do it.
Change-Id: I1277733afe0649ae1324d11cac84826a1056e308
Reviewed-on: https://boringssl-review.googlesource.com/6812
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
This can be used to get some initial corpus for fuzzing.
Change-Id: Ifcd365995b54d202c4a2674f49e7b28515f36025
Reviewed-on: https://boringssl-review.googlesource.com/7289
Reviewed-by: Adam Langley <agl@google.com>
It's useful to make sure our fuzzer mode works. Not all tests pass, but most
do. (Notably the negative tests for everything we've disabled don't work.) We
can also use then use runner to record fuzzer-mode transcripts with the ciphers
correctly nulled.
Change-Id: Ie41230d654970ce6cf612c0a9d3adf01005522c6
Reviewed-on: https://boringssl-review.googlesource.com/7288
Reviewed-by: Adam Langley <agl@google.com>
If running the stack through a fuzzer, we would like execution to be
completely deterministic. This is gated on a
BORINGSSL_UNSAFE_FUZZER_MODE #ifdef.
For now, this just uses the zero ChaCha20 key and a global counter. As
needed, we can extend this to a thread-local counter and a separate
ChaCha20 stream and counter per input length.
Change-Id: Ic6c9d8a25e70d68e5dc6804e2c234faf48e51395
Reviewed-on: https://boringssl-review.googlesource.com/7286
Reviewed-by: Adam Langley <agl@google.com>
A len(tests) should have been len(testCases), the code never added to the
sync.WaitGroup, and feeding tests to the tests channel blocks on the tests
completing, so with one worker the results didn't stream. (And if the results
channel wasn't large enough, we'd deadlock.)
Change-Id: Iee37507b9706b14cffddd9c1b55fc311ee9b666d
Reviewed-on: https://boringssl-review.googlesource.com/7292
Reviewed-by: Adam Langley <agl@google.com>
Both sides' signature and Finished checks still occur, but the results
are ignored. Also, all ciphers behave like the NULL cipher.
Conveniently, this isn't that much code since all ciphers and their size
computations funnel into SSL_AEAD_CTX.
This does carry some risk that we'll mess up this code. Up until now, we've
tried to avoid test-only changes to the SSL stack.
There is little risk that anyone will ship a BORINGSSL_UNSAFE_FUZZER_MODE build
for anything since it doesn't interop anyway. There is some risk that we'll end
up messing up the disableable checks. However, both skipped checks have
negative tests in runner (see tests that set InvalidSKXSignature and
BadFinished). For good measure, I've added a server variant of the existing
BadFinished test to this CL, although they hit the same code.
Change-Id: I37f6b4d62b43bc08fab7411965589b423d86f4b8
Reviewed-on: https://boringssl-review.googlesource.com/7287
Reviewed-by: Adam Langley <agl@google.com>
There was some uncertainty about what the code is doing with |$end0|
and whether it was necessary for |$len| to be a multiple of 16 or 96.
Hopefully these added comments make it clear that the code is correct
except for the caveat regarding low memory addresses.
Change-Id: Iea546a59dc7aeb400f50ac5d2d7b9cb88ace9027
Reviewed-on: https://boringssl-review.googlesource.com/7194
Reviewed-by: Adam Langley <agl@google.com>
Use -num-workers to run multiple workers in parallel when running tests.
Change-Id: Iee5554ee78ec8d77700a0df5a297bd2515d34dca
Reviewed-on: https://boringssl-review.googlesource.com/7285
Reviewed-by: David Benjamin <davidben@google.com>
Node.js calls it but handles it failing. Since we have abstracted this
in the state machine, we mightn't even be using a cipher suite where the
server's key can be expressed as an EVP_PKEY.
Change-Id: Ic3f013dc9bcd7170a9eb2c7535378d478b985849
Reviewed-on: https://boringssl-review.googlesource.com/7272
Reviewed-by: David Benjamin <davidben@google.com>
This was dropped in d27441a9cb due to lack
of use, but node.js now needs it.
Change-Id: I1e207d4b46fc746cfae309a0ea7bbbc04ea785e8
Reviewed-on: https://boringssl-review.googlesource.com/7270
Reviewed-by: David Benjamin <davidben@google.com>
Found by libFuzzer combined with some experimental unsafe-fuzzer-mode patches
(to be uploaded once I've cleaned them up a bit) to disable all those pesky
cryptographic checks in the protocol.
Change-Id: I9153164fa56a0c2262c4740a3236c2b49a596b1b
Reviewed-on: https://boringssl-review.googlesource.com/7282
Reviewed-by: Adam Langley <agl@google.com>
If LeakSanitizer fires something on a test that's expected to fail, runner will
swallow it. Have stderr output always end in a "--- DONE ---" marker and treat
all output following that as a test failure.
Change-Id: Ia8fd9dfcaf48dd23972ab8f906d240bcb6badfe2
Reviewed-on: https://boringssl-review.googlesource.com/7281
Reviewed-by: Adam Langley <agl@google.com>
See also upstream's dc22d6b37e8058a4334e6f98932c2623cd3d8d0d. (Though I'm not
sure why they didn't need to fix cmov.)
Change-Id: I2a194a8aea1734d4c1e7f6a0536a636379381627
Reviewed-on: https://boringssl-review.googlesource.com/7280
Reviewed-by: Adam Langley <agl@google.com>
This is an attempt to make MSVC happy. Currently it's saying:
..\tool\speed.cc(508) : error C2536: 'SpeedSPAKE2::<lambda_…>::SpeedSPAKE2::<lambda_…>::alice_msg' : cannot specify explicit initializer for arrays
Change-Id: Ifba1df26b5d734f142668a41834645c1549f9f52
Reviewed-on: https://boringssl-review.googlesource.com/7248
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 64333004a41a9f4aa587b8e5401420fb70d00687.)
RT#4284.
This case should be impossible to hit because |EC_POINT_add| doesn't use
this function and trying to add equal inputs should never occur during a
multiplication. Support for this exists because the pattern has been
copied from the first 64-bit P-224 and P-256 work that Emilia, Bodo and
I did. There it seemed like a reasonable defense-in-depth in case the
code changed in the future.
Change-Id: I7ff138669c5468b7d7a5153429bec728cb67e338
Reviewed-on: https://boringssl-review.googlesource.com/7246
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 515f3be47a0b58eec808cf365bc5e8ef6917266b)
Additional hardening following on from CVE-2016-0702.
Change-Id: I19a6739b401887a42eb335fe5838379dc8d04100
Reviewed-on: https://boringssl-review.googlesource.com/7245
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 25d14c6c29b53907bf614b9964d43cd98401a7fc.)
At the same time remove miniscule bias in final subtraction. Performance
penalty varies from platform to platform, and even with key length. For
rsa2048 sign it was observed to be 4% for Sandy Bridge and 7% on
Broadwell.
(This is part of the fix for CVE-2016-0702.)
Change-Id: I43a13d592c4a589d04c17c33c0ca40c2d7375522
Reviewed-on: https://boringssl-review.googlesource.com/7244
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 08ea966c01a39e38ef89e8920d53085e4807a43a)
Performance penalty is 2%.
(This is part of the fix for CVE-2016-0702.)
Change-Id: Id3b6262c5d3201dd64b93bdd34601a51794a9275
Reviewed-on: https://boringssl-review.googlesource.com/7243
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's ef98503eeef5c108018081ace902d28e609f7772.)
Performance penalty is 2% on Linux and 5% on Windows.
(This is part of the fix for CVE-2016-0702.)
Change-Id: If82f95131c93168282a46ac5a35e2b007cc2bd67
Reviewed-on: https://boringssl-review.googlesource.com/7242
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 708dc2f1291e104fe4eef810bb8ffc1fae5b19c1.)
Performance penalty varies from platform to platform, and even key
length. For rsa2048 sign it was observed to reach almost 10%.
This is part of the fix for CVE-2016-0702.
Change-Id: Ie0860bf3e531196f03102db1bc48eeaf30ab1d58
Reviewed-on: https://boringssl-review.googlesource.com/7241
Reviewed-by: Adam Langley <agl@google.com>
(Imported from upstream's 10c639a8a56c90bec9e332c7ca76ef552b3952ac)
Change-Id: Ia8203eeae9d274249595a6e352ec2f77a97ca5d5
Reviewed-on: https://boringssl-review.googlesource.com/7227
Reviewed-by: David Benjamin <davidben@google.com>
If d2i_PrivateKey hit the PKCS#8 codepath, it didn't enforce that the key was
of the specified type.
Note that this requires tweaking d2i_AutoPrivateKey slightly. A PKCS #8
PrivateKeyInfo may have 3 or 4 elements (optional attributes), so we were
relying on this bug for d2i_AutoPrivateKey to work.
Change-Id: If50b7a742f535d208e944ba37c3a585689d1da43
Reviewed-on: https://boringssl-review.googlesource.com/7253
Reviewed-by: Adam Langley <agl@google.com>
They fail the newly-added in-place tests. Since we don't have bots for them
yet, verified manually that the arm and aarch64 code is fine.
Change-Id: Ic6f4060f63e713e09707af05e6b7736b7b65c5df
Reviewed-on: https://boringssl-review.googlesource.com/7252
Reviewed-by: Adam Langley <agl@google.com>
Cases where the input and output buffers overlap are always a little
odd. This change adds a test to ensures that the (generic) AEADs
function in these situations.
Change-Id: I6f1987a5e10ddef6b2b8f037a6d50737a120bc99
Reviewed-on: https://boringssl-review.googlesource.com/7195
Reviewed-by: David Benjamin <davidben@google.com>
Forgot to delete a line.
Change-Id: Ia1fb2904398816d495045dc237337f0be5b09272
Reviewed-on: https://boringssl-review.googlesource.com/7250
Reviewed-by: Steven Valdez <svaldez@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
(Imported from upstream's 04f2a0b50d219aafcef2fa718d91462b587aa23d)
Change-Id: Ie840edeb1fc9d5a4273f137467e3ef16528c9668
Reviewed-on: https://boringssl-review.googlesource.com/7234
Reviewed-by: David Benjamin <davidben@google.com>
This isn't a problem when called from EVP, since the buffer is
aligned in the EVP_CIPHER_CTX. The increment counter code is also
fixed to deal with overflow.
(Imported from upstream's 6533a0b8d1ed12aa5f7dfd7a429eec67c5486bb5)
Change-Id: I8d7191c3d3873db254a551085d2358d90bc8397a
Reviewed-on: https://boringssl-review.googlesource.com/7233
Reviewed-by: David Benjamin <davidben@google.com>
