Add a check for history section location to find-doc-nits
Check that the HISTORY section is located after the SEE ALSO section,
this is a much more frequent order in OpenSSL manual pages (and UNIX
manual pages in general).
Also check that SEE ALSO comes after EXAMPLES, so that the tool can
ensure the correct manual section sequence.
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 573ac8f2228241771f727ecd8ff10f54073536d3)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8736)
Make check_example_location() in find-doc-nits generic
Change to check_section_location(), a generic function to ensure that
section SECTION appears before section BEFORE in the man pages.
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(cherry picked from commit 95f92d57755a9bfc83135a585da69d497f7293d9)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8736)
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/8736)
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/8736)
Don't use coordinate blinding when scalar is group order
This happens in ec_key_simple_check_key and EC_GROUP_check.
Since the the group order is not a secret scalar, it is
unnecessary to use coordinate blinding.
Fixes: #8731 Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8734)
Fixed linux_x86_icc compiler errors in EC code related to __uint128_t/__int128_t
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8697)
Jakub Wilk [Wed, 10 Apr 2019 10:00:20 +0000 (12:00 +0200)]
Fix typos
CLA: trivial
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8714)
Richard Levitte [Thu, 4 Apr 2019 23:22:14 +0000 (01:22 +0200)]
EVP_*Update: ensure that input NULL with length 0 isn't passed
Even with custome ciphers, the combination in == NULL && inl == 0
should not be passed down to the backend cipher function. The reason
is that these are the values passed by EVP_*Final, and some of the
backend cipher functions do check for these to see if a "final" call
is made.
Fixes #8675
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8676)
Richard Levitte [Mon, 1 Apr 2019 04:40:33 +0000 (06:40 +0200)]
Rework DSO API conditions and configuration option
'no-dso' is meaningless, as it doesn't get any macro defined.
Therefore, we remove all checks of OPENSSL_NO_DSO. However, there may
be some odd platforms with no DSO scheme. For those, we generate the
internal macro DSO_NONE aand use it.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8622)
Matt Caswell [Mon, 8 Apr 2019 10:22:37 +0000 (11:22 +0100)]
Fix crash in X509_STORE_CTX_get_by_subject
If using a custom X509_LOOKUP_METHOD then calls to
X509_STORE_CTX_get_by_subject may crash due to an incorrectly initialised
X509_OBJECT being passed to the callback get_by_subject function.
Fixes #8673
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8698)
Dan Campbell [Thu, 4 Apr 2019 21:15:33 +0000 (15:15 -0600)]
s_client starttls: fix handling of multiline reply
Fixes #8645
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8654)
It was assumed that the config functionality returned a boolean.
However, it may return a negative number on error, so we need to take
that into account.
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8679)
Tomas Mraz [Thu, 21 Mar 2019 12:17:29 +0000 (13:17 +0100)]
Use AI_ADDRCONFIG hint with getaddrinfo if available.
This prevents failure of openssl s_server socket binding to wildcard
address on hosts with disabled IPv6.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8550)
rand_win.c: loosen version requirements for BCryptGenRandom
BCryptGenRandom() is available for Windows Vista and newer versions, see
https://docs.microsoft.com/en-us/windows/desktop/api/bcrypt/nf-bcrypt-bcryptgenrandom
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8639)
Soujyu Tanaka [Wed, 27 Mar 2019 08:30:47 +0000 (17:30 +0900)]
For the lack of GetModuleHandleEx(), we use DSO route for WinCE.
Revert win32_pathbyaddr() which is used in DSO_dsobyaddr().
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8596)
Soujyu Tanaka [Wed, 27 Mar 2019 07:21:58 +0000 (16:21 +0900)]
Circumvent a problem of lacking GetEnvironmentVariable() in WindowsCE.
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8596)
Soujyu Tanaka [Wed, 27 Mar 2019 07:15:31 +0000 (16:15 +0900)]
Avoid linking error for InitializeCriticalSectionAndSpinCount().
Replace it with InitializeCriticalSection()
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8596)
Soujyu Tanaka [Wed, 27 Mar 2019 06:55:32 +0000 (15:55 +0900)]
Avoid linking error on WCE700 for _InterlockedExchangeAdd().
This implementation is referenced to https://www.boost.org/doc/libs/1_69_0/boost/detail/interlocked.hpp
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8596)
Jake Massimo [Wed, 27 Mar 2019 04:13:08 +0000 (04:13 +0000)]
Increase rounds of Miller-Rabin testing DH_check
DH_check is used to test the validity of Diffie-Hellman parameter sets (p, q, g). Among the tests performed are primality tests on p and q, for this BN_is_prime_ex is called with the rounds of Miller-Rabin set as default. This will therefore use the average case error estimates derived from the function BN_prime_checks_for_size based on the bit size of the number tested.
However, these bounds are only accurate on testing random input. Within this testing scenario, where we are checking the validity of a DH parameter set, we can not assert that these parameters are randomly generated. Thus we must treat them as if they are adversarial in nature and increase the rounds of Miller-Rabin performed.
Generally, each round of Miller-Rabin can declare a composite number prime with probability at most (1/4), thus 64 rounds is sufficient in thwarting known generation techniques (even in safe prime settings - see https://eprint.iacr.org/2019/032 for full analysis). The choice of 64 rounds is also consistent with SRP_NUMBER_ITERATIONS_FOR_PRIME 64 as used in srp_Verify_N_and_g in openssl/apps/s_client.c.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8593)
Matt Caswell [Tue, 26 Mar 2019 15:25:15 +0000 (15:25 +0000)]
Tolerate 0 byte input length for Update functions
We treat that as automatic success. Other EVP_*Update functions already do
this (e.g. EVP_EncryptUpdate, EVP_DecryptUpdate etc). EVP_EncodeUpdate is
a bit of an anomoly. That treats 0 byte input length as an error.
Fixes #8576
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8587)
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/8421)
Bernd Edlinger [Wed, 20 Mar 2019 21:02:58 +0000 (22:02 +0100)]
Modify the RSA_private_decrypt functions to check the padding in
constant time with a memory access pattern that does not depend
on secret information.
[extended tests]
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8543)
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8526)
Reorganized signature-scheme detection in 'apps/s_cb.c:security_callback_debug' callback-function.
So far, it only handled hash-and-algorithm pairs from TLS1.2,
now it also handles 'schemes' defined in TLS1.3 like 0x0807=ed25519 or
0x0809=rsa_pss_pss_sha256
Now it prints information in one of these formats:
Richard Levitte [Wed, 20 Mar 2019 09:18:13 +0000 (10:18 +0100)]
OPENSSL_config(): restore error agnosticism
Great effort has been made to make initialization more configurable.
However, the behavior of OPENSSL_config() was lost in the process,
having it suddenly generate errors it didn't previously, which is not
how it's documented to behave.
A simple setting of default flags fixes this problem.
Fixes #8528
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8533)
Shane Lontis [Tue, 19 Mar 2019 22:13:55 +0000 (08:13 +1000)]
Updated doc for BN_clear, BN_CTX_end when param is NULL
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8532)
Vitezslav Cizek [Tue, 5 Mar 2019 21:52:33 +0000 (22:52 +0100)]
apps/speed.c: properly address NO_EC2M on systems without SIGALRM
The ecdh_c array is allocated of the same size as ecdh_choices,
whose size depends on whether the support for binary curves is enabled
or not. (The same goes for ecdsa_c).
On systems without SIGALRM, ecdh_c is indexed by predefined constants
intended for representing the index of the ciphers in the ecdh_choices
array.
However, in case of NO_EC2M some of the #defined constants won't match
and would actually access the ecdh_c out-of-bounds.
Use enum instead of a macro to define the curve indexes so they're
within the bounds of the ecdh_c array.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8422)
Vitezslav Cizek [Tue, 5 Mar 2019 16:14:33 +0000 (17:14 +0100)]
apps/speed.c: skip binary curves when compiling with OPENSSL_NO_EC2M
openssl speed doesn't take into account that the library could be
compiled without the support for the binary curves and happily uses
them, which results in EC_GROUP_new_by_curve_name() errors.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8422)
The indentation in the Configure file is currently very strange when
viewed in an editor with a tab width of four spaces, because it has
mixed tab-and-whitespace indentation, which was apparently done with
a tab width of eight spaces.
This commit converts all tabs to spaces using expand(1) with default
settings. To verify that there are only whitespace changes, use
git show --ignore-space-change <this commit>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8512)
Hua Zhang [Wed, 13 Mar 2019 06:28:44 +0000 (14:28 +0800)]
Fix compiling error for mips32r6 and mips64r6
There are some compiling errors for mips32r6 and mips64r6:
crypto/bn/bn-mips.S:56: Error: opcode not supported on this processor: mips2 (mips2) `mulu $1,$12,$7'
crypto/mips_arch.h: Assembler messages:
crypto/mips_arch.h:15: Error: junk at end of line, first unrecognized character is `&'
Signed-off-by: Hua Zhang <hua.zhang1974@hotmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8464)
Shane Lontis [Mon, 18 Mar 2019 23:58:09 +0000 (09:58 +1000)]
Added NULL check to BN_clear() & BN_CTX_end()
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8518)
Daniel Axtens [Sun, 17 Mar 2019 23:22:44 +0000 (10:22 +1100)]
PPC assembly pack: fix copy-paste error in CTR mode
There are two copy-paste errors in handling CTR mode. When dealing
with a 2 or 3 block tail, the code branches to the CBC decryption exit
path, rather than to the CTR exit path.
This can lead to data corruption: in the Linux kernel we have a copy
of this file, and the bug leads to corruption of the IV, which leads
to data corruption when we call the encryption function again later to
encrypt subsequent blocks.
Originally reported to the Linux kernel by Ondrej Mosnáček <omosnacek@gmail.com>
CLA: trivial
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8510)
Shane Lontis [Mon, 18 Mar 2019 00:39:07 +0000 (10:39 +1000)]
coverity fixes for bntest.c
Reviewed-by: Paul Yang <yang.yang@baishancloud.com> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8508)
VMS: only use the high precision on VMS v8.4 and up
Fixes #8487
Amends #7230
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8488)
Matt Caswell [Tue, 12 Mar 2019 10:21:39 +0000 (10:21 +0000)]
Fix memory leaks in pkread.c demo file
Also make various changes to bring the file into line with current coding
style.
Fixes #8456
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Paul Yang <yang.yang@baishancloud.com>
(Merged from https://github.com/openssl/openssl/pull/8457)
Shane Lontis [Wed, 6 Mar 2019 02:57:09 +0000 (12:57 +1000)]
fix truncation of integers on 32bit AIX
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8417)
A. Schulze [Sat, 9 Mar 2019 18:05:31 +0000 (19:05 +0100)]
Fix two spelling errors
CLA: trivial
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/8447)
Matt Caswell [Wed, 6 Mar 2019 11:51:28 +0000 (11:51 +0000)]
Add a test for underflow in ecp_nistp521.c
The previous commit fixed an underflow that may occur in ecp_nistp521.c.
This commit adds a test for that condition. It is heavily based on an
original test harness by Billy Brumley.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/8405)
Matt Caswell [Tue, 5 Mar 2019 13:26:45 +0000 (13:26 +0000)]
Avoid an underflow in ecp_nistp521.c
The function felem_diff_128_64 in ecp_nistp521.c substracts the number |in|
from |out| mod p. In order to avoid underflow it first adds 32p mod p
(which is equivalent to 0 mod p) to |out|. The comments and variable naming
suggest that the original author intended to add 64p mod p. In fact it
has been shown that with certain unusual co-ordinates it is possible to
cause an underflow in this function when only adding 32p mod p while
performing a point double operation. By changing this to 64p mod p the
underflow is avoided.
It turns out to be quite difficult to construct points that satisfy the
underflow criteria although this has been done and the underflow
demonstrated. However none of these points are actually on the curve.
Finding points that satisfy the underflow criteria and are also *on* the
curve is considered significantly more difficult. For this reason we do
not believe that this issue is currently practically exploitable and
therefore no CVE has been assigned.
This only impacts builds using the enable-ec_nistp_64_gcc_128 Configure
option.
With thanks to Bo-Yin Yang, Billy Brumley and Dr Liu for their significant
help in investigating this issue.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/8405)
Matt Caswell [Tue, 5 Mar 2019 15:41:27 +0000 (15:41 +0000)]
Update ChaCha20-Poly1305 documentation
Correctly describe the maximum IV length.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8406)
Matt Caswell [Tue, 5 Mar 2019 14:51:07 +0000 (14:51 +0000)]
Test an overlong ChaCha20-Poly1305 nonce
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8406)
Matt Caswell [Tue, 5 Mar 2019 14:39:15 +0000 (14:39 +0000)]
Prevent over long nonces in ChaCha20-Poly1305
ChaCha20-Poly1305 is an AEAD cipher, and requires a unique nonce input for
every encryption operation. RFC 7539 specifies that the nonce value (IV)
should be 96 bits (12 bytes). OpenSSL allows a variable nonce length and
front pads the nonce with 0 bytes if it is less than 12 bytes. However it
also incorrectly allows a nonce to be set of up to 16 bytes. In this case
only the last 12 bytes are significant and any additional leading bytes are
ignored.
It is a requirement of using this cipher that nonce values are unique.
Messages encrypted using a reused nonce value are susceptible to serious
confidentiality and integrity attacks. If an application changes the
default nonce length to be longer than 12 bytes and then makes a change to
the leading bytes of the nonce expecting the new value to be a new unique
nonce then such an application could inadvertently encrypt messages with a
reused nonce.
Additionally the ignored bytes in a long nonce are not covered by the
integrity guarantee of this cipher. Any application that relies on the
integrity of these ignored leading bytes of a long nonce may be further
affected.
Any OpenSSL internal use of this cipher, including in SSL/TLS, is safe
because no such use sets such a long nonce value. However user
applications that use this cipher directly and set a non-default nonce
length to be longer than 12 bytes may be vulnerable.
CVE-2019-1543
Fixes #8345
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8406)
Matt Caswell [Fri, 1 Mar 2019 15:40:20 +0000 (15:40 +0000)]
Don't write the tick_identity to the session
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)
Vitezslav Cizek [Thu, 28 Feb 2019 12:47:18 +0000 (13:47 +0100)]
openssl_strerror_r: Fix handling of GNU strerror_r
GNU strerror_r may return either a pointer to a string that the function
stores in buf, or a pointer to some (immutable) static string in which case
buf is unused.
In such a case we need to set buf manually.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8371)
Richard Levitte [Thu, 14 Feb 2019 15:26:40 +0000 (16:26 +0100)]
Configure: make --strict-warnings a regular user provided compiler option
This makes `--strict-warnings` into a compiler pseudo-option, i.e. it
gets treated the same way as any other compiler option given on the
configuration command line, but is retroactively replaced by actual
compiler warning options, depending on what compiler is used.
This makes it easier to see in what order options are given to the
compiler from the configuration command line, i.e. this:
./config -Wall --strict-warnings
would give the compiler flags in the same order as they're given,
i.e.:
Shane Lontis [Tue, 19 Feb 2019 03:56:33 +0000 (13:56 +1000)]
cfi build fixes in x86-64 ghash assembly
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8281)
Paul Yang [Mon, 25 Feb 2019 09:34:03 +0000 (17:34 +0800)]
Fix the default digest algorthm of SM2
Currently SM2 shares the ameth with EC, so the current default digest
algorithm returned is SHA256. This fixes the default digest algorithm of
SM2 to SM3, which is the only valid digest algorithm for SM2 signature.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8186)
Reviewed-by: Paul Yang <yang.yang@baishancloud.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8319)
cipher_init may be called on an already initialized context, without a
necessary cleanup. This separates cleanup from initialization, closing
an eventual open session before creating a new one.
Move the /dev/crypto session cleanup code to its own function.
Signed-off-by: Eneas U de Queiroz <cote2004-github@yahoo.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/8306)
Richard Levitte [Tue, 26 Feb 2019 10:22:16 +0000 (11:22 +0100)]
Disable 02-test_errstr.t on msys/mingw as well as MSWin32
There is too high a risk that perl and OpenSSL are linked with
different C RTLs, and thereby get different messages for even the most
mundane error numbers.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8343)
Richard Levitte [Mon, 25 Feb 2019 18:27:42 +0000 (19:27 +0100)]
Rearrange the inclusion of curve448/curve448_lcl.h
The real cause for this change is that test/ec_internal_test.c
includes ec_lcl.h, and including curve448/curve448_lcl.h from there
doesn't work so well with compilers who always do inclusions relative
to the C file being compiled.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8334)
Matt Caswell [Mon, 25 Feb 2019 11:28:32 +0000 (11:28 +0000)]
Ensure bn_cmp_words can handle the case where n == 0
Thanks to David Benjamin who reported this, performed the analysis and
suggested the patch. I have incorporated some of his analysis in the
comments below.
This issue can cause an out-of-bounds read. It is believed that this was
not reachable until the recent "fixed top" changes. Analysis has so far
only identified one code path that can encounter this - although it is
possible that others may be found. The one code path only impacts 1.0.2 in
certain builds. The fuzzer found a path in RSA where iqmp is too large. If
the input is all zeros, the RSA CRT logic will multiply a padded zero by
iqmp. Two mitigating factors:
- Private keys which trip this are invalid (iqmp is not reduced mod p).
Only systems which take untrusted private keys care.
- In OpenSSL 1.1.x, there is a check which rejects the oversize iqmp,
so the bug is only reproducible in 1.0.2 so far.
Fortunately, the bug appears to be relatively harmless. The consequences of
bn_cmp_word's misbehavior are:
- OpenSSL may crash if the buffers are page-aligned and the previous page is
non-existent.
- OpenSSL will incorrectly treat two BN_ULONG buffers as not equal when they
are equal.
- Side channel concerns.
The first is indeed a concern and is a DoS bug. The second is fine in this
context. bn_cmp_word and bn_cmp_part_words are used to compute abs(a0 - a1)
in Karatsuba. If a0 = a1, it does not matter whether we use a0 - a1 or
a1 - a0. The third would be worth thinking about, but it is overshadowed
by the entire Karatsuba implementation not being constant time.
Due to the difficulty of tripping this and the low impact no CVE is felt
necessary for this issue.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Viktor Dukhovni <viktor@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8326)
Richard Levitte [Thu, 21 Feb 2019 17:25:50 +0000 (18:25 +0100)]
Windows: Call TerminateProcess, not ExitProcess
Ty Baen-Price explains:
> Problem and Resolution:
> The following lines of code make use of the Microsoft API ExitProcess:
>
> ```
> Apps\Speed.c line 335: ExitProcess(ret);
> Ms\uplink.c line 22: ExitProcess(1);
> ```
>
> These function calls are made after fatal errors are detected and
> program termination is desired. ExitProcess(), however causes
> _orderly_ shutdown of a process and all its threads, i.e. it unloads
> all dlls and runs all destructors. See MSDN for details of exactly
> what happens
> (https://msdn.microsoft.com/en-us/library/windows/desktop/ms682658(v=vs.85).aspx).
> The MSDN page states that ExitProcess should never be called unless
> it is _known to be safe_ to call it. These calls should simply be
> replaced with calls to TerminateProcess(), which is what should be
> called for _disorderly_ shutdown.
>
> An example of usage:
>
> ```
> TerminateProcess(GetCurrentProcess(), exitcode);
> ```
>
> Effect of Problem:
> Because of a compilation error (wrong c++ runtime), my program
> executed the uplink.c ExitProcess() call. This caused the single
> OpenSSL thread to start executing the destructors of all my dlls,
> and their objects. Unfortunately, about 30 other threads were
> happily using those objects at that time, eventually causing a
> 0xC0000005 ACCESS_VIOLATION. Obviously an ACCESS_VIOLATION is the
> best case scenario, as I'm sure you can imagine at the consequences
> of undiscovered memory corruption, even in a terminating process.
And on the subject of `TerminateProcess()` being asynchronous:
> That is technically true, but I think it's probably synchronous
> "enough" for your purposes, since a call to TerminateProcess
> suspends execution of all threads in the target process. This means
> it's really only asynchronous if you're calling TerminateProcess one
> some _other_ process. If you're calling TerminateProcess on your own
> process, you'll never return from the TerminateProcess call.
Fixes #2489
Was originally RT-4526
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8301)
Matt Caswell [Thu, 21 Feb 2019 16:02:24 +0000 (16:02 +0000)]
Don't restrict the number of KeyUpdate messages we can process
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)
engines/dasync: add explaining comments about AES-128-CBC-HMAC-SHA1
Fixes #7950
It was reported that there might be a null pointer dereference in the
implementation of the dasync_aes_128_cbc_hmac_sha1() cipher, because
EVP_aes_128_cbc_hmac_sha1() can return a null pointer if AES-NI is
not available. It took some analysis to find out that this is not
an issue in practice, and these comments explain the reason to comfort
further NPD hunters.
Detected by GitHub user @wurongxin1987 using the Sourcebrella Pinpoint
static analyzer.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/8305)
projects / openssl / log