This should fix 2 issues detected by Coverity and introduced with
https://github.com/openssl/openssl/pull/11807
- CID 1463577: Memory - corruptions (ARRAY_VS_SINGLETON)
- CID 1463573: Memory - corruptions (ARRAY_VS_SINGLETON)
In practice the tests seem to show that they both aren't real issues,
yet I believe this small change should appease the scanner and at the
same time improve clarity for the reader.
Here is the original report:
```
** CID 1463577: Memory - corruptions (ARRAY_VS_SINGLETON)
________________________________________________________________________________________________________
*** CID 1463577: Memory - corruptions (ARRAY_VS_SINGLETON)
/crypto/ec/ec_lib.c: 1123 in EC_POINT_mul()
1117
1118 if (group->meth->mul != NULL)
1119 ret = group->meth->mul(group, r, g_scalar, point != NULL
1120 && p_scalar != NULL, &point, &p_scalar, ctx);
1121 else
1122 /* use default */
CID 1463577: Memory - corruptions (ARRAY_VS_SINGLETON)
Passing "&point" to function "ec_wNAF_mul" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
1123 ret = ec_wNAF_mul(group, r, g_scalar, point != NULL
1124 && p_scalar != NULL, &point, &p_scalar, ctx);
1125
1126 #ifndef FIPS_MODULE
1127 BN_CTX_free(new_ctx);
1128 #endif
** CID 1463573: Memory - corruptions (ARRAY_VS_SINGLETON)
________________________________________________________________________________________________________
*** CID 1463573: Memory - corruptions (ARRAY_VS_SINGLETON)
/crypto/ec/ec_lib.c: 1123 in EC_POINT_mul()
1117
1118 if (group->meth->mul != NULL)
1119 ret = group->meth->mul(group, r, g_scalar, point != NULL
1120 && p_scalar != NULL, &point, &p_scalar, ctx);
1121 else
1122 /* use default */
CID 1463573: Memory - corruptions (ARRAY_VS_SINGLETON)
Passing "&p_scalar" to function "ec_wNAF_mul" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.
1123 ret = ec_wNAF_mul(group, r, g_scalar, point != NULL
1124 && p_scalar != NULL, &point, &p_scalar, ctx);
1125
1126 #ifndef FIPS_MODULE
1127 BN_CTX_free(new_ctx);
1128 #endif
```
Reviewed-by: Kurt Roeckx <kurt@roeckx.be>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11919)
At various points we need to be able to retrieve the current library
context so we store it in the ECX_KEY structure.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11635)
ECX keys can very easily crete the public key from the private key.
Therefore when we import ecx keys it is sufficent to just have the private
key.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11635)
This macro is used to determine if certain pieces of code should
become part of the FIPS module or not. The old name was confusing.
Fixes#11538
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11539)
Previously import_to just took an EVP_PKEY as the argument. However we
need to some additional context data as well - specifically the libctx.
Therefore we pass an EVP_PKEY_CTX instead to hold the combination of
both of these things.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11536)
Don't wrap conditionally-compiled files in global ifndef tests.
Instead, test if the feature is disabled and, if so, do not
compile it.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11263)
The Ed448 private key deconding needs to use a library ctx. So we
implement a priv_decode_with_libctx function for it.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11494)
The macros are converted to functions, and are modified to support
provider implementations.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11328)
Ed25519 needs to fetch a digest and so needs to use the correct libctx.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11496)
The EC export_to function calls EC_POINT_point2buf that can later
generate a random number in some circumstances. Therefore we pass in a
BN_CTX associated with the library context. This means we have to change
the export_to function signature to accept the library context.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11493)
This was pointed out by a false-positive
-fsanitizer warning ;-)
However from the cryptographical POV the
code is wrong:
A point R^0 on the wrong curve
is infinity on the wrong curve.
[extended tests]
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11475)
a9612d6c03 introduced possible memory leaks in EC_GROUP_cmp and EC_POINTs_mul, and a possible BN_CTX_end without BN_CTX_start in ec_field_inverse_mod_ord.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11452)
- Convert to affine coords on ladder entry. This lets us use more efficient
ladder step formulae.
- Convert to affine coords on ladder exit. This prevents the current code
awkwardness where conversion happens twice during serialization: first to
fetch the buffer size, then again to fetch the coords.
- Instead of projectively blinding the input point, blind both accumulators
independently.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
(Merged from https://github.com/openssl/openssl/pull/11435)
Since this is public, it is best to make the underlying structure opaque.
This means converting from stack allocation to dynamic allocation for all
usages.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11390)
The catalyst for this is the difficult of passing BNs through the other
OSSL_PARAM APIs.
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
(Merged from https://github.com/openssl/openssl/pull/11390)
The provider key export functions for EC_KEY assumed that a public key
is always present, and would fail if not. This blocks any attempt to
export a key structure with only domain parameters.
This is similar to earlier work done in EVP_PKEY_ASN1_METHODs.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11394)
Downgrading EVP_PKEYs from containing provider side internal keys to
containing legacy keys demands support in the EVP_PKEY_ASN1_METHOD.
This became a bit elaborate because the code would be almost exactly
the same as the import functions int EVP_KEYMGMT. Therefore, we end
up moving most of the code to common backend support files that can be
used both by legacy backend code and by our providers.
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11375)
We can find no reason why everyone should have to call both
EC_curve_nist2nid() and ec_curve_name2nid() to find the NID for a
name, and it's too easy to forget EC_curve_nist2nid(), so we make life
simpler.
One could argue that FIPS only allows a limited set of names, but that
now gets handled internally, and those who really want to be really
sure to only get the NIST names can still do so with EC_curve_nist2nid()
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11391)
The export-to-provider functions for DH, DSA and EC_KEY assumed that a
public key is always present, and would fail if not. This blocks any
attempt to export a key structure with only domain parameters.
While fixing this, we also modify the selection declaration to
evp_keymgmt_import() to be more adaptive, the diverse selection bits
are now added when the corresponding data is added to the OSSL_PARAM
array.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11374)
This is largely based on the existing X25519 and X448 serializers - but
a few adjustments were necessary so that we can identify what type of key
we are using. Previously we used the keylen for this but X25519 and
ED25519 have the same keylen.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11272)
Note: This PR has not attempted to move the curves into the provider dispatch table.
Mappings between the curve name / nid have been added to the inbuilt curve table.
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11133)
The EVP_PKEY_ASN1_METHOD function export_to() must check that the key
we're trying to export has a known libcrypto method, i.e. is a built
in RSA_METHOD, DSA_METHOD, etc. Otherwise, the method may be defined
by the calling application, by an engine, by another library, and we
simply cannot know all the quirks hidden behind that method, if we
have access to the key data, or much anything.
Such keys are simply deemed impossible to export to provider keys,
i.e. have export_to() return 0. This cascades back to functions like
evp_pkey_export_to_provider() and evp_pkey_upgrade_to_provider() and
their callers. In most cases, this is fine, but if these get mixed in
with provider side keys in any function, that function will fail.
Fixes#11179Fixes#9915
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/11193)
At the moment we only provider support for these algorithms in the default
provider. These algorithms only support "one shot" EVP_DigestSign() and
EVP_DigestVerify() as per the existing libcrypto versions.
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/11261)
CLA: trivial
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
Reviewed-by: Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
(Merged from https://github.com/openssl/openssl/pull/11175)
We should never leak the bit length of the secret scalar in the key,
so we always set the `BN_FLG_CONSTTIME` flag on the internal `BIGNUM`
holding the secret scalar.
This is important also because `BN_dup()` (and `BN_copy()`) do not
propagate the `BN_FLG_CONSTTIME` flag from the source `BIGNUM`, and
this brings an extra risk of inadvertently losing the flag, even when
the called specifically set it.
The propagation has been turned on and off a few times in the past
years because in some conditions has shown unintended consequences in
some code paths, so at the moment we can't fix this in the BN layer.
In `EC_KEY_set_private_key()` we can work around the propagation by
manually setting the flag after `BN_dup()` as we know for sure that
inside the EC module the `BN_FLG_CONSTTIME` is always treated
correctly and should not generate unintended consequences.
Setting the `BN_FLG_CONSTTIME` flag alone is never enough, we also have
to preallocate the `BIGNUM` internal buffer to a fixed public size big
enough that operations performed during the processing never trigger
a realloc which would leak the size of the scalar through memory
accesses.
Fixed Length
------------
The order of the large prime subgroup of the curve is our choice for
a fixed public size, as that is generally the upper bound for
generating a private key in EC cryptosystems and should fit all valid
secret scalars.
For preallocating the `BIGNUM` storage we look at the number of "words"
required for the internal representation of the order, and we
preallocate 2 extra "words" in case any of the subsequent processing
might temporarily overflow the order length.
Future work
-----------
A separate commit addresses further hardening of `BN_copy()` (and
indirectly `BN_dup()`).
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10631)
For EC keys it is particularly important to avoid leaking the bit length
of the secret scalar.
Key import/export should never leak the bit length of the secret
scalar in the key.
For this reason, on export we use padded BIGNUMs with fixed length,
using the new `ossl_param_bld_push_BN_pad()`.
When importing we also should make sure that, even if short lived,
the newly created BIGNUM is marked with the BN_FLG_CONSTTIME flag as
soon as possible, so that any processing of this BIGNUM might opt for
constant time implementations in the backend.
Setting the BN_FLG_CONSTTIME flag alone is never enough, we also have
to preallocate the BIGNUM internal buffer to a fixed size big enough
that operations performed during the processing never trigger a
realloc which would leak the size of the scalar through memory
accesses.
Fixed length
------------
The order of the large prime subgroup of the curve is our choice for
a fixed public size, as that is generally the upper bound for
generating a private key in EC cryptosystems and should fit all valid
secret scalars.
For padding on export we just use the bit length of the order
converted to bytes (rounding up).
For preallocating the BIGNUM storage we look at the number of "words"
required for the internal representation of the order, and we
preallocate 2 extra "words" in case any of the subsequent processing
might temporarily overflow the order length.
Future work
-----------
To ensure the flag and fixed size preallocation persists upon
`EC_KEY_set_private_key()`, we need to further harden
`EC_KEY_set_private_key()` and `BN_copy()`.
This is done in separate commits.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10631)
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10631)
A pair of internal functions related to EC_KEY handling could benefit
from declaring `EC_KEY *` variables as `const`, providing clarity for
callers and readers of the code, in addition to enlisting the compiler
in preventing some mistakes.
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10631)
In https://github.com/openssl/openssl/pull/10883, I'd meant to exclude
the perlasm drivers since they aren't opening pipes and do not
particularly need it, but I only noticed x86_64-xlate.pl, so
arm-xlate.pl and ppc-xlate.pl got the change.
That seems to have been fine, so be consistent and also apply the change
to x86_64-xlate.pl. Checking for errors is generally a good idea.
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: David Benjamin <davidben@google.com>
(Merged from https://github.com/openssl/openssl/pull/10930)
Add ref counting and control how we allocate storage for the private key.
We will need this type in following commits where we move the ecx code
to be provider aware.
Reviewed-by: Patrick Steuer <patrick.steuer@de.ibm.com>
Reviewed-by: Shane Lontis <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10964)
