Commit Graph

1632 Commits

Author SHA1 Message Date
David Benjamin
183eba4dc9 Convert ecdsa_test to the new ECDSA_SIG API.
BUG=499653

Change-Id: Idc50c79ad597f74226ae41f9c0839ac96a8950ae
Reviewed-on: https://boringssl-review.googlesource.com/5270
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:29:14 +00:00
David Benjamin
87897a8cea Implement ECDSA_SIG_{parse,marshal} with crypto/bytestring.
This is the first structure to be implemented with the new BIGNUM ASN.1
routines. Object reuse in the legacy d2i/i2d functions is implemented by
releasing whatever was in *out before and setting it to the
newly-allocated object. As with the new d2i_SSL_SESSION, this is a
weaker form of object reuse, but should suffice for reasonable callers.

As ECDSA_SIG is more likely to be parsed alone than as part of another
structure (and using CBB is slightly tedious), add convenient functions
which take byte arrays. For consistency with SSL_SESSION, they are named
to/from_bytes. from_bytes, unlike the CBS variant, rejects trailing
data.

Note this changes some test expectations: BER signatures now push an
error code. That they didn't do this was probably a mistake.

BUG=499653

Change-Id: I9ec74db53e70d9a989412cc9e2b599be0454caec
Reviewed-on: https://boringssl-review.googlesource.com/5269
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 02:28:42 +00:00
Adam Langley
6df1ac9092 Fix Windows build.
Now that 11c0f8e54c has landed, none of
the cases of the switch in |ssl3_ctrl| ever break and so the “return 1”
at the end of the function is unreachable. MSVC is unhappy about that.

Change-Id: I001dc63831ba60d93b622ac095297e2febc5f078
2015-07-06 19:01:48 -07:00
David Benjamin
f3cdc5030e Remove some unused constants.
This is certainly far from exhaustive, but get rid of these.

Change-Id: Ie96925bcd452873ed8399b68e1e71d63e5a0929b
Reviewed-on: https://boringssl-review.googlesource.com/5357
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:56:38 +00:00
David Benjamin
71d2e54099 Clear key_method in ssl_cert_clear_certs.
Since it resets leaf, private key, and chain, it makes sense to also
clear custom key method tables.

Change-Id: If511b8f15a44674c31d068d36984e9189c5a9071
Reviewed-on: https://boringssl-review.googlesource.com/5356
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:56:11 +00:00
David Benjamin
11c0f8e54c Promote certificate-related ctrl macros to functions.
Also document them in the process. Almost done!

BUG=404754

Change-Id: I3333c7e9ea6b4a4844f1cfd02bff8b5161b16143
Reviewed-on: https://boringssl-review.googlesource.com/5355
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:55:39 +00:00
David Benjamin
7481d39bf7 Document APIs relating to configuring certificates and private keys.
The APIs that are CTRL macros will be documented (and converted to
functions) in a follow-up.

Change-Id: I7d086db1768aa3c16e8d7775b0c818b72918f4c2
Reviewed-on: https://boringssl-review.googlesource.com/5354
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:52:39 +00:00
David Benjamin
b2a9d6ab78 Remove SSL_build_cert_chain.
This is unused. It seems to be distinct from the automatic chain
building and was added in 1.0.2. Seems to be an awful lot of machinery
that consumers ought to configure anyway.

BUG=486295

Change-Id: If3d4a2761f61c5b2252b37d4692089112fc0ec21
Reviewed-on: https://boringssl-review.googlesource.com/5353
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:23:18 +00:00
David Benjamin
4462809623 Remove SSL_CTX_select_current_cert.
Without certificate slots this function doesn't do anything. It's new in
1.02 and thus unused, so get rid of it rather than maintain a
compatibility stub.

BUG=486295

Change-Id: I798fce7e4307724756ad4e14046f1abac74f53ed
Reviewed-on: https://boringssl-review.googlesource.com/5352
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:22:32 +00:00
David Benjamin
d1d8078025 Fold away certificate slots mechanism.
This allows us to remove the confusing EVP_PKEY argument to the
SSL_PRIVATE_KEY_METHOD wrapper functions. It also simplifies some of the
book-keeping around the CERT structure, as well as the API for
configuring certificates themselves. The current one is a little odd as
some functions automatically route to the slot while others affect the
most recently touched slot. Others still (extra_certs) apply to all
slots, making them not terribly useful.

