Following on from CVE-2017-3733, this removes the OPENSSL_assert() check
that failed and replaces it with a soft assert, and an explicit check of
value with an error return if it fails.
Reviewed-by: Richard Levitte <levitte@openssl.org>
In 1.1.0 changing the ciphersuite during a renegotiation can result in
a crash leading to a DoS attack. In master this does not occur with TLS
(instead you get an internal error, which is still wrong but not a security
issue) - but the problem still exists in the DTLS code.
The problem is caused by changing the flag indicating whether to use ETM
or not immediately on negotiation of ETM, rather than at CCS. Therefore,
during a renegotiation, if the ETM state is changing (usually due to a
change of ciphersuite), then an error/crash will occur.
Due to the fact that there are separate CCS messages for read and write
we actually now need two flags to determine whether to use ETM or not.
CVE-2017-3733
Reviewed-by: Richard Levitte <levitte@openssl.org>
If s->s3->tmp.new_cipher is NULL then a crash can occur. This can happen
if an alert gets sent after version negotiation (i.e. we have selected
TLSv1.3 and ended up in tls13_enc), but before a ciphersuite has been
selected.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2575)
The record layer was making decisions that should really be left to the
state machine around unexpected handshake messages that are received after
the initial handshake (i.e. renegotiation related messages). This commit
removes that code from the record layer and updates the state machine
accordingly. This simplifies the state machine and paves the way for
handling other messages post-handshake such as the NewSessionTicket in
TLSv1.3.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2259)
Remove duplicate defines from EVP source files.
Most of them were in evp.h, which is always included.
Add new ones evp_int.h
EVP_CIPH_FLAG_TLS1_1_MULTIBLOCK is now always defined in evp.h, so
remove conditionals on it
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2201)
This comes from a comment in GH issue #1027. Andy wrote the code,
Rich made the PR.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2253)
TLSv1.3 freezes the record layer version and ensures that it is always set
to TLSv1.0. Some implementations check this.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
We were not incrementing the sequence number every time we sent/received
a record.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2157)
Otherwise the client will try to process it again. The second time around
it will try and move the record data into handshake fragment storage and
realise that there is no data left. At that point it marks it as read
anyway. However, it is a bug that we go around the loop a second time, so
we prevent that.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2200)
SSL_clear() was resetting numwpipes to 0, but not freeing any allocated
memory for existing write buffers.
Fixes#2026
Reviewed-by: Rich Salz <rsalz@openssl.org>
Improves the readability of the code, and reduces the liklihood of errors.
Also made a few minor style changes.
Reviewed-by: Rich Salz <rsalz@openssl.org>
At the moment the msg callback only received the record header with the
outer record type in it. We never pass the inner record type - we probably
need to at some point.
Reviewed-by: Rich Salz <rsalz@openssl.org>
This updates the record layer to use the TLSv1.3 style nonce construciton.
It also updates TLSProxy and ossltest to be able to recognise the new
layout.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Travis is reporting one file at a time shadowed variable warnings where
"read" has been used. This attempts to go through all of libssl and replace
"read" with "readbytes" to fix all the problems in one go.
Reviewed-by: Rich Salz <rsalz@openssl.org>
TLS1.0 and TLS1.1 say you SHOULD ignore unrecognised record types, but
TLS 1.2 says you MUST send an unexpected message alert. We swap to the
TLS 1.2 behaviour for all protocol versions to prevent issues where no
progress is being made and the peer continually sends unrecognised record
types, using up resources processing them.
Issue reported by 郭志攀
Reviewed-by: Tim Hudson <tjh@openssl.org>
The function ssl3_read_n() takes a parameter |clearold| which, if set,
causes any old data in the read buffer to be forgotten, and any unread data
to be moved to the start of the buffer. This is supposed to happen when we
first read the record header.
However, the data move was only taking place if there was not already
sufficient data in the buffer to satisfy the request. If read_ahead is set
then the record header could be in the buffer already from when we read the
preceding record. So with read_ahead we can get into a situation where even
though |clearold| is set, the data does not get moved to the start of the
read buffer when we read the record header. This means there is insufficient
room in the read buffer to consume the rest of the record body, resulting in
an internal error.
This commit moves the |clearold| processing to earlier in ssl3_read_n()
to ensure that it always takes place.
Reviewed-by: Richard Levitte <levitte@openssl.org>
A zero return from BIO_read()/BIO_write() could mean that an IO operation
is retryable. A zero return from SSL_read()/SSL_write() means that the
connection has been closed down (either cleanly or not). Therefore we
should not propagate a zero return value from BIO_read()/BIO_write() back
up the stack to SSL_read()/SSL_write(). This could result in a retryable
failure being treated as fatal.
Reviewed-by: Richard Levitte <levitte@openssl.org>
