Fixed issue where DRBG_CTR fails if NO_DF is used - when entropy is called
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Kurt Roeckx <kurt@roeckx.be> Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/6778)
Andy Polyakov [Fri, 20 Jul 2018 11:23:42 +0000 (13:23 +0200)]
crypto/init.c: use destructor_key even as guard in OPENSSL_thread_stop.
Problem was that Windows threads that were terminating before libcrypto
was initialized were referencing uninitialized or possibly even
unrelated thread local storage index.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6752)
David Benjamin [Tue, 17 Jul 2018 17:20:28 +0000 (13:20 -0400)]
Remove zero special-case in BN_mod_exp_mont.
A number intended to treat the base as secret should not be branching on
whether it is zero. Test-wise, this is covered by existing tests in bnmod.txt.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6733)
asn1_encode : x, y => 0 | x,0 | y
(because of DER encoding rules when x and y have high bit set)
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/6694)
Richard Levitte [Mon, 23 Jul 2018 20:29:22 +0000 (22:29 +0200)]
def_load_bio(): Free |biosk| more carefully
If there's anything in the |biosk| stack, the first element is always
the input BIO. It should never be freed in this function, so we must
take careful steps not to do so inadvertently when freeing the stack.
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6769)
Richard Levitte [Mon, 23 Jul 2018 11:25:45 +0000 (13:25 +0200)]
Make sure the 'tsget' script is called 'tsget.pl' everywhere
The result is that we don't have to produce different names on
different platforms, and we won't have confusion on Windows depending
on if the script was built with mingw or with MSVC.
Partial fix for #3254
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6764)
Matt Caswell [Wed, 18 Jul 2018 14:22:06 +0000 (15:22 +0100)]
Add a note about aborts encountered while sending early_data
In some circumstances it is possible for a client to have a session
reporting a max early data value that is greater than the server will
support. In such cases the client could encounter an aborted connection.
Fixes #6735
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6740)
ecp_nistz256_set_from_affine is called when application attempts to use
custom generator, i.e. rarely. Even though it was wrong, it didn't
affect point operations, they were just not as fast as expected.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6738)
The ecp_nistz256_scatter_w7 function is called when application
attempts to use custom generator, i.e. rarely. Even though non-x86_64
versions were wrong, it didn't affect point operations, they were just
not as fast as expected.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6738)
Andy Polyakov [Sat, 21 Jul 2018 11:50:14 +0000 (13:50 +0200)]
apps/dsaparam.c: fix -C output.
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
(Merged from https://github.com/openssl/openssl/pull/6758)
Benjamin Kaduk [Wed, 30 May 2018 16:12:22 +0000 (11:12 -0500)]
Add TODO comment for a nonsensical public API
The API used to set what SNI value to send in the ClientHello
can also be used on server SSL objects, with undocumented and
un-useful behavior. Unfortunately, when generic SSL_METHODs
are used, s->server is still set, prior to the start of the
handshake, so we cannot prevent this nonsensical usage at the
present time. Leave a note to revisit this when ABI-breaking
changes are permitted.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6378)
Benjamin Kaduk [Wed, 30 May 2018 14:49:29 +0000 (09:49 -0500)]
Normalize SNI hostname handling for SSL and SSL_SESSION
In particular, adhere to the rule that we must not modify any
property of an SSL_SESSION object once it is (or might be) in
a session cache. Such modifications are thread-unsafe and have
been observed to cause crashes at runtime.
To effect this change, standardize on the property that
SSL_SESSION->ext.hostname is set only when that SNI value
has been negotiated by both parties for use with that session.
For session resumption this is trivially the case, so only new
handshakes are affected.
On the client, the new semantics are that the SSL->ext.hostname is
for storing the value configured by the caller, and this value is
used when constructing the ClientHello. On the server, SSL->ext.hostname
is used to hold the value received from the client. Only if the
SNI negotiation is successful will the hostname be stored into the
session object; the server can do this after it sends the ServerHello,
and the client after it has received and processed the ServerHello.
This obviates the need to remove the hostname from the session object
in case of failed negotiation (a change that was introduced in commit 9fb6cb810b769abbd60f11ef6e936a4e4456b19d in order to allow TLS 1.3
early data when SNI was present in the ClientHello but not the session
being resumed), which was modifying cached sessions in certain cases.
(In TLS 1.3 we always produce a new SSL_SESSION object for new
connections, even in the case of resumption, so no TLS 1.3 handshakes
were affected.)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6378)
Benjamin Kaduk [Wed, 30 May 2018 14:28:03 +0000 (09:28 -0500)]
const-ify some input SSL * arguments
These tiny functions only read from the input SSL, and we are
about to use them from functions that only have a const SSL* available,
so propagate const a bit further.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6378)
Andy Polyakov [Sun, 15 Jul 2018 15:59:59 +0000 (17:59 +0200)]
CHANGES: mention blinding reverting in ECDSA. [skip ci]
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6664)
Andy Polyakov [Thu, 12 Jul 2018 20:27:43 +0000 (22:27 +0200)]
ec/ecdsa_ossl.c: switch to fixed-length Montgomery multiplication.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6664)
Andy Polyakov [Fri, 6 Jul 2018 14:13:29 +0000 (16:13 +0200)]
ec/ecdsa_ossl.c: formatting and readability fixes.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6664)
Andy Polyakov [Fri, 6 Jul 2018 13:55:34 +0000 (15:55 +0200)]
ec/ecdsa_ossl.c: revert blinding in ECDSA signature.
Originally suggested solution for "Return Of the Hidden Number Problem"
is arguably too expensive. While it has marginal impact on slower
curves, none to ~6%, optimized implementations suffer real penalties.
Most notably sign with P-256 went more than 2 times[!] slower. Instead,
just implement constant-time BN_mod_add_quick.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6664)
Andy Polyakov [Mon, 16 Jul 2018 16:17:44 +0000 (18:17 +0200)]
bn/bn_lib.c address Coverity nit in bn2binpad.
It was false positive, but one can as well view it as readability issue.
Switch even to unsigned indices because % BN_BYTES takes 4-6 instructions
with signed dividend vs. 1 (one) with unsigned.
Matt Caswell [Tue, 17 Jul 2018 15:31:07 +0000 (16:31 +0100)]
Check that the public key OID matches the sig alg
Using the rsa_pss_rsae_sha256 sig alg should imply that the key OID is
rsaEncryption. Similarly rsa_pss_pss_sha256 implies the key OID is
rsassaPss. However we did not check this and incorrectly tolerated a key
OID that did not match the sig alg sent by the peer.
Fixes #6611
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6732)
Matt Caswell [Tue, 17 Jul 2018 10:43:30 +0000 (11:43 +0100)]
Skip the GOST test where appropriate
The GOST ciphers are dynamically loaded via the GOST engine, so we must
be able to support that. The engine also uses DSA and CMS symbols, so we
skip the test on no-dsa or no-cms.
Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6730)
Matt Caswell [Mon, 16 Jul 2018 13:57:35 +0000 (14:57 +0100)]
Don't remove sessions from the cache during PHA in TLSv1.3
If we issue new tickets due to post-handshake authentication there is no
reason to remove previous tickets from the cache. The code that did that
only removed the last session anyway - so if more than one ticket got
issued then those other tickets are still valid.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6722)
EC2M Lopez-Dahab ladder: use it also for ECDSA verify
By default `ec_scalar_mul_ladder` (which uses the Lopez-Dahab ladder
implementation) is used only for (k * Generator) or (k * VariablePoint).
ECDSA verification uses (a * Generator + b * VariablePoint): this commit
forces the use of `ec_scalar_mul_ladder` also for the ECDSA verification
path, while using the default wNAF implementation for any other case.
With this commit `ec_scalar_mul_ladder` loses the static attribute, and
is added to ec_lcl.h so EC_METHODs can directly use it.
While working on a new custom EC_POINTs_mul implementation, I realized
that many checks (e.g. all the points being compatible with the given
EC_GROUP, creating a temporary BN_CTX if `ctx == NULL`, check for the
corner case `scalar == NULL && num == 0`) were duplicated again and
again in every single implementation (and actually some
implementations lacked some of the tests).
I thought that it makes way more sense for those checks that are
independent from the actual implementation and should always be done, to
be moved in the EC_POINTs_mul wrapper: so this commit also includes
these changes.
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6690)
This commit uses the new ladder scaffold to implement a specialized
ladder step based on differential addition-and-doubling in mixed
Lopez-Dahab projective coordinates, modified to independently blind the
operands.
The arithmetic in `ladder_pre`, `ladder_step` and `ladder_post` is
auto generated with tooling:
- see, e.g., "Guide to ECC" Alg 3.40 for reference about the
`ladder_pre` implementation;
- see https://www.hyperelliptic.org/EFD/g12o/auto-code/shortw/xz/ladder/mladd-2003-s.op3
for the differential addition-and-doubling formulas implemented in
`ladder_step`;
- see, e.g., "Fast Multiplication on Elliptic Curves over GF(2**m)
without Precomputation" (Lopez and Dahab, CHES 1999) Appendix Alg Mxy
for the `ladder_post` implementation to recover the `(x,y)` result in
affine coordinates.
Co-authored-by: Billy Brumley <bbrumley@gmail.com> Co-authored-by: Sohaib ul Hassan <soh.19.hassan@gmail.com> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6690)
EC point multiplication: add `ladder` scaffold
for specialized Montgomery ladder implementations
PR #6009 and #6070 replaced the default EC point multiplication path for
prime and binary curves with a unified Montgomery ladder implementation
with various timing attack defenses (for the common paths when a secret
scalar is feed to the point multiplication).
The newly introduced default implementation directly used
EC_POINT_add/dbl in the main loop.
The scaffolding introduced by this commit allows EC_METHODs to define a
specialized `ladder_step` function to improve performances by taking
advantage of efficient formulas for differential addition-and-doubling
and different coordinate systems.
- `ladder_pre` is executed before the main loop of the ladder: by
default it copies the input point P into S, and doubles it into R.
Specialized implementations could, e.g., use this hook to transition
to different coordinate systems before copying and doubling;
- `ladder_step` is the core of the Montgomery ladder loop: by default it
computes `S := R+S; R := 2R;`, but specific implementations could,
e.g., implement a more efficient formula for differential
addition-and-doubling;
- `ladder_post` is executed after the Montgomery ladder loop: by default
it's a noop, but specialized implementations could, e.g., use this
hook to transition back from the coordinate system used for optimizing
the differential addition-and-doubling or recover the y coordinate of
the result point.
This commit also renames `ec_mul_consttime` to `ec_scalar_mul_ladder`,
as it better corresponds to what this function does: nothing can be
truly said about the constant-timeness of the overall execution of this
function, given that the underlying operations are not necessarily
constant-time themselves.
What this implementation ensures is that the same fixed sequence of
operations is executed for each scalar multiplication (for a given
EC_GROUP), with no dependency on the value of the input scalar.
Co-authored-by: Sohaib ul Hassan <soh.19.hassan@gmail.com> Co-authored-by: Billy Brumley <bbrumley@gmail.com> Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6690)
Run `make update ERROR_REBUILD=-rebuild` to remove some stale error
codes for SM2 (which is now using its own submodule for error codes,
i.e., `SM2_*`).
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6690)
Andy Polyakov [Sun, 4 Feb 2018 14:20:29 +0000 (15:20 +0100)]
bn/bn_lib.c: make BN_bn2binpad computationally constant-time.
"Computationally constant-time" means that it might still leak
information about input's length, but only in cases when input
is missing complete BN_ULONG limbs. But even then leak is possible
only if attacker can observe memory access pattern with limb
granularity.
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5254)
Matt Caswell [Wed, 4 Jul 2018 15:48:56 +0000 (16:48 +0100)]
As a server don't select TLSv1.3 if we're not capable of it
Check that we are either configured for PSK, or that we have a TLSv1.3
capable certificate type. DSA certs can't be used in TLSv1.3 and we
don't (currently) allow GOST ones either (owing to the lack of standard
sig algs).
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6650)
Change the description for BN_hex2bn() so that it uses the same BIGNUM argument name as its prototype.
CLA: trivial
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6712)
Andy Polyakov [Fri, 6 Jul 2018 13:13:15 +0000 (15:13 +0200)]
bn/bn_{mont|exp}.c: switch to zero-padded intermediate vectors.
Note that exported functions maintain original behaviour, so that
external callers won't observe difference. While internally we can
now perform Montogomery multiplication on fixed-length vectors, fixed
at modulus size. The new functions, bn_to_mont_fixed_top and
bn_mul_mont_fixed_top, are declared in bn_int.h, because one can use
them even outside bn, e.g. in RSA, DSA, ECDSA...
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6662)
Andy Polyakov [Fri, 6 Jul 2018 13:02:29 +0000 (15:02 +0200)]
bn/bn_lib.c: add BN_FLG_FIXED_TOP flag.
The new flag marks vectors that were not treated with bn_correct_top,
in other words such vectors are permitted to be zero padded. For now
it's BN_DEBUG-only flag, as initial use case for zero-padded vectors
would be controlled Montgomery multiplication/exponentiation, not
general purpose. For general purpose use another type might be more
appropriate. Advantage of this suggestion is that it's possible to
back-port it...
bn/bn_div.c: fix memory sanitizer problem.
bn/bn_sqr.c: harmonize with BN_mul.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6662)
Andy Polyakov [Fri, 6 Jul 2018 12:54:34 +0000 (14:54 +0200)]
bn/bn_mont.c: improve readability of post-condition code.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6662)
Andy Polyakov [Fri, 6 Jul 2018 11:46:07 +0000 (13:46 +0200)]
bn/bn_mont.c: move boundary condition check closer to caller.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6662)
Andy Polyakov [Fri, 6 Jul 2018 11:16:40 +0000 (13:16 +0200)]
bn/bn_lib.c: remove bn_check_top from bn_expand2.
Trouble is that addition is postponing expansion till carry is
calculated, and if addition carries, top word can be zero, which
triggers assertion in bn_check_top.
Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/6662)
Patrick Steuer [Fri, 2 Feb 2018 10:09:25 +0000 (11:09 +0100)]
apps/speed.c: let EVP_Update_loop_ccm behave more like EVP_Update_loop
Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5246)
Richard Levitte [Wed, 11 Jul 2018 09:05:15 +0000 (11:05 +0200)]
Windows: avoid using 'rem' in the nmake makefile
To avoid the possibility that someone creates rem.exe, rem.bat or
rem.cmd, simply don't use it. In the cases it was used, it was to
avoid empty lines, but it turns out that nmake handles those fine, so
no harm done.
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/6686)
Richard Levitte [Tue, 10 Jul 2018 12:12:33 +0000 (14:12 +0200)]
Windows: fix echo for nmake
It seems that nmake first tries to run executables on its own, and
only pass commands to cmd if that fails. That means it's possible to
have nmake run something like 'echo.exe' when the builtin 'echo'
command was expected, which might give us unexpected results.
To get around this, we create our own echoing script and call it
explicitly from the nmake makefile.
Fixes #6670
Reviewed-by: Andy Polyakov <appro@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/6686)
The sense of the check for build-time support for most hashes was inverted.
CLA: trivial
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/6673)
Richard Levitte [Sun, 8 Jul 2018 10:00:06 +0000 (12:00 +0200)]
Keep supporting the env / make variable PERL
OpenSSL 1.1.0 supports the use of this environment variable for
passing to the build files. For the sake of backward compatibility,
we keep it.
Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/6668)