Ivan Filenko [Sun, 25 Feb 2018 13:49:27 +0000 (16:49 +0300)]
Fix typo in ASN1_STRING_length doc
CLA: trivial
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/5458)
bio_b64.c: prevent base64 filter BIO from decoding out-of-bound data
Fixes #5405, #1381
The base64 filter BIO reads its input in chunks of B64_BLOCK_SIZE bytes.
When processing input in PEM format it can happen in rare cases that
- the trailing PEM marker crosses the boundary of a chunk, and
- the beginning of the following chunk contains valid base64 encoded data.
This happened in issue #5405, where the PEM marker was split into
"-----END CER" and "TIFICATE-----" at the end of the first chunk.
The decoding of the first chunk terminated correctly at the '-' character,
which is treated as an EOF marker, and b64_read() returned. However,
when called the second time, b64_read() read the next chunk and interpreted
the string "TIFICATE" as valid base64 encoded data, adding 6 extra bytes
'4c 81 48 08 04 c4'.
This patch restores the assignment of the error code to 'ctx->cont', which
was deleted accidentally in commit 5562cfaca4f3 and which prevents b64_read()
from reading additional data on subsequent calls.
This issue was observed and reported by Annie Yousar.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5422)
Pavel Kopyl [Sun, 10 Dec 2017 19:57:43 +0000 (22:57 +0300)]
do_body: fix heap-use-after-free.
The memory pointed to by the 'push' is freed by the
X509_NAME_ENTRY_free() in do_body(). The second time
it is referenced to (indirectly) in certify_cert:X509_REQ_free().
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4896)
X509v3_add_ext: free 'sk' if the memory pointed to by it
was malloc-ed inside this function.
X509V3_EXT_add_nconf_sk: return an error if X509v3_add_ext() fails.
This prevents use of a freed memory in do_body:sk_X509_EXTENSION_num().
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4896)
Andy Polyakov [Thu, 1 Feb 2018 21:03:59 +0000 (22:03 +0100)]
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.
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)
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/5192)
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/5115)
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)
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/5019)
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 794bf5f756ad4748735e9b333c40d2b1bf685c36)
Richard Levitte [Sat, 18 Nov 2017 16:54:57 +0000 (17:54 +0100)]
Configure: use a better method to identify gcc and derivates
Looking for 'gcc' and 'clang' in the output from the C compiler is
uncertain. Some versions report argv[0], which might be /usr/bin/cc
(for example), and others might mention gcc without being gcc or a
derivate.
Better then to fetch predefined macros and checking if __GNUC__ and
__clang__ are defined.
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4755)
FdaSilvaYY [Fri, 8 Dec 2017 15:25:38 +0000 (10:25 -0500)]
Fix an incoherent test.
Pointer 'o' is set inside a local buffer, so it can't be NULL.
Also fix coding style and add comments
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4754)
(cherry picked from commit cef115ff0ca4255d3decc1dda87c5418a961fd2c)
The call to FIPS_crypto_set_id_callback() was added in revision a43cfd7bb1fc681d563e,
but there is no prototype for it in <openssl/fips.h>.
Signed-off-by: Dr. Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4870)
Matt Caswell [Wed, 29 Nov 2017 14:04:01 +0000 (14:04 +0000)]
Don't allow read/write after fatal error
OpenSSL 1.0.2 (starting from version 1.0.2b) introduced an "error state"
mechanism. The intent was that if a fatal error occurred during a handshake
then OpenSSL would move into the error state and would immediately fail if
you attempted to continue the handshake. This works as designed for the
explicit handshake functions (SSL_do_handshake(), SSL_accept() and
SSL_connect()), however due to a bug it does not work correctly if
SSL_read() or SSL_write() is called directly. In that scenario, if the
handshake fails then a fatal error will be returned in the initial function
call. If SSL_read()/SSL_write() is subsequently called by the application
for the same SSL object then it will succeed and the data is passed without
being decrypted/encrypted directly from the SSL/TLS record layer.
In order to exploit this issue an attacker would have to trick an
application into behaving incorrectly by issuing an SSL_read()/SSL_write()
after having already received a fatal error.
Thanks to David Benjamin (Google) for reporting this issue and suggesting
this fix.
MerQGh [Mon, 4 Dec 2017 06:20:51 +0000 (09:20 +0300)]
Update eng_fat.c
This line will allow use private keys, which created by Crypto Pro, to
sign with OpenSSL.
CLA: trivial
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4836)
Andy Polyakov [Sat, 11 Nov 2017 15:27:48 +0000 (16:27 +0100)]
Configure: add back /WX to VC-WIN32.
We had /WX (treat warnings as errors) in VC-WIN32 for long time. At
some point it was somehow omitted. It's argued that it allows to
keep better focus on new code, which motivates the comeback...
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4718)
Long Qin [Tue, 7 Nov 2017 06:59:20 +0000 (14:59 +0800)]
lhash.c: Replace Unicode EN DASH with the ASCII char '-'.
* addressing", Proc. 6th Conference on Very Large Databases: 212–223
^
The EN DASH ('–') in this line is one UTF-8 character (hex: e2 80 93).
Under some code page setting (e.g. 936), Visual Studio may report C4819
warning: The file contains a character that cannot be represented in the
current code page.
Replace this character with the ASCII char '-' (Hex Code: 2D).
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4691)
Richard Levitte [Fri, 10 Nov 2017 12:26:10 +0000 (13:26 +0100)]
ssltest.c: cb_ticket2 appears to not return a value when it "should"
cb_ticket2() does an exit, and should therefore not need to return anything.
Some compilers don't detect that, or don't care, and warn about a non-void
function without a return statement.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4713)
Andy Polyakov [Sun, 5 Nov 2017 16:08:16 +0000 (17:08 +0100)]
{aes-armv4|bsaes-armv7|sha256-armv4}.pl: make it work with binutils-2.29
It's not clear if it's a feature or bug, but binutils-2.29[.1]
interprets 'adr' instruction with Thumb2 code reference differently,
in a way that affects calculation of addresses of constants' tables.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/4673)
Matt Caswell [Mon, 6 Nov 2017 11:18:35 +0000 (11:18 +0000)]
Don't error with -1 for BIGNUM exp operations
The man pages say that BIGNUM arithmetic operations fail with a 0 return.
However some functions were returning -1 on error. In master and 1.1.0 they
already return 0, so this brings 1.0.2 in line.
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/4682)
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4670)
David Benjamin [Mon, 23 Oct 2017 23:13:05 +0000 (19:13 -0400)]
Fix weak digest in TLS 1.2 with SNI.
1ce95f19601bbc6bfd24092c76c8f8105124e857 was incomplete and did not
handle the case when SSL_set_SSL_CTX was called from the cert_cb
callback rather than the SNI callback. The consequence is any server
using OpenSSL 1.0.2 and the cert_cb callback for SNI only ever signs a
weak digest, SHA-1, even when connecting to clients which use secure
ones.
Fix this and add regression tests for both this and the original issue.
Fixes #4554.
Reviewed-by: Emilia Käsper <emilia@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4577)
Matt Caswell [Fri, 20 Oct 2017 16:11:03 +0000 (17:11 +0100)]
Don't use strcasecmp and strncasecmp for IA5 strings
The functions strcasecmp() and strncasecmp() will use locale specific rules
when performing comparison. This could cause some problems in certain
locales. For example in the Turkish locale an 'I' character is not the
uppercase version of 'i'. However IA5 strings should not use locale specific
rules, i.e. for an IA5 string 'I' is uppercase 'i' even if using the
Turkish locale.
This fixes a bug in name constraints checking reported by Thomas Pornin
(NCCGroup).
This is not considered a security issue because it would require both a
Turkish locale (or other locale with similar issues) and malfeasance by
a trusted name-constrained CA for a certificate to pass name constraints
in error. The constraints also have to be for excluded sub-trees which are
extremely rare. Failure to match permitted subtrees is a bug, not a
vulnerability.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4570)
Matt Caswell [Wed, 18 Oct 2017 13:07:57 +0000 (14:07 +0100)]
Don't make any changes to the lhash structure if we are going to fail
The lhash expand() function can fail if realloc fails. The previous
implementation made changes to the structure and then attempted to do a
realloc. If the realloc failed then it attempted to undo the changes it
had just made. Unfortunately changes to lh->p were not undone correctly,
ultimately causing subsequent expand() calls to increment num_nodes to a
value higher than num_alloc_nodes, which can cause out-of-bounds reads/
writes. This is not considered a security issue because an attacker cannot
cause realloc to fail.
This commit moves the realloc call to near the beginning of the function
before any other changes are made to the lhash structure. That way if a
failure occurs we can immediately fail without having to undo anything.
Thanks to Pavel Kopyl (Samsung) for reporting this issue.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4551)
Richard Levitte [Tue, 24 Oct 2017 11:42:41 +0000 (13:42 +0200)]
asn1_item_embed_new(): don't free an embedded item
The previous change with this intention didn't quite do it. An
embedded item must not be freed itself, but might potentially contain
non-embedded elements, which must be freed.
So instead of calling ASN1_item_ex_free(), where we can't pass the
combine flag, we call asn1_item_embed_free() directly.
This changes asn1_item_embed_free() from being a static function to
being a private non-static function.
Xiangyu Bu [Wed, 18 Oct 2017 00:10:53 +0000 (17:10 -0700)]
Fix memory leak in GENERAL_NAME_set0_othername.
CLA: trivial
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4544)
Rich Salz [Thu, 19 Oct 2017 12:22:12 +0000 (08:22 -0400)]
Additional name for all commands
Add openssl-foo as a name for the openssl "foo" command.
Recommended by a usability study conducted by Martin Ukrop at CRoCS, FI MU Fixes: #4548 Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/4557)
Matt Caswell [Wed, 27 Sep 2017 10:13:47 +0000 (11:13 +0100)]
Ensure we test all parameters for BN_FLG_CONSTTIME
RSA_setup_blinding() calls BN_BLINDING_create_param() which later calls
BN_mod_exp() as follows:
BN_mod_exp(ret->A, ret->A, ret->e, ret->mod, ctx)
ret->mod will have BN_FLG_CONSTTIME set, but ret->e does not. In
BN_mod_exp() we only test the third param for the existence of this flag.
We should test all the inputs.
Thanks to Samuel Weiser (samuel.weiser@iaik.tugraz.at) for reporting this
issue.
This typically only happens once at key load, so this is unlikely to be
exploitable in any real scenario.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4477)
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Stephen Henson <steve@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4453)
Functions to retrieve the function pointer of an existing method: this
can be used to create a method which intercepts or modifies the behaviour
of an existing method while retaining most of the existing behaviour.
Bernd Edlinger [Mon, 2 Oct 2017 15:24:17 +0000 (17:24 +0200)]
Fix the return type of felem_is_zero_int which should be int.
Change argument type of xxxelem_is_zero_int to const void*
to avoid the need of type casts.
Fixes #4413
Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4450)
David Benjamin [Mon, 18 Sep 2017 19:58:41 +0000 (15:58 -0400)]
Fix overflow in c2i_ASN1_BIT_STRING.
c2i_ASN1_BIT_STRING takes length as a long but uses it as an int. Check
bounds before doing so. Previously, excessively large inputs to the
function could write a single byte outside the target buffer. (This is
unreachable as asn1_ex_c2i already uses int for the length.)
Thanks to NCC for finding this issue. Fix written by Martin Kreichgauer.
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Andy Polyakov <appro@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/4385)