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

Avoid using short exponents with ElGamal #2790

Merged
merged 1 commit into from Aug 15, 2021
Merged

Conversation

randombit
Copy link
Owner

Some off-brand PGP implementation generates keys where p - 1 is smooth, as a result short exponents can leak enough information about k to allow decryption.

Needs backport for 2.x

cc @ronaldtse

Some off-brand PGP implementation generates keys where p - 1 is
smooth, as a result short exponents can leak enough information about
k to allow decryption.
@codecov-commenter
Copy link

codecov-commenter commented Aug 3, 2021

Codecov Report

Merging #2790 (9a23e4e) into master (f63555c) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2790      +/-   ##
==========================================
+ Coverage   92.32%   92.33%   +0.01%     
==========================================
  Files         568      568              
  Lines       63086    63086              
  Branches     6180     6178       -2     
==========================================
+ Hits        58244    58251       +7     
+ Misses       4810     4803       -7     
  Partials       32       32              
Impacted Files Coverage Δ
src/lib/pubkey/elgamal/elgamal.cpp 95.94% <100.00%> (ø)
src/lib/pubkey/dl_group/dl_group.cpp 92.65% <0.00%> (+1.04%) ⬆️
src/lib/asn1/der_enc.cpp 83.22% <0.00%> (+2.48%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f63555c...9a23e4e. Read the comment docs.

@ronaldtse
Copy link

Thank you for the ping @randombit !

CC @ni4 @dewyatt @antonsviridenko

@antonsviridenko
Copy link

@randombit what measures should be taken by rnp developers to address this issue?

@randombit randombit merged commit 1df7edc into master Aug 15, 2021
22 checks passed
/*
Some ElGamal implementations foolishly use prime fields where p - 1 is
smooth, as a result it is unsafe to use short exponents.
*/
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi. I'm one of the discoverers of the vulnerability (described here and here).

It seems there is a misunderstanding on the nature of the vulnerability (or maybe it's just an improper use of the word "smooth"): the affected public keys strictly follow El Gamal's original description, and do nothing "foolish". In particular, p-1 is not smooth: it contains a large prime factor, but also a large smooth factor, just like in DSA keys. The blame cannot fall solely on the library that does public key generation: it's a complex issue involving the whole OpenPGP ecosystem.

This patch properly fixes the vulnerability, but the comment is misleading. You may want to edit it, so that in, say, 10 years time some maintainer doesn't feel it's ok to ignore "foolish" implementations. Note that there is nothing you can do against really foolish implementations that use truly smooth p-1: for these, message recovery is always possible no matter what the encrypting party does.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I did in fact just skim and clearly misunderstand the paper. I thought the issue was these keys had chosen p completely at random rather than via a structured way and due to poor luck had generated a p where p - 1 truly had only small factors. I will edit the comment.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not very important, I'm just being a 🤓: if you took a p of 2048 bits at random, the chances that all the prime factors of p-1 are smaller than, say, 200 bits are ≈10^-11. You would need some criminally poor luck for that to happen!

Relatedly, we would definitely appreciate if the comment and/or commit message contained a link to our paper 😊

randombit added a commit that referenced this pull request Sep 14, 2021
@randombit randombit deleted the jack/elgamal-no-short-exp branch September 14, 2021 13:20
woodsts pushed a commit to woodsts/buildroot that referenced this pull request Sep 18, 2021
Fixes the following security issue:

- CVE-2021-40529: The ElGamal implementation in Botan through 2.18.1, as
  used in Thunderbird and other products, allows plaintext recovery because,
  during interaction between two cryptographic libraries, a certain
  dangerous combination of the prime defined by the receiver's public key,
  the generator defined by the receiver's public key, and the sender's
  ephemeral exponents can lead to a cross-configuration attack against
  OpenPGP

For more details, see the upstream bug and issue writeup:
- randombit/botan#2790
- https://ibm.github.io/system-security-research-updates/2021/07/20/insecurity-elgamal-pt1

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
randombit added a commit that referenced this pull request Sep 18, 2021
woodsts pushed a commit to woodsts/buildroot that referenced this pull request Sep 29, 2021
Fixes the following security issue:

- CVE-2021-40529: The ElGamal implementation in Botan through 2.18.1, as
  used in Thunderbird and other products, allows plaintext recovery because,
  during interaction between two cryptographic libraries, a certain
  dangerous combination of the prime defined by the receiver's public key,
  the generator defined by the receiver's public key, and the sender's
  ephemeral exponents can lead to a cross-configuration attack against
  OpenPGP

For more details, see the upstream bug and issue writeup:
- randombit/botan#2790
- https://ibm.github.io/system-security-research-updates/2021/07/20/insecurity-elgamal-pt1

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
(cherry picked from commit 31c9408)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
woodsts pushed a commit to woodsts/buildroot that referenced this pull request Sep 29, 2021
Fixes the following security issue:

- CVE-2021-40529: The ElGamal implementation in Botan through 2.18.1, as
  used in Thunderbird and other products, allows plaintext recovery because,
  during interaction between two cryptographic libraries, a certain
  dangerous combination of the prime defined by the receiver's public key,
  the generator defined by the receiver's public key, and the sender's
  ephemeral exponents can lead to a cross-configuration attack against
  OpenPGP

For more details, see the upstream bug and issue writeup:
- randombit/botan#2790
- https://ibm.github.io/system-security-research-updates/2021/07/20/insecurity-elgamal-pt1

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
(cherry picked from commit 31c9408)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
woodsts pushed a commit to woodsts/buildroot that referenced this pull request Sep 29, 2021
Fixes the following security issue:

- CVE-2021-40529: The ElGamal implementation in Botan through 2.18.1, as
  used in Thunderbird and other products, allows plaintext recovery because,
  during interaction between two cryptographic libraries, a certain
  dangerous combination of the prime defined by the receiver's public key,
  the generator defined by the receiver's public key, and the sender's
  ephemeral exponents can lead to a cross-configuration attack against
  OpenPGP

For more details, see the upstream bug and issue writeup:
- randombit/botan#2790
- https://ibm.github.io/system-security-research-updates/2021/07/20/insecurity-elgamal-pt1

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
(cherry picked from commit 31c9408)
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
bkircher pushed a commit to bkircher/botan that referenced this pull request Nov 7, 2021
bkircher pushed a commit to bkircher/botan that referenced this pull request Nov 7, 2021
Backport of randombit#2790. Fixes CVE-2021-40529: ElGamal implementation allows
plaintext recovery.
bkircher pushed a commit to bkircher/botan that referenced this pull request Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants