After the custom key method support, the EVP_PKEY parameter is somewhat
confusing (to be resolved with the certificate slots removal) as it must
always refer to a private key. ssl3_cert_verify_hash is sometimes used
with the peer's public key. If custom keys were supported on the server,
this would break.
Fix this by passing a pkey_type parameter and letting the caller decide
whether this uses SSL_PRIVATE_KEY_METHOD or not.
Change-Id: I673b92579a84b4561f28026ec0b1c78a6bfee440
Reviewed-on: https://boringssl-review.googlesource.com/5341
Reviewed-by: Adam Langley <agl@google.com>
If ssl_do_write takes more than one iteration, ssl3_output_cert_chain
would be called an extra time. This is very unlikely in practice because
of the buffer BIO.
Change-Id: Ic1ae9752a8837bb404429fc60306c659208c6185
Reviewed-on: https://boringssl-review.googlesource.com/5340
Reviewed-by: Adam Langley <agl@google.com>
RSA and ECDSA will both require being able to convert ASN.1 INTEGERs to
and from DER. Don't bother handling negative BIGNUMs for now. It doesn't
seem necessary and saves bothering with two's-complement vs
sign-and-magnitude.
BUG=499653
Change-Id: I1e80052067ed528809493af73b04f82539d564ff
Reviewed-on: https://boringssl-review.googlesource.com/5268
Reviewed-by: Adam Langley <agl@google.com>
Reason for change: Define _POSIX_C_SOURCE such that it demands correct
posix interfaces, netdb.h declares interfaces such as
getaddrinfo if __USE_POSIX, i.e. POSIX.1:1990 or later.
However, these interfaces were new in the 2001 edition of POSIX
therefore ask for Extension from POSIX.1:2001 since we use addrinfo
structure here.
Change-Id: Icb1c92745d1a0ca958108ae80c270c630628729e
Signed-off-by: Khem Raj <raj.khem@gmail.com>
Reviewed-on: https://boringssl-review.googlesource.com/5253
Reviewed-by: Adam Langley <agl@google.com>
Missed that there were two of them.
Change-Id: Ibab169ef1f75be9c5ad1ffa0f232629e76a4512d
Reviewed-on: https://boringssl-review.googlesource.com/5332
Reviewed-by: Adam Langley <agl@google.com>
It (incorrectly) thinks some variables are uninitialized. It also gets confused
about some const parameters.
Change-Id: Ie2b3a5336692e7293cf03d6a4cd5345d30b628b3
Reviewed-on: https://boringssl-review.googlesource.com/5330
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>
The SSL23_ST_foo macros are only used in ssl_stat.c.
However, these states are never set and can be removed.
Move the two remaining SSLv2 client hello record macros to ssl3.h
Change-Id: I76055405a9050cf873b4d1cbc689e54dd3490b8a
Reviewed-on: https://boringssl-review.googlesource.com/4160
Reviewed-by: Adam Langley <agl@google.com>
All callers have been moved to EVP_PKEY_up_ref. (Neither spelling exists
upstream so we only had our own callers to move.)
Change-Id: I267f14054780fe3d6dc1170b7b6ae3811a0d1a9a
Reviewed-on: https://boringssl-review.googlesource.com/5291
Reviewed-by: Adam Langley <agl@google.com>
May as well. Depending on the implementation chosen in cipher/e_aes.c,
AES_encrypt may or may not be hit, so test this entry point explicitly.
Change-Id: Icb02bf3f4b6e5ecbb9e5111f44fbb1b267ead6c3
Reviewed-on: https://boringssl-review.googlesource.com/5312
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>
One tedious thing about using CBB is that you can't safely CBB_cleanup
until CBB_init is successful, which breaks the general 'goto err' style
of cleanup. This makes it possible:
CBB_zero ~ EVP_MD_CTX_init
CBB_init ~ EVP_DigestInit
CBB_cleanup ~ EVP_MD_CTX_cleanup
Change-Id: I085ecc4405715368886dc4de02285a47e7fc4c52
Reviewed-on: https://boringssl-review.googlesource.com/5267
Reviewed-by: Adam Langley <agl@google.com>
While I'm here, make them consistent with the keys.
Change-Id: Ib2804dd4f18bbb3b3735fb7772fca590e0d6d624
Reviewed-on: https://boringssl-review.googlesource.com/5266
Reviewed-by: Adam Langley <agl@google.com>
They weren't valid DER. Some lengths were encoded with one more byte
than necessary.
Change-Id: I94c8c525ade835fdeca115af98ab7e5910d2aeb2
Reviewed-on: https://boringssl-review.googlesource.com/5265
Reviewed-by: Adam Langley <agl@google.com>
kData5 was meant to test lengths that are too long, but the input
gets rejected earlier for not using short-form encoding. Switch it to
testing a badly encoded element of length 128, the shortest element that
uses long-form encoding.
Change-Id: I35f4df89bfa7a681698eda569c525b5871288487
Reviewed-on: https://boringssl-review.googlesource.com/5264
Reviewed-by: Adam Langley <agl@google.com>
Rather than parse with d2i_ECDSA_SIG and reserialize, this is cleaner.
It's also clearer that i2d_PublicKey isn't being used for DER.
Change-Id: Iac57fb6badd1dfed1e66984e95a31f609b1538a4
Reviewed-on: https://boringssl-review.googlesource.com/5263
Reviewed-by: Adam Langley <agl@google.com>
Less chance of problems should the prototype ever change. This doesn't
make it any more or less a circular dependency. (It actually isn't;
crypto/chacha doesn't use crypto/rand and CMakeLists.txt actually puts
rand above chacha anyway.)
Change-Id: Ia80289f801f76551737233f158755aac99ddd74a
Reviewed-on: https://boringssl-review.googlesource.com/5262
Reviewed-by: Adam Langley <agl@google.com>
The name is confusing. EC keys aren't serialized to DER.
DSA keys are also weird, but left alone for now. i2d_DSAPublicKey either
serializes to a DSAPublicKey per RFC 3279 if write_params is 0 or what
seems to be an OpenSSL-specific format that includes the group if
write_params is 1. See upstream's
ea6b07b54c1f8fc2275a121cdda071e2df7bd6c1.
Change-Id: I0d15140acc2d688a563b615fc6a9e3abec929753
Reviewed-on: https://boringssl-review.googlesource.com/5261
Reviewed-by: Adam Langley <agl@google.com>
They're all forward-declared. There's no need to use the struct names.
Change-Id: I435ae2f5971128f08c730317ca644d97239f3b54
Reviewed-on: https://boringssl-review.googlesource.com/5260
Reviewed-by: Adam Langley <agl@google.com>
Use more sensible variable names. Also move some work between the helpers and
s3_srvr.c a little; the session lookup functions now only return a new session.
Whether to send a ticket is now an additional output to avoid the enum
explosion around renewal. The actual SSL state is not modified.
This is somewhat cleaner as s3_srvr.c may still reject a session for other
reasons, so we avoid setting ssl->session and ssl->verify_result to a session
that wouldn't be used. (They get fixed up in ssl_get_new_session, so it didn't
actually matter.)
Change-Id: Ib52fabbe993b5e2b7408395a02cdea3dee66df7b
Reviewed-on: https://boringssl-review.googlesource.com/5235
Reviewed-by: Adam Langley <agl@google.com>
This change also switches the behaviour of the client. Previously the
client would send the SCSV rather than the extension, but now it'll only
do that for SSLv3 connections.
Change-Id: I67a04b8abbef2234747c0dac450458deb6b0cd0a
Reviewed-on: https://boringssl-review.googlesource.com/5143
Reviewed-by: Adam Langley <agl@google.com>
It's still the case that we have many old compilers that can't cope with
anything else ☹.
Change-Id: Ie5a1987cd5164bdbde0c17effaa62aecb7d12352
Reviewed-on: https://boringssl-review.googlesource.com/5320
Reviewed-by: Adam Langley <agl@google.com>
Rather than four massive functions that handle every extension,
organise the code by extension with four smaller functions for each.
Change-Id: I876b31dacb05aca9884ed3ae7c48462e6ffe3b49
Reviewed-on: https://boringssl-review.googlesource.com/5142
Reviewed-by: Adam Langley <agl@google.com>
This test shouldn't trigger a renegotiation: the test is trying to
assert that without the legacy-server flag set, a server that doesn't
echo the renegotiation extension can't be connected to.
Change-Id: I1368d15ebc8f296f3ff07040c0e6c48fdb49e56f
Reviewed-on: https://boringssl-review.googlesource.com/5141
Reviewed-by: Adam Langley <agl@google.com>
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>
Having them spread between ssl.h and tls1.h isn't terribly enlightening.
Change-Id: I5fec4b8e5260312b22bcef21bd4db7a8a8149ad8
Reviewed-on: https://boringssl-review.googlesource.com/5234
Reviewed-by: Adam Langley <agl@google.com>
Using the original numerical order made more sense before they were changed to
doesnt_exist.
BUG=404754
Change-Id: I2971eff7c6fbe7c5d340b103de71bbfa180f1f96
Reviewed-on: https://boringssl-review.googlesource.com/5232
Reviewed-by: Adam Langley <agl@google.com>
Otherwise another thread may cause the session to be destroyed first.
Change-Id: I2084a28ece11540e1b8f289553161d99395e2d1f
Reviewed-on: https://boringssl-review.googlesource.com/5231
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>
The callback arguments are required to be NULL.
Change-Id: I266ec46efdaca411a7f0c2b645883b2c5bec1c96
Reviewed-on: https://boringssl-review.googlesource.com/5160
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>
They'll probably stay that way too, so document it as being an ignored
parameter.
Change-Id: Iff385715f5413290a7186c38ea9ef2dd4fce9b38
Reviewed-on: https://boringssl-review.googlesource.com/5175
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