Consumers with complex needs should use cert_cb or the early callback
(select_certificate_cb) to configure whatever they like based on the
ClientHello.

BUG=486295

Change-Id: Ice29ffeb867fa4959898b70dfc50fc00137f01f3
Reviewed-on: https://boringssl-review.googlesource.com/5351
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:22:13 +00:00
David Benjamin
570364800c Remove SSL_CTX_get_extra_chain_certs_only.
This is in preparation for folding away certificate slots. extra_certs
and the slot-specific certificate chain will be the same.
SSL_CTX_get_extra_chain_certs already falls back to the slot-specific
chain if missing. SSL_CTX_get_extra_chain_certs_only is similar but
never falls back. This isn't very useful and is confusing with them
merged, so remove it.

BUG=486295

Change-Id: Ic708105bcf453dfe4e1969353d7eb7547ed2981b
Reviewed-on: https://boringssl-review.googlesource.com/5350
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:16:20 +00:00
David Benjamin
bb20f52383 Merge the RSA_ENC and RSA_SIGN certificate slots.
The distinction was not well-enforced in the code. In fact, it wasn't
even possible to use the RSA_SIGN slot because ssl_set_pkey and
ssl_set_cert would always use the RSA_ENC slot.

A follow-up will fold away the mechanism altogether, but this is an easy
initial simplfication.

BUG=486295

Change-Id: I66b5bf3e6dc243dac7c75924c1c1983538e49060
Reviewed-on: https://boringssl-review.googlesource.com/5349
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:15:41 +00:00
David Benjamin
0fc431a0d7 Prune NIDs from TLS_SIGALGS.
There's no need to store more than the TLS values.

Change-Id: I1a93c7c6aa3254caf7cc09969da52713e6f8acf4
Reviewed-on: https://boringssl-review.googlesource.com/5348
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:14:40 +00:00
David Benjamin
ba16a1e405 Remove SSL_get_(shared_)sigalgs.
These are new as of 1.0.2, not terribly useful of APIs, and are the only
reason we have to retain so many NIDs in the TLS_SIGALGS structure.

Change-Id: I7237becca09acc2ec2be441ca17364f062253893
Reviewed-on: https://boringssl-review.googlesource.com/5347
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:12:24 +00:00
David Benjamin
a31c5bf2cc Make pem_str const-correct.
They're always constant literals.

Change-Id: I8acaaf2a8c95b02bc8b9b13740ce40044a483394
Reviewed-on: https://boringssl-review.googlesource.com/5346
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:58 +00:00
David Benjamin
42ae3adcf6 Remove info field on EVP_PKEY_ASN1_METHOD.
Nothing ever reads it.

Change-Id: Id375c461aa2feb6877a14e19eb2daefec7a03f89
Reviewed-on: https://boringssl-review.googlesource.com/5345
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:46 +00:00
David Benjamin
1504304147 Remove dead SRP SSL_CTRL values.
Change-Id: Id2d3a2a0a3826566376bac2efeefbb54fc55d227
Reviewed-on: https://boringssl-review.googlesource.com/5344
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:30 +00:00
David Benjamin
7d97fd3a91 Remove SSL_get_server_tmp_key.
It's never used and is partially broken right now; EVP_PKEY_DH doesn't
work.

Change-Id: Id6262cd868153ef731e3f4d679b2ca308cfb12a3
Reviewed-on: https://boringssl-review.googlesource.com/5343
Reviewed-by: Adam Langley <agl@google.com>
2015-07-07 01:11:10 +00:00
David Benjamin
396a441421 ssl3_cert_verify_hash should take the EVP_PKEY type.
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>
2015-07-07 01:10:35 +00:00
David Benjamin
67dc83851b Fix ssl3_send_client_certificate state machine.
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>
2015-07-07 01:09:45 +00:00
David Benjamin
b9c579db6d Add crypto/bytestring-based BIGNUM DER functions.
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>
2015-07-07 00:47:39 +00:00
Khem Raj
241364c6f4 Demand for newer POSIX macro
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>
2015-07-06 22:41:16 +00:00
David Benjamin
dfa4069a16 Fix MSVC build some more.
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>
2015-07-02 00:02:27 +00:00
David Benjamin
3e536334be Fix Windows build.
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>
2015-07-01 23:01:05 +00:00
Matt Braithwaite
6a1275b627 typedef |COMP_METHOD| to void, for compatibility.
Change-Id: I2965c0d698e25f4eb08ddd9fc000b0e4adf6f2c0
Reviewed-on: https://boringssl-review.googlesource.com/5240
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:49:06 +00:00
David Benjamin
fd5603666c Call ERR_clear_error after functions that are expected to fail.
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>
2015-07-01 21:47:44 +00:00
Håvard Molland
047d6f0f4d Remove ssl2.h and ssl23.h.
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>
2015-07-01 21:47:01 +00:00
David Benjamin
cce5a98efb Remove EVP_PKEY_dup.
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>
2015-07-01 21:38:39 +00:00
David Benjamin
76e9a2ab97 Add tests for low-level AES functions.
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>
2015-07-01 21:37:57 +00:00
David Benjamin
f078639d5a Test both streaming and single-shot EVP_CIPHER operations.
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>
2015-07-01 21:36:43 +00:00
David Benjamin
deedad1238 Fix documentation typo.
Change-Id: Iaa05acf1f775b0b35cc99e5f2c048f4e70f0a899
Reviewed-on: https://boringssl-review.googlesource.com/5300
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 21:33:29 +00:00
David Benjamin
a8653208ec Add CBB_zero to set a CBB to the zero state.
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>
2015-07-01 19:45:43 +00:00
David Benjamin
d63307199a Re-wrap kOAEPCipherText from rsa_test.
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>
2015-07-01 19:40:22 +00:00
David Benjamin
cb41d77029 Fix kTwoPrimeKey and kSixPrimeKey samples in rsa_test.
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>
2015-07-01 19:40:04 +00:00
David Benjamin
d13a5e15d4 Fix crypto/bytestring test for too long lengths.
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>
2015-07-01 19:39:16 +00:00
David Benjamin
961ad6ad2c Sign channel IDs with ECDSA_do_sign.
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>
2015-07-01 19:38:04 +00:00
David Benjamin
1db476e7a5 Include openssl/chacha.h rather than duplicate the prototype.
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>
2015-07-01 19:35:56 +00:00
David Benjamin
7e5f5944c8 Document i2d_PublicKey better.
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>
2015-07-01 19:35:26 +00:00
David Benjamin
507b819304 Use typedef names in evp.h.
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>
2015-07-01 19:33:44 +00:00
David Benjamin
e3aa1d9dd4 Cleanup ticket processing and session lookup.
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>
2015-07-01 19:33:23 +00:00
Adam Langley
5021b223d8 Convert the renegotiation extension to the new system.
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>
2015-07-01 19:30:53 +00:00
Adam Langley
4cfa96bd26 Move compile-time asserts to the top-level.
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>
2015-07-01 18:57:57 +00:00
Adam Langley
614c66a2f8 Add infrastructure for better extension handling.
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>
2015-07-01 18:25:28 +00:00
Adam Langley
be9eda4a88 Fix Renegotiate-Client-NoExt test.
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>
2015-07-01 17:56:19 +00:00
David Benjamin
705076ac91 Handle EINTR in open and dup calls in urandom.c.
Per review comment in
https://boringssl-review.googlesource.com/#/c/5302/7/crypto/rand/urandom.c

Change-Id: I9c279524a452cb97c60354213cbc6e2aeabe0bfa
Reviewed-on: https://boringssl-review.googlesource.com/5311
Reviewed-by: Adam Langley <agl@google.com>
2015-07-01 00:50:05 +00:00
David Benjamin
de24aadc5b Add RAND_set_urandom_fd.
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>
2015-06-30 21:41:42 +00:00
David Benjamin
daeafc22c6 Move the session ticket functions to ssl.h.
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>
2015-06-25 22:40:38 +00:00
David Benjamin
6cacac033b Promote SSL_CTX_[gs]et_tlsext_ticket_keys to functions.
BUG=404754

Change-Id: Iae75a7ab24d4aa3b30edf578cbfc1058aeadd863
Reviewed-on: https://boringssl-review.googlesource.com/5233
Reviewed-by: Adam Langley <agl@google.com>
2015-06-25 22:39:36 +00:00
David Benjamin
cb3872f546 Sort the preprocessor compatibility section.
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>
2015-06-25 22:36:37 +00:00
David Benjamin
bed63548d1 Increment the reference count of sessions inside the lock.
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>
2015-06-25 22:35:51 +00:00