Remove unused DSA methods

There are no accessors to set them, so this has been involved in a bunch
of dead logic ever since we made DSA opaque a few years ago.

ok jsing

Make the openssl_dsa_meth static const

Move openssl_dsa_meth below the methods it uses

no functional change

Make the bn_rand_interval() API a bit more ergonomic

Provide bn_rand_in_range() which is a slightly tweaked version of what was
previously called bn_rand_range().

The way bn_rand_range() is called

Make the bn_rand_interval() API a bit more ergonomic

Provide bn_rand_in_range() which is a slightly tweaked version of what was
previously called bn_rand_range().

The way bn_rand_range() is called in libcrypto, the lower bound is always
expressible as a word. In fact, most of the time it is 1, the DH code uses
a 2, the MR tests in BPSW use 3 and an exceptinally high number appears in
the Tonelli-Shanks implementation where we use 32. Converting these lower
bounds to BIGNUMs on the call site is annoying so let bn_rand_interval()
do that internally and route that through bn_rand_in_range(). This way we
can avoid using BN_sub_word().

Adjust the bn_isqrt() test to use bn_rand_in_range() since that's the
only caller that uses actual BIGNUMs as lower bounds.

ok jsing

show more ...

hide symbols in dsa

ok tb@

Replace the remaining BN_copy() with bn_copy()

ok jsing

Cap the number of iterations in DSA signing

The DSA standard specifies an infinite loop: if either r or s is zero
in the signature calculation, a new random number k shall be generated
and the whole

Cap the number of iterations in DSA signing

The DSA standard specifies an infinite loop: if either r or s is zero
in the signature calculation, a new random number k shall be generated
and the whole thing is to be redone. The rationale is that, as the
standard puts it, "[i]t is extremely unlikely that r = 0 or s = 0 if
signatures are generated properly."

The problem is... There is no cheap way to know that the DSA domain
parameters we are handed are actually DSA domain parameters, so even
if all our calculations are carefully done to do all the checks needed,
we cannot know if we generate the signatures properly. For this we would
need to do two primality checks as well as various congruences and
divisibility properties. Doing this easily leads to DoS, so nobody does
it.

Unfortunately, it is relatively easy to generate parameters that pass
all sorts of sanity checks and will always compute s = 0 since g
is nilpotent. Thus, as unlikely as it is, if we are in the mathematical
model, in practice it is very possible to ensure that s = 0.

Read David Benjamin's glorious commit message for more information
https://boringssl-review.googlesource.com/c/boringssl/+/57228

Thanks to Guido Vranken for reporting this issue, also thanks to
Hanno Boeck who apparently found and reported similar problems earlier.

ok beck jsing

show more ...

Call dsa_check_keys() before signing or verifying

We already had some checks on both sides, but they were less precise
and differed between the functions. The code here is messy enough, so
any simpl

Call dsa_check_keys() before signing or verifying

We already had some checks on both sides, but they were less precise
and differed between the functions. The code here is messy enough, so
any simplification is helpful...

ok beck jsing

show more ...

Merge dsa_sign.c and dsa_vrf.c into dsa_ossl.c

discussed with jsing

Clean up and simplify BIGNUM handling in DSA code.

This adds missing BN_CTX_start()/BN_CTX_end() calls, removes NULL checks
before BN_CTX_end()/BN_CTX_free() (since they're NULL safe) and calls
BN_f

Clean up and simplify BIGNUM handling in DSA code.

This adds missing BN_CTX_start()/BN_CTX_end() calls, removes NULL checks
before BN_CTX_end()/BN_CTX_free() (since they're NULL safe) and calls
BN_free() instead of BN_clear_free() (which does the same thing).

Also replace stack allocated BIGNUMs with calls to BN_CTX_get(), using the
BN_CTX that is already available.

ok tb@

show more ...

Make internal header file names consistent

Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names
used for internal headers. Move all these headers we inherited from
OpenSSL to *_l

Make internal header file names consistent

Libcrypto currently has a mess of *_lcl.h, *_locl.h, and *_local.h names
used for internal headers. Move all these headers we inherited from
OpenSSL to *_local.h, reserving the name *_internal.h for our own code.
Similarly, move dtls_locl.h and ssl_locl.h to dtls_local and ssl_local.h.
constant_time_locl.h is moved to constant_time.h since it's special.

Adjust all .c files in libcrypto, libssl and regress.

The diff is mechanical with the exception of tls13_quic.c, where
#include <ssl_locl.h> was fixed manually.

discussed with jsing,
no objection bcook

show more ...

Fix whitespace

Minor tweaks

i is a silly name for BN_num_bits(dsa->q); move a comment for readability.

Prepare the move of DSA_SIG, DSA_METHOD and DSA to dsa_locl.h by
including the local header where it will be needed.

discussed with jsing

Readability tweaks for comments that explain the blinding.

Remove the blinding later to avoid leaking information on the length
of kinv.

Pointed out and fix suggested by David Schrammel and Samuel Weiser

ok jsing

unrevert the use of bn_rand_interval().

ok beck jsing

revert use of bn_rand_interval due to failures with ECDHE and TLS

Make use of bn_rand_interval() where appropriate.

ok beck jsing

Use a blinding value when generating a DSA signature, in order to reduce
the possibility of a side-channel attack leaking the private key.

Suggested by Keegan Ryan at NCC Group.

With input from and

Use a blinding value when generating a DSA signature, in order to reduce
the possibility of a side-channel attack leaking the private key.

Suggested by Keegan Ryan at NCC Group.

With input from and ok tb@

show more ...

Clarify the digest truncation comment in DSA signature generation.

Requested by and ok tb@

Pull up the code that converts the digest to a BIGNUM - this only needs
to occur once and not be repeated if the signature generation has to be
repeated.

ok tb@

Fix a potential leak/incorrect return value in DSA signature generation.

In the very unlikely case where we have to repeat the signature generation,
the DSA_SIG return value has already been allocat

Fix a potential leak/incorrect return value in DSA signature generation.

In the very unlikely case where we have to repeat the signature generation,
the DSA_SIG return value has already been allocated. This will either
result in a leak when we allocate again on the next iteration, or it
will give a false success (with missing signature values) if any error
occurs on the next iteration.

ok tb@

show more ...

style(9), comments and whitespace.

Avoid a timing side-channel leak when generating DSA and ECDSA signatures.

This is caused by an attempt to do fast modular arithmetic, which
introduces branches that leak information regarding secre

Avoid a timing side-channel leak when generating DSA and ECDSA signatures.

This is caused by an attempt to do fast modular arithmetic, which
introduces branches that leak information regarding secret values.

Issue identified and reported by Keegan Ryan of NCC Group.

ok beck@ tb@

show more ...