kestra-io / kestra-io/plugin-crypto

fix(openpgp): verify SEIP message integrity (MDC) in Decrypt

Open
#123 0 comments 0 reactions 0 assignees View on GitHub
area/plugin good first issue kind/security
Dominant language
Java
Stars
2
Forks
5
Avg merge
1d 20h
Merged PRs (30d)
4

Description

### What happens

`Decrypt` consumes the decrypted payload from `PGPPublicKeyEncryptedData.getDataStream(...)` but never calls `encData.verify()` afterwards. For a Symmetrically Encrypted Integrity Protected (SEIP) packet, that call is what checks the Modification Detection Code trailer. Without it, an attacker who can modify the ciphertext in transit can tamper with the encrypted payload and `Decrypt` will emit the corrupted plaintext as a successful task output rather than failing.

Discovered during review of #121 ([review comment](https://github.com/kestra-io/plugin-crypto/pull/121#pullrequestreview-5087912625)). Pre-existing, so it was deliberately left out of that PR's scope.

### Why it still matters after #121

#121 makes signature verification actually work, so a *signed* message is now protected — a tampered payload fails `sig.verify(...)`. The gap remains for the common case of an **encrypted-but-unsigned** message, where `signUsersKey` is not set: there is currently no integrity check at all on that path.

### Acceptance criteria

- [ ] `encData.verify()` is called after the data stream is fully consumed, and a `false` result (or `PGPException`) fails the task with a clear message distinguishing tampering from a decryption-key problem.
- [ ] Handle the legacy non-integrity-protected case explicitly: `encData.isIntegrityProtected()` is `false` for old SED packets, where `verify()` must not be called. Decide and document whether such messages are rejected outright or accepted with a warning log.
- [ ] Test: an encrypted, unsigned message whose ciphertext bytes were flipped fails decryption instead of yielding corrupt output.
- [ ] Test: a well-formed encrypted, unsigned message still decrypts unchanged (no regression on the `runUnsigned` path).
- [ ] The output file is not published to internal storage when the integrity check fails.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at Decrypt and the PGPPublicKeyEncryptedData.getDataStream(...) path; trace where the stream is fully consumed and how runUnsigned publishes output. Check the OpenPGP API behavior of verify() and isIntegrityProtected(), then add coverage for tampered and unchanged unsigned messages. Done means failures prevent task success and internal-storage publication, while legacy SED behavior is explicit.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
cryptography, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.