For CBC ciphersuites using Mac-then-encrypt we have to be careful about
removing the MAC from the record in constant time. Currently that happens
immediately before MAC verification. Instead we move this responsibility
to the various protocol "enc" functions so that MAC removal is handled at
the same time as padding removal.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/12288)
Partially fixes#11209.
Before OpenSSL 3.0 in case when peer does not send close_notify,
the behaviour was to set SSL_ERROR_SYSCALL error with errno 0.
This behaviour has changed. The SSL_OP_IGNORE_UNEXPECTED_EOF restores
the old behaviour for compatibility's sake.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11735)
The alignment calculation in ssl3_setup_write incorrectly results in an
alignment allowance of
(-SSL3_RT_HEADER_LENGTH) & (SSL3_ALIGN_PAYLOAD - 1) bytes. This equals 3
in almost all cases. The maximum alignment actually used in do_ssl3_write
is (SSL3_ALIGN_PAYLOAD - 1). This equals 7 bytes in almost all cases. So
there is a potential to overrun the buffer by up to 4 bytes.
Fortunately, the encryption overhead allowed for is 80 bytes which
consists of 16 bytes for the cipher block size and 64 bytes for the MAC
output. However the biggest MAC that we ever produce is HMAC-384 which is
48 bytes - so we have a headroom of 16 bytes (i.e. more than the 4 bytes
of potential overrun).
Thanks to Nagesh Hegde for reporting this.
Fixes#11766
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/11768)
This API requests that the TLS stack generate a (TLS 1.3)
NewSessionTicket message the next time it is safe to do so (i.e., we do
not have other data pending write, which could be mid-record). For
efficiency, defer actually generating/writing the ticket until there
is other data to write, to avoid producing server-to-client traffic when
not needed.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11416)
HMACs used via the legacy EVP_DigestSign interface are strange in
that they use legacy codepath's which eventually (under the covers)
transform the operation into a new style EVP_MAC. This can mean the
digest in use can be a legacy one, so we need to be careful with any
digest we extract from the ctx.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11511)
With KTLS, writes to an SSL connection store the application buffer
pointer directly in the 'buf' member instead of allocating a separate
buffer to hold the encrypted data. As a result,
ssl3_release_write_buffer() has to avoid freeing these 'buf' pointers.
Previously, ssl3_release_write_buffer() checked for KTLS being enabled
on the write BIO to determine if a buffer should be freed. However, a
buffer can outlive a BIO. For example, 'openssl s_time' creates new
write BIOs when reusing sessions. Since the new BIO did not have KTLS
enabled at the start of a connection, ssl3_release_write_buffer()
would incorrectly try to free the 'buf' pointer from the previous KTLS
connection. To fix, track the state of 'buf' explicitly in
SSL3_BUFFER to determine if the 'buf' should be freed or simply
cleared.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10489)
Allow for encryption overhead in early DTLS size check
and send overflow if validated record is too long
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11096)
If we hit an EOF while reading in libssl then we will report an error
back to the application (SSL_ERROR_SYSCALL) but errno will be 0. We add
an error to the stack (which means we instead return SSL_ERROR_SSL) and
therefore give a hint as to what went wrong.
Contains a partial fix for #10880
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/10907)
Now that libssl knows about libctx we should use it wherever we generate
a random number.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10927)
The kernel will generate the MAC when transmitting the frame. Doing
so here causes the MAC to be included as part of the plain text that
the kernel MACs and encrypts. Note that this path is not taken when
using stitched cipher suites.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10045)
The decryption failed alert was deprecated a long time ago. It can
provide an attacker too much information to be able to distinguish between
MAC failures and decryption failures and can lead to oracle attacks.
Instead we should always use the bad_record_mac alert for these issues.
This fixes one instance that still exists. It does not represent a
security issue in this case because it is only ever sent if the record is
publicly invalid, i.e. we have detected it is invalid without using any
secret material.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10093)
Apart from public and internal header files, there is a third type called
local header files, which are located next to source files in the source
directory. Currently, they have different suffixes like
'*_lcl.h', '*_local.h', or '*_int.h'
This commit changes the different suffixes to '*_local.h' uniformly.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9333)
At some point in the past do_ssl3_write() used to return the number of
bytes written, or a value <= 0 on error. It now just returns a success/
error code and writes the number of bytes written to |tmpwrit|.
The SSL_MODE_RELEASE_BUFFERS code was still looking at the return code
for the number of bytes written rather than |tmpwrit|. This has the effect
that the buffers are not released when they are supposed to be.
Fixes#9490
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9505)
If we receive a KeyUpdate message (update requested) from the peer while
we are in the middle of a write, we should defer sending the responding
KeyUpdate message until after the current write is complete. We do this
by waiting to send the KeyUpdate until the next time we write and there is
no pending write data.
This does imply a subtle change in behaviour. Firstly the responding
KeyUpdate message won't be sent straight away as it is now. Secondly if
the peer sends multiple KeyUpdates without us doing any writing then we
will only send one response, as opposed to previously where we sent a
response for each KeyUpdate received.
Fixes#8677
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/8773)
With the removal of SSLv2, the s3 structure is always allocated, so
there is little point in having it be an allocated pointer. Collapse
the ssl3_state_st structure into ssl_st and fixup any references.
This should be faster than going through an indirection and due to
fewer allocations, but I'm not seeing any significant performance
improvement; it seems to be within the margin of error in timing.
Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7888)
If we were using a different type of BIO than a socket BIO then
BIO_get_ktls_send() and BIO_get_ktls_recv() could return the wrong
result.
The above occurred even if KTLS was disabled at compile time - so we should
additionally ensure that those macros do nothing if KTLS is disabled.
Finally we make the logic in ssl3_get_record() a little more robust when
KTLS has been disabled.
[extended tests]
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8793)
This patch adds support for the Linux TLS Rx socket option.
It completes the previous patch for TLS Tx offload.
If the socket option is successful, then the receive data-path of the TCP
socket is implemented by the kernel.
We choose to set this option at the earliest - just after CCS is complete.
Change-Id: I59741e04d89dddca7fb138e88fffcc1259b30132
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7848)
This commit erroneously kept the DTLS timer running after the end of the
handshake. This is not correct behaviour and shold be reverted.
This reverts commit f7506416b1.
Fixes#7998
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8047)
This patch adds support for the Linux TLS Tx socket option.
If the socket option is successful, then the data-path of the TCP socket
is implemented by the kernel.
We choose to set this option at the earliest - just after CCS is complete.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5253)
Since 1fb9fdc30 we may attempt to buffer a record from the next epoch
that has already been buffered. Prior to that this never occurred.
We simply ignore a failure to buffer a duplicated record.
Fixes#6902
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7414)
Previously when a ClientHello arrives with a valid cookie using
DTLSv1_listen() we only "peeked" at the message and left it on the
underlying fd. This works fine for single threaded applications but for
multi-threaded apps this does not work since the fd is typically reused for
the server thread, while a new fd is created and connected for the client.
By "peeking" we leave the message on the server fd, and consequently we
think we've received another valid ClientHello and so we create yet another
fd for the client, and so on until we run out of fds.
In this new approach we remove the ClientHello and buffer it in the SSL
object.
Fixes#6934
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7375)
Rather than using init_buf we use the record layer read and write buffers
in DTLSv1_listen(). These seem more appropriate anyway and will help with
the next commit.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/7375)
If we've sent a close_notify then we are restricted about what we can do
in response to handshake messages that we receive. However we can sensibly
process NewSessionTicket messages. We can also process a KeyUpdate message
as long as we also ignore any request for us to update our sending keys.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7114)
At certain points in the handshake we could receive either a plaintext or
an encrypted alert from the client. We should tolerate both where
appropriate.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6887)
If we sent early_data and then received back an HRR, the enc_write_ctx
was stale resulting in errors if an alert needed to be sent.
Thanks to Quarkslab for reporting this.
In any case it makes little sense to encrypt alerts using the
client_early_traffic_secret, so we add special handling for alerts sent
after early_data. All such alerts are sent in plaintext.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6887)
Previoulsy we just had max_early_data which controlled both the value of
max early_data that we advertise in tickets *and* the amount of early_data
that we are willing to receive from clients. This doesn't work too well in
the case where we want to reduce a previously advertised max_early_data
value. In that case clients with old, stale tickets may attempt to send us
more early data than we are willing to receive. Instead of rejecting the
early data we abort the connection if that happens.
To avoid this we introduce a new "recv_max_early_data" value. The old
max_early_data becomes the value that is advertised in tickets while
recv_max_early_data is the maximum we will tolerate from clients.
Fixes#6647
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/6655)
Currently if you encounter application data while waiting for a
close_notify from the peer, and you have called SSL_shutdown() then
you will get a -1 return (fatal error) and SSL_ERROR_SYSCALL from
SSL_get_error(). This isn't accurate (it should be SSL_ERROR_SSL) and
isn't persistent (you can call SSL_shutdown() again and it might then work).
We change this into a proper fatal error that is persistent.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6340)
In the case where we are shutdown for writing and awaiting a close_notify
back from a subsequent SSL_shutdown() call we skip over handshake data
that is received. This should not be treated as an error - instead it
should be signalled with SSL_ERROR_WANT_READ.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6340)
If we've sent a close_notify and we're waiting for one back we drop
incoming records until we see the close_notify we're looking for. If
SSL_MODE_AUTO_RETRY is on, then we should immediately try and read the
next record.
Fixes#6262
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6340)
Commit 4aa5a5669 accidentally missed off the catch all case of ignoring all
warning alerts that are otherwise unhandled. This breaks the SSLv3 tests
which send a "no certificate" warning alert.
Fixes#6496
[extended tests]
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/6509)
In TLSv1.3 we should ignore the severity level of an alert according to
the spec.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6370)
The TLS code marks records as read when its finished using a record. The DTLS code did
not do that. However SSL_has_pending() relies on it. So we should make DTLS consistent.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6159)
During a full handshake the server is the last one to "speak". The timer
should continue to run until we know that the client has received our last
flight (e.g. because we receive some application data).
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6170)
DTLS was not correctly returning the number of pending bytes left in
a call to SSL_pending(). This makes the detection of truncated packets
almost impossible.
Fixes#5478
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6020)
Since the public and private DRBG are per thread we don't need one
per ssl object anymore. It could also try to get entropy from a DRBG
that's really from an other thread because the SSL object moved to an
other thread.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5547)
According to TLSv1.3 draft-24 the record version for ClientHello2 should
be TLS1.2, and not TLS1.0 as it is now.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5377)
If a server receives an unexpected ClientHello then we may or may not
accept it. Make sure all such decisions are made in the state machine
and not in the record layer. This also removes a disparity between the
TLS and the DTLS code. The TLS code was making this decision in the
record layer, while the DTLS code was making it later.
Finally it also solves a problem where a warning alert was being sent
during tls_setup_handshake() and the function was returning a failure
return code. This is problematic because it can be called from a
transition function - which we only allow fatal errors to occur in.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5190)
In the case of a protocol version alert being sent by a peer the record
version number may not be what we are expecting. In DTLS records with an
unexpected version number are silently discarded. This probably isn't
appropriate for alerts, so we tolerate a mismatch in the minor version
number.
This resolves an issue reported on openssl-users where an OpenSSL server
chose DTLS1.0 but the client was DTLS1.2 only and sent a protocol_version
alert with a 1.2 record number. This was silently ignored by the server.
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5018)
The CCS may be sent at different times based on whether or not we
sent an HRR earlier. In order to make that decision this commit
also updates things to make sure we remember whether an HRR was
used or not.
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4701)
It's argued that /WX allows to keep better focus on new code, which
motivates its comeback...
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4721)
If SSL_read() is called with a zero length buffer, and we read a zero length
record then we should mark that record as read.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4685)
Since return is inconsistent, I removed unnecessary parentheses and
unified them.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4541)
Previously if a client received an HRR then we would do version negotiation
immediately - because we know we are going to get TLSv1.3. However this
causes a problem when we emit the 2nd ClientHello because we start changing
a whole load of stuff to ommit things that aren't relevant for < TLSv1.3.
The spec requires that the 2nd ClientHello is the same except for changes
required from the HRR. Therefore the simplest thing to do is to defer the
version negotiation until we receive the ServerHello.
Fixes#4292
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4527)
Removed e_os.h from all bar three headers (apps/apps.h crypto/bio/bio_lcl.h and
ssl/ssl_locl.h).
Added e_os.h into the files that need it now.
Directly reference internal/nelem.h when required.
Reviewed-by: Andy Polyakov <appro@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4188)
Remove GETPID_IS_MEANINGLESS and osslargused.
Move socket-related things to new file internal/sockets.h; this is now
only needed by four(!!!) files. Compiles should be a bit faster.
Remove USE_SOCKETS ifdef's
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4209)
Give each SSL object it's own DRBG, chained to the parent global
DRBG which is used only as a source of randomness into the per-SSL
DRBG. This is used for all session, ticket, and pre-master secret keys.
It is NOT used for ECDH key generation which use only the global
DRBG. (Doing that without changing the API is tricky, if not impossible.)
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4050)
Move the definition of ossl_assert() out of e_os.h which is intended for OS
specific things. Instead it is moved into internal/cryptlib.h.
This also changes the definition to remove the (int) cast.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4073)
This patch removes the prototype of function RECORD_LAYER_set_write_sequence from record_locl.h, since this function is not defined.
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4051)
We prevent compression both when the server is parsing the ClientHello
and when the client is constructing the ClientHello. A 1.3 ServerHello
has no way to hand us back a compression method, and we already check
that the server does not try to give us back a compression method that
we did not request, so these checks seem sufficient.
Weaken the INSTALL note slightly, as we do now expect to interoperate
with other implementations.
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3131)
Signed-off-by: Paul Yang <paulyang.inf@gmail.com>
Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3622)
The check for SSL3_FLAGS_ALLOW_UNSAFE_LEGACY_RENEGOTIATION is
inconsistent. Most places check SSL->options, one place is checking
SSL_CTX->options; fix that.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
GH: #3523
The return code from tls1_mac is supposed to be a boolean 0 for fail, 1 for
success. In one place we returned -1 on error. This would cause code calling
the mac function to erroneously see this as a success (because a non-zero
value is being treated as success in all call sites).
Fortunately, AFAICT, the place that returns -1 can only happen on an
internal error so is not under attacker control. Additionally this code only
appears in master. In 1.1.0 the return codes are treated differently.
Therefore there are no security implications.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3495)
