Repository navigation
Stop the obfuscation marker being emitted twice - #33
Merged
Merged
Conversation
An obfuscated message began and ended with the marker repeated:
00023CD100023CD1wV4DTmk...=x9Yz00023CD100023CD1
Blank lines were being replaced with the marker alongside the armor header and
footer, and armored output carries one blank line after the version header and
another at the end. So each real marker was followed, or preceded, by a second.
Ordinary lines are joined with no separator, so the body was never affected —
only the two ends.
This is not only untidy. The setting exists to make a message harder to
recognise as PGP, and a doubled sixteen-character hex run is a more distinctive
signature than a single one, so the duplication worked against the feature it
belonged to.
Blank lines are now dropped, like the version header immediately above them.
Nothing depended on them: every use of the marker either tests for it at the
start of a message or strips every occurrence, and deobfuscation discards all
newlines and re-chunks the base64 from scratch.
Messages already sent carry the doubled markers and cannot be recalled, so
there is a test pinning that they still deobfuscate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An obfuscated message began and ended with the marker repeated:
Cause
Not every line break — only blank lines.
obfuscateMarkersreplaced them with the marker alongside the armor header and footer, and armored output happens to carry a blank line immediately after theVersion:header and another at the end, both sitting next to a real marker:-----BEGIN PGP MESSAGE-----00023CD1Version: BCPG v1.81line.isBlank()00023CD1←wV4DTmk...-----END PGP MESSAGE-----00023CD1line.isBlank()00023CD1←Ordinary lines are joined with no separator, so the body was never affected — only the two ends.
Why it is worth fixing rather than tolerating
The setting's own description is "make messages harder to detect". A doubled sixteen-character hex run is a more distinctive signature than a single eight-character one, so the duplication worked against the feature it belonged to.
The fix
Blank lines are dropped, like the
Version:header immediately above them. Nothing depended on them being marked: every use of the marker either tests for it at the start of a message or strips every occurrence, and deobfuscation discards all newlines and re-chunks the base64 from scratch, so the blank line's structural role is rebuilt rather than preserved.Backward compatibility
Messages already sent carry the doubled markers and cannot be recalled. Deobfuscation removes every occurrence, so they still work — and there is a test pinning that, because it would be easy to tidy the deobfuscator later and silently break every message sent before this release.
Tests
Seven, covering what the feature emits as well as whether it round trips:
-----BEGIN,-----ENDorBCPGversion string survivesdecryptMessageand the notification listener both detect by leading markerMutation-checked: restoring the old behaviour fails two of them. 63 tests total, lint clean.
obfuscateMarkersbecomesinternal+@VisibleForTesting, matchingdeobfuscateMarkers.Note
No version bump here — that goes in the release PR when you cut the next version, same as 0.4 and 0.5.