Commit Graph

26 Commits

Author SHA1 Message Date
Matthew Braithwaite
d2ed382e64 Serialize SSL configuration in handoff and check it on application.
A split SSL handshake may involve 2 binaries, potentially built at
different versions: call them the "handoff/handback" binary and the
"handshake" binary.  We would like to guarantee that the
handoff/handback binary does not make any promises that the handshake
binary cannot keep.

As a start, this commit serializes |kCiphers| to the handoff message.
When the handoff message is applied to an |SSL|, any configured
ciphers not listed in the handoff message will be removed, in order to
prevent them from being negotiated.

Subsequent commits will apply the same approach to other lists of features.

Change-Id: Idf6dbeadb750c076ab0509c09b9d3f22eb162b9c
Reviewed-on: https://boringssl-review.googlesource.com/c/29264
Reviewed-by: Matt Braithwaite <mab@google.com>
2018-11-02 19:45:42 +00:00
David Benjamin
8525ff31ee Another batch of bools.
Change-Id: I5a7688b6e635e7ee6fc16173f9919bff16c4d59d
Reviewed-on: https://boringssl-review.googlesource.com/31604
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-09-07 15:06:55 +00:00
Joshua Liebow-Feeser
8c7c6356e6 Support symbol prefixes
- In base.h, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols.h
- In all .S files, if BORINGSSL_PREFIX is defined, include
  boringssl_prefix_symbols_asm.h
- In base.h, BSSL_NAMESPACE_BEGIN and BSSL_NAMESPACE_END are
  defined with appropriate values depending on whether
  BORINGSSL_PREFIX is defined; these macros are used in place
  of 'namespace bssl {' and '}'
- Add util/make_prefix_headers.go, which takes a list of symbols
  and auto-generates the header files mentioned above
- In CMakeLists.txt, if BORINGSSL_PREFIX and BORINGSSL_PREFIX_SYMBOLS
  are defined, run util/make_prefix_headers.go to generate header
  files
- In various CMakeLists.txt files, add "global_target" that all
  targets depend on to give us a place to hook logic that must run
  before all other targets (in particular, the header file generation
  logic)
- Document this in BUILDING.md, including the fact that it is
  the caller's responsibility to provide the symbol list and keep it
  up to date
- Note that this scheme has not been tested on Windows, and likely
  does not work on it; Windows support will need to be added in a
  future commit

Change-Id: If66a7157f46b5b66230ef91e15826b910cf979a2
Reviewed-on: https://boringssl-review.googlesource.com/31364
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-09-06 20:07:52 +00:00
Michał Janiszewski
c4f3b8a22a Add a compile time verification ciphers are sorted for bsearch()
Change-Id: Ibdf5b5194087a44088563141f1de2801cade231b
Reviewed-on: https://boringssl-review.googlesource.com/30144
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-07-30 22:17:31 +00:00
David Benjamin
c59b9aace6 Remove more remnants of SSLv3.
Mostly in comments, but there is one special-case around renegotiation_info
that can now be removed.

Change-Id: I2a9114cbff05e0cfff95fe93270fe42379728012
Reviewed-on: https://boringssl-review.googlesource.com/29824
Reviewed-by: Steven Valdez <svaldez@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-07-17 20:02:35 +00:00
David Benjamin
0ce090acd6 A bunch more scopers.
Change-Id: I5c8dbfec4a404d8d1501725a90b383eb3e05c664
Reviewed-on: https://boringssl-review.googlesource.com/29591
Reviewed-by: Adam Langley <agl@google.com>
2018-07-06 19:43:08 +00:00
David Benjamin
9bb15f58f7 Remove SSL 3.0 implementation.
Update-Note: SSL_CTX_set_min_proto_version(SSL3_VERSION) now fails.
   SSL_OP_NO_SSLv3 is now zero. Internal SSL3-specific "AEAD"s are gone.

Change-Id: I34edb160be40a5eea3e2e0fdea562c6e2adda229
Reviewed-on: https://boringssl-review.googlesource.com/29444
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2018-06-28 16:54:58 +00:00
Adam Langley
bcfb49914b Add special AES-GCM AEAD for TLS 1.3.
This change adds an AES-GCM AEAD that enforces nonce uniqueness inside
the FIPS module, like we have for TLS 1.2. While TLS 1.3 has not yet
been mentioned in the FIPS 140 IG, we expect it to be in the next ~12
months and so are preparing for that.

Change-Id: I65a7d8196b08dc0033bdde5c844a73059da13d9e
Reviewed-on: https://boringssl-review.googlesource.com/29224
Commit-Queue: Adam Langley <agl@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: David Benjamin <davidben@google.com>
2018-06-25 10:23:22 +00:00
David Benjamin
0ca921431a Temporarily restore SHA256 and SHA384 cipher suite aliases.
https://boringssl-review.googlesource.com/27944 inadvertently caused
SHA256 and SHA384 aliases to be rejected in
SSL_CTX_set_strict_cipher_list. While this is the desired end state, in
case the removal needs to be reverted, we should probably defer this to
post-removal cleanup.

Otherwise we might update someone's "ALL:!SHA256" cipher string to
account for the removal, and then revert the removal underneath them.

Change-Id: Id516a27a2ecefb5871485d0ae18067b5bbb536bb
Reviewed-on: https://boringssl-review.googlesource.com/28004
Reviewed-by: Adam Langley <agl@google.com>
2018-05-03 15:48:50 +00:00
David Benjamin
6e678eeb6e Remove legacy SHA-2 CBC ciphers.
All CBC ciphers in TLS are broken and insecure. TLS 1.2 introduced
AEAD-based ciphers which avoid their many problems. It also introduced
new CBC ciphers based on HMAC-SHA256 and HMAC-SHA384 that share the same
flaws as the original HMAC-SHA1 ones. These serve no purpose. Old
clients don't support them, they have the highest overhead of all TLS
ciphers, and new clients can use AEADs anyway.

Remove them from libssl. This is the smaller, more easily reverted
portion of the removal. If it survives a week or so, we can unwind a lot
more code elsewhere in libcrypto. This removal will allow us to clear
some indirect calls from crypto/cipher_extra/tls_cbc.c, aligning with
the recommendations here:

https://github.com/HACS-workshop/spectre-mitigations/blob/master/crypto_guidelines.md#2-avoid-indirect-branches-in-constant-time-code

Update-Note: The following cipher suites are removed:
- TLS_RSA_WITH_AES_128_CBC_SHA256
- TLS_RSA_WITH_AES_256_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384
- TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256
- TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384

Change-Id: I7ade0fc1fa2464626560d156659893899aab6f77
Reviewed-on: https://boringssl-review.googlesource.com/27944
Reviewed-by: Adam Langley <agl@google.com>
2018-05-02 19:21:56 +00:00
David Benjamin
48b276db3d Give ssl_cipher_preference_list_st a destructor.
Change-Id: I578a284c6a8cae773a97d3d30ad8a5cd13f56164
Reviewed-on: https://boringssl-review.googlesource.com/27491
Commit-Queue: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Steven Valdez <svaldez@google.com>
2018-04-24 19:55:29 +00:00
Adam Langley
fa3e9c3385 Add |SSL_COMP_get[0_name|_id]|.
These functions are needed by MySQL 8.0:
https://github.com/mysql/mysql-server/blob/8.0/vio/viossl.cc#L459

Change-Id: I4f13fa26cfe695229d6c8df80bcfc218408184da
Reviewed-on: https://boringssl-review.googlesource.com/26544
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2018-03-15 17:34:33 +00:00
Adam Langley
e745b25dcb Remove trailing whitespace from ssl/.
Change-Id: Ibcb27e1e5b14294c9d877db89ae62ef138e9e061
Reviewed-on: https://boringssl-review.googlesource.com/26184
Reviewed-by: Adam Langley <agl@google.com>
2018-02-26 22:05:13 +00:00
David Benjamin
3b903f252a Move the SSL_eNULL special-case into the matching function.
This avoids needing to keep track of which rules do and don't need it.

Change-Id: Id086b0622305f7f4acd3892f5d24d8e0c970febb
Reviewed-on: https://boringssl-review.googlesource.com/22468
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-11-01 16:45:06 +00:00
David Benjamin
5be3a74c49 Remove supports_cipher hook.
RC4 is gone. The only remaining exception was the dumb SSL_eNULL cipher,
which works fine in DTLS. It doesn't seem worth the trouble to retain
this special-case.

Change-Id: I31023b71192808e4d21e82109255dc4d6d381df8
Reviewed-on: https://boringssl-review.googlesource.com/22467
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-11-01 16:44:46 +00:00
David Benjamin
f496249405 Switch int to bool in ssl_cipher.cc.
Change-Id: I815f9fa77e08f72b0130ea9ef0dda751bf2ed7a6
Reviewed-on: https://boringssl-review.googlesource.com/20826
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
2017-10-02 20:41:20 +00:00
David Benjamin
b1b76aee3c Add SSL_CIPHER_get_prf_nid.
draft-ietf-quic-tls needs access to the cipher's PRF hash to size its
keys correctly.

Change-Id: Ie4851f990e5e1be724f262f608f7195f7ca837ca
Reviewed-on: https://boringssl-review.googlesource.com/20624
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-21 21:44:15 +00:00
Martin Kreichgauer
c0e15d1d9d Zero memory in |OPENSSL_free|.
Allocations by |OPENSSL_malloc| are prefixed with their length.
|OPENSSL_free| zeros the allocation before calling free(), eliminating
the need for a separate call to |OPENSSL_cleanse| for sensitive data.

This change will be followed up by the cleanup in
https://boringssl-review.googlesource.com/c/boringssl/+/19824.

Change-Id: Ie272f07e9248d7d78af9aea81dacec0fdb7484c4
Reviewed-on: https://boringssl-review.googlesource.com/19544
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-09-06 19:22:46 +00:00
David Benjamin
c11ea942b7 Convert comments in ssl.
That's the last of it!

Change-Id: I93d1f5ab7e95b2ad105c34b24297a0bf77625263
Reviewed-on: https://boringssl-review.googlesource.com/19784
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-29 21:33:32 +00:00
David Benjamin
e3bb51cb23 Remove deprecated cipher property APIs.
Consumers have been switched to the new ones.

Change-Id: I7a8ec6308775a105a490882c97955daed12a2c2c
Reviewed-on: https://boringssl-review.googlesource.com/19605
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-28 17:47:25 +00:00
David Benjamin
348f0d8db9 Add OpenSSL 1.1.0's cipher property functions.
Other projects are starting to use them. Having two APIs for the same
thing is silly, so deprecate all our old ones.

Change-Id: Iaf6b6995bc9e4b624140d5c645000fbf2cb08162
Reviewed-on: https://boringssl-review.googlesource.com/19064
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-11 02:08:58 +00:00
David Benjamin
ca9e8f52f1 Tidy up handshake digest logic.
Use SSL_SESSION_get_digest instead of the lower level function where
applicable. Also, remove the failure case (Ivan Maidanski points out in
https://android-review.googlesource.com/c/337852/1/src/ssl/t1_enc.c that
this unreachable codepath is a memory leak) by passing in an SSL_CIPHER
to make it more locally obvious that other values are impossible.

Change-Id: Ie624049d47ab0d24f32b405390d6251c7343d7d6
Reviewed-on: https://boringssl-review.googlesource.com/19024
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-08-09 19:13:15 +00:00
Adam Langley
22df69103f Document the behaviour of non-standard separators in cipher strings.
OpenSSL allows spaces, commas and semi-colons to be used as separators
in cipher strings, in addition to the usual colons.

This change documents that spaces cannot be used in equal-preference
groups and forbids these alternative separators in strict mode.

Change-Id: I3879e25aed54539c281511627e6a282e9463bdc3
Reviewed-on: https://boringssl-review.googlesource.com/18424
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
2017-07-25 20:48:44 +00:00
David Benjamin
e39ac8fb59 Switch BORINGSSL_INTERNAL_CXX_TYPES in favor of subclassing games.
The previous attempt around the 'struct ssl_st' compatibility mess
offended OSS-Fuzz and UBSan because one compilation unit passed a
function pointer with ssl_st* and another called it with
bssl::SSLConnection*.

Linkers don't retain such types, of course, but to silence this alert,
instead make C-visible types be separate from the implementation and
subclass the public type. This does mean we risk polluting the symbol
namespace, but hopefully the compiler is smart enough to inline the
visible struct's constructor and destructor.

Bug: 132
Change-Id: Ia75a89b3a22a202883ad671a630b72d0aeef680e
Reviewed-on: https://boringssl-review.googlesource.com/18224
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Steven Valdez <svaldez@google.com>
Reviewed-by: Steven Valdez <svaldez@google.com>
2017-07-20 17:24:12 +00:00
David Benjamin
86e95b852e Move libssl's internals into the bssl namespace.
This is horrible, but everything else I tried was worse. The goal with
this CL is to take the extern "C" out of ssl/internal.h and move most
symbols to namespace bssl, so we can start using C++ helpers and
destructors without worry.

Complications:

- Public API functions must be extern "C" and match their declaration in
  ssl.h, which is unnamespaced. C++ really does not want you to
  interleave namespaced and unnamespaced things. One can actually write
  a namespaced extern "C" function, but this means, from C++'s
  perspective, the function is namespaced. Trying to namespace the
  public header would worked but ended up too deep a rabbithole.

- Our STACK_OF macros do not work right in namespaces.

- The typedefs for our exposed but opaque types are visible in the
  header files and copied into consuming projects as forward
  declarations. We ultimately want to give SSL a destructor, but
  clobbering an unnamespaced ssl_st::~ssl_st seems bad manners.

- MSVC complains about ambiguous names if one typedefs SSL to bssl::SSL.

This CL opts for:

- ssl/*.cc must begin with #define BORINGSSL_INTERNAL_CXX_TYPES. This
  informs the public headers to create forward declarations which are
  compatible with our namespaces.

- For now, C++-defined type FOO ends up at bssl::FOO with a typedef
  outside. Later I imagine we'll rename many of them.

- Internal functions get namespace bssl, so we stop worrying about
  stomping the tls1_prf symbol. Exported C functions are stuck as they
  are. Rather than try anything weird, bite the bullet and reorder files
  which have a mix of public and private functions. I expect that over
  time, the public functions will become fairly small as we move logic
  to more idiomatic C++.

  Files without any public C functions can just be written normally.

- To avoid MSVC troubles, some bssl types are renamed to CPlusPlusStyle
  in advance of them being made idiomatic C++.

Bug: 132
Change-Id: Ic931895e117c38b14ff8d6e5a273e868796c7581
Reviewed-on: https://boringssl-review.googlesource.com/18124
Reviewed-by: David Benjamin <davidben@google.com>
2017-07-19 19:10:59 +00:00
David Benjamin
e64d2c74fa Convert ssl_buffer, ssl_cert, and ssl_cipher to C++.
ssl_cipher required fixing the types of the cipher masks.

Bug: 132
Change-Id: I0428d853b25fe4674ac3cad87a8eb92c6c8659e3
Reviewed-on: https://boringssl-review.googlesource.com/17746
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
2017-07-13 15:10:43 +00:00