Viktor Dukhovni [Wed, 14 Feb 2018 03:43:15 +0000 (22:43 -0500)]
Avoid fragile aliasing of SHA224/384 update/final
This is purported to save a few cycles, but makes the code less
obvious and more brittle, and in fact breaks on platforms where for
ABI continuity reasons there is a SHA2 implementation in libc, and
so EVP needs to call those to avoid conflicts.
A sufficiently good optimizer could simply generate the same entry
points for:
foo(...) { ... }
and
bar(...) { return foo(...); }
but, even without that, the different is negligible, with the
"winner" varying from run to run (openssl speed -evp sha384):
Richard Levitte [Fri, 16 Jun 2017 01:46:41 +0000 (03:46 +0200)]
Add the target 'build_all_generated'
This new target is used to build all generated files and only that.
This can be used to prepare everything that requires things like perl
for a system that lacks perl and then move everything to that system
and do the rest of the build there.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3695)
Matt Caswell [Thu, 8 Feb 2018 14:48:51 +0000 (14:48 +0000)]
Don't calculate the Finished MAC twice
In <= TLSv1.2 a Finished message always comes immediately after a CCS
except in the case of NPN where there is an additional message between
the CCS and Finished. Historically we always calculated the Finished MAC
when we processed the CCS. However to deal with NPN we also calculated it
when we receive the Finished message. Really this should only have been
done if we hand negotiated NPN.
This simplifies the code to only calculate the MAC when we receive the
Finished. In 1.1.1 we need to do it this way anyway because there is no
CCS (except in middlebox compat mode) in TLSv1.3.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5286)
Richard Levitte [Thu, 8 Feb 2018 11:31:05 +0000 (12:31 +0100)]
util/mkdef.pl: use better array in search of 'DEPRECATEDIN_'
%disabled_algorithms isn't necessarily initialised with the "algos"
'DEPRECATEDIN_1_1_0' etc. However, we know that @known_algorithms has
them all, so use that to find them instead.
Fixes #5157
(where this was reported)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5282)
Richard Levitte [Wed, 24 Jan 2018 13:17:39 +0000 (14:17 +0100)]
util/mkdef.pl: Trust configdata.pm
This script kept its own database of disablable algorithms, which is a
maintenance problem, as it's not always perfectly in sync with what
Configure does. However, we do have all the data in configdata.pm,
produced by Configure, so let's use that instead.
Also, make sure to parse the *err.h header files, as they contain
function declarations that might not be present elsewhere.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5157)
Rich Salz [Tue, 6 Feb 2018 20:27:03 +0000 (15:27 -0500)]
Remove unused file
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5267)
(cherry picked from commit a3a5d1b73db46274e2cdedaad42fda5ce5cfb2da)
David Benjamin [Wed, 31 Jan 2018 19:47:41 +0000 (14:47 -0500)]
Fix timing leak in BN_from_montgomery_word.
BN_from_montgomery_word doesn't have a constant memory access pattern.
Replace the pointer trick with a constant-time select. There is, of
course, still the bn_correct_top leak pervasive in BIGNUM itself.
See also https://boringssl-review.googlesource.com/22904 from BoringSSL.
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/5228)
David Benjamin [Tue, 23 Jan 2018 18:57:10 +0000 (13:57 -0500)]
Don't leak the exponent bit width in BN_mod_exp_mont_consttime.
The exponent here is one of d, dmp1, or dmq1 for RSA. This value and its
bit length are both secret. The only public upper bound is the bit width
of the corresponding modulus (RSA n, p, and q, respectively).
Although BN_num_bits is constant-time (sort of; see bn_correct_top notes
in preceding patch), this does not fix the root problem, which is that
the windows are based on the minimal bit width, not the upper bound. We
could use BN_num_bits(m), but BN_mod_exp_mont_consttime is public API
and may be called with larger exponents. Instead, use all top*BN_BITS2
bits in the BIGNUM. This is still sensitive to the long-standing
bn_correct_top leak, but we need to fix that regardless.
This may cause us to do a handful of extra multiplications for RSA keys
which are just above a whole number of words, but that is not a standard
RSA key size.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
David Benjamin [Tue, 23 Jan 2018 18:46:53 +0000 (13:46 -0500)]
Make BN_num_bits_word constant-time.
(This patch was written by Andy Polyakov. I only wrote the commit
message. Mistakes in the analysis are my fault.)
BN_num_bits, by way of BN_num_bits_word, currently leaks the
most-significant word of its argument via branching and memory access
pattern.
BN_num_bits is called on RSA prime factors in various places. These have
public bit lengths, but all bits beyond the high bit are secret. This
fully resolves those cases.
There are a few places where BN_num_bits is called on an input where the
bit length is also secret. This does *not* fully resolve those cases as
we still only look at the top word. Today, that is guaranteed to be
non-zero, but only because of the long-standing bn_correct_top timing
leak. Once that is fixed, a constant-time BN_num_bits on such inputs
must count bits on each word.
Instead, those cases should not call BN_num_bits at all. In particular,
BN_mod_exp_mont_consttime uses the exponent bit width to pick windows,
but it should be using the maximum bit width. The next patch will fix
this.
Thanks to Dinghao Wu, Danfeng Zhang, Shuai Wang, Pei Wang, and Xiao Liu
for reporting this issue.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5154)
added macro to create version number
use the macro to build OPENSSL_VERSION_AT_LEAST(maj,min,fix) so that
customers of libssl (such as ruby-openssl) do not need to be so aware of
openssl version numbers.
includes updates to ssl(7) and OPENSSL_VERSION_NUMBER(3) man page
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5212)
(cherry picked from commit 3c5a61dd0f9d9a9eac098419bcaf47d1c296ca81)
Matt Caswell [Wed, 10 May 2017 20:46:14 +0000 (16:46 -0400)]
Add the SSL_OP_NO_RENEGOTIATION option to 1.1.0
This is based on a heavily modified version of commit db0f35dda by Todd
Short from the master branch.
We are adding this because it used to be possible to disable reneg using
the flag SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS in 1.0.2. This is no longer
possible because of the opacity work.
A point to note about this is that if an application built against new
1.1.0 headers (that know about the new option SSL_OP_NO_RENEGOTIATION
option) is run using an older version of 1.1.0 (that doesn't know about
the option) then the option will be accepted but nothing will happen, i.e.
renegotiation will not be prevented. There's probably not much we can do
about that.
Fixes #4739
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4901)
Matt Caswell [Mon, 29 Jan 2018 14:55:44 +0000 (14:55 +0000)]
Make sure we check an incoming reneg ClientHello in DTLS
In TLS we have a check to make sure an incoming reneg ClientHello is
acceptable. The equivalent check is missing in the DTLS code. This means
that if a client does not signal the ability to handle secure reneg in the
initial handshake, then a subsequent reneg handshake should be rejected by
the server. In the DTLS case the reneg was being allowed if the the 2nd
ClientHello had a renegotiation_info extension. This is incorrect.
While incorrect, this does not represent a security issue because if
the renegotiation_info extension is present in the second ClientHello it
also has to be *correct*. Therefore this will only work if both the client
and server believe they are renegotiating, and both know the previous
Finished result. This is not the case in an insecure rengotiation attack.
I have also tidied up the check in the TLS code and given a better check
for determining whether we are renegotiating or not.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5191)
Benjamin Kaduk [Wed, 24 Jan 2018 19:54:46 +0000 (13:54 -0600)]
Fix strict-warnings build on FreeBSD
The cryptodev engine is only available for OpenBSD and FreeBSD,
but for the OS version-specific checks the OpenBSD macro is not
defined on FreeBSD. When building with -Werror and -Wundef (enabled
by strict-warnings), the FreeBSD build fails:
crypto/engine/eng_cryptodev.c:47:7: error: 'OpenBSD' is not defined,
evaluates to 0
[-Werror,-Wundef]
\# if (OpenBSD >= 200112) || ((__FreeBSD_version >= 470101 &&
\# __FreeBSD_versi...
^
The reverse case would be true on OpenBSD (__FreeBSD_version would
not be defined), but since the boolean will short-circuit and this
code is only executed on OpenBSD and FreeBSD, and the line is
already pretty long, leave that out for now.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5160)
Christian Heimes [Sun, 21 Jan 2018 09:37:59 +0000 (10:37 +0100)]
Fix signature of min/max proto getter
The getters for min and max proto version wrongly passed NULL instead of
0 as third argument to SSL_ctrl() and SSL_CTX_ctrl(). The third argument
is not used, but the error results in a compiler warning:
warning: passing argument 3 of ‘SSL_CTX_ctrl’ makes integer from pointer without a cast [-Wint-conversion]
int v = SSL_CTX_get_max_proto_version(self->ctx);
See https://github.com/openssl/openssl/pull/4364
Signed-off-by: Christian Heimes <christian@python.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/5128)
Richard Levitte [Tue, 23 Jan 2018 18:13:48 +0000 (19:13 +0100)]
Configure: ensure that a DEPEND generates the correct inclusion directory
We incorrectly assumed that explicit dependencies meant that the
source directory would be added for inclusion. However, if the
dependent file is generated, it's stored in the build directory, and
that should be used for inclusion rather than the source directory.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5153)
Richard Levitte [Mon, 22 Jan 2018 18:03:37 +0000 (19:03 +0100)]
Have EVP_PKEY_asn1_find_str() work more like EVP_PKEY_asn1_find()
EVP_PKEY_asn1_find_str() would search through standard asn1 methods
first, then those added by the application, which EVP_PKEY_asn1_find()
worked the other way around. Also, EVP_PKEY_asn1_find_str() didn't
handle aliases.
This change brings EVP_PKEY_asn1_find_str() closer to EVP_PKEY_asn1_find().
Fixes #5086
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5137)
Matt Caswell [Fri, 19 Jan 2018 14:34:56 +0000 (14:34 +0000)]
Don't allow an empty Subject when creating a Certificate
Misconfiguration (e.g. an empty policy section in the config file) can
lead to an empty Subject. Since certificates should have unique Subjects
this should not be allowed.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5114)
Richard Levitte [Wed, 17 Jan 2018 10:22:47 +0000 (11:22 +0100)]
Create one permanent proxy socket per TLSProxy::Proxy instance
On Windows, we sometimes see a behavior with SO_REUSEADDR where there
remains lingering listening sockets on the same address and port as a
newly created one.
To avoid this scenario, we don't create a new proxy port for each new
client run. Instead, we create one proxy socket when the proxy object
is created, and close it when destroying that object.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5095)
Richard Levitte [Thu, 11 Jan 2018 21:01:44 +0000 (22:01 +0100)]
Cygwin is POSIX, don't say it isn't
More to the point, Cygwin is a POSIX API. In our library, the use of
a POSIX API is marked by defining the macro OPENSSL_SYS_UNIX.
Therefore, that macro shouldn't be undefined when building for Cygwin.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5060)
Richard Levitte [Thu, 18 Jan 2018 09:54:48 +0000 (10:54 +0100)]
TLSProxy::Proxy: Don't use ReuseAddr on Windows
On Windows, we sometimes see a behavior with SO_REUSEADDR where there
remains lingering listening sockets on the same address and port as a
newly created one.
An easy solution is not to use ReuseAddr on Windows.
Thanks Bernd Edlinger for the suggestion.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/5103)
Richard Levitte [Wed, 17 Jan 2018 20:27:33 +0000 (21:27 +0100)]
TLSProxy::Proxy: don't waste time redirecting STDOUT and STDERR
On Windows, it seems that doing so in a forked (pseudo-)process
sometimes affects the parent, and thereby hides all the results that
are supposed to be seen by the running test framework (the "ok" and
"not ok" lines).
It turns out that our redirection isn't necessary, as the test
framework seems to swallow it all in non-verbose mode anyway.
It's possible that we did need this at some point, but the framework
has undergone some refinement since then...
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5100)
Matt Caswell [Mon, 15 Jan 2018 11:23:07 +0000 (11:23 +0000)]
Revert BN_copy() flag copy semantics change
Commit 9f9442918a changed the semantics of BN_copy() to additionally
copy the BN_FLG_CONSTTIME flag if it is set. This turns out to be
ill advised as it has unintended consequences. For example calling
BN_mod_inverse_no_branch() can sometimes return a result with the flag
set and sometimes not as a result. This can lead to later failures if we
go down code branches that do not support constant time, but check for
the presence of the flag.
The original commit was made due to an issue in BN_MOD_CTX_set(). The
original PR fixed the problem in that function, but it was changed in
review to fix it in BN_copy() instead. The solution seems to be to revert
the BN_copy() change and go back to the originally proposed way.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/5080)
Richard Levitte [Mon, 15 Jan 2018 09:40:24 +0000 (10:40 +0100)]
Fix intermittent Windows and Cygwin failures in s_server
The same kind of failure that has already been observed on the
s_client can sometimes also be observed on s_server, so we need to add
the same kind of 50ms delay as was previously added on s_client.
Richard Levitte [Sun, 14 Jan 2018 16:15:32 +0000 (17:15 +0100)]
Fix intermittent Cygwin failures in s_client
This was identified for Windows almost two years ago for VC and
msys/mingw. It seems that Cygwin suffers from the same issue, and
since Cygwin doesn't define OPENSSL_SYS_WINDOWS, we need to make a
special case to have a 50ms pause before closing the TLS connection.
Matt Caswell [Fri, 5 Jan 2018 10:12:29 +0000 (10:12 +0000)]
Tolerate DTLS alerts with an incorrect version number
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)
Various small build improvements on mkdef.pl, progs.pl
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4994)
Patrick Steuer [Mon, 8 Jan 2018 18:42:32 +0000 (13:42 -0500)]
Document OPENSSL_ENGINES environment variable
In apps/engine.pod and crypto/ENGINE_add.pod
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4956)
Rich Salz [Sun, 7 Jan 2018 03:32:59 +0000 (22:32 -0500)]
Add fingerprint text, remove MD5
Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/4906)
(Cherry-picked from commit 9422d45de2b70cabec5f6e6a5c812e0647e6d3ab)
Daniel Bevenius [Sun, 17 Dec 2017 21:04:48 +0000 (07:04 +1000)]
Add comments to NULL func ptrs in bio_method_st
This commit adds comments to bio_method_st definitions where the
function pointers are defined as NULL. Most of the structs have comments
but some where missing and not all consitent.
CLA: trivial
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4881)
Benjamin Kaduk [Thu, 7 Dec 2017 22:37:54 +0000 (16:37 -0600)]
Wrap more of ocspapitest.c in OPENSSL_NO_OCSP
make_dummy_resp() uses OCSP types, and get_cert_and_key() is unused
once make_dummy_resp() is compiled out, so neither can be included
in the build when OCSP is disabled and strict warnings are active.
Benjamin Kaduk [Thu, 19 Oct 2017 19:44:10 +0000 (14:44 -0500)]
Add OCSP API test executable
Some of the OCSP APIs (such as the recently added OCSP_resp_get0_signer)
do not really merit inclusion in the ocsp(1) utility, but we should still
have unit tests for them.
For now, only test OCSP_resp_get0_signer(), but it should be easy to
add more tests in the future.
Provide an X509 cert and private key in the test's data directory
to use for signing responses, since constructing those on the fly
is more effort than is needed.
Benjamin Kaduk [Wed, 18 Oct 2017 20:29:18 +0000 (15:29 -0500)]
Add an API to get the signer of an OCSP response
Add a new function OCSP_resp_get0_signer() that looks in the
certs bundled with the response as well as in additional certificates
provided as a function argument, returning the certificate that signed
the given response (if present).
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4916)
Richard Levitte [Tue, 12 Dec 2017 01:05:38 +0000 (02:05 +0100)]
Fix leak in ERR_get_state() when OPENSSL_init_crypto() isn't called yet
If OPENSSL_init_crypto() hasn't been called yet when ERR_get_state()
is called, it need to be called early, so the base initialization is
done. On some platforms (those who support DSO functionality and
don't define OPENSSL_USE_NODELETE), that includes a call of
ERR_set_mark(), which calls this function again.
Furthermore, we know that ossl_init_thread_start(), which is called
later in ERR_get_state(), calls OPENSSL_init_crypto(0, NULL), except
that's too late.
Here's what happens without an early call of OPENSSL_init_crypto():
=> ERR_get_state():
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated.
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# Here, base_inited is set to 1
# before ERR_set_mark() call
=> ERR_set_mark():
=> ERR_get_state():
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated!!!!!
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
<=
=> CRYPTO_thread_set_local():
<=
<=
<=
<= 1
<=
=> CRYPTO_thread_set_local() # previous value removed!
<=
Result: double allocation, and we have a leak.
By calling the base OPENSSL_init_crypto() early, we get this instead:
=> ERR_get_state():
=> OPENSSL_init_crypto():
# Here, base_inited is set to 1
# before ERR_set_mark() call
=> ERR_set_mark():
=> ERR_get_state():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
=> CRYPTO_THREAD_get_local():
<= NULL;
# no state is found, so it gets allocated
# let's assume we got 0xDEADBEEF
=> ossl_init_thread_start():
=> OPENSSL_init_crypto():
# base_inited is 1,
# so no more init to be done
<= 1
<= 1
=> CRYPTO_thread_set_local():
<=
<=
<=
<= 1
=> CRYPTO_THREAD_get_local():
<= 0xDEADBEEF
<= 0xDEADBEEF
Result: no leak.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4913)