The PSK identity hint should be stored in the SSL_SESSION structure
and not in the parent context (which will overwrite values used
by other SSL structures with the same SSL_CTX).
Use BUF_strndup when copying identity as it may not be null terminated.
Reviewed-by: Tim Hudson <tjh@openssl.org>
It is valid for an extension block to be present in a ClientHello, but to
be of zero length.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
Fix error handling in ssl_session_dup, as well as incorrect setting up of
the session ticket. Follow on from CVE-2015-1791.
Thanks to LibreSSL project for reporting these issues.
Conflicts:
ssl/ssl_sess.c
Reviewed-by: Tim Hudson <tjh@openssl.org>
It should not be possible for DTLS message fragments to span multiple
packets. However previously if the message header fitted exactly into one
packet, and the fragment body was in the next packet then this would work.
Obviously this would fail if packets get re-ordered mid-flight.
Reviewed-by: Tim Hudson <tjh@openssl.org>
This adds additional checks to the processing of extensions in a ClientHello
to ensure that either no extensions are present, or if they are then they
take up the exact amount of space expected.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Stephen Henson <steve@openssl.org>
Conflicts:
ssl/t1_lib.c
Remove a comment that suggested further clean up was required.
DH_free() performs the necessary cleanup.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit f3d889523e)
Ensure OPENSSL_cleanse() is called on the premaster secret value calculated for GOST.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit b7ee4815f2)
Conflicts:
ssl/s3_srvr.c
Ensure the Kerberos pre-master secret has OPENSSL_cleanse called on it.
With thanks to the Open Crypto Audit Project for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
The session object on the client side is initially created during
construction of the ClientHello. If the client is DTLS1.2 capable then it
will store 1.2 as the version for the session. However if the server is only
DTLS1.0 capable then when the ServerHello comes back the client switches to
using DTLS1.0 from then on. However the session version does not get
updated. Therefore when the client attempts to resume that session the
server throws an alert because of an incorrect protocol version.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(cherry picked from commit 7322abf5ce)
Conflicts:
ssl/s3_clnt.c
If a NewSessionTicket is received by a multi-threaded client when
attempting to reuse a previous ticket then a race condition can occur
potentially leading to a double free of the ticket data.
CVE-2015-1791
This also fixes RT#3808 where a session ID is changed for a session already
in the client session cache. Since the session ID is the key to the cache
this breaks the cache access.
Parts of this patch were inspired by this Akamai change:
c0bf69a791
Reviewed-by: Rich Salz <rsalz@openssl.org>
dtls1_get_message has an |mt| variable which is the type of the message that
is being requested. If it is negative then any message type is allowed.
However the value of |mt| is not checked in one of the main code paths, so a
peer can send a message of a completely different type and it will be
processed as if it was the message type that we were expecting. This has
very little practical consequences because the current behaviour will still
fail when the format of the message isn't as expected.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 8c2b1d872b)
Also add more ciphersuite test coverage, and a negative test for
512-bit DHE.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 1ee85aab75)
The size of the SRP extension can never be negative (the variable
|size| is unsigned). Therefore don't check if it is less than zero.
RT#3862
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 9c89d29083)
If the record received is for a version that we don't support, previously we
were sending an alert back. However if the incoming record already looks
like an alert then probably we shouldn't do that. So suppress an outgoing
alert if it looks like we've got one incoming.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
We had updates of certain header files in both Makefile.org and the
Makefile in the directory the header file lived in. This is error
prone and also sometimes generates slightly different results (usually
just a comment that differs) depending on which way the update was
done.
This removes the file update targets from the top level Makefile, adds
an update: target in all Makefiles and has it depend on the depend: or
local_depend: targets, whichever is appropriate, so we don't get a
double run through the whole file tree.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 0f539dc1a2)
Conflicts:
Makefile.org
apps/Makefile
test/Makefile
The certificate masks are used to select which ciphersuite we are going to
use. The variables |emask_k| and |emask_a| relate to export grade key
exchange and authentication respecitively. The variables |mask_k| and
|mask_a| are the equivalent versions for non-export grade. This fixes an
instance where the two usages of export/non-export were mixed up. In
practice it makes little difference since it still works!
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit fdfe8b06ae)
Remove support for the two export grade static DH ciphersuites. These two
ciphersuites were newly added (along with a number of other static DH
ciphersuites) to 1.0.2. However the two export ones have *never* worked
since they were introduced. It seems strange in any case to be adding new
export ciphersuites, and given "logjam" it also does not seem correct to
fix them.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 13f8eb4730)
Conflicts:
CHANGES
If a client receives a bad hello request in DTLS then the alert is not
sent correctly.
RT#2801
Signed-off-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
(cherry picked from commit 4dc1aa0436)
40 bit ciphers are limited to 512 bit RSA, 56 bit ciphers to 1024 bit.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit ac38115c1a)
Also reorder preferences to prefer prime curves to binary curves, and P-256 to everything else.
The result:
$ openssl s_server -named_curves "auto"
This command will negotiate an ECDHE ciphersuite with P-256:
$ openssl s_client
This command will negotiate P-384:
$ openssl s_client -curves "P-384"
This command will not negotiate ECDHE because P-224 is disabled with "auto":
$ openssl s_client -curves "P-224"
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Since the client has no way of communicating her supported parameter
range to the server, connections to servers that choose weak DH will
simply fail.
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Currently we set change_cipher_spec_ok to 1 before calling
ssl3_get_cert_verify(). This is because this message is optional and if it
is not sent then the next thing we would expect to get is the CCS. However,
although it is optional, we do actually know whether we should be receiving
one in advance. If we have received a client cert then we should expect
a CertificateVerify message. By the time we get to this point we will
already have bombed out if we didn't get a Certificate when we should have
done, so it is safe just to check whether |peer| is NULL or not. If it is
we won't get a CertificateVerify, otherwise we will. Therefore we should
change the logic so that we only attempt to get the CertificateVerify if
we are expecting one, and not allow a CCS in this scenario.
Whilst this is good practice for TLS it is even more important for DTLS.
In DTLS messages can be lost. Therefore we may be in a situation where a
CertificateVerify message does not arrive even though one was sent. In that
case the next message the server will receive will be the CCS. This could
also happen if messages get re-ordered in-flight. In DTLS if
|change_cipher_spec_ok| is not set and a CCS is received it is ignored.
However if |change_cipher_spec_ok| *is* set then a CCS arrival will
immediately move the server into the next epoch. Any messages arriving for
the previous epoch will be ignored. This means that, in this scenario, the
handshake can never complete. The client will attempt to retransmit
missing messages, but the server will ignore them because they are the wrong
epoch. The server meanwhile will still be waiting for the CertificateVerify
which is never going to arrive.
RT#2958
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit a0bd649336)
If sk_SSL_CIPHER_new_null() returns NULL then ssl_bytes_to_cipher_list()
should also return NULL.
Based on an original patch by mrpre <mrpre@163.com>.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 14def5f537)
Ensure all fatal errors transition into the new error state for DTLS.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cefc93910c)
Conflicts:
ssl/d1_srvr.c
Ensure all fatal errors transition into the new error state on the client
side.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cc273a9361)
Conflicts:
ssl/s3_clnt.c
Ensure all fatal errors transition into the new error state on the server
side.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit cf9b0b6fb2)
Conflicts:
ssl/s3_srvr.c
Reusing an SSL object when it has encountered a fatal error can
have bad consequences. This is a bug in application code not libssl
but libssl should be more forgiving and not crash.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit a89db885e0)
Conflicts:
ssl/s3_srvr.c
ssl/ssl_stat.c
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit cb0f400b0c)
The return value is checked for 0. This is currently safe but we should
really check for <= 0 since -1 is frequently used for error conditions.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c427570e50)
Conflicts:
ssl/ssl_locl.h
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
this is true for all SSLv3 ciphersuites, this fact is far from obvious by
looking at the code. If this were not the case then an integer overflow
would occur, leading to a subsequent buffer overflow. Therefore I have
added an explicit sanity check to ensure header_length is always valid.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit 29b0a15a48)
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
least 13 bytes long. Add sanity checks to ensure that the length is at
least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(cherry picked from commit c826988109)
Conflicts:
ssl/record/ssl3_record.c
Fortify flagged up a problem in n_do_ssl_write() in SSLv2. Analysing the
code I do not believe there is a real problem here. However the logic flows
are complicated enough that a sanity check of |len| is probably worthwhile.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
EAP-FAST session resumption relies on handshake message lookahead
to determine server intentions. Commits
980bc1ec61
and
7b3ba508af
removed the lookahead so broke session resumption.
This change partially reverts the commits and brings the lookahead back
in reduced capacity for TLS + EAP-FAST only. Since EAP-FAST does not
support regular session tickets, the lookahead now only checks for a
Finished message.
Regular handshakes are unaffected by this change.
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 6e3d015363)
A 0-length ciphers list is never permitted. The old code only used to
reject an empty ciphers list for connections with a session ID. It
would later error out on a NULL structure, so this change just moves
the alert closer to the problem source.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 3ae91cfb32)
If OpenSSL is configured with no-tlsext then ssl_get_prev_session can read
past the end of the ClientHello message if the session_id length in the
ClientHello is invalid. This should not cause any security issues since the
underlying buffer is 16k in size. It should never be possible to overrun by
that many bytes.
This is probably made redundant by the previous commit - but you can never be
too careful.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit 5e0a80c1c9)
The ClientHello processing is insufficiently rigorous in its checks to make
sure that we don't read past the end of the message. This does not have
security implications due to the size of the underlying buffer - but still
needs to be fixed.
With thanks to Qinghao Tang for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(cherry picked from commit c9642eb1ff79a30e2c7632ef8267cc34cc2b0d79)
There is no indication that the timing differences are exploitable in
OpenSSL, and indeed there is some indication (Usenix '14) that they
are too small to be exploitable. Nevertheless, be careful and apply
the same countermeasures as in s3_srvr.c
Thanks to Nimrod Aviram, Sebastian Schinzel and Yuval Shavitt for
reporting this issue.
Reviewed-by: Richard Levitte <levitte@openssl.org>
In ssl3_send_new_session_ticket the message to be sent is constructed. We
skip adding the length of the session ticket initially, then call
ssl_set_handshake_header, and finally go back and add in the length of the
ticket. Unfortunately, in DTLS, ssl_set_handshake_header also has the side
effect of buffering the message for subsequent retransmission if required.
By adding the ticket length after the call to ssl_set_handshake_header the
message that is buffered is incomplete, causing an invalid message to be
sent on retransmission.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 4f9fab6bd0)
Conflicts:
ssl/s3_srvr.c
In DTLS, immediately prior to epoch change, the write_sequence is supposed
to be stored in s->d1->last_write_sequence. The write_sequence is then reset
back to 00000000. In the event of retransmits of records from the previous
epoch, the last_write_sequence is restored. This commit fixes a bug in
DTLS1.2 where the write_sequence was being reset before last_write_sequence
was saved, and therefore retransmits are sent with incorrect sequence
numbers.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit d5d0a1cb13)
The certificate already contains the DH parameters in that case.
ssl3_send_server_key_exchange() would fail in that case anyway.
Reviewed-by: Matt Caswell <matt@openssl.org>
(cherry picked from commit 93f1c13619)
Don't check that the curve appears in the list of acceptable curves for the
peer, if they didn't send us such a list (RFC 4492 does not require that the
extension be sent).
Reviewed-by: Emilia Käsper <emilia@openssl.org>
(cherry picked from commit b79d24101e)
