Android never did this - they patched out the point in the code that set
the SSL3_FLAGS_DELAY_CLIENT_FINISHED flag when doing False Start.
Also, from the unittests it appears that NSS doesn't do this either.
Thus this change brings BoringSSL into line with existing behaviour.
SSL3_FLAGS_DELAY_CLIENT_FINISHED wasn't introduced with False Start,
it's an option in vanilla OpenSSL. But I can't find anything that uses
it and, since it's going to be untested, I've removed it completely in
this change.
Change-Id: I910537bfa35e74ab88778b83612cf5607d485969
Reviewed-on: https://boringssl-review.googlesource.com/1221
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
Got one of the conditions flipped.
Change-Id: I327a9c13e42865459e8d69a431b0d3a2bc6b54a5
Reviewed-on: https://boringssl-review.googlesource.com/1210
Reviewed-by: Adam Langley <agl@google.com>
Also fix some DTLS cookie bugs. rcvd_cookie is never referenced after being
saved (and the length isn't saved, so it couldn't be used anyway), and the
cookie verification failed to check the length.
For convenience, add a CBS_mem_equal helper function. Saves a bit of
repetition.
Change-Id: I187137733b069f0ac8d8b1bf151eeb80d388b971
Reviewed-on: https://boringssl-review.googlesource.com/1174
Reviewed-by: Adam Langley <agl@google.com>
This avoids duplicating the code to build the final premaster in PSK and
ECDHE_PSK. It also ports it to CBB for an initial trial of the API. Computing
the premaster secret now proceeds in four steps:
1. If a PSK key exchange (alg_a), look up the pre-shared key.
2. Compute the premaster secret based on alg_k. If PSK, it's all zeros.
3. If a PSK key exchange (alg_a), wrap the premaster in a struct with the
pre-shared key.
4. Use the possibly modified premaster to compute the master secret.
Change-Id: Ib511dd2724cbed42c82b82a676f641114cec5470
Reviewed-on: https://boringssl-review.googlesource.com/1173
Reviewed-by: Adam Langley <agl@google.com>
More consistent with ssl3_send_server_key_exchange and the message name.
Change-Id: If0f435a89bdf117297d349099708fff0bd5a6e98
Reviewed-on: https://boringssl-review.googlesource.com/1170
Reviewed-by: Adam Langley <agl@google.com>
This avoids having to do the CBS_skip dance and is better about returning the
right alert.
Change-Id: Id84eba307d7c67269ccbc07a38d9044b6f4f7c6c
Reviewed-on: https://boringssl-review.googlesource.com/1169
Reviewed-by: Adam Langley <agl@google.com>
Also tidy up some variable names and update RSA_verify call for it no longer
returning -1. Add CBS helper functions for dealing with C strings.
Change-Id: Ibc398d27714744f5d99d4f94ae38210cbc89471a
Reviewed-on: https://boringssl-review.googlesource.com/1164
Reviewed-by: Adam Langley <agl@google.com>
It's current a void* and gets explicitly cast everywhere. Make it a uint8_t and
only add the casts when converting it come init_buf, which internally stores a
char*.
Change-Id: I28bed129e46ed37ee1ce378d5c3bd0738fc1177f
Reviewed-on: https://boringssl-review.googlesource.com/1163
Reviewed-by: Adam Langley <agl@google.com>
Missing ServerKeyExchange is handled, but only because it hits an
ERR_R_INTERNAL_ERROR in ssl3_send_client_key_exchange in trying to find the
server ECDH parameters. Be strict about requiring it for ECDHE.
Change-Id: Ifce5b73c8bd14746b8a2185f479d550e9e3f84df
Reviewed-on: https://boringssl-review.googlesource.com/1157
Reviewed-by: Adam Langley <agl@google.com>
Introduce a ssl_cipher_has_server_public_key to save the repeated
NULL/PSK/RSA_PSK[*] check. Don't allow skipping to ServerKeyExchange when
expecting Certificate; the messages expected are determined by the cipher
suite. The ssl3_get_server_public_key call is already guarded.
As the previous test demonstrates, this is safe because of the
ssl3_check_cert_and_algorithm call, but avoid the looseness in the parsing
there.
[*] NB: we don't implement RSA_PSK, and OpenSSL has never implemented it.
Change-Id: I0571e6bcbeb8eb883f77878bdc98d1aa3a287cf3
Reviewed-on: https://boringssl-review.googlesource.com/1156
Reviewed-by: Adam Langley <agl@google.com>
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>
This can't happen because we don't implement RSA_PSK, but we probably should
check here.
Probably |sess_cert| shouldn't be attached to SSL_SESSION anyway; it's only
relevant when initializing the session and if it's accessed afterwards, it'll
be shared and cause problems.
Change-Id: Id868e523195f33c22e057f9b89dc02fe68e9b554
Reviewed-on: https://boringssl-review.googlesource.com/1153
Reviewed-by: Adam Langley <agl@google.com>
Fixes bug introduced in c26c802a89. Only one of
the two halves got flipped.
Change-Id: I0b3905ab22b0f83f093e1720af85594b1a970a7f
Reviewed-on: https://boringssl-review.googlesource.com/1152
Reviewed-by: Adam Langley <agl@google.com>
Instead of, in the pre-TLS-1.2 case, reaching into the EVP and manually
signing, compute the digest separately from signing. Then use EVP_PKEY_sign.
This will make it easier to implement https://crbug.com/347404 by having only
one signing codepath as well as make that logic simpler.
Also add a bounds check while we're here, although the buffer is too large to
actually matter.
runner.go client auth tests should cover code changes.
Change-Id: I7d87181bbcc5a761660412452e508d24c4725327
Reviewed-on: https://boringssl-review.googlesource.com/1122
Reviewed-by: Adam Langley <agl@google.com>
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>
Along the way, clean up the certificate types code to not have the
hard-coded fixed-size array.
Change-Id: If3e5978f7c5099478a3dfa37a0a7059072f5454a
Reviewed-on: https://boringssl-review.googlesource.com/1103
Reviewed-by: Adam Langley <agl@google.com>
SSL_OP_NETSCAPE_CA_DN_BUG is not included in SSL_OP_ALL.
Change-Id: I1635ad2721ed2742b1dff189d68bfc67a1c840a6
Reviewed-on: https://boringssl-review.googlesource.com/1102
Reviewed-by: Adam Langley <agl@google.com>
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>
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>
This gives us systematic bounds-checking on all the parses. Also adds a
convenience function, CBS_memdup, for saving the current contents of a CBS.
Change-Id: I17dad74575f03121aee3f771037b8806ff99d0c3
Reviewed-on: https://boringssl-review.googlesource.com/1031
Reviewed-by: Adam Langley <agl@google.com>
Allow CCS after finished has been sent by client: at this point
keys have been correctly set up so it is OK to accept CCS from
server. Without this renegotiation can sometimes fail.
PR#3400
(Imported from upstream's 90d94ce39ecc2fad7fb2b8eb6bde0c669a65ee81)
If application uses tls_session_secret_cb for session resumption set the
CCS_OK flag.
(Imported from upstream's a21f350a76b34b66dcaf9c1676baec945f32e980)
Only accept change cipher spec when it is expected instead of at any
time. This prevents premature setting of session keys before the master
secret is determined which an attacker could use as a MITM attack.
Thanks to KIKUCHI Masashi (Lepidum Co. Ltd.) for reporting this issue
and providing the initial fix this patch is based on.
(Imported from upstream's 77719aefb8f549ccc7f04222174889615d62057b)
Fix a bug in handling of 128 byte long PSK identity in
psk_client_callback.
OpenSSL supports PSK identities of up to (and including) 128 bytes in
length. PSK identity is obtained via the psk_client_callback,
implementors of which are expected to provide a NULL-terminated
identity. However, the callback is invoked with only 128 bytes of
storage thus making it impossible to return a 128 byte long identity and
the required additional NULL byte.
This CL fixes the issue by passing in a 129 byte long buffer into the
psk_client_callback. As a safety precaution, this CL also zeroes out the
buffer before passing it into the callback, uses strnlen for obtaining
the length of the identity returned by the callback, and aborts the
handshake if the identity (without the NULL terminator) is longer than
128 bytes.
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.
Fix limit checks in ssl_add_clienthello_tlsext and
ssl_add_serverhello_tlsext.
Some of the limit checks reference p rather than ret. p is the original
buffer position, not the current one. Fix those and rename p to orig so
it's clearer.
(Called "cut through" for historical reasons in this patch.)
Enables SSL3+ clients to send application data immediately following the
Finished message even when negotiating full-handshakes. With this
patch, clients can negotiate SSL connections in 1-RTT even when
performing full-handshakes.
Initial fork from f2d678e6e89b6508147086610e985d4e8416e867 (1.0.2 beta).
(This change contains substantial changes from the original and
effectively starts a new history.)
