Fix #41: encrypt attachment bytes with the HRBC2 binary envelope - #89
Merged
Conversation
'files upload --encrypted' stamped the resource is_encrypted and uploaded
the file in the clear. The flag was a lie the user could tell the server.
It now seals the bytes on this machine first.
- crypto: SealBytes/OpenBytes/IsBinaryEnvelope for the binary envelope
('HRBC2' magic + iv + ciphertext + tag), AES-256-GCM under the master
key with NO AAD - matching web, macOS/iOS, Android and Windows, whose
integrity binding is the sha256 content address over the ciphertext.
Pins the first binary known-answer vector, computed independently.
- files upload --encrypted: reads the file, seals it, uploads the
envelope. Fails closed without HARBOR_PASSPHRASE rather than shipping
plaintext under an encrypted label. Filename and MIME are resolved from
the original and stay plaintext, as on every other client.
- files download: sniffs the magic (the presigned path returns no
is_encrypted field) and decrypts transparently. Plaintext blobs still
stream rather than buffer. With no passphrase it refuses instead of
writing unusable ciphertext, matching the other four clients;
--ciphertext is the explicit opt-out.
- files check --help: says why --file can never match an encrypted blob.
- crypto/README.md: pins the finalized attachment AAD rule (none) and the
33-byte sniff minimum.
… regression test - decryptDownload's comment said all four other clients refuse to hand over bytes they cannot read. Only web and macOS/iOS do: Android opens the presigned URL in a browser, and Windows' in-note save path writes ciphertext even when unlocked. Reworded to say what is actually true and why refusing is still the parity-correct choice. Comments in this repo get read as contract facts, so a wrong one is worse than none. - Extract uploadEncrypted so a test can point it at a mock server and assert the multipart body carries the envelope and NOT the plaintext. Nothing covered that wiring: swapping the call back to UploadFile would have restored the original bug with every test still green. Verified the new test fails when the plaintext upload is reintroduced. - filesKey: cover the no-keystore branch and the happy path. - A plaintext file starting with the ASCII bytes 'HRBC2' sniffs as an envelope and cannot decrypt; that error now names --ciphertext instead of only insisting the key is wrong. - Downgrade the 'shipped twice' claim to the one occurrence verified.
4 tasks
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.
Closes #41
harbor files upload --encryptedstamped the resourceis_encryptedand uploaded the file in the clear. The flag was a lie the user could tell the server. This implements the real thing.What changed
crypto— the binary envelope.SealBytes/OpenBytes/IsBinaryEnvelope:AES-256-GCM under the master key, no AAD. That is the decisive interop decision the issue asked to finalize, and it is not a fresh choice — web, macOS/iOS, Android and Windows all pass no AAD here, because an attachment's integrity is bound instead by its content address, which the server computes as
sha256over the whole envelope. Binding AAD would make CLI-written files unreadable on all four. Now pinned incrypto/README.md.files upload --encryptedreads the file, seals it, and uploads the envelope. It fails closed withoutHARBOR_PASSPHRASE— uploading plaintext while stamping it encrypted is worse than not uploading. Filename and MIME are resolved from the original file before sealing (sniffing the envelope would record everything asoctet-stream) and stay plaintext on the record, matching the other clients.files downloadsniffs the leading bytes for the magic and decrypts transparently. It sniffs rather than trusting metadata because the presigned-download path returns nois_encryptedfield — and sniffing is what the other clients do. Plaintext blobs still stream; only an envelope is buffered, which GCM requires anyway since the tag is at the end.With no passphrase it refuses rather than writing ciphertext into a file the user thinks is their document. The issue's acceptance criteria allowed "ciphertext (or a clear, non-failing message)", so this is a deliberate deviation, recorded on #41.
To be accurate about parity: web and macOS/iOS refuse; Android and Windows do not — Android fires
ACTION_VIEWat the presigned URL so the browser saves the raw envelope, and Windows' in-note save path writes ciphertext even when unlocked. Those read as bugs on those clients rather than a different design, so refusing here matches the intended behaviour rather than diverging from a settled one.--ciphertextis the explicit opt-out for backups and moving bytes between machines.files check --helpnow says why--filecan never match an encrypted blob (the hash covers ciphertext, and every seal uses a fresh nonce).On the "or refuse" option in #81
#81 offered a cheaper alternative — make
notes encryptrefuse notes with attachments. This PR takes the real path instead, because it builds the primitives that the honest version of #81 needs. Note this PR does not close #81: that issue is aboutnotes encryptre-encrypting an existing note's attachments (download → re-wrap → re-upload → rewrite embeds → call the swap endpoint).SealBytes/OpenBytesare the foundation for it; the seal-path wiring is still to come.Verification
Full suite, lint clean:
New tests. KATs pin the binary envelope byte for byte (seal and open); a no-AAD test fails if anyone "hardens"
SealBytesby binding AAD; round-trip covers 0/1/15/16/17/4096 bytes and asserts a fresh nonce each time;OpenBytesis pinned to distinguishErrNotEnvelope("never encrypted") fromErrDecrypt("will not open"); and the ≥33-byte sniff minimum is pinned, because reading fewer bytes makes every input answer "not encrypted" — a bug that has shipped on another Harbor client before.End-to-end against a real server — throwaway account on a disposable environment, isolated
HOME, all eight checks passed.The plaintext is gone from what the server stores:
Decrypted by a standalone Python script importing none of this repo's code (Argon2id → unwrap the keystore → AES-GCM open with no AAD):
Round-trip, refusal, and the opt-out:
Plaintext uploads unchanged — no
HRBC2prefix, stored bytes identical to the original, hash still equalssha256(plaintext), clean round-trip. A 4096-byte random binary round-tripped identical, with the same 33-byte delta.Behaviour changes worth knowing
files check --help.Also noted while testing
harbor files delete, andDELETE /api/v1/files/<hash>404s — so an unwanted blob cannot be cleared. Combined with encrypted uploads not deduplicating, repeated encrypted testing burns quota with no way to reclaim it. Out of scope here; worth its own issue.HARBOR_TOKENlimitations filed as Headless auth: 'whoami' ignores HARBOR_TOKEN, and 'crypto setup' can't run under it at all #88 reproduced exactly as described, and were worked around with a hand-writtencredentials.json.