Commit Graph

31 Commits

Author SHA1 Message Date
David Benjamin
beb47022b0 Remove redundant s->server assignments in handshake.
It should be set correctly prior to entering the handshake. Don't mask bugs by
assigning it.

Change-Id: Ib9bca8fad68916b3b242aad8819e3760e59e777a
Reviewed-on: https://boringssl-review.googlesource.com/2443
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:35:38 +00:00
David Benjamin
cde8abae14 Merge client/server SSL_METHODs into the generic one.
Supporting both schemes seems pointless. Now that s->server and s->state are
set appropriately late and get_ssl_method is gone, the only difference is that
the client/server ones have non-functional ssl_accept or ssl_connect hooks. We
can't lose the generic ones, so let's unify on that.

Note: this means a static linker will no longer drop the client or server
handshake code if unused by a consumer linking statically. However, Chromium
needs the server half anyway for DTLS and WebRTC, so that's probably a lost
cause. Android also exposes server APIs.

Change-Id: I290f5fb4ed558f59fadb5d1f84e9d9c405004c23
Reviewed-on: https://boringssl-review.googlesource.com/2440
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:35:15 +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
533ef7304d Remove SSL_clear calls in handshake functions.
If the state is SSL_ST_BEFORE, the SSL* was just initialized. Otherwise, we
don't want to call SSL_clear. The one case I found where we do is if a
handshake message is received and someone sets
SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS. This is apparently intended for external
consumers to set, but I see no code in Google that does.

Which is fortunate because it'll trigger SSL_clear. This retains the BIOs but
drops all connection state, including the record. If the client just initiated
renego, that's the ClientHello that's lost. The connection then hangs: the now
reset SSL* wants a ClientHello (under the null cipher because that too's been
dropped) while the peer wants an encrypted ServerHello.

Change-Id: Iddb3e0bb86d39d98155b060f9273a0856f2d1409
Reviewed-on: https://boringssl-review.googlesource.com/2436
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:32:39 +00:00
David Benjamin
8c88153465 Remove a place where SSL_clear cleans up after client/server confusion.
SSL_clear sets s->state and dtls1_clear sets cookie_len on the server. Setting
cookie_len on the server seems to serve no purpose but to let the callback know
how large the buffer is. This can be done just before calling the callback.

It also avoids a bug where the cookie check can be bypassed, should the server
not specify an app_verify_cookie_cb, by supplying a cookie of all zeros of the
maximum size. (Zero is fine because an empty cookie is rejected.)

The goal here is to avoid needing the SSL_clear calls in the handshake
functions. They are currently needed to fix the cookie_len setting when using
the generic method. (They get set wrong and then flipped back.)

Change-Id: I5095891bc0f7df62d83a9c84312fcf0b84826faa
Reviewed-on: https://boringssl-review.googlesource.com/2435
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:31:57 +00:00
David Benjamin
e58a71b9b3 Trim impossible state combinations.
SSL_ST_BEFORE is never standalone. As of upstream's
413c4f45ed0508d2242638696b7665f499d68265, SSL_ST_BEFORE is only ever set paired
with SSL_ST_ACCEPT or SSL_ST_CONNECT.

Conversely, SSL_ST_OK is never paired with SSL_ST_ACCEPT or SSL_ST_CONNECT. As
far as I can tell, this combination has never been possible.

Change-Id: Ifbc8f147be821026cf59f3d5038f0dbad3b0a1d2
Reviewed-on: https://boringssl-review.googlesource.com/2433
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:31:00 +00:00
David Benjamin
0b145c29a3 Don't assign handshake_func in the handshake functions.
It should already be assigned, as of upstream's
b31b04d951e9b65bde29657e1ae057b76f0f0a73. I believe these assignments are part
of the reason it used to appear to work. Replace them with assertions. So the
assertions are actually valid, check in SSL_connect / SSL_accept that they are
never called if the socket had been placed in the opposite state. (Or we'd be
in another place where it would have appeared to work with the handshake
functions fixing things afterwards.)

Now the only places handshake_func is set are in SSL_set_{connect,accept}_state
and the method switches.

Change-Id: Ib249212bf4aa889b94c35965a62ca06bdbcf52e1
Reviewed-on: https://boringssl-review.googlesource.com/2432
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:30:49 +00:00
David Benjamin
0f1e64bf7f Remove method swap in SSL_set_session.
This is a bit of cleanup that probably should have been done at the same time
as 30ddb434bf.

For now, version negotiation is implemented with a method swap. It also
performs this swap on SSL_set_session, but this was neutered in
30ddb434bf. Rather than hackishly neuter it,
remove it outright.  In addition, remove SSL_set_ssl_method. Now all method
swaps are internal: SSLv23_method switch to a version-specific method and
SSL_clear undoing it.

Note that this does change behavior: if an SSL* is created with one
version-specific method and we SSL_set_session to a session from a /different/
version, we would switch to the /other/ version-specific method. This is
extremely confusing, so it's unlikely anyone was actually expecting it.
Version-specific methods in general don't work well.

Change-Id: I72a5c1f321ca9aeb1b52ebe0317072950ba25092
Reviewed-on: https://boringssl-review.googlesource.com/2390
Reviewed-by: Adam Langley <agl@google.com>
2014-12-02 19:26:30 +00:00
Adam Langley
69a01608f3 Add malloc failure tests.
This commit fixes a number of crashes caused by malloc failures. They
were found using the -malloc-test=0 option to runner.go which runs tests
many times, causing a different allocation call to fail in each case.

(This test only works on Linux and only looks for crashes caused by
allocation failures, not memory leaks or other errors.)

This is not the complete set of crashes! More can be found by collecting
core dumps from running with -malloc-test=0.

Change-Id: Ia61d19f51e373bccb7bc604642c51e043a74bd83
Reviewed-on: https://boringssl-review.googlesource.com/2320
Reviewed-by: Adam Langley <agl@google.com>
2014-11-19 01:24:46 +00:00
David Benjamin
688d8dfe48 Remove psk_identity_hint from SSL_SESSION.
There's not much point in retaining the identity hint in the SSL_SESSION. This
avoids the complexity around setting psk_identity hint on either the SSL or the
SSL_SESSION. Introduce a peer_psk_identity_hint for the client to store the one
received from the server.

This changes the semantics of SSL_get_psk_identity_hint; it now only returns
the value configured for the server. The client learns the hint through the
callback. This is compatible with the one use of this API in conscrypt (it
pulls the hint back out to pass to a callback).

Change-Id: I6d9131636b47f13ac5800b4451436a057021054a
Reviewed-on: https://boringssl-review.googlesource.com/2213
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 23:59:47 +00:00
David Benjamin
60e7992764 Remove DTLSv1_listen.
This was added in http://rt.openssl.org/Ticket/Display.html?id=2033 to support
a mode where a DTLS socket would statelessly perform the ClientHello /
HelloVerifyRequest portion of the handshake, to be handed off to a socket
specific to this peer address.

This is not used by WebRTC or other current consumers. If we need to support
something like this, it would be cleaner to do the listen portion (cookieless
ClientHello + HelloVerifyRequest) externally and then spin up an SSL instance
on receipt of a cookied ClientHello. This would require a slightly more complex
BIO to replay the second ClientHello but would avoid peppering the DTLS
handshake state with a special short-circuiting mode.

Change-Id: I7a413932edfb62f8b9368912a9a0621d4155f1aa
Reviewed-on: https://boringssl-review.googlesource.com/2220
Reviewed-by: Adam Langley <agl@google.com>
2014-11-10 22:39:24 +00:00
Adam Langley
7571292eac Extended master secret support.
This change implements support for the extended master secret. See
https://tools.ietf.org/html/draft-ietf-tls-session-hash-01
https://secure-resumption.com/

Change-Id: Ifc7327763149ab0894b4f1d48cdc35e0f1093b93
Reviewed-on: https://boringssl-review.googlesource.com/1930
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
2014-10-24 21:19:44 +00:00
David Benjamin
6c7aed048c Client-side OCSP stapling support.
Remove the old implementation which was excessively general. This mirrors the
SCT support and adds a single boolean flag to request an OCSP response with no
responder IDs, extensions, or frills. The response, if received, is stored on
the SSL_SESSION so that it is available for (re)validation on session
resumption; Chromium revalidates the saved auth parameters on resume.

Server support is unimplemented for now. This API will also need to be adjusted
in the future if we implement RFC 6961.

Change-Id: I533c029b7f7ea622d814d05f934fdace2da85cb1
Reviewed-on: https://boringssl-review.googlesource.com/1671
Reviewed-by: Adam Langley <agl@google.com>
2014-08-29 00:39:33 +00:00
David Benjamin
854dd654d1 Refactor server-side CertificateVerify handling.
This moves CertificateVerify digest processing to the new
SSL_GET_MESSAGE_DONT_HASH_MESSAGE flag. It also refactors it similarly to
ssl3_send_cert_verify and moves that logic to a common ssl3_cert_verify_hash
function to compute the handshake hash.

This removes a large chunk of duplicate (and divergent!) logic between TLS and
DTLS. It also removes TLS1_FLAGS_KEEP_HANDSHAKE.

Change-Id: Ia63c94f7d76d901bc9c4c33454fbfede411adf63
Reviewed-on: https://boringssl-review.googlesource.com/1633
Reviewed-by: Adam Langley <agl@google.com>
2014-08-27 01:55:27 +00:00
David Benjamin
8da990677b Rename some message functions for consistency.
Make the get/send functions match.

ssl3_client_hello -> ssl3_send_client_hello.
ssl3_send_newsession_ticket -> ssl3_send_new_session_ticket.
ssl3_send_client_verify -> ssl3_send_cert_verify

Change-Id: Iea5579479b8a8f392167b8fb3b7e9fe961d0f007
Reviewed-on: https://boringssl-review.googlesource.com/1613
Reviewed-by: Adam Langley <agl@google.com>
2014-08-26 21:09:40 +00:00
David Benjamin
92909a6206 Remove MS SGC support on the the server.
It doesn't appear to have ever been implemented on the client. The server code
stopped working anyway because it now skips the ssl_get_message call, so we
never cash in on the reuse_message, attempt to reprocess the repeated
ClientHello, and reject it thinking it's a second MS SGC restart.

Change-Id: Id536846e08460143f6fc0a550bdcc1b26b506b04
Reviewed-on: https://boringssl-review.googlesource.com/1580
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:06:01 +00:00
David Benjamin
b52e3dded2 Move common code between two branches.
Some follow-up cleanup to bd30f8e34a.

Change-Id: Id996c6c43f42f4db410e1e612ee1e5e8e0233356
Reviewed-on: https://boringssl-review.googlesource.com/1569
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 19:05:02 +00:00
David Benjamin
bd30f8e34a Remove support on both sides for *_fixed_(ec)dh client auth.
In the fixed_ecdh case, it wasn't even implemented, but there was stub code for
it. It complicates the ClientKeyExchange (the client parameters become implicit
in the certificate) and isn't used.

Change-Id: I3627a37042539c90e05e59cd0cb3cd6c56225561
Reviewed-on: https://boringssl-review.googlesource.com/1563
Reviewed-by: Adam Langley <agl@google.com>
2014-08-20 02:07:00 +00:00
David Benjamin
cc23df53da Remove SSL_OP_CISCO_ANYCONNECT.
I see no internal users and the existence of a THIRD version encoding
complicates all version-checking logic. Also convert another version check to
SSL_IS_DTLS that was missed earlier.

Change-Id: I60d215f57d44880f6e6877889307dc39dbf838f7
Reviewed-on: https://boringssl-review.googlesource.com/1550
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:57:01 +00:00
David Benjamin
fb4ea28bb8 Tidy DTLS cookie callback types.
const-correctness, unsigned long -> size_t.

Change-Id: Ic0c2685a48a0f98396c5753b6077c6c0c3b92326
Reviewed-on: https://boringssl-review.googlesource.com/1540
Reviewed-by: Adam Langley <agl@google.com>
2014-08-18 17:52:21 +00:00
David Benjamin
95fcaa4f4b Fix server-side ClientHello state machine.
- DTLS server code didn't account for the new ClientHello state. This looks
  like it only matters if a DTLS server uses select_certificate_cb and returns
  asynchronously.

- State A transitions immediately to B and is redundant. No code distinguishes
  A and B.

- The ssl_get_message call transitions to the second state (originally C). This
  makes the explicit transition to C a no-op. More of a problem,
  ssl_get_message may return asynchronously and remain in its second state if the
  handshake body had not completed yet. Fix this by splitting state C in two.
  Combined with the above change, this results in only the top few states getting
  reshuffled.

This fixes the server async tests.

Change-Id: I46703bcd205988b118217b6424ba4f88e731be5a
Reviewed-on: https://boringssl-review.googlesource.com/1412
Reviewed-by: Adam Langley <agl@google.com>
2014-08-05 18:07:13 +00:00
Adam Langley
482b4f189f Fix DTLS certificate requesting code.
Use same logic when determining when to expect a client certificate for
both TLS and DTLS.

PR#3452

(Imported from upstream's 666a597ffb9bcf3ba2d49e711fcca28df91eff9d)

Change-Id: Ia267255a32c0b3b9a7da1c53f13ef6f620ff5ec1
2014-07-28 17:05:14 -07: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
b9cc33a4d6 Remove SSL_OP_EPHEMERAL_RSA quirk.
Also fix a place where fixes for the condition for sending ServerKeyExchange in
s3_srvr.c were never propogated to d1_srvr.c. Tidy up that logic to use
ssl_cipher_requires_server_key_exchange and simplify the PSK check.

Change-Id: Ie36d378f733e59a8df405bc869f2346af59bd574
Reviewed-on: https://boringssl-review.googlesource.com/1283
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:50 +00:00
David Benjamin
ff175b4a46 unifdef OPENSSL_NO_PSK.
Get those out of the way.

Change-Id: I4cc8c34cf637379ad734c43623f76ae72f22014e
Reviewed-on: https://boringssl-review.googlesource.com/1282
Reviewed-by: Adam Langley <agl@google.com>
2014-07-24 21:11:05 +00:00
David Benjamin
d26aea6c96 Remove remnants of KRB5 support.
This drops the bits of logic that allowed Certificate messages to be optional
for a KRB5 cipher suite.

Change-Id: I2a71b7c13d7e76f4f5542d4074169f80f3617240
Reviewed-on: https://boringssl-review.googlesource.com/1154
Reviewed-by: Adam Langley <agl@google.com>
2014-07-14 20:49:57 +00:00
David Benjamin
6dbd73db5d Remove OPENSSL_NO_TLSEXT compilation option.
Mostly done with unifdef.

Change-Id: I876f79f9e96d77628d696b09694363d07aee6b74
Reviewed-on: https://boringssl-review.googlesource.com/1096
Reviewed-by: Adam Langley <agl@google.com>
2014-07-07 20:31:37 +00:00
David Benjamin
13ab3e3ce1 Remove heartbeat extension.
Change-Id: I0273a31e49c5367b89b9899553e3ebe13ec50687
Reviewed-on: https://boringssl-review.googlesource.com/1050
Reviewed-by: Adam Langley <agl@google.com>
2014-06-26 20:48:19 +00:00
Adam Langley
0289c73051 Fix TLS-PSK identity hint implementation issues.
PSK identity hint can be stored in SSL_CTX and in SSL/SSL_SESSION,
similar to other TLS parameters, with the value in SSL/SSL_SESSION
taking precedence over the one in SSL_CTX. The value in SSL_CTX is
shared (used as the default) between all SSL instances associated
with that SSL_CTX, whereas the value in SSL/SSL_SESSION is confined
to that particular TLS/SSL connection/session.

The existing implementation of TLS-PSK does not correctly distinguish
between PSK identity hint in SSL_CTX and in SSL/SSL_SESSION. This
change fixes these issues:
1. SSL_use_psk_identity_hint does nothing and returns "success" when
   the SSL object does not have an associated SSL_SESSION.
2. On the client, the hint in SSL_CTX (which is shared between
   multiple SSL instances) is overwritten with the hint received from
   server or reset to NULL if no hint was received.
3. On the client, psk_client_callback is invoked with the hint from
   SSL_CTX rather than from current SSL/SSL_SESSION (i.e., the one
   received from the server). Issue #2 above masks this issue.
4. On the server, the hint in SSL/SSL_SESSION is ignored and the hint
   from SSL_CTX is sent to the client.
5. On the server, the hint in SSL/SSL_SESSION is reset to the one in
   SSL_CTX after the ClientKeyExchange message step.

This change fixes the issues by:
* Adding storage for the hint in the SSL object. The idea being that
  the hint in the associated SSL_SESSION takes precedence.
* Reading the hint during the handshake only from the associated
  SSL_SESSION object.
* Initializing the hint in SSL object with the one from the SSL_CTX
  object.
* Initializing the hint in SSL_SESSION object with the one from the
  SSL object.
* Making SSL_use_psk_identity_hint and SSL_get_psk_identity_hint
  set/get the hint to/from SSL_SESSION associated with the provided
  SSL object, or, if no SSL_SESSION is available, set/get the hint
  to/from the provided SSL object.
* Removing code which resets the hint during handshake.
2014-06-20 13:17:36 -07:00
Adam Langley
95c29f3cd1 Inital import.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).

(This change contains substantial changes from the original and
effectively starts a new history.)
2014-06-20 13:17:32 -07:00