Commit Graph

51 Commits

Author SHA1 Message Date
David Benjamin
b31040d0d8 Get rid of CERT_PKEY slots in SESS_CERT.
This doesn't even change behavior. Unlike local configuration, the peer
can never have multiple certificates anyway. (Even with a renego, the
SESS_CERT is created anew.)

This does lose the implicit certificate type check, but the certificate
type is already checked in ssl3_get_server_certificate and later checked
post-facto in ssl3_check_cert_and_algorithm (except that one seems to
have some bugs like it accepts ECDSA certificates for RSA cipher suites,
to be cleaned up in a follow-up). Either way, we have the certificate
mismatch tests for this.

BUG=486295

Change-Id: I437bb723bb310ad54ee4150eda67c1cfe43377b3
Reviewed-on: https://boringssl-review.googlesource.com/5044
Reviewed-by: Adam Langley <agl@google.com>
2015-06-08 22:13:45 +00:00
Adam Langley
4bdb6e43fa Remove remaining calls to the old lock functions.
|SSL_CTX| and |X509_STORE| have grown their own locks. Several static
locks have been added to hack around not being able to use a
|CRYPTO_once_t| in public headers. Lastly, support for calling
|SSL_CTX_set_generate_session_id| concurrently with active connections
has been removed. No other property of an |SSL_CTX| works like that.

Change-Id: Iff5fe3ee3fdd6ea9c9daee96f850b107ad8a6bca
Reviewed-on: https://boringssl-review.googlesource.com/4775
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:18:13 +00:00
Adam Langley
0da323a8b8 Convert reference counts in crypto/
This change converts the reference counts in crypto/ to use
|CRYPTO_refcount_t|. The reference counts in |X509_PKEY| and |X509_INFO|
were never actually used and so were dropped.

Change-Id: I75d572cdac1f8c1083c482e29c9519282d7fd16c
Reviewed-on: https://boringssl-review.googlesource.com/4772
Reviewed-by: Adam Langley <agl@google.com>
2015-05-20 19:15:26 +00:00
David Benjamin
9a10f8fd88 Switch EVP_PKEY_dup calls to EVP_PKEY_up_ref.
Keep internal callers up-to-date with deprecations.

Change-Id: I7ee171afc669592d170f83bd4064857d59332878
Reviewed-on: https://boringssl-review.googlesource.com/4640
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:57:09 +00:00
David Benjamin
6abb37016e Remove ciphers_raw.
With SSL_get0_raw_cipherlist gone, there's no need to hold onto it.

Change-Id: I258f8bfe21cc354211a777660df680df6c49df2a
Reviewed-on: https://boringssl-review.googlesource.com/4616
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:56:31 +00:00
David Benjamin
60da0cd7c6 Fix STACK_OF pointer style.
clang-format got a little confused there.

Change-Id: I46df523e8a7813a2b4e243da3df22851b3393873
Reviewed-on: https://boringssl-review.googlesource.com/4614
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:55:16 +00:00
David Benjamin
605641ed95 Move the NULL case in ssl_add_cert_chain up.
It's only called for client certificates with NULL. The interaction with
extra_certs is more obvious if we handle that case externally. (We
shouldn't attach extra_certs if there is no leaf.)

Change-Id: I9dc26f32f582be8c48a4da9aae0ceee8741813dc
Reviewed-on: https://boringssl-review.googlesource.com/4613
Reviewed-by: Adam Langley <agl@google.com>
2015-05-06 22:53:53 +00:00
David Benjamin
9362b6e235 Errors are uint32_t, not unsigned long.
Change-Id: Ic2339b771d949a555b8d05a3b24dc2e990b9d8d3
Reviewed-on: https://boringssl-review.googlesource.com/4555
Reviewed-by: Adam Langley <agl@google.com>
2015-05-05 18:48:01 +00:00
David Benjamin
2755a3eda3 Remove unnecessary NULL checks, part 5.
Finally, the ssl stack.

Change-Id: Iea10e302825947da36ad46eaf3e8e2bce060fde2
Reviewed-on: https://boringssl-review.googlesource.com/4518
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 23:16:19 +00:00
David Benjamin
ed8fbad170 Remove SSL cert_flags.
These are never used and no flags are defined anyway.

Change-Id: I206dc2838c5f68d87559a702dcb299b208cc7e1e
Reviewed-on: https://boringssl-review.googlesource.com/4493
Reviewed-by: Adam Langley <agl@google.com>
2015-05-04 22:48:13 +00:00
David Benjamin
dd978784d7 Always enable ecdh_auto.
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.

Currently, by default, server sockets can only use the plain RSA key exchange.

BUG=481139

Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 20:51:05 +00:00
David Benjamin
f0ae170021 Include-what-you-use ssl/internal.h.
The rest of ssl/ still includes things everywhere, but this at least fixes the
includes that were implicit from ssl/internal.h.

Change-Id: I7ed22590aca0fe78af84fd99a3e557f4b05f6782
Reviewed-on: https://boringssl-review.googlesource.com/4281
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:15:02 +00:00
David Benjamin
2ee94aabf5 Rename ssl_locl.h to internal.h
Match the other internal headers.

Change-Id: Iff7e2dd06a1a7bf993053d0464cc15638ace3aaa
Reviewed-on: https://boringssl-review.googlesource.com/4280
Reviewed-by: Adam Langley <agl@google.com>
2015-04-10 22:14:09 +00:00
David Benjamin
c0f763b080 Simplify server-side ECDH curve selection.
There's multiple sets of APIs for selecting the curve. Fold away
SSL_OP_SINGLE_ECDH_USE as failing to set it is either a no-op or a bug. With
that gone, the consumer only needs to control the selection of a curve, with
key generation from then on being uniform. Also clean up the interaction
between the three API modes in s3_srvr.c; they were already mutually exclusive
due to tls1_check_ec_tmp_key.

This also removes all callers of EC_KEY_dup (and thus CRYPTO_dup_ex_data)
within the library.

Change-Id: I477b13bd9e77eb03d944ef631dd521639968dc8c
Reviewed-on: https://boringssl-review.googlesource.com/4200
Reviewed-by: Adam Langley <agl@google.com>
2015-04-02 18:37:06 +00:00
Håvard Molland
ab2479a08a Clean up error reporting.
Quite a few functions reported wrong function names when pushing
to the error stack.

Change-Id: I84d89dbefd2ecdc89ffb09799e673bae17be0e0f
Reviewed-on: https://boringssl-review.googlesource.com/4080
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-03-20 22:12:59 +00:00
David Benjamin
b85a4c2923 Remove unnecessary NULL initializations in ssl_cert_dup.
A casual grep would suggest this function has the same problems as
CVE-2015-0291, but the structure is memset to 0, so the calls are unnecessary.
Also use BUF_memdup rather than an OPENSSL_malloc + mempcy pair.

Change-Id: Id605374d99cff32e2dccb7f9b8a9da226faf7715
Reviewed-on: https://boringssl-review.googlesource.com/4051
Reviewed-by: Adam Langley <agl@google.com>
2015-03-19 19:52:57 +00:00
David Benjamin
a5a3eeb9cc Remove ssl_cert_inst()
It created the cert structure in SSL_CTX or SSL if it was NULL, but they can
never be NULL as the comments already said.

(Imported from upstream's 2c3823491d8812560922a58677e3ad2db4b2ec8d.)

Change-Id: I97c7bb306d6f3c18597850db9f08023b2ef74839
Reviewed-on: https://boringssl-review.googlesource.com/4042
Reviewed-by: Adam Langley <agl@google.com>
2015-03-19 11:35:46 +00:00
David Benjamin
6eb000dbee Add in missing curly braces part 3.
Everything else.

Change-Id: Iac02b144465b4e7b6d69ea22ff2aaf52695ae732
2015-02-11 15:14:46 -08:00
Adam Langley
a307dfd29f Add (void) to some macros to satisfy compiler.
More modern versions of GCC (at least with aarch64) are warning about an
unused value in these locations. It's incorrect, but I guess that the
macro is confusing it.

Using a (void) tag is a little ugly but solves the problem.

Change-Id: If6ba5083ab6e501c81e7743ae1ed99a89565e57c
Reviewed-on: https://boringssl-review.googlesource.com/2810
Reviewed-by: Adam Langley <agl@google.com>
2015-01-12 23:46:03 +00:00
Adam Langley
fcf25833bc Reformat the rest of ssl/.
Change-Id: I7dc264f7e29b3ba8be4c717583467edf71bf8dd9
2014-12-18 17:43:03 -08:00
Adam Langley
2481975857 Reformat d1_{srtp|srvr}.c and s3_both.c
Change-Id: I4dc1463b75b12e15673da32e4945f83aaea123e6
2014-12-15 18:42:07 -08:00
David Benjamin
8278184631 Remove redundant checks in ssl_cert_dup.
PR#3613

(Imported from upstream's fc3968a25ce0c16cab8730ec0d68a59856158029)

We don't care about GOST, but removing redundant code is reasonable. Also
switch that CRYPTO_add to EVP_PKEY_dup. Missed a spot.

Change-Id: I768ec546d987fb3d8bc3decf7ebf1a5590fbb6c2
Reviewed-on: https://boringssl-review.googlesource.com/2477
Reviewed-by: Adam Langley <agl@google.com>
2014-12-05 17:27:23 +00:00
David Benjamin
63246e8a99 Remove s->type from SSL.
It's redundant with s->server.

Change-Id: Idb4ca44618477b54f3be5f0630f0295f0708b0f4
Reviewed-on: https://boringssl-review.googlesource.com/2438
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:34:28 +00:00
David Benjamin
ec2f27dee1 Account for EVP_PKEY capabilities in selecting hash functions.
tls1_process_sigalgs now only determines the intersection between the peer
algorithms and those configured locally. That list is queried later to
determine the hash algorithm to use when signing CertificateVerify or
ServerKeyExchange.

This is needed to support client auth on Windows where smartcards or CAPI may
not support all hash functions.

As a bonus, this does away with more connection-global state. This avoids the
current situation where digests are chosen before keys are known (for
CertificateVerify) or for slots that don't exist.

Change-Id: Iec3619a103d691291d8ebe08ef77d574f2faf0e8
Reviewed-on: https://boringssl-review.googlesource.com/2280
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:22:33 +00:00
David Benjamin
033e5f47d1 Remove CERT_PKEY::valid_flags.
CERT_PKEY_SIGN isn't meaningful since, without strict mode, we always fall back
to SHA-1 anyway. So the digest is never NULL when CERT_PKEY_SIGN is computed.
The entire valid_flags is now back to it's pre-1.0.2 check of seeing if the
certificate and key are configured.

This finally removes the sensitivity between valid_flags and selecting the
digest, so we can defer choosing the digest all we like.

Change-Id: I9f9952498f512d7f0cc799497f7c5b52145a48af
Reviewed-on: https://boringssl-review.googlesource.com/2288
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:22:23 +00:00
David Benjamin
f31e681acf Clean up ssl_set_cert_masks.
It doesn't depend on the cipher now that export ciphers are gone. It need only
be called once. Also remove the valid bit; nothing ever reads it. Its output is
also only used within a function, so make mask_k and mask_a local variables.

So all the configuration-based checks are in one place, change the input
parameter from CERT to SSL and move the PSK and ECDHE checks to the mask
computation. This avoids having to evaluate the temporary EC key for each
cipher.

The remaining uses are on the client which uses them differently (disabled
features rather than enabled ones). Those too may as well be local variables,
so leave a TODO.

Change-Id: Ibcb574341795d4016ea749f0290a793eed798874
Reviewed-on: https://boringssl-review.googlesource.com/2287
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:21:52 +00:00
David Benjamin
675227e0d2 Remove CERT_PKEY_EXPLICIT_SIGN flag.
This is maintained just to distinguish whether the digest was negotiated or we
simply fell back to assuming SHA-1 support. No code is sensitive to this flag
and it adds complexity because it is set at a different time, for now, from the
rest of valid_flags.

The flag is new in OpenSSL 1.0.2, so nothing external could be sensitive to it.

Change-Id: I9304e358d56f44d912d78beabf14316d456bf389
Reviewed-on: https://boringssl-review.googlesource.com/2282
Reviewed-by: Adam Langley <agl@google.com>
2014-11-18 22:19:06 +00:00
David Benjamin
525a0fe315 Remove client-side support for ServerKeyExchange in the RSA key exchange.
Server-side support was removed in 77a942b7fe,
but client-side support was retained as it appeared NSS supported this.
However, this is not the case: ssl3_HandleServerKeyExchange only allows a
ServerKeyExchange message if hs.ws is in an appropriate state.
ssl3_AuthCertificate only sets it to allow ServerKeyExchange if it is a key
exchange that normally uses it or if is_limited is set. is_limited is only set
for the export cipher suites.

Thus we can safely remove this without waiting on gathering UMA data.

BUG=chromium:400587

Change-Id: I9aefb742dbb2d99c13340ab48017e1ceee04bc2f
Reviewed-on: https://boringssl-review.googlesource.com/2230
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:00:09 +00:00
David Benjamin
3f383908e2 Properly clean up on ssl_cert_dup failure.
Caught by scan-build.

Change-Id: I7c09b176d6a9e5d4fcd6e4fba184ac0679983cff
Reviewed-on: https://boringssl-review.googlesource.com/2200
Reviewed-by: Adam Langley <agl@google.com>
2014-11-06 01:28:30 +00:00
David Benjamin
fb3ff2c66c Don't compare signed vs. unsigned.
This resolves a pile of MSVC warnings in Chromium.

Change-Id: Ib9a29cb88d8ed8ec4118d153260f775be059a803
Reviewed-on: https://boringssl-review.googlesource.com/1865
Reviewed-by: Adam Langley <agl@google.com>
2014-10-01 02:17:38 +00:00
David Benjamin
457112e197 unifdef a bunch of OPENSSL_NO_* ifdefs.
Get all this stuff out of the way.

- OPENSSL_NO_MD5
- OPENSSL_NO_SHA
- OPENSSL_NO_EC
- OPENSSL_NO_ECDSA
- OPENSSL_NO_ECDH
- OPENSSL_NO_NEXTPROTONEG
- OPENSSL_NO_DH
- OPENSSL_NO_SSL3
- OPENSSL_NO_RC4
- OPENSSL_NO_RSA

Also manually removed a couple instances of OPENSSL_NO_DSA that seemed to be
confused anyway. Did some minor manual cleanup. (Removed a few now-pointless
'if (0)'s.)

Change-Id: Id540ba97ee22ff2309ab20ceb24c7eabe766d4c4
Reviewed-on: https://boringssl-review.googlesource.com/1662
Reviewed-by: Adam Langley <agl@google.com>
2014-08-28 00:41:34 +00:00
David Benjamin
422d3a4015 Remove some unused state and code.
Change-Id: I1f917d1fe70ef5046eeea8fed27cc402bcd674b9
Reviewed-on: https://boringssl-review.googlesource.com/1568
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:04:32 +00:00
David Benjamin
a7d1363fcb Prune removed key types from SSL_PKEY_*.
Remove all the logic managing key types that aren't being used anymore.

Change-Id: I101369164588048e64ba1c84a6b8aac8f3a221cd
Reviewed-on: https://boringssl-review.googlesource.com/1567
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:15:32 +00:00
David Benjamin
335d10d201 Remove Suite B mode.
It was added in OpenSSL 1.0.2, so nothing can be depending on it yet. If we
really want a Suite B profile, it seems better to generate a configuration for
the rest of the system rather than pepper the codebase with checks.

Change-Id: I1be3ebed0e87cbfe236ade4174dcf5bbc7e10dd5
Reviewed-on: https://boringssl-review.googlesource.com/1517
Reviewed-by: Adam Langley <agl@google.com>
2014-08-14 22:00:16 +00:00
David Benjamin
67454b6f76 Don't X509_up_ref X509_STOREs.
Change-Id: Ic78bec93aedcc06c1496fe374e1c1c77ef70ea4b
Reviewed-on: https://boringssl-review.googlesource.com/1416
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 02:44:31 +00:00
David Benjamin
150c617cfc Add X509_up_ref and use it internally.
Avoid needing to manually increment the reference count and using the right
lock, both here and in Chromium.

Change-Id: If116ebc224cfb1c4711f7e2c06f1fd2c97af21dd
Reviewed-on: https://boringssl-review.googlesource.com/1415
Reviewed-by: Adam Langley <agl@google.com>
2014-08-07 00:06:34 +00:00
David Benjamin
44dbcc0804 Remove SSL_get_client_certificate_types.
Chromium is no longer using it.

Change-Id: If56340627d2024ff3fb8561405dd0cfc6f4787cb
Reviewed-on: https://boringssl-review.googlesource.com/1346
Reviewed-by: Adam Langley <agl@google.com>
2014-07-30 00:44:16 +00:00
David Benjamin
77a942b7fe Don't use the RSA key exchange with a signing-only key.
This removes the last case where the server generates an RSA key for the
ServerKeyExchange. Remove the code for this. Client support to accept them
still remains.

Leave the APIs for now, but they don't do anything anymore.

Change-Id: I84439e034cc575719f5bc9b3e501165e12b62107
Reviewed-on: https://boringssl-review.googlesource.com/1286
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:35:40 +00:00
David Benjamin
060d9d2c56 Remove support code for export cipher suites.
Now the only case where temporary RSA keys are used on the server end is
non-signing keys.

Change-Id: I55f6c206e798dd28548c386fdffd555ccc395477
Reviewed-on: https://boringssl-review.googlesource.com/1285
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:14:08 +00:00
David Benjamin
676d1e780e Separate client and server certificate_types.
This is the first of reorganizing state between connection state and handshake
state. The existing set are retained in cert_st for the server; they are server
configuration. The client gets a copy in s->s3->tmp alongside other handshake
state.

With other handshake state moved there, hopefully we can reset that state in
one go and possibly not even maintain it when there is no handshake in
progress.  Rather than currently where we sometimes confused connection state
and handshake state and have to reset as appropriate on renegotiate.

While I'm here, document the fields and name them something more useful than
'ctypes'.

Change-Id: Ib927579f0004fc5c6854fce2127625df669b2b6d
Reviewed-on: https://boringssl-review.googlesource.com/1113
Reviewed-by: Adam Langley <agl@google.com>
2014-07-09 19:51:08 +00:00
David Benjamin
398ba895fb Remove SSL_copy_session_id.
This is the only codepath that allowed a cert_st to be shared between two
ssl_st's. Given that the cert_st currently contains some per-connection and
even per-handshake state, this probably doesn't work.

Remove the function altogether and don't ref-count cert_st.

Change-Id: I66d5346117cb59b6063e7b9b893d1c4b40cb6867
Reviewed-on: https://boringssl-review.googlesource.com/1110
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 22:43:56 +00:00
David Benjamin
5468b23797 Remove rest of DANE code.
We pulled in some of upstream's removal, but not the rest of it.

Change-Id: I8ea6681848f3c59955b7d2ce935c077d024245be
Reviewed-on: https://boringssl-review.googlesource.com/1099
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:32:04 +00:00
Alex Chernyakhovsky
3c5034e97c Remove OPENSSL_NO_RSA
Building without RSA support is unreasonable. Changes were made by
running

find . -type f -name *.c | xargs unifdef -m -U OPENSSL_NO_RSA
find . -type f -name *.h | xargs unifdef -m -U OPENSSL_NO_RSA

using unifdef 2.10 and some newlines were removed manually.

Change-Id: Iea559e2d4b3d1053f28a4a9cc2f7a3d1f6cabd61
Reviewed-on: https://boringssl-review.googlesource.com/1095
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:20:53 +00:00
David Benjamin
2b0aeecabf Remove authz extension (RFC5878)
Found no users of the functions which control the feature. (Also I don't
particularly want to port all of that to CBS...)

Change-Id: I55da42c44d57252bd47bdcb30431be5e6e90dc56
Reviewed-on: https://boringssl-review.googlesource.com/1061
Reviewed-by: Adam Langley <agl@google.com>
2014-07-01 20:48:38 +00:00
David Benjamin
d7e23c17a9 Remove serverinfo and custom extensions support.
If we need an extension, we can implement it in-library.

Change-Id: I0eac5affcd8e7252b998b6c86ed2068234134b08
Reviewed-on: https://boringssl-review.googlesource.com/1051
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:51:50 +00:00
David Benjamin
5c57c60beb Deprecate SSL_get_client_certificate_types.
OpenSSL added SSL_get0_certificate_types and fixed the truncation (and
subsequent parse error) by adding an alternate copy of the data.
http://git.openssl.org/gitweb/?p=openssl.git;a=commit;h=9f27b1eec3175305e62eed87faa80e231f319ca0

Make SSL_get_client_certificate_types call SSL_get0_certificate_types to query
the new list. Remove when Chromium is switched over.

Also remove a now unnecessary cast because SSL_get_client_certificate_types
fixed the type of tmp.ctypes. Further fix it to use a size_t and match the
cert_st copy OpenSSL added.

BUG=388000

Change-Id: Ic6653e10e5a3c3ac6b3fe2a2322f388d6ffb0a06
2014-06-23 18:37:12 -04:00
Adam Langley
64c2223310 Update chain building function.
Don't clear verification errors from the error queue unless
SSL_BUILD_CHAIN_FLAG_CLEAR_ERROR is set.

If errors occur during verification and
SSL_BUILD_CHAIN_FLAG_IGNORE_ERROR is set return 2 so applications can
issue warnings.

(Imported from upstream's 2dd6976f6d02f98b30c376951ac38f780a86b3b5)
2014-06-20 13:17:39 -07:00
Adam Langley
dff7b9e11b Allow duplicate certs in ssl_build_cert_chain
(Imported from upstream's 662239183da08d687dc939211ac09d0a5c3a5b93)
2014-06-20 13:17:39 -07:00
Adam Langley
f669c2d91a New chain building flags.
New flags to build certificate chains. The can be used to rearrange
the chain so all an application needs to do is add all certificates
in arbitrary order and then build the chain to check and correct them.

Add verify error code when building chain.

(Imported from upstream's c5ea65b157e17743c881b9e348524b0281b3d39f)
2014-06-20 13:17:38 -07:00
Adam Langley
28acbbc2d6 Add SSL_get_client_certificate_types.
Exposes the certificate_types parameter in a CertificateRequest.

Transplanted from https://codereview.chromium.org/254723002/
2014-06-20 13:17:36 -07:00