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

Fix forward compatibility of keys, SKESKs, and detached/cleartext signatures and ECDH #1656

Merged
merged 4 commits into from Jul 10, 2023

Conversation

larabr
Copy link
Collaborator

@larabr larabr commented Jun 23, 2023

Relevant for forward compatibility when:

  • verifying detached or cleartext signatures that include e.g. v4 and v6 packets. Non-detached signatures are not affected. Compatibility was likely broken in Add UnparseablePacket to properly deal with key blocks that include malformed/unsupported packets #1522 .
  • parsing messages with sym. encrypted session keys (SKESK) that rely on unsupported S2K types (e.g. argon2, as far as openpgp.js v5 is concerned)
  • parsing keys that include ECDH subkeys with unknown KDF params version (e.g. end-to-end encrypted forwarding use-case)

@larabr larabr changed the title Fix verification of detached signatures that include unknown signature packet versions Fix forward compatibility of SKESK packets and detached/cleartext signatures Jun 26, 2023
@larabr larabr requested a review from twiss June 26, 2023 14:04
@larabr larabr force-pushed the fix-fwd-compat-detached-verify branch 2 times, most recently from 05f0564 to dd006d1 Compare June 29, 2023 14:41
try {
offset += this.s2k.read(bytes.subarray(offset, bytes.length));
} catch (err) {
throw new UnsupportedError('Unsupported or invalid S2K');
Copy link
Member

@twiss twiss Jun 29, 2023

Choose a reason for hiding this comment

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

Can we change the type of the errors in s2k.read instead? (And add a try/catch around the enums.read there, I guess)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we just have to merge this after #1658

@larabr larabr force-pushed the fix-fwd-compat-detached-verify branch from dd006d1 to 225ec7d Compare June 29, 2023 17:34
@larabr larabr requested a review from twiss June 29, 2023 17:35
@larabr larabr changed the title Fix forward compatibility of SKESK packets and detached/cleartext signatures Fix forward compatibility of keys, SKESKs, and detached/cleartext signatures and ECDH Jun 30, 2023
@larabr larabr force-pushed the fix-fwd-compat-detached-verify branch from 368f0e4 to b638edd Compare July 7, 2023 18:07
…e packet versions

Relevant for forward compatibility when verifying detached signatures
that include e.g. v4 and v6 packets
These messages should still be decrypt-able if they include at least one
supported ESK packet.
Keys with such subkeys should still be usable, as long as they have
a supported primary key.
@larabr larabr force-pushed the fix-fwd-compat-detached-verify branch from b638edd to 32caf41 Compare July 10, 2023 13:44
@larabr larabr merged commit b6170aa into openpgpjs:main Jul 10, 2023
12 checks passed
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

2 participants