summaryrefslogtreecommitdiffstats
path: root/crypto/ec
AgeCommit message (Collapse)Author
2020-02-17Also check for errors in x86_64-xlate.pl.David Benjamin
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)
2020-02-11Add S390 support for provider based X25519/X448Matt Caswell
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)
2020-02-11Add X25519/X448 Key Exchange to the default providerMatt Caswell
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)
2020-02-11Implement a stricter ECX_KEY typeMatt Caswell
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)
2020-02-06Params: change UTF8 construct calls to avoid explicit strlen(3) calls.Pauli
It is better, safer and smaller to let the library routine handle the strlen(3) call. Added a note to the documentation suggesting this. Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from https://github.com/openssl/openssl/pull/11019)
2020-02-04Deprecate the ECDSA and EV_KEY_METHOD functions.Pauli
Use of the low level ECDSA and EC_KEY_METHOD functions has been informally discouraged for a long time. We now formally deprecate them. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10960)
2020-02-04Deprecate the ECDH functions.Pauli
Use of the low level ECDH functions has been informally discouraged for a long time. We now formally deprecate them. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10960)
2020-02-02Make SM3 a mandatory hash function for SM2.Richard Levitte
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> (Merged from https://github.com/openssl/openssl/pull/10942)
2020-01-23Check ECC-CDH is compliant with SP800-56A-r3Shane Lontis
Added comments and cleared an intermediate result. KAT tests already exist in evppkey.txt (Search for "KAS_ECC_CDH_PrimitiveTest") Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10838)
2020-01-22Do not silently truncate files on perlasm errorsDavid Benjamin
If one of the perlasm xlate drivers crashes, OpenSSL's build will currently swallow the error and silently truncate the output to however far the driver got. This will hopefully fail to build, but better to check such things. Handle this by checking for errors when closing STDOUT (which is a pipe to the xlate driver). Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Tim Hudson <tjh@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/10883)
2020-01-19Deprecate the low level SHA functions.Pauli
Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10791)
2020-01-17For all assembler scripts where it matters, recognise clang > 9.xRichard Levitte
Fixes #10853 Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/10855)
2020-01-07Make ECDSA_size() use consistent asn1 encoder.Shane Lontis
ECDSA signature lengths are calculated using i2d_ECDSA_SIG(). i2d_ECDSA_SIG() was changed in a previous PR to use a custom ASN1 encoder (using WPACKET) so that the normal ASN1 encoder does not need to be pulled into the provider boundary. For consistency ECDSA_size() has been changed to also use i2d_ECDSA_SIG() - this can now be used directly inside the FIPS provider. Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> (Merged from https://github.com/openssl/openssl/pull/10577)
2020-01-05Fix incorrect return code on ECDSA key verificationAndrew Hoang
ECDSA_do_verify() is a function that verifies a ECDSA signature given a hash and a public EC key. The function is supposed to return 1 on valid signature, 0 on invalid signature and -1 on error. Previously, we returned 0 if the key did not have a verify_sig method. This is actually an error case and not an invalid signature. Consequently, this patch updates the return code to -1. Fixes #8766 Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> 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/10693)
2020-01-05Fix side channel in ecp_nistz256-armv8.plFangming.Fang
This change addresses a potential side-channel vulnerability in the internals of nistz256 low level operations for armv8. Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Fix side channel in the ecp_nistz256.c reference implementationBernd Edlinger
This is only used if configured with ./config -DECP_NISTZ256_REFERENCE_IMPLEMENTATION Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Improve side channel fix in ecp_nistz256-x86_64.plBernd Edlinger
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Fix side channel in ecp_nistz256-armv4.plBernd Edlinger
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Fix side channel in ecp_nistz256-x86.plBernd Edlinger
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Avoid leaking intermediate states in point doubling special case.David Benjamin
Cherry picked from https://github.com/google/boringssl/commit/12d9ed670da3edd64ce8175cfe0e091982989c18 Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9239)
2020-01-05Fix potential SCA vulnerability in some EC_METHODsNicola Tuveri
This commit addresses a potential side-channel vulnerability in the internals of some elliptic curve low level operations. The side-channel leakage appears to be tiny, so the severity of this issue is rather low. The issue was reported by David Schrammel and Samuel Weiser. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9239)
2019-12-23Add some missing cfi frame info in x25519-x86_64.plBernd Edlinger
Reviewed-by: Richard Levitte <levitte@openssl.org> Reviewed-by: Kurt Roeckx <kurt@roeckx.be> (Merged from https://github.com/openssl/openssl/pull/10676)
2019-12-23Add some missing cfi frame info in ecp_nistz256-x86_64.plBernd Edlinger
Reviewed-by: Kurt Roeckx <kurt@roeckx.be> (Merged from https://github.com/openssl/openssl/pull/10672)
2019-11-21ECDSA: don't clear free memory after verify.Pauli
Verifications are public, there is no need to clear the used storage before freeing it. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10475)
2019-11-13Fix EC_POINT_bn2point() for BN_zero()Nicola Tuveri
EC_POINT_bn2point() rejected BIGNUMs with a zero value. This behavior indirectly caused failures when converting a point at infinity through EC_POINT_point2hex() and then back to a point with EC_POINT_hex2point(). With this change such BIGNUMs are treated like any other and exported to an octet buffer filled with zero. It is then EC_POINT_oct2point() (either the default implementation or the custom one in group->meth->oct2point) to determine if such encoding maps to a valid point (generally the point at infinity is encoded as 0x00). Fixes #10258 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10329)
2019-11-07Update source files for deprecation at 3.0Richard Levitte
Previous macros suggested that from 3.0, we're only allowed to deprecate things at a major version. However, there's no policy stating this, but there is for removal, saying that to remove something, it must have been deprecated for 5 years, and that removal can only happen at a major version. Meanwhile, the semantic versioning rule is that deprecation should trigger a MINOR version update, which is reflected in the macro names as of this change. Reviewed-by: Tim Hudson <tjh@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10364)
2019-11-05s390x assembly pack: process x25519 and x448 non-canonical valuesPatrick Steuer
...in constant time. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10339)
2019-11-05s390x assembly pack: fix x448 handling of non-canonical valuesPatrick Steuer
The s390x x448 implementation does not correctly reduce non-canonical values i.e., u-coordinates >= p = 2^448 - 2^224 - 1. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10339)
2019-11-01s390x: fix build errorsPatrick Steuer
ecp_s390x_nistp.c and ecx_meth.c need to include s390x_arch.h. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10317)
2019-10-23Deprecate EC_GROUP_clear_free()Nicola Tuveri
There is nothing confidential in `EC_GROUP` so really having a `EC_GROUP_clear_free` function at all does not make much sense anymore. See https://github.com/openssl/openssl/issues/9822 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9874)
2019-10-23Avoid using EC_GROUP_clear_free() internallyNicola Tuveri
There is nothing confidential in `EC_GROUP` so really having a `EC_GROUP_clear_free` function at all does not make much sense anymore. See https://github.com/openssl/openssl/issues/9822 Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9874)
2019-10-16Fix missing Assembler definesShane Lontis
Implementations are now spread across several libraries, so the assembler related defines need to be applied to all affected libraries and modules. AES_ASM define was missing from libimplementations.a which disabled AESNI aarch64 changes were made by xkqian. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10180)
2019-10-15[ec_asn1.c] Avoid injecting seed when built-in matchesNicola Tuveri
An unintended consequence of https://github.com/openssl/openssl/pull/9808 is that when an explicit parameters curve is matched against one of the well-known builtin curves we automatically inherit also the associated seed parameter, even if the input parameters excluded such parameter. This later affects the serialization of such parsed keys, causing their input DER encoding and output DER encoding to differ due to the additional optional field. This does not cause problems internally but could affect external applications, as reported in https://github.com/openssl/openssl/pull/9811#issuecomment-536153288 This commit fixes the issue by conditionally clearing the seed field if the original input parameters did not include it. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> (Merged from https://github.com/openssl/openssl/pull/10140)
2019-10-10Rework how our providers are builtRichard Levitte
We put almost everything in these internal static libraries: libcommon Block building code that can be used by all our implementations, legacy and non-legacy alike. libimplementations All non-legacy algorithm implementations and only them. All the code that ends up here is agnostic to the definitions of FIPS_MODE. liblegacy All legacy implementations. libnonfips Support code for the algorithm implementations. Built with FIPS_MODE undefined. Any code that checks that FIPS_MODE isn't defined must end up in this library. libfips Support code for the algorithm implementations. Built with FIPS_MODE defined. Any code that checks that FIPS_MODE is defined must end up in this library. The FIPS provider module is built from providers/fips/*.c and linked with libimplementations, libcommon and libfips. The Legacy provider module is built from providers/legacy/*.c and linked with liblegacy, libcommon and libcrypto. If module building is disabled, the object files from liblegacy and libcommon are added to libcrypto and the Legacy provider becomes a built-in provider. The Default provider module is built-in, so it ends up being linked with libimplementations, libcommon and libnonfips. For libcrypto in form of static library, the object files from those other libraries are simply being added to libcrypto. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10088)
2019-10-09Explicitly test against NULL; do not use !p or similarRich Salz
Also added blanks lines after declarations in a couple of places. Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9916)
2019-09-28Fix header file include guard namesDr. Matthias St. Pierre
Make the include guards consistent by renaming them systematically according to the naming conventions below For the public header files (in the 'include/openssl' directory), the guard names try to match the path specified in the include directives, with all letters converted to upper case and '/' and '.' replaced by '_'. For the private header files files, an extra 'OSSL_' is added as prefix. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9333)
2019-09-28Reorganize local header filesDr. Matthias St. Pierre
Apart from public and internal header files, there is a third type called local header files, which are located next to source files in the source directory. Currently, they have different suffixes like '*_lcl.h', '*_local.h', or '*_int.h' This commit changes the different suffixes to '*_local.h' uniformly. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9333)
2019-09-28Reorganize private crypto header filesDr. Matthias St. Pierre
Currently, there are two different directories which contain internal header files of libcrypto which are meant to be shared internally: While header files in 'include/internal' are intended to be shared between libcrypto and libssl, the files in 'crypto/include/internal' are intended to be shared inside libcrypto only. To make things complicated, the include search path is set up in such a way that the directive #include "internal/file.h" could refer to a file in either of these two directoroes. This makes it necessary in some cases to add a '_int.h' suffix to some files to resolve this ambiguity: #include "internal/file.h" # located in 'include/internal' #include "internal/file_int.h" # located in 'crypto/include/internal' This commit moves the private crypto headers from 'crypto/include/internal' to 'include/crypto' As a result, the include directives become unambiguous #include "internal/file.h" # located in 'include/internal' #include "crypto/file.h" # located in 'include/crypto' hence the superfluous '_int.h' suffixes can be stripped. The files 'store_int.h' and 'store.h' need to be treated specially; they are joined into a single file. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9333)
2019-09-25s390x assembly pack: accelerate X25519, X448, Ed25519 and Ed448Patrick Steuer
using PCC and KDSA instructions. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10004)
2019-09-25s390x assembly pack: cleanse only sensitive fieldsPatrick Steuer
of instruction parameter blocks. Signed-off-by: Patrick Steuer <patrick.steuer@de.ibm.com> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/10004)
2019-09-16Unify all assembler file generatorsRichard Levitte
They now generally conform to the following argument sequence: script.pl "$(PERLASM_SCHEME)" [ C preprocessor arguments ... ] \ $(PROCESSOR) <output file> However, in the spirit of being able to use these scripts manually, they also allow for no argument, or for only the flavour, or for only the output file. This is done by only using the last argument as output file if it's a file (it has an extension), and only using the first argument as flavour if it isn't a file (it doesn't have an extension). While we're at it, we make all $xlate calls the same, i.e. the $output argument is always quoted, and we always die on error when trying to start $xlate. There's a perl lesson in this, regarding operator priority... This will always succeed, even when it fails: open FOO, "something" || die "ERR: $!"; The reason is that '||' has higher priority than list operators (a function is essentially a list operator and gobbles up everything following it that isn't lower priority), and since a non-empty string is always true, so that ends up being exactly the same as: open FOO, "something"; This, however, will fail if "something" can't be opened: open FOO, "something" or die "ERR: $!"; The reason is that 'or' has lower priority that list operators, i.e. it's performed after the 'open' call. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9884)
2019-09-16build.info: For all assembler generators, remove all argumentsRichard Levitte
Since the arguments are now generated in the build file templates, they should be removed from the build.info files. Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9884)
2019-09-16clearing the ecx private key memoryManishPatidar1
Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9830)
2019-09-13Fix potential memory leaks with BN_to_ASN1_INTEGERBernd Edlinger
Reviewed-by: Paul Dale <paul.dale@oracle.com> Reviewed-by: Matt Caswell <matt@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9833)
2019-09-12util/mkerr.pl: make it not depend on the function codeRichard Levitte
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)
2019-09-11Usages of KDFs converted to use the name macrosPauli
Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/9814)
2019-09-09[ec] Match built-in curves on EC_GROUP_new_from_ecparametersNicola Tuveri
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. _______________________________________________________________________________ Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Paul Dale <paul.dale@oracle.com> (Merged from https://github.com/openssl/openssl/pull/9808)
2019-09-07[ec/ecp_nistp*.c] restyle: use {} around `else` tooNicola Tuveri
Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511)
2019-09-07[ec/ecp_nistp*.c] remove flip_endian()Nicola Tuveri
Replace flip_endian() by using the little endian specific BN_bn2lebinpad() and BN_lebin2bn(). Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511)
2019-09-07Fix a SCA leak using BN_bn2bin()Nicola Tuveri
BN_bn2bin() is not constant-time and leaks the number of bits in the processed BIGNUM. The specialized methods in ecp_nistp224.c, ecp_nistp256.c and ecp_nistp521.c internally used BN_bn2bin() to convert scalars into the internal fixed length representation. This can leak during ECDSA/ECDH key generation or handling the nonce while generating an ECDSA signature, when using these implementations. The amount and risk of leaked information useful for a SCA attack varies for each of the three curves, as it depends mainly on the ratio between the bitlength of the curve subgroup order (governing the size of the secret nonce/key) and the limb size for the internal BIGNUM representation (which depends on the compilation target architecture). To fix this, we replace BN_bn2bin() with BN_bn2binpad(), bounding the output length to the width of the internal representation buffer: this length is public. Internally the final implementation of both BN_bn2binpad() and BN_bn2bin() already has masking in place to avoid leaking bn->top through memory access patterns. Memory access pattern still leaks bn->dmax, the size of the lazily allocated buffer for representing the BIGNUM, which is inevitable with the current BIGNUM architecture: reading past bn->dmax would be an out-of-bound read. As such, it's the caller responsibility to ensure that bn->dmax does not leak secret information, by explicitly expanding the internal BIGNUM buffer to a public value sufficient to avoid any lazy reallocation while manipulating it: this is already done at the top level alongside setting the BN_FLG_CONSTTIME. Finally, the internal implementation of BN_bn2binpad() indirectly calls BN_num_bits() via BN_num_bytes(): the current implementation of BN_num_bits() can leak information to a SCA attacker, and is addressed in the next commit. Thanks to David Schrammel and Samuel Weiser for reporting this issue through responsible disclosure. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Bernd Edlinger <bernd.edlinger@hotmail.de> (Merged from https://github.com/openssl/openssl/pull/9511)