Commit Graph

33 Commits

Author SHA1 Message Date
David Benjamin
de94238217 Fix SSL_get_{read,write}_sequence.
I switched up the endianness. Add some tests to make sure those work right.

Also tweak the DTLS semantics. SSL_get_read_sequence should return the highest
sequence number received so far. Include the epoch number in both so we don't
need a second API for it.

Change-Id: I9901a1665b41224c46fadb7ce0b0881dcb466bcc
Reviewed-on: https://boringssl-review.googlesource.com/7141
Reviewed-by: Adam Langley <agl@google.com>
2016-02-17 22:05:29 +00:00
David Benjamin
fb974e6cb3 Use initializer lists to specify cipher rule tests.
This is significantly less of a nuisance than having to explicitly type out
kRule5, kExpected5.

Change-Id: I61820c26a159c71e09000fbe0bf91e30da42205e
Reviewed-on: https://boringssl-review.googlesource.com/7000
Reviewed-by: Adam Langley <agl@google.com>
2016-02-16 18:42:07 +00:00
David Benjamin
4f6acaf0da Use more C++11 features.
Finally, we can stick ScopedFOO in containers.

Change-Id: I3ed166575822af9f182e8be8f4db723e1f08ea31
Reviewed-on: https://boringssl-review.googlesource.com/6553
Reviewed-by: Adam Langley <alangley@gmail.com>
2016-01-28 00:52:37 +00:00
David Benjamin
13414b3a04 Implement draft-ietf-tls-chacha20-poly1305-04.
Only ECDHE-based ciphers are implemented. To ease the transition, the
pre-standard cipher shares a name with the standard one. The cipher rule parser
is hacked up to match the name to both ciphers. From the perspective of the
cipher suite configuration language, there is only one cipher.

This does mean it is impossible to disable the old variant without a code
change, but this situation will be very short-lived, so this is fine.

Also take this opportunity to make the CK and TXT names align with convention.

Change-Id: Ie819819c55bce8ff58e533f1dbc8bef5af955c21
Reviewed-on: https://boringssl-review.googlesource.com/6686
Reviewed-by: Adam Langley <agl@google.com>
2015-12-16 23:34:56 +00:00
David Benjamin
d6e9eec3f8 Remove algo_strength.
FIPS is the same as HIGH (but for CHACHA20), so those are redundant.
Likewise, MEDIUM vs HIGH was just RC4. Remove those in favor of
redefining those legacy rules to mean this.

One less field to keep track of in each cipher.

Change-Id: I2b2489cffb9e16efb0ac7d7290c173cac061432a
Reviewed-on: https://boringssl-review.googlesource.com/6515
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:30:44 +00:00
David Benjamin
dcb6ef0f0b Remove algorithm_ssl.
It's redundant with other cipher properties. We can express these in code.
Cipher rule matching gets a little bit complicated due to the confusing legacy
protocol version cipher rules, so add some tests for it. (It's really hard to
grep for uses of them, so I've kept them working to be safe.)

Change-Id: Ic6b3fcd55d76d4a51b31bf7ae629a2da50a7450e
Reviewed-on: https://boringssl-review.googlesource.com/6453
Reviewed-by: Adam Langley <agl@google.com>
2015-11-19 01:28:24 +00:00
David Benjamin
ef14b2d86e Remove stl_compat.h.
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>
2015-11-11 22:19:36 +00:00
Brian Smith
271777f5ac Refactor ChaCha20-Poly1305 AEAD nonce handling.
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>
2015-10-27 01:01:42 +00:00
David Benjamin
0a870c2f7e Correctly free SSL_SESSIONs in ssl_test.
That was silly.

Change-Id: I375c04f725cbb75f9e04fce386e20c4de5e7ae0c
Reviewed-on: https://boringssl-review.googlesource.com/6352
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:49:16 +00:00
David Benjamin
1269ddd377 Never use the internal session cache for a client.
The internal session cache is keyed on session ID, so this is completely
useless for clients (indeed we never look it up internally). Along the way,
tidy up ssl_update_cache to be more readable. The slight behavior change is
that SSL_CTX_add_session's return code no longer controls the external
callback. It's not clear to me what that could have accomplished. (It can only
fail on allocation error. We only call it for new sessions, so the duplicate
case is impossible.)

The one thing of value the internal cache might have provided is managing the
timeout. The SSL_CTX_flush_sessions logic would flip the not_resumable bit and
cause us not to offer expired sessions (modulo SSL_CTX_flush_sessions's delay
and any discrepancies between the two caches). Instead, just check expiration
when deciding whether or not to offer a session.

This way clients that set SSL_SESS_CACHE_CLIENT blindly don't accidentally
consume gobs of memory.

BUG=531194

Change-Id: If97485beab21874f37737edc44df24e61ce23705
Reviewed-on: https://boringssl-review.googlesource.com/6321
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:27:28 +00:00
David Benjamin
0f653957c1 Add tests for the internal session cache behavior.
In doing so, fix the documentation for SSL_CTX_add_session and
SSL_CTX_remove_session. I misread the code and documented the behavior
on session ID collision wrong.

Change-Id: I6f364305e1f092b9eb0b1402962fd04577269d30
Reviewed-on: https://boringssl-review.googlesource.com/6319
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 19:18:44 +00:00
David Benjamin
7a1eefd3cd Deprecate SSL_library_init.
It just calls CRYPTO_library_init and doesn't do anything else. If
anything, I'd like to make CRYPTO_library_init completely go away too.
We have CRYPTO_once now, so I think it's safe to assume that, if ssl/
ever grows initialization needs beyond that of crypto/, we can hide it
behind a CRYPTO_once and not burden callers.

Change-Id: I63dc362e0e9e98deec5516f4620d1672151a91b6
Reviewed-on: https://boringssl-review.googlesource.com/6311
Reviewed-by: Adam Langley <alangley@gmail.com>
2015-10-26 18:36:23 +00:00
David Benjamin
1d128f369c Make SSL_get_client_CA_list slightly more OpenSSL-compatible.
SSL_get_client_CA_list is one of those dreaded functions which may query either
configuration state or handshake state. Moreover, it does so based on
|ssl->server|, which may not be configured until later. Also check
|ssl->handshake_func| to make sure |ssl| is not in an indeterminate state.

This also fixes a bug where SSL_get_client_CA_list wouldn't work in DTLS due to
the incorrect |ssl->version| check.

Change-Id: Ie564dbfeecd2c8257fd6bcb148bc5db827390c77
Reviewed-on: https://boringssl-review.googlesource.com/5827
Reviewed-by: Adam Langley <agl@google.com>
2015-09-11 22:30:55 +00:00
Matt Braithwaite
af096751e8 Restore the NULL-SHA ciphersuite. (Alas.)
Change-Id: Ia5398f3b86a13fb20dba053f730b51a0e57b9aa4
Reviewed-on: https://boringssl-review.googlesource.com/5791
Reviewed-by: Adam Langley <agl@google.com>
2015-09-11 22:18:08 +00:00
David Benjamin
26416e9dde Remove the last of SESS_CERT.
Move cert_chain to the SSL_SESSION. Now everything on an SSL_SESSION is
properly serialized. The cert_chain field is, unfortunately, messed up
since it means different things between client and server.

There exists code which calls SSL_get_peer_cert_chain as both client and
server and assumes the existing semantics for each. Since that function
doesn't return a newly-allocated STACK_OF(X509), normalizing between the
two formats is a nuisance (we'd either need to store both cert_chain and
cert_chain_full on the SSL_SESSION or create one of the two variants
on-demand and stash it into the SSL).

This CL does not resolve this and retains the client/server difference
in SSL_SESSION. The SSL_SESSION serialization is a little inefficient
(two copies of the leaf certificate) for a client, but clients don't
typically serialize sessions. Should we wish to resolve it in the
future, we can use a different tag number. Because this was historically
unserialized, existing code must already allow for cert_chain not being
preserved across i2d/d2i.

In keeping with the semantics of retain_only_sha256_of_client_certs,
cert_chain is not retained when that flag is set.

Change-Id: Ieb72fc62c3076dd59750219e550902f1ad039651
Reviewed-on: https://boringssl-review.googlesource.com/5759
Reviewed-by: Adam Langley <agl@google.com>
2015-08-28 22:45:59 +00:00
Sigbjorn Vik
2b23d24c38 Make key exchange strength available.
This change stores the size of the group/modulus (for RSA/DHE) or curve
ID (for ECDHE) in the |SSL_SESSION|. This makes it available for UIs
where desired.

Change-Id: I354141da432a08f71704c9683f298b361362483d
Reviewed-on: https://boringssl-review.googlesource.com/5280
Reviewed-by: Adam Langley <agl@google.com>
2015-08-18 22:09:27 +00:00
David Benjamin
422fe08672 Add tests for the padding extension.
This sort of test is more suitable for ssl_test than runner. This should
stress all the various cases around padding. Use tickets rather than
hostnames to inflate the ClientHello because there's a fairly tight
maximum hostname length.

Change-Id: Ibd43aaa7acb9bf5fa00a9d2548d2101e5bb147d3
Reviewed-on: https://boringssl-review.googlesource.com/5480
Reviewed-by: Adam Langley <agl@google.com>
2015-07-29 19:20:53 +00:00
David Benjamin
fd67aa8c95 Add SSL_SESSION_from_bytes.
Mirrors SSL_SESSION_to_bytes. It avoids having to deal with object-reuse, the
non-size_t length parameter, and trailing data. Both it and the object-reuse
variant back onto an unexposed SSL_SESSION_parse which reads a CBS.

Note that this changes the object reuse story slightly. It's now merely an
optional output pointer that frees its old contents. No d2i_SSL_SESSION
consumer in Google that's built does reuse, much less reuse with the assumption
that the top-level object won't be overridden.

Change-Id: I5cb8522f96909bb222cab0f342423f2dd7814282
Reviewed-on: https://boringssl-review.googlesource.com/5121
Reviewed-by: Adam Langley <agl@google.com>
2015-06-16 18:12:39 +00:00
Adam Langley
85bc5601ee Add ECDHE-PSK-AES{128,256}-SHA cipher suites.
If we're going to have PSK and use standard cipher suites, this might be
the best that we can do for the moment.

Change-Id: I35d9831b2991dc5b23c9e24d98cdc0db95919d39
Reviewed-on: https://boringssl-review.googlesource.com/5052
Reviewed-by: Adam Langley <agl@google.com>
2015-06-09 18:10:42 +00:00
Adam Langley
1feb42a2fb Drop ECDHE-PSK-AES-128-GCM.
This is the best PSK cipher suite, but it's non-standard and nobody is
using it. Trivial to bring back in the future if we have need of it.

Change-Id: Ie78790f102027c67d1c9b19994bfb10a2095ba92
Reviewed-on: https://boringssl-review.googlesource.com/5051
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2015-06-09 18:08:52 +00:00
David Benjamin
338e06771c Reject sessions with the wrong structure version.
The old upstream logic actually didn't do this, but 1.1.0's new code does.
Given that the version has never changed and even unknown fields were rejected
by the old code, this seems a safe and prudent thing to do.

Change-Id: I09071585e5183993b358c10ad36fc206f8bceeda
Reviewed-on: https://boringssl-review.googlesource.com/4942
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 20:30:02 +00:00
David Benjamin
f297e021f1 Reject unknown fields in d2i_SSL_SESSION.
The original OpenSSL implementation did the same. M_ASN1_D2I_Finish checks
this. Forwards compatibility with future sessions with unknown fields is
probably not desirable.

Change-Id: I116a8c482cbcc47c3fcc31515c4a3718f66cf268
Reviewed-on: https://boringssl-review.googlesource.com/4941
Reviewed-by: Adam Langley <agl@google.com>
2015-06-01 20:29:07 +00:00
David Benjamin
3fa65f0f05 Fix some malloc test crashs.
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>
2015-05-21 18:00:10 +00:00
David Benjamin
687937304b Revert "Temporarily break a handful of tests."
This reverts commit a921d550d0.
2015-05-04 20:21:32 -04:00
David Benjamin
a921d550d0 Temporarily break a handful of tests.
This will be reverted in a minute. The bots should run both suites of tests and
report the names of all failing tests in the summary.

Change-Id: Ibe351017dfa8ccfd182b3c88eee413cd2cbdeaf0
2015-05-04 20:17:28 -04:00
David Benjamin
67be048e1a Convert ec_test to C++
Change-Id: I5e25ddbc87370b58d9b6fc410f51e259947df8dd
Reviewed-on: https://boringssl-review.googlesource.com/4468
Reviewed-by: Adam Langley <agl@google.com>
2015-04-28 21:00:34 +00:00
David Benjamin
83f182c6c5 Revert "Temporary introduce a memory leak in ssl_test"
This reverts commit 88d0f91952.
2015-04-21 06:32:24 -04:00
David Benjamin
88d0f91952 Temporary introduce a memory leak in ssl_test
Making sure the ASan bot notices. This will be reverted in a minute.

Change-Id: Ib8acf21948e3638d54ab09f3df4c339c4ea3d649
2015-04-21 06:30:59 -04:00
David Benjamin
0344dafb71 Tidy cipher rule processing.
Rather than shoehorn real ciphers and cipher aliases into the same type (that's
what cipher->valid is used for), treat them separately. Make
ssl_cipher_apply_rule match ciphers by cipher_id (the parameter was ignored and
we assumed that masks uniquely identify a cipher) and remove the special cases
around zero for all the masks. This requires us to remember which fields
default to 0 and which default to ~0u, but the logic is much clearer.

Finally, now that ciphers and cipher aliases are different, don't process rules
which sum together an actual cipher with cipher aliases. This would AND
together the masks for the alias with the values in the cipher and do something
weird around alg_ssl. (alg_ssl is just weird in general, as everyone trying to
disable SSLv3 in OpenSSL recently discovered.)

With all that, we can finally remove cipher->valid which was always one.

Change-Id: Iefcfe159bd6c22dbaea3a5f1517bd82f756dcfe1
Reviewed-on: https://boringssl-review.googlesource.com/4284
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 22:05:10 +00:00
Brian Smith
83a82981dc Rename BIO_print_errors_fp back to ERR_print_errors_fp & refactor it.
A previous change in BoringSSL renamed ERR_print_errors_fp to
BIO_print_errors_fp as part of refactoring the code to improve the
layering of modules within BoringSSL. Rename it back for better
compatibility with code that was using the function under the original
name. Move its definition back to crypto/err using an implementation
that avoids depending on crypto/bio.

Change-Id: Iee7703bb1eb4a3d640aff6485712bea71d7c1052
Reviewed-on: https://boringssl-review.googlesource.com/4310
Reviewed-by: Adam Langley <agl@google.com>
2015-04-13 20:23:29 +00:00
David Benjamin
32fbdf2025 Remove anonymous cipher suites.
These are the remaining untested cipher suites. Rather than add support in
runner.go, just remove them altogether. Grepping for this is a little tricky,
but nothing enables aNULL (all occurrences disable it), and all occurrences of
["ALL:] seem to be either unused or explicitly disable anonymous ciphers.

Change-Id: I4fd4b8dc6a273d6c04a26e93839641ddf738343f
Reviewed-on: https://boringssl-review.googlesource.com/4258
Reviewed-by: Adam Langley <agl@google.com>
2015-04-08 23:29:07 +00:00
David Benjamin
e12c4378e9 Fix leak in ssl_test.
SSL_CIPHER_get_rfc_name still returns an allocated string.

Change-Id: Ie2f14626c1ff22d0ea613b22439b7de5c04c9062
Reviewed-on: https://boringssl-review.googlesource.com/4190
Reviewed-by: Adam Langley <agl@google.com>
2015-04-01 18:22:23 +00:00
David Benjamin
1d77e56b29 Convert ssl_test to C++.
Change-Id: Ic8f3cd5c6a89e07bbae43b1599a01fedf119b081
Reviewed-on: https://boringssl-review.googlesource.com/4121
Reviewed-by: Adam Langley <agl@google.com>
2015-03-31 23:03:54 +00:00