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)
(cherry picked from commit 37133290832ac2d1389926eba7325125fdacbe8d)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/9653)
(cherry picked from commit e7c27a6c3716843f8412fd96311b70ac84b785f9)
According to RFC8446 CertificateEntry in Certificate message contains
extensions that were not present in the Certificate message in RFC5246.
CLA: trivial
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9994)
(cherry picked from commit 65c76cd2c9e8da9468dd490b334e56c51dbef582)
Make the include guards consistent by renaming them systematically according
to the naming conventions below
The public header files (in the 'include/openssl' directory) are not changed
in 1.1.1, because it is a stable release.
For the private header files files, the guard names try to match the path
specified in the include directives, with all letters converted to upper case
and '/' and '.' replaced by '_'. An extra 'OSSL_' is added as prefix.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9681)
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/9681)
If a TLSv1.3 server configured to respond to the status_request extension
also attempted to send a CertificateRequest then it was incorrectly
inserting a non zero length status_request extension into that message.
The TLSv1.3 RFC does allow that extension in that message but it must
always be zero length.
In fact we should not be sending the extension at all in that message
because we don't support it.
Fixes#9767
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9780)
(cherry picked from commit debb64a0ca43969eb3f043aa8895a4faa7f12b6e)
In commit 6aca8d1a5 ("Honour mandatory digest on private key in
has_usable_cert()") I added two checks for the capabilities of the
EVP_PKEY being used. One of them was wrong, as it should only be
checking the signature of the X.509 cert (by its issuer) against the
sigalgs given in a TLS v1.3 signature_algorithms_cert extension.
Remove it.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9705)
The function SSL_check_chain() can be used by applications to check that
a cert and chain is compatible with the negotiated parameters. This could
be useful (for example) from the certificate callback. Unfortunately this
function was applying TLSv1.2 sig algs rules and did not work correctly if
TLSv1.3 was negotiated.
We refactor tls_choose_sigalg to split it up and create a new function
find_sig_alg which can (optionally) take a certificate and key as
parameters and find an appropriate sig alg if one exists. If the cert and
key are not supplied then we try to find a cert and key from the ones we
have available that matches the shared sig algs.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9443)
Fix a few places where calling ossl_isdigit does the wrong thing on
EBCDIC based systems.
Replaced with ascii_isdigit.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9556)
(cherry picked from commit 48102247ff)
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)
(cherry picked from commit 8bbf63e48f)
tls_parse_stoc_key_share was generating a new EVP_PKEY public/private
keypair and then overrides it with the server public key, so the
generation was a waste anyway. Instead, it should create a
parameters-only EVP_PKEY.
(This is a consequence of OpenSSL using the same type for empty key,
empty key with key type, empty key with key type + parameters, public
key, and private key. As a result, it's easy to mistakenly mix such
things up, as happened here.)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/9445)
(cherry picked from commit 166c0b98fd)
Also, use define rather than sizeof
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9377)
(cherry picked from commit fe9edc9d39)
It was only ever in cert_st because ssl_st was a public structure
and could not be modified without breaking the API. However, both
structures are now opaque, and thus we can freely change their layout
without breaking applications. In this case, keeping the shared
sigalgs in the SSL object prevents complications wherein they would
inadvertently get cleared during SSL_set_SSL_CTX() (e.g., as run
during a cert_cb).
Fixes#9099
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9157)
(cherry picked from commit 29948ac80c)
This reverts commit 524006dd1b.
While this change did prevent the sigalgs from getting inadvertently
clobbered by SSL_set_SSL_CTX(), it also caused the sigalgs to not be
set when the cert_cb runs. This, in turn, caused significant breakage,
such as SSL_check_chain() failing to find any valid chain. An alternate
approach to fixing the issue from #7244 will follow.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9157)
(cherry picked from commit 6f34d7bc7d)
The previous 2 commits moved supported groups and ciphers out of the
session object to avoid race conditions. We now also move ecpointformats
for consistency. There does not seem to be a race condition with access
to this data since it is only ever set in a non-resumption handshake.
However, there is no reason for it to be in the session.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
Similarly to the previous commit we were storing the peer offered list
of ciphers in the session. In practice there is no need for this
information to be avilable from one resumption to the next since this
list is specific to a particular handshake. Since the session object is
supposed to be immutable we should not be updating it once we have decided
to resume. The solution is to remove the session list out of the session
object.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
In TLSv1.3 the supported groups can be negotiated each time a handshake
occurs, regardless of whether we are resuming or not. We should not store
the supported groups information in the session because session objects
can be shared between multiple threads and we can end up with race
conditions. For most users this won't be seen because, by default, we
use stateless tickets in TLSv1.3 which don't get shared. However if you
use SSL_OP_NO_TICKET (to get stateful tickets in TLSv1.3) then this can
happen.
The answer is to move the supported the supported group information into
the SSL object instead.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9176)
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)
(cherry picked from commit feb9e31c40)
This function only returns a status and does not modify the parameter.
Since similar function are already taking const parameters, also
change this function to have a const parameter.
Fixes#8934
CLA: trivial
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8945)
(cherry picked from commit c04b66b18d)
Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8756)
(cherry picked from commit 6fda11ae5a)
Sessions must be immutable once they can be shared with multiple threads.
We were breaking that rule by writing the ticket index into it during the
handshake. This can lead to incorrect behaviour, including failed
connections in multi-threaded environments.
Reported by David Benjamin.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8383)
(cherry picked from commit c96ce52ce2)
Prior to this commit we were keeping a count of how many KeyUpdates we
have processed and failing if we had had too many. This simplistic approach
is not sufficient for long running connections. Since many KeyUpdates
would not be a particular good DoS route anyway, the simplest solution is
to simply remove the key update count.
Fixes#8068
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/8299)
(cherry picked from commit 3409a5ff8a)
The original 1.1.1 design was to use SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal start/end of a post-handshake message
exchange in TLSv1.3. Unfortunately experience has shown that this confuses
some applications who mistake it for a TLSv1.2 renegotiation. This means
that KeyUpdate messages are not handled properly.
This commit removes the use of SSL_CB_HANDSHAKE_START and
SSL_CB_HANDSHAKE_DONE to signal the start/end of a post-handshake
message exchange. Individual post-handshake messages are still signalled in
the normal way.
This is a potentially breaking change if there are any applications already
written that expect to see these TLSv1.3 events. However, without it,
KeyUpdate is not currently usable for many applications.
Fixes#8069
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8096)
(cherry picked from commit 4af5836b55)
set_cipher_list() sets TLSv1.2 (and below) ciphers, and its success or
failure should not depend on whether set_ciphersuites() has been used to
setup TLSv1.3 ciphers.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7759)
(cherry picked from commit 3c83c5ba4f)
When computing the end-point shared secret, don't take the
terminating NULL character into account.
Please note that this fix breaks interoperability with older
versions of OpenSSL, which are not fixed.
Fixes#7956
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7957)
(cherry picked from commit 09d62b336d)
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)
(cherry picked from commit bcc1f3e2ba)
We were setting a limit of SSL3_RT_MAX_PLAIN_LENGTH on the size of the
ClientHello. AFAIK there is nothing in the standards that requires this
limit.
The limit goes all the way back to when support for extensions was first
added for TLSv1.0. It got converted into a WPACKET max size in 1.1.1. Most
likely it was originally added to avoid the complexity of having to grow
the init_buf in the middle of adding extensions. With WPACKET this is
irrelevant since it will grow automatically.
This issue came up when an attempt was made to send a very large
certificate_authorities extension in the ClientHello.
We should just remove the limit.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7424)
(cherry picked from commit 7835e97b6f)
1. In addition to overriding the default application name,
one can now also override the configuration file name
and flags passed to CONF_modules_load_file().
2. By default we still keep going when configuration file
processing fails. But, applications that want to be
strict about initialization errors can now make explicit
flag choices via non-null OPENSSL_INIT_SETTINGS that omit
the CONF_MFLAGS_IGNORE_RETURN_CODES flag (which had so far
been both undocumented and unused).
3. In OPENSSL_init_ssl() do not request OPENSSL_INIT_LOAD_CONFIG
if the options already include OPENSSL_INIT_NO_LOAD_CONFIG.
4. Don't set up atexit() handlers when called with opts equal to
OPENSSL_INIT_BASE_ONLY (this flag should only be used alone).
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7969)
The cryptopro extension is supposed to be unsolicited and appears in the
ServerHello only. Additionally it is unofficial and unregistered - therefore
we should really treat it like any other unknown extension if we see it in
the ClientHello.
Fixes#7747
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/7984)
(cherry picked from commit 23fed8ba0e)
We have a number of instances where there are multiple "init" functions for
a single CRYPTO_ONCE variable, e.g. to load config automatically or to not
load config automatically. Unfortunately the RUN_ONCE mechanism was not
correctly giving the right return value where an alternative init function
was being used.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7983)
Fix some issues in tls13_hkdf_expand() which impact the above function
for TLSv1.3. In particular test that we can use the maximum label length
in TLSv1.3.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/7755)
(cherry picked from commit 0fb2815b87)
