Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Denial of Service (DoS) via Large Public Key #166

Closed
missytake opened this issue Jul 14, 2022 · 5 comments · Fixed by #176
Closed

Denial of Service (DoS) via Large Public Key #166

missytake opened this issue Jul 14, 2022 · 5 comments · Fixed by #176

Comments

@missytake
Copy link

This issue was not discovered by me, but during and audit, see "L3" in https://delta.chat/assets/1907-otf-deltachat-rpgp-rustrsa-gb-reportv1.pdf:

"The RSA library allows operating upon large keys, which can consume a large amount of
computation time. An attacker who can force an application to encrypt with a million-byte RSA
public key can force the application into a Denial of Service (DoS) condition."

"The assessment team recommends exposing a higher-level API which performs additional
security checks. For instance, key sizes may be limited to 4096 bits by default but can be
overridden if necessary."

@tarcieri
Copy link
Member

tarcieri commented Jul 19, 2022

Thanks for opening a tracking issue for this.

I agree that something like 4096 or 8192 would be a reasonable limit for a "checked" API.

My understanding is some applications (i.e. OpenPGP) need to support significantly larger keys. I'm not sure what the sanity limit should actually be there (IIRC @dignifiedquire mentioned up to 16384 iirc?)

So perhaps the default API could be changed to have a sanity limit, and an additional *_unchecked API could be added which doesn't enforce the sanity check in order to support exotic key sizes.

@dignifiedquire
Copy link
Member

So for gpg the default limit seems to be 8KiB, with a compile time option to go larger than that: https://wiki.gnupg.org/LargeKeys.
I have not heard of any use case other than pgp for larger keys, so I think we could follow the same approach

  • default: up to 4kib
  • medium-keys: up to 8kib
  • ´large-keys`: up to 16kib

@tarcieri
Copy link
Member

@dignifiedquire how about reducing that to just two options, one which allows keys <= 4096-bits, and "large keys" mode which supports up to 16384-bits?

tarcieri added a commit that referenced this issue Jul 30, 2022
- Ensure modulus is 16384-bits or fewer. See #166
- Increase maximum public exponent. Closes #155
@tarcieri
Copy link
Member

tarcieri commented Jul 30, 2022

#170 restricts the maximum modulus size to 16384 bits. That's a stopgap, but not sufficient to close this issue.

To go further, we need to relegate keys that large to the previously proposed "large keys" mode, and set a lower bound which is used by default. I'm still thinking 4096-bits there.

Edit: opened #171 which restricts public keys to 4096-bits by default.

tarcieri added a commit that referenced this issue Jul 30, 2022
This commit fixes #166 by enforcing a 4096-bit upper limit by default,
which prevents potential DoS by using maliciously large RSA keys.

The PKCS#1/PKCS#8 parsers use this API, limiting the size of keys parsed
from these formats to 4096-bits.

An `RsaPrivateKey::new_large` constructor has been added which enforces
the 16384-bit limit added in #170. This can be used for unusual
applications that need to support larger keys.

`RsaPrivateKey::from_components` uses the `::new_large` constructor, so
private keys follow the 16384-bit limit only.

The `RsaPrivateKey::MAX_SIZE` and `RsaPrivateKey::MAX_SIZE_LARGE`
inherent constants specify the maximum allowed sizes.
tarcieri added a commit that referenced this issue Aug 7, 2022
- Ensure modulus is 16384-bits or fewer. See #166
- Increase maximum public exponent. Closes #155
tarcieri added a commit that referenced this issue Aug 8, 2022
This constructor accepts a configurable maximum key size which can be
used in applications that need to deal with unusually large RSA public
keys, such as OpenPGP.

With the ability to configure an upper limit, this makes it possible to
lower the default maximum key size to 4096-bits, which is a reasonable
upper limit for most applications.

Fixes #166
tarcieri added a commit that referenced this issue Aug 8, 2022
This constructor accepts a configurable maximum key size which can be
used in applications that need to deal with unusually large RSA public
keys, such as OpenPGP.

With the ability to configure an upper limit, this makes it possible to
lower the default maximum key size to 4096-bits, which is a reasonable
upper limit for most applications.

Fixes #166
tarcieri added a commit that referenced this issue Aug 9, 2022
This constructor accepts a configurable maximum key size which can be
used in applications that need to deal with unusually large RSA public
keys, such as OpenPGP.

With the ability to configure an upper limit, this makes it possible to
lower the default maximum key size to 4096-bits, which is a reasonable
upper limit for most applications.

Fixes #166
@tarcieri
Copy link
Member

tarcieri commented Aug 9, 2022

#176 now restricts the max public key size to 4096-bits, unless the special RsaPublicKey::new_with_max_size function is used to override the limit.

Note we place no limit on max RsaPrivateKey size. However, I can't think of a threat model where we should consider attacks based on a malicious private key. It seems like if that's a possibility, you have bigger problems than DoS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants