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

Restrict S2K types #213

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Restrict S2K types #213

merged 1 commit into from
Jul 5, 2024

Conversation

twiss
Copy link
Member

@twiss twiss commented Jul 4, 2024

RFC9580 says that:

Argon2 is only used with AEAD (S2K usage octet 253).  An
implementation MUST NOT create and MUST reject as malformed any
secret key packet where the S2K usage octet is not AEAD (253) and
the S2K specifier type is Argon2.

Therefore, we disallow reading and writing Argon2 keys without AEAD.

And:

[The Simple and Salted S2K methods] are used only for reading in
backwards compatibility mode.

So, we disallow encrypting keys using those methods.

Since V6 keys don't need backwards compatibility, we also disallow reading Simple S2K there. We still allow reading Salted S2K since the spec says it may be used "when [the password] is high entropy".

@twiss twiss requested a review from lubux July 4, 2024 21:21
@twiss twiss force-pushed the restrict-s2k-types branch from 1b33c40 to 6028a9f Compare July 4, 2024 21:25
Copy link
Contributor

@lubux lubux left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@twiss twiss merged commit 3df78a3 into main Jul 5, 2024
8 checks passed
@twiss twiss deleted the restrict-s2k-types branch July 5, 2024 08:36
twiss added a commit that referenced this pull request Jul 18, 2024
Partially reverts #213 by
allowing Salted S2K for high-entropy passphrases, as already enforced
by openpgp/s2k/s2k.go:195.
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.

2 participants