Richard Levitte [Thu, 12 Sep 2019 21:58:07 +0000 (23:58 +0200)]
Build files: Unify standard arguments for assembler generating scrips
- Make the last argument always be the output file.
- Make the first argument always be the flavour, even if there is no
flavour (i.e. it might become the empty string).
- Make the next to last argument to be $(PROCESSOR) if that one has a
value.
- Remaining arguments are C prepropressor arguments.
Perl scripts that should handle this may use the following code:
$output = pop;
$flavour = shift;
if ($ARGV[$#ARGV] eq '386') {
# Do 386 specific things
} else {
# Do whatever else, with the knowledge the @ARGV contains
# C preprocessor arguments
}
Some scripts don't care about anything than $output, and that's ok.
Some scripts do care, but handle it a little differently, and that's
ok too (notably, the x86 scripts call asm_init() with the first and
the last argument after having popped $output).
As long as they handle the argument order right, they are going to
be fine.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9884)
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/9894)
Fix S390X bad size_t that causes memory trash in legacy ciphers
This caused a SEGV inside tls13_enc() when using chacha_poly.
The tls code assigns the iv_length to a size_t (even though it is an int).
This is actually really bad since it could be -1, which will then trash the iv buffer.
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/9890)
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9870)
Richard Levitte [Fri, 26 Jul 2019 16:11:55 +0000 (18:11 +0200)]
Deprecate ERR_get_state()
Internally, we still need this function, so we make it internal and
then add a new ERR_get_state() that simply calls the internal variant,
unless it's "removed" by configuration.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9462)
Richard Levitte [Sun, 25 Aug 2019 08:46:22 +0000 (10:46 +0200)]
confdata.pm.in: New template for configdata.pm
To have the configdata.pm text embedded in Configure was kind of ugly,
and becomes clearer if put into a template file, configdata.pm.in. We
can then use OpenSSL::Template to generate it.
We also modify configdata.pm to be the build file generator, and run
it from Configure. The benefit with that is that developers who
tinker and play with the build file can do a "factory reset" without
having to go through the configuration process, i.e. they can re-use
the config data the already have.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9693)
Richard Levitte [Sun, 25 Aug 2019 08:44:41 +0000 (10:44 +0200)]
util/dofile.pl, util/perl/OpenSSL/Template.pm: move parts of dofile.pl
We make a module OpenSSL::Template from the central parts of
util/dofile.pl, and also reduce the amount of ugly code with more
proper use of Text::Template. OpenSSL::Template is a simply subclass
of Text::Template.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9693)
Richard Levitte [Wed, 4 Sep 2019 20:14:21 +0000 (22:14 +0200)]
Modernise ERR_print_errors_cb()
ERR_print_errors_cb() used functionality that isn't suitable any more,
as that functionality couldn't integrate the error record function
name strings. We therefore refactor it a bit to use better adapted
methods.
Fixes #9756
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9756)
Richard Levitte [Wed, 4 Sep 2019 20:04:08 +0000 (22:04 +0200)]
Modernise the ERR functionality further (new functions and deprecations)
ERR_func_error_string() essentially returns NULL, and since all
function codes are now removed for all intents and purposes, this
function has fallen out of use and cannot be modified to suit the
data, since its only function is to interpret an error code.
To compensate for the loss of error code, we instead provide new
functions that extracts the function name strings from an error
record:
Similarly, the once all encompasing functions
ERR_peek_last_error_line_data(), ERR_peek_error_line_data() and
ERR_get_error_line_data() lack the capability of getting the function
name string, so we deprecate those and add these functions to replace
them:
Richard Levitte [Wed, 4 Sep 2019 19:55:58 +0000 (21:55 +0200)]
include/openssl/err.h: Depend on OPENSSL_NO_FILENAMES, not OPENSSL_NO_ERR
The configuration option 'no-err' is documented to be used to avoid
loading error related string tables. For some reason, it was also
used to define if ERR_PUT_error() would pass the source file name and
line information or not.
The configuration option 'no-filenames' is documented to be used to
avoid passing the source file name and line anywhere. So, the
definition of ERR_PUT_error() should depend on OPENSSL_NO_FILENAMES
rather than OPENSSL_NO_ERR.
Furthermore, the definition of OPENSSL_FILE and OPENSSL_LINE depends
on if OPENSSL_NO_FILENAMES is defined or not, so there was never any
need to do extra macro gymnastics in include/openssl/err.h, so we
simply remove it and use OPENSSL_FILE and OPENSSL_LINE directly.
Finally, the macro OPENSSL_FUNC is unaffected by all these
configuration options, so it should be used in all macros that call
ERR_set_debug().
Fixes #9756
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9756)
Richard Levitte [Wed, 4 Sep 2019 19:51:17 +0000 (21:51 +0200)]
include/openssl/macros.h: Remove the PEDANTIC OPENSSL_FUNC definition
There was a section to define OPENSSL_FUNC that depended on PEDANTIC
being defined. That is an internal build macro that should never
appear in a public header. The solution was simple, replace it with
a check of __STRICT_ANSI__.
Fixes #9756
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9756)
Richard Levitte [Wed, 4 Sep 2019 19:45:56 +0000 (21:45 +0200)]
util/mkerr.pl: make it not depend on the function code
The output C code was made to use ERR_func_error_string() to see if a
string table was already loaded or not. Since this function returns
NULL always, this check became useless.
Change it to use ERR_reason_error_string() instead, as there's no
reason to believe we will get rid of reason strings, ever.
To top it off, we rebuild all affected C sources.
Fixes #9756
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9756)
Richard Levitte [Wed, 11 Sep 2019 09:05:11 +0000 (11:05 +0200)]
Rework test/run_tests.pl to support selective verbosity and TAP copy
This includes a complete rework of how we use TAP::Harness, by adding
a TAP::Parser subclass that allows additional callbacks to be passed
to perform what we need. The TAP::Parser callbacks we add are:
ALL to print all the TAP output to a file (conditionally)
to collect all the TAP output to an array (conditionally)
EOF to print all the collected TAP output (if there is any)
if any subtest failed
To get TAP output to file, the environment variable HARNESS_TAP_COPY
must be defined, with a file name as value. That file will be
overwritten unconditionally.
To get TAP output displayed on failure, the make variable VERBOSE_FAILURE
or VF must be defined with a non-emoty value.
Additionally, the output of test recipe names has been changed to only
display its basename.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9862)
Richard Levitte [Sat, 31 Aug 2019 07:30:43 +0000 (09:30 +0200)]
doc/man3/OSSL_PARAM.pod: add details about multiple elements with same key
Usually, each element in an OSSL_PARAM array will have a unique key.
However, there may be some rare cases when a responder will handle
multiple elements with the same key. This adds a short passage
explaining this case.
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9741)
Richard Levitte [Mon, 9 Sep 2019 09:51:01 +0000 (11:51 +0200)]
Rework the perl fallback functionality
The module with_fallback.pm was kind of clunky and required a transfer
module. This change replaces if with a much more generic pragma type
module, which simply appends given directories to @INC (as opposed to
the 'lib' pragma, which prepends the directories to @INC).
This also supports having a file MODULES.txt with sub-directories to
modules. This ensures that we don't have to spray individual module
paths throughout our perl code, but can have them collected in one
place.
(do note that there is a 'fallback' module on CPAN. However, it isn't
part of the core perl, and it has no support the any MODULES.txt kind
of construct)
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9826)
Matt Caswell [Wed, 11 Sep 2019 14:03:39 +0000 (15:03 +0100)]
Avoid passing NULL to memcpy
It is undefined behaviour to send NULL as either the src, or dest params
in memcpy.
In pkey_kdf.c we had a check to ensure that the src address is non-NULL.
However in some situations it is possible that the dest address could also
be NULL. Specifically in the case where the datalen is 0 and we are using
a newly allocated BUF_MEM.
We add a check of datalen to avoid the undefined behaviour.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9868)
Matt Caswell [Wed, 11 Sep 2019 10:52:47 +0000 (11:52 +0100)]
Pass the correct ctx to provider KDF functions
Make sure we pass the provider side ctx and not the libcrypto side ctx.
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9865)
drbg: fix issue where DRBG_CTR fails if NO_DF is used (2nd attempt)
Since commit 7c226dfc434d a chained DRBG does not add additional
data anymore when reseeding from its parent. The reason is that
the size of the additional data exceeded the allowed size when
no derivation function was used.
This commit provides an alternative fix: instead of adding the
entire DRBG's complete state, we just add the DRBG's address
in memory, thereby providing some distinction between the different
DRBG instances.
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
drbg: ensure fork-safety without using a pthread_atfork handler
When the new OpenSSL CSPRNG was introduced in version 1.1.1,
it was announced in the release notes that it would be fork-safe,
which the old CSPRNG hadn't been.
The fork-safety was implemented using a fork count, which was
incremented by a pthread_atfork handler. Initially, this handler
was enabled by default. Unfortunately, the default behaviour
had to be changed for other reasons in commit b5319bdbd095, so
the new OpenSSL CSPRNG failed to keep its promise.
This commit restores the fork-safety using a different approach.
It replaces the fork count by a fork id, which coincides with
the process id on UNIX-like operating systems and is zero on other
operating systems. It is used to detect when an automatic reseed
after a fork is necessary.
To prevent a future regression, it also adds a test to verify that
the child reseeds after fork.
CVE-2019-1549
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9832)
Bernd Edlinger [Sat, 31 Aug 2019 22:16:28 +0000 (00:16 +0200)]
Fix a padding oracle in PKCS7_dataDecode and CMS_decrypt_set1_pkey
An attack is simple, if the first CMS_recipientInfo is valid but the
second CMS_recipientInfo is chosen ciphertext. If the second
recipientInfo decodes to PKCS #1 v1.5 form plaintext, the correct
encryption key will be replaced by garbage, and the message cannot be
decoded, but if the RSA decryption fails, the correct encryption key is
used and the recipient will not notice the attack.
As a work around for this potential attack the length of the decrypted
key must be equal to the cipher default key length, in case the
certifiate is not given and all recipientInfo are tried out.
The old behaviour can be re-enabled in the CMS code by setting the
CMS_DEBUG_DECRYPT flag.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9777)
Due to the dynamic allocation that was added to rand_pool_add_begin
this function could now return a null pointer where it was previously
guaranteed to succeed. But the return value of this function does
not need to be checked by design.
Move rand_pool_grow from rand_pool_add_begin to rand_pool_bytes_needed.
Make an allocation error persistent to avoid falling back to less secure
or blocking entropy sources.
Fixes: a6a66e4511ee ("Make rand_pool buffers more dynamic in their sizing.") 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/9687)
Bernd Edlinger [Sat, 24 Aug 2019 09:38:32 +0000 (11:38 +0200)]
Fix a strict warnings error in rand_pool_acquire_entropy
There was a warning about unused variables in this config:
./config --strict-warnings --with-rand-seed=rdcpu
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/9687)
Matt Caswell [Wed, 4 Sep 2019 11:46:02 +0000 (12:46 +0100)]
Revise EVP_PKEY param handling
We add new functions for getting parameters and discovering the gettable
and settable parameters. We also make EVP_PKEY_CTX_get_signature_md() a
function and implement it in terms of the new functions.
This enables applications to discover the set of parameters that are
supported for a given algorithm implementation.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9753)
Matt Caswell [Fri, 30 Aug 2019 12:33:10 +0000 (13:33 +0100)]
Add the ability to perform signatures in a provider
This makes EVP_PKEY_sign and EVP_PKEY_sign_init provider aware. It
also introduces the new type EVP_SIGNATURE to represent signature
algorithms. This also automatically makes the EVP_Sign* APIs provider
aware because they use EVP_Digest* (which is already provider aware)
and EVP_PKEY_sign(_init) under the covers.
At this stage there are no signature algorithms in any providers. That
will come in the following commits.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9753)
- Replace a `TEST_true()` with `!TEST_false()` to avoid reporting
confusing errors
- We tend to use `if (!TEST_foo() || !TEST_bar())` and it's a bit
confusing to switch to `if(!(TEST_foo() && TEST_bar()))`: replace it
with the more common style
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/9813)
Jakub Zelenka [Sun, 8 Sep 2019 16:32:07 +0000 (17:32 +0100)]
Fix typos in the OSSL_METHOD_STORE doc
Reviewed-by: Paul Yang <kaishen.yy@antfin.com> Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9825)
[ec] Match built-in curves on EC_GROUP_new_from_ecparameters
Description
-----------
Upon `EC_GROUP_new_from_ecparameters()` check if the parameters match any
of the built-in curves. If that is the case, return a new
`EC_GROUP_new_by_curve_name()` object instead of the explicit parameters
`EC_GROUP`.
This affects all users of `EC_GROUP_new_from_ecparameters()`:
- direct calls to `EC_GROUP_new_from_ecparameters()`
- direct calls to `EC_GROUP_new_from_ecpkparameters()` with an explicit
parameters argument
- ASN.1 parsing of explicit parameters keys (as it eventually
ends up calling `EC_GROUP_new_from_ecpkparameters()`)
A parsed explicit parameter key will still be marked with the
`OPENSSL_EC_EXPLICIT_CURVE` ASN.1 flag on load, so, unless
programmatically forced otherwise, if the key is eventually serialized
the output will still be encoded with explicit parameters, even if
internally it is treated as a named curve `EC_GROUP`.
Before this change, creating any `EC_GROUP` object using
`EC_GROUP_new_from_ecparameters()`, yielded an object associated with
the default generic `EC_METHOD`, but this was never guaranteed in the
documentation.
After this commit, users of the library that intentionally want to
create an `EC_GROUP` object using a specific `EC_METHOD` can still
explicitly call `EC_GROUP_new(foo_method)` and then manually set the
curve parameters using `EC_GROUP_set_*()`.
Motivation
----------
This has obvious performance benefits for the built-in curves with
specialized `EC_METHOD`s and subtle but important security benefits:
- the specialized methods have better security hardening than the
generic implementations
- optional fields in the parameter encoding, like the `cofactor`, cannot
be leveraged by an attacker to force execution of the less secure
code-paths for single point scalar multiplication
- in general, this leads to reducing the attack surface
Check the manuscript at https://arxiv.org/abs/1909.01785 for an in depth
analysis of the issues related to this commit.
It should be noted that `libssl` does not allow to negotiate explicit
parameters (as per RFC 8422), so it is not directly affected by the
consequences of using explicit parameters that this commit fixes.
On the other hand, we detected external applications and users in the
wild that use explicit parameters by default (and sometimes using 0 as
the cofactor value, which is technically not a valid value per the
specification, but is tolerated by parsers for wider compatibility given
that the field is optional).
These external users of `libcrypto` are exposed to these vulnerabilities
and their security will benefit from this commit.
Related commits
---------------
While this commit is beneficial for users using built-in curves and
explicit parameters encoding for serialized keys, commit b783beeadf6b80bc431e6f3230b5d5585c87ef87 (and its equivalents for the
1.0.2, 1.1.0 and 1.1.1 stable branches) fixes the consequences of the
invalid cofactor values more in general also for other curves
(CVE-2019-1547).
The following list covers commits in `master` that are related to the
vulnerabilities presented in the manuscript motivating this commit:
- d2baf88c43 [crypto/rsa] Set the constant-time flag in multi-prime RSA too
- 311e903d84 [crypto/asn1] Fix multiple SCA vulnerabilities during RSA key validation.
- b783beeadf [crypto/ec] for ECC parameters with NULL or zero cofactor, compute it
- 724339ff44 Fix SCA vulnerability when using PVK and MSBLOB key formats
Note that the PRs that contributed the listed commits also include other
commits providing related testing and documentation, in addition to
links to PRs and commits backporting the fixes to the 1.0.2, 1.1.0 and
1.1.1 branches.
Responsible Disclosure
----------------------
This and the other issues presented in https://arxiv.org/abs/1909.01785
were reported by Cesar Pereida GarcĂa, Sohaib ul Hassan, Nicola Tuveri,
Iaroslav Gridin, Alejandro Cabrera Aldaya and Billy Bob Brumley from the
NISEC group at Tampere University, FINLAND.
The OpenSSL Security Team evaluated the security risk for this
vulnerability as low, and encouraged to propose fixes using public Pull
Requests.
Configure: clang: move -Wno-unknown-warning-option to the front
While gcc ignores unknown options of the type '-Wno-xxx', clang by default issues
a warning [-Wunknown-warning-option] (see [3]), which together with '-Werror'
causes the build to fail. This turned out to be a problem on the 1.0.2 stable branch
in the case of the '-Wextended-offsetof' option, which was removed in version 6.0.0,
but needs to be kept here in order to support older clang versions, too (see #9446).
Incidentally, master and 1.1.1 branch already contained the -Wno-unknown-warning-option
option. Due to its special role and its importance, this commit adds an explaining
commit message and moves the option to the front.
[extended tests]
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/9447)
Billy Brumley [Fri, 6 Sep 2019 14:26:40 +0000 (17:26 +0300)]
CHANGES entry: for ECC parameters with NULL or zero cofactor, compute it
This is a forward port from https://github.com/openssl/openssl/pull/9781
of the CHANGES entry for the functionality added in
https://github.com/openssl/openssl/pull/9727
Billy Brumley [Fri, 6 Sep 2019 14:26:08 +0000 (17:26 +0300)]
[test] computing ECC cofactors: regression test
This is a forward port from
https://github.com/openssl/openssl/pull/9781
for the test logic introduced by
https://github.com/openssl/openssl/pull/9727
As @mattcaswell commented
(https://github.com/openssl/openssl/pull/9781#discussion_r321621541):
> These `TEST_true` calls should be `!TEST_false` because we are
> *expecting* a failure.
> The difference is that the test framework will print error details if
> the test doesn't give the expected answer.
> So by using `TEST_true` instead of `!TEST_false` we'll get error
> details printed, but the test will succeed anyway.