OpenSSL 1.1.0 made this structure opaque. I don't think we particularly
need to make it opaque, but external code uses it. Also add
RSA_test_flags.
Change-Id: I136d38e72ec4664c78f4d1720ec691f5760090c1
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50605
Reviewed-by: Adam Langley <agl@google.com>
The bulk of RSA_check_key is spent in bn_div_consttime, which is a naive
but constant-time long-division algorithm for the few places that divide
by a secret even divisor: RSA keygen and RSA import. RSA import is
somewhat performance-sensitive, so pick some low-hanging fruit:
The main observation is that, in all but one call site, the bit width of
the divisor is public. That means, for an N-bit divisor, we can skip the
first N-1 iterations of long division because an N-1-bit remainder
cannot exceed the N-bit divisor.
One minor nuisance is bn_lcm_consttime, used in RSA keygen has a case
that does *not* have a public bit width. Apply the optimization there
would leak information. I've implemented this as an optional public
lower bound on num_bits(divisor), which all but that call fills in.
Before:
Did 5060 RSA 2048 private key parse operations in 1058526us (4780.2 ops/sec)
Did 1551 RSA 4096 private key parse operations in 1082343us (1433.0 ops/sec)
After:
Did 11532 RSA 2048 private key parse operations in 1084145us (10637.0 ops/sec) [+122.5%]
Did 3542 RSA 4096 private key parse operations in 1036374us (3417.7 ops/sec) [+138.5%]
Bug: b/192484677
Change-Id: I893ebb8886aeb8200a1a365673b56c49774221a2
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/49106
Reviewed-by: Adam Langley <agl@google.com>
We usually call the parameter 'digest', but people sometimes think they
can skip the hashing for short inputs are short. I also suspect the term
'digest' is less common. Add warnings about this.
There were also some cases where we called it 'in' and even 'msg'. This
CL fixes those to say 'digest'. Finally, RSA_{sign,verify}_raw are
documented to be building blocks of signature schemes, rather than
signature schemes themselves.
It's unfortunate that EVP_PKEY_sign means "sign a digest", while
EVP_DigestSign means "sign, likely internally digesting it as the first
step", but we're a bit stuck there.
Change-Id: I4c38afff9b6196e2789cf27653fe5e5e8c68c1bf
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47504
Reviewed-by: Adam Langley <agl@google.com>
https://boringssl-review.googlesource.com/c/boringssl/+/42504 aligned
RSA private key checks, but I missed the public key ones. We have two
different sets of RSA public key checks right now. One in the parser
just checks for e = 1 and even e. The other, when using the key, checks
for overly large e and n.
Align the two. Now parsing RSA public keys calls RSA_check_key and the
extra checks on e are added to RSA_check_key. Note RSA private key
parsing already called RSA_check_key. The consequences are:
First, RSA public keys with large n, large e, or n < e will be rejected
at parse time. Previously, they would be parsed but all operations on
them would fail. This aligns with our existing behavior for parsing
private keys.
Second, operations on RSA public keys with even e will fail. They
already failed to parse, but it was possible to manually construct such
a key. Previously, operations wouldn't explicitly fail, but they
wouldn't do anything useful because even exponents are not invertible.
(Encrypting would produce something undecryptable and the private key
would have a hard time reliably producing signatures we'd accept.) There
is no change to RSA private keys with even e. Those would already fail
the (e, d) consistency check and the fault check.
Third, operations on RSA public keys with e = 1 will fail. They already
failed to parse, but it was possible to manually construct such a key
and "verify" signatures or "encrypt" messages. However, with e = 1,
those operations are no-ops.
Finally, RSA private keys with e = d = 1 will be rejected at parse and
use. This is the only case that affects private keys because e = d = 1
are inverses, just pointless. Uses paired with RSA public key parsing
(e.g. our TLS library checks consistency with a certificate public key)
are not affected. Those already rejected such keys because we rejected
them in the public key parser. This CL aligns the private half.
This doesn't close https://crbug.com/boringssl/316, but we won't be able
to resolve that without a consistent story for what keys are valid.
Update-Note: See above.
Bug: 316
Change-Id: Ic27df18c4f48e5e3e57a17d6fe39399e2f8d5c68
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47524
Reviewed-by: Adam Langley <agl@google.com>
It's insufficient to signal an error when the PWCT fails. We
additionally need to ensure that the invalid key material is not
returned.
Change-Id: Ic5ff719a688985a61c52540ce6d1ed279a493d27
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/44306
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: David Benjamin <davidben@google.com>
Most asymmetric operations scale superlinearly, which makes them
potential DoS vectors. This (and other problems) are mitigated with
fixed sizes, like RSA-2048, P-256, or curve25519.
In older algorithms like RSA and DSA, these sizes are conventions rather
than well-defined algorithms. "Everyone" uses RSA-2048, but code which
imports an RSA key may see an arbitrary key size, possibly from an
untrusted source. This is commonly a public key, so we bound RSA key
sizes in check_modulus_and_exponent_sizes.
However, some applications import external private keys, and may need
tighter bounds. These typically parse the key then check the result.
However, parsing itself can perform superlinear work (RSA_check_key or
recovering the DSA public key).
This CL does the following:
- Rename check_modulus_and_exponent_sizes to rsa_check_public_key and
additionally call it from RSA_check_key.
- Fix a bug where RSA_check_key, on CRT-less keys, did not bound d, and
bound p and q before multiplying (quadratic).
- Our DSA verifier had stricter checks on q (160-, 224-, and 256-bit
only) than our DSA signer (multiple of 8 bits). Aligner the signer to
the verifier's checks.
- Validate DSA group sizes on parse, as well as priv_key < q, to bound
the running time.
Ideally these invariants would be checked exactly once at construction,
but our RSA and DSA implementations suffer from some OpenSSL's API
mistakes (https://crbug.com/boringssl/316), which means it is hard to
consistently enforce invariants. This CL focuses on the parser, but
later I'd like to better rationalize the freeze_private_key logic.
Performance of parsing RSA and DSA keys, gathered on my laptop.
Did 15130 RSA-2048 parse operations in 5022458us (3012.5 ops/sec)
Did 4888 RSA-4096 parse operations in 5060606us (965.9 ops/sec)
Did 354 RSA-16384 parse operations in 5043565us (70.2 ops/sec)
Did 88 RSA-32768 parse operations in 5038293us (17.5 ops/sec) [rejected by this CL]
Did 35000 DSA-1024/256 parse operations in 5030447us (6957.6 ops/sec)
Did 11316 DSA-2048/256 parse operations in 5094664us (2221.1 ops/sec)
Did 5488 DSA-3072/256 parse operations in 5096032us (1076.9 ops/sec)
Did 3172 DSA-4096/256 parse operations in 5041220us (629.2 ops/sec)
Did 840 DSA-8192/256 parse operations in 5070616us (165.7 ops/sec)
Did 285 DSA-10000/256 parse operations in 5004033us (57.0 ops/sec)
Did 74 DSA-20000/256 parse operations in 5066299us (14.6 ops/sec) [rejected by this CL]
Update-Note: Some invalid or overly large RSA and DSA keys may
previously have been accepted that are now rejected at parse time. For
public keys, this only moves the error from verification to parsing. In
some private key cases, we would previously allow signing with those
keys, but the resulting signatures would not be accepted by BoringSSL
anyway. This CL makes us behave more consistently.
Bug: oss-fuzz:24730
Change-Id: I4ad2003ee61138b693e65d3da4c6aa00bc165251
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42504
Reviewed-by: Adam Langley <agl@google.com>
David Benjamin
Shelley Vohr
Adam Langley