Skip to content

Don't panic on unknown AEAD mode in v5/v6 SKESK packets - #322

Merged
twiss merged 1 commit into
ProtonMail:mainfrom
knQzx:reject-unknown-aead-skesk
Aug 14, 2026
Merged

twiss merged 1 commit into
ProtonMail:mainfrom
knQzx:reject-unknown-aead-skesk

Conversation

@knQzx

@knQzx knQzx commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

An unknown AEAD mode in a v5/v6 symmetric-key encrypted session key gives a zero tag length, which later divides by the tag length. This rejects unknown modes during parsing.

@twiss

twiss commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Could we return an error while decrypting instead of while parsing, for example by modifying getEncryptedKeyAeadInstance, AEADMode.new and AEADMode.New to return an error instead of panicking on an unknown AEAD mode?

@knQzx

knQzx commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

done. AEADMode.New, AEADMode.new and getEncryptedKeyAeadInstance now return an error instead of panicking, and the parse-time check is gone, so an unknown mode only fails when decrypting

getSymmetricallyEncryptedAeadInstance had to return an error too since it goes through the same path. it also built the nonce slice before constructing the aead, which panicked on an unknown mode before ever reaching mode.new, so that's reordered

@knQzx
knQzx force-pushed the reject-unknown-aead-skesk branch from 850cfd0 to 2ec63f9 Compare August 13, 2026 16:43
@twiss
twiss force-pushed the reject-unknown-aead-skesk branch from 2ec63f9 to 0daa61e Compare August 14, 2026 09:36
@twiss twiss changed the title reject unknown aead mode in v5/v6 skesk parse Don't panic on unknown AEAD mode in v5/v6 SKESK packets Aug 14, 2026

@twiss twiss left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks!

@twiss
twiss merged commit d1dc24f into ProtonMail:main Aug 14, 2026
9 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.

2 participants