diff --git a/client/files.go b/client/files.go index f73c2bc..3b0098d 100644 --- a/client/files.go +++ b/client/files.go @@ -4,6 +4,7 @@ package client import ( + "bytes" "fmt" "io" "mime" @@ -55,6 +56,24 @@ func (c *Client) UploadFile(path, mimeType, filename string, isEncrypted bool) ( if mimeType == "" { mimeType = detectMIME(path, f) } + return c.uploadMultipart(f, mimeType, filename, isEncrypted) +} + +// UploadBytes uploads content already held in memory. It exists for the +// client-side-encryption path: the bytes on the wire are the HRBC2 envelope, not +// what is on disk, so there is no file to stream and the caller has already +// resolved the MIME type and filename from the original. +// +// Those two stay PLAINTEXT on the resource record, matching every other Harbor +// client — an accepted metadata leak. The server hashes whatever it receives, so +// the resulting content address covers the ciphertext. +func (c *Client) UploadBytes(content []byte, mimeType, filename string, isEncrypted bool) ([]byte, error) { + return c.uploadMultipart(bytes.NewReader(content), mimeType, filename, isEncrypted) +} + +// uploadMultipart is the shared body of UploadFile and UploadBytes: build the +// form fields and POST the multipart request. +func (c *Client) uploadMultipart(content io.Reader, mimeType, filename string, isEncrypted bool) ([]byte, error) { fields := map[string]string{"filename": filename} if mimeType != "" { fields["mime"] = mimeType @@ -62,7 +81,20 @@ func (c *Client) UploadFile(path, mimeType, filename string, isEncrypted bool) ( if isEncrypted { fields["is_encrypted"] = "true" } - return c.doMultipart("/files/upload", fields, "file", filename, f) + return c.doMultipart("/files/upload", fields, "file", filename, content) +} + +// DetectMIME resolves a path's MIME type exactly as UploadFile would, so a +// caller that transforms the bytes before upload (client-side encryption) can +// still record the ORIGINAL type rather than the ciphertext's. Returns "" when +// it cannot do better than application/octet-stream. +func DetectMIME(path string) string { + f, err := os.Open(path) + if err != nil { + return "" + } + defer f.Close() + return detectMIME(path, f) } // detectMIME guesses a file's MIME type: first by extension (covers png, jpg, diff --git a/client/files_test.go b/client/files_test.go index a0ceffd..02d0449 100644 --- a/client/files_test.go +++ b/client/files_test.go @@ -209,3 +209,59 @@ func TestFetchURLDownloadError(t *testing.T) { } } } + +// TestUploadBytesMultipart proves the in-memory upload path sends exactly the +// bytes it is given (no re-reading from disk), stamps is_encrypted, and keeps +// the caller's filename and MIME rather than deriving them from the ciphertext. +func TestUploadBytesMultipart(t *testing.T) { + var rec recordedRequest + srv := newTestServer(t, &rec, 201, `{"hash":"abc","filename":"secret.pdf","is_encrypted":true}`) + defer srv.Close() + + sealed := []byte("HRBC2\x00\x01\x02ciphertext-not-plaintext") + if _, err := testClient(srv.URL).UploadBytes(sealed, "application/pdf", "secret.pdf", true); err != nil { + t.Fatalf("UploadBytes error: %v", err) + } + if rec.Path != "/files/upload" { + t.Errorf("path = %s", rec.Path) + } + if !strings.HasPrefix(rec.ContentType, "multipart/form-data") { + t.Errorf("content-type = %q", rec.ContentType) + } + body := string(rec.Body) + for _, want := range []string{"ciphertext-not-plaintext", "secret.pdf", "application/pdf", "is_encrypted", "true"} { + if !strings.Contains(body, want) { + t.Errorf("multipart body missing %q: %s", want, body) + } + } +} + +// TestUploadBytesOmitsEncryptedFlag proves is_encrypted is sent only when true, +// so an ordinary upload is not mislabelled. +func TestUploadBytesOmitsEncryptedFlag(t *testing.T) { + var rec recordedRequest + srv := newTestServer(t, &rec, 201, `{"hash":"abc"}`) + defer srv.Close() + + if _, err := testClient(srv.URL).UploadBytes([]byte("plain"), "text/plain", "a.txt", false); err != nil { + t.Fatalf("UploadBytes error: %v", err) + } + if strings.Contains(string(rec.Body), "is_encrypted") { + t.Errorf("is_encrypted should be omitted when false: %s", rec.Body) + } +} + +// TestDetectMIME resolves the original file's type, which the encrypted upload +// path needs before it replaces the bytes with an opaque envelope. +func TestDetectMIME(t *testing.T) { + path := filepath.Join(t.TempDir(), "a.txt") + if err := os.WriteFile(path, []byte("hello"), 0644); err != nil { + t.Fatal(err) + } + if got := DetectMIME(path); !strings.HasPrefix(got, "text/plain") { + t.Errorf("DetectMIME = %q, want text/plain…", got) + } + if got := DetectMIME(filepath.Join(t.TempDir(), "missing")); got != "" { + t.Errorf("DetectMIME(missing) = %q, want \"\"", got) + } +} diff --git a/cmd/assets/skill/reference.md b/cmd/assets/skill/reference.md index 464dbd7..3078279 100644 --- a/cmd/assets/skill/reference.md +++ b/cmd/assets/skill/reference.md @@ -274,7 +274,16 @@ Content-addressed (sha256) blobs. | `harbor files list` | List with linked notes | `--mime`, `--note-id`, `--ocr-status`, `--encrypted`, `--updated-since`, paging | | `harbor files get ` | Presigned URL + metadata (no bytes) | | | `harbor files check` | Does a blob exist? | `--hash` (+`--size`) or `--file` (hash computed locally) | -| `harbor files download ` | Download bytes | `--output` (`-` = stdout), `--raw` | +| `harbor files download ` | Download bytes | `--output` (`-` = stdout), `--raw`, `--ciphertext` | + +**Encrypted attachments.** `files upload --encrypted` seals the bytes on this +machine (HRBC2 binary envelope, master key) before uploading — it needs +`HARBOR_PASSPHRASE` and refuses rather than uploading in the clear. `files +download` detects an encrypted blob and decrypts it automatically when the +passphrase is set; without it the download is refused, and `--ciphertext` writes +the raw envelope instead. Filename and MIME stay plaintext on the record; the +stored size is 33 bytes larger than the original, and `files check --file` can +never match an encrypted upload because the content hash covers the ciphertext. --- diff --git a/cmd/files.go b/cmd/files.go index aac2cbc..d0fe484 100644 --- a/cmd/files.go +++ b/cmd/files.go @@ -4,15 +4,19 @@ package cmd import ( + "bytes" "crypto/sha256" "encoding/hex" "errors" "fmt" "io" "os" + "path/filepath" "strings" "github.com/HarborMyNotes/harbor-cli/client" + "github.com/HarborMyNotes/harbor-cli/config" + "github.com/HarborMyNotes/harbor-cli/crypto" "github.com/spf13/cobra" ) @@ -66,7 +70,14 @@ var filesListCmd = &cobra.Command{ var filesCheckCmd = &cobra.Command{ Use: "check", Short: "Check whether a blob already exists", - Long: "Check by --hash (and optional --size), or pass --file to compute the sha256 and size locally.", + Long: `Check by --hash (and optional --size), or pass --file to compute the sha256 and +size locally. + +--file hashes the file as it sits on disk, so it only answers for uploads that +were NOT encrypted. An encrypted blob is stored as an HRBC2 envelope and its +content address covers that ciphertext, which carries a fresh nonce every time — +so a file uploaded with 'files upload --encrypted' will always report "does not +exist" here, and encrypted uploads never deduplicate.`, Example: ` harbor files check --hash e3b0c442...b855 harbor files check --file diagram.png`, RunE: func(cmd *cobra.Command, args []string) error { @@ -100,22 +111,85 @@ var filesUploadCmd = &cobra.Command{ Use: "upload ", Short: "Upload a file", Args: cobra.ExactArgs(1), + Long: `Upload a file and get back its content-addressed resource record. + +With --encrypted the bytes are sealed on this machine before they leave it: the +file is wrapped in an HRBC2 binary envelope under your master key, and the server +only ever sees ciphertext. It needs HARBOR_PASSPHRASE and refuses rather than +uploading anything in the clear. + +The filename and MIME type are recorded as they were, in plaintext — the same +accepted trade every other Harbor client makes, so the file stays recognisable in +listings. The stored size is the envelope's (33 bytes larger than the original), +and because the content hash covers the ciphertext, an encrypted upload can never +deduplicate against an existing blob.`, Example: ` harbor files upload diagram.png - harbor files upload report.pdf --mime application/pdf`, + harbor files upload report.pdf --mime application/pdf + harbor files upload secrets.pdf --encrypted`, RunE: func(cmd *cobra.Command, args []string) error { - c, _, err := loadClientFromConfig() + c, creds, err := loadClientFromConfig() if err != nil { return err } - data, err := c.UploadFile(args[0], stringFlag(cmd, "mime"), stringFlag(cmd, "filename"), boolFlag(cmd, "encrypted")) + path, mimeType, filename := args[0], stringFlag(cmd, "mime"), stringFlag(cmd, "filename") + + if !boolFlag(cmd, "encrypted") { + data, uerr := c.UploadFile(path, mimeType, filename, false) + if uerr != nil { + return mapFileError(uerr) + } + printResult(data, displayResource) + return nil + } + + // Fail closed: without the key we would otherwise upload the file in the + // clear while stamping it is_encrypted, which is worse than not uploading. + key, err := filesKey(c, creds) + if err != nil { + return err + } + data, err := uploadEncrypted(c, key, path, mimeType, filename) if err != nil { - return mapFileError(err) + return err } printResult(data, displayResource) return nil }, } +// uploadEncrypted seals a file on this machine and uploads the envelope, so the +// server never receives the plaintext. +// +// It is a named function rather than inline RunE so a test can point it at a +// mock server and assert what actually goes on the wire. That matters more here +// than usual: the bug this replaced was an upload that stamped the resource +// is_encrypted while sending the file in the clear, and every unit test still +// passed. Asserting the multipart body carries the envelope and NOT the +// plaintext is the only check that catches a regression to it. +func uploadEncrypted(c *client.Client, key []byte, path, mimeType, filename string) ([]byte, error) { + content, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("cannot read file: %w", err) + } + // Resolve both from the ORIGINAL file, before sealing — sniffing the + // envelope would record every encrypted upload as octet-stream. + if filename == "" { + filename = filepath.Base(path) + } + if mimeType == "" { + mimeType = client.DetectMIME(path) + } + sealed, err := crypto.SealBytes(key, content) + if err != nil { + return nil, fmt.Errorf("encrypting %s: %w", filepath.Base(path), err) + } + data, err := c.UploadBytes(sealed, mimeType, filename, true) + if err != nil { + return nil, mapFileError(err) + } + return data, nil +} + // filesGetCmd shows the presigned download URL + basic metadata for a blob. var filesGetCmd = &cobra.Command{ Use: "get ", @@ -141,11 +215,18 @@ var filesDownloadCmd = &cobra.Command{ Use: "download ", Short: "Download a file's bytes", Args: cobra.ExactArgs(1), - Long: "Download a blob. By default it follows a presigned URL; --raw streams through the API instead. Writes to --output (default: the stored filename, or - for stdout).", + Long: `Download a blob. By default it follows a presigned URL; --raw streams through +the API instead. Writes to --output (default: the stored filename, or - for stdout). + +Encrypted files are decrypted automatically when HARBOR_PASSPHRASE is set. When +it is not, the download is refused rather than writing ciphertext you cannot use +— pass --ciphertext to write the raw envelope anyway (for backups or moving bytes +between machines).`, Example: ` harbor files download e3b0... --output diagram.png - harbor files download e3b0... --raw --output -`, + harbor files download e3b0... --raw --output - + harbor files download e3b0... --ciphertext --output sealed.bin`, RunE: func(cmd *cobra.Command, args []string) error { - c, _, err := loadClientFromConfig() + c, creds, err := loadClientFromConfig() if err != nil { return err } @@ -184,7 +265,11 @@ var filesDownloadCmd = &cobra.Command{ if out == "" { out = suggestedName } - n, err := writeOutput(out, body) + content, err := decryptDownload(c, creds, body, boolFlag(cmd, "ciphertext")) + if err != nil { + return err + } + n, err := writeOutput(out, content) if err != nil { return err } @@ -195,6 +280,91 @@ var filesDownloadCmd = &cobra.Command{ }, } +// filesKey unlocks the master key for an encrypted upload, turning the two +// sentinel unlock failures into actionable refusals. It fails closed on purpose: +// the alternative is uploading a file in the clear while stamping the resource +// is_encrypted, which leaves the user believing a plaintext blob is sealed. +func filesKey(c *client.Client, creds *config.Credentials) ([]byte, error) { + key, err := unlockMasterKey(c, creds) + if err == nil { + return key, nil + } + switch { + case errors.Is(err, errPassphraseNotSet): + return nil, fmt.Errorf("--encrypted needs your encryption passphrase and %s is not set, so nothing was uploaded.\n\n"+ + " export %s=$(op read \"op://Vault/Harbor/passphrase\")\n\n"+ + "Uploading anyway would put the file on the server in the clear while marking it encrypted", + passphraseEnv, passphraseEnv) + case errors.Is(err, errNoKeystore): + return nil, errors.New("this account has no encryption keys yet, so nothing was uploaded — run 'harbor crypto setup' first (or 'harbor crypto sync' if you set them up on another device)") + } + return nil, fmt.Errorf("could not unlock encryption, so nothing was uploaded: %w", err) +} + +// decryptDownload transparently unwraps an encrypted blob on its way to disk. +// +// It sniffs the leading bytes for the HRBC2 binary magic rather than trusting +// resource metadata, because the presigned-download path returns no is_encrypted +// field — and sniffing is what the other clients do too. A plaintext blob is +// passed straight through as a stream, so ordinary downloads keep their memory +// profile; only an envelope is buffered, which AES-GCM requires anyway since the +// authentication tag lives at the end. +// +// With no passphrase it REFUSES rather than writing ciphertext into a file the +// user will think is their document. That matches web and macOS/iOS, which both +// decline to hand over bytes they cannot read. Android and Windows still have +// surfaces that pass the raw envelope through (Android opens the presigned URL +// in a browser; Windows' in-note save path writes ciphertext even when +// unlocked) — those are bugs on those clients, not a different design, so +// refusing here is the parity-correct behaviour rather than a deviation. +// wantCiphertext is the explicit opt-out for backups and moving bytes between +// machines. +func decryptDownload(c *client.Client, creds *config.Credentials, body io.Reader, wantCiphertext bool) (io.Reader, error) { + head := make([]byte, crypto.BinaryEnvelopeMinBytes) + n, err := io.ReadFull(body, head) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + return nil, err + } + head = head[:n] + rest := io.MultiReader(bytes.NewReader(head), body) + + if !crypto.IsBinaryEnvelope(head) { + return rest, nil + } + if wantCiphertext { + fmt.Fprintln(os.Stderr, dim("This file is encrypted; writing the raw envelope as asked (--ciphertext).")) + return rest, nil + } + + key, err := unlockMasterKey(c, creds) + if err != nil { + switch { + case errors.Is(err, errPassphraseNotSet): + return nil, fmt.Errorf("this file is encrypted and %s is not set, so nothing was written.\n\n"+ + " export %s=$(op read \"op://Vault/Harbor/passphrase\")\n\n"+ + "Re-run with --ciphertext to write the sealed bytes instead", + passphraseEnv, passphraseEnv) + case errors.Is(err, errNoKeystore): + return nil, errors.New("this file is encrypted but this account has no encryption keys cached — run 'harbor crypto sync' first, or re-run with --ciphertext to write the sealed bytes") + } + return nil, fmt.Errorf("this file is encrypted and the key could not be unlocked, so nothing was written: %w", err) + } + + sealed, err := io.ReadAll(rest) + if err != nil { + return nil, err + } + plain, err := crypto.OpenBytes(key, sealed) + if err != nil { + // Also reachable for a plaintext file that happens to begin with the + // ASCII bytes "HRBC2" — magic-sniffing cannot tell those apart, so name + // the escape hatch rather than insisting the key is wrong. + return nil, fmt.Errorf("this file did not decrypt with your key, so nothing was written "+ + "(if it was never encrypted, re-run with --ciphertext to write it as stored): %w", err) + } + return bytes.NewReader(plain), nil +} + // mapFileError gives friendly messages for file-specific codes. func mapFileError(err error) error { var apiErr *client.APIError @@ -351,10 +521,11 @@ func init() { filesUploadCmd.Flags().String("mime", "", "MIME type (server sniffs when omitted)") filesUploadCmd.Flags().String("filename", "", "Stored filename (defaults to the base name)") - filesUploadCmd.Flags().Bool("encrypted", false, "Mark the upload as client-encrypted (opaque bytes)") + filesUploadCmd.Flags().Bool("encrypted", false, "Encrypt the bytes on this machine before uploading (requires HARBOR_PASSPHRASE)") filesDownloadCmd.Flags().String("output", "", "Output path, or - for stdout (default: the stored filename)") filesDownloadCmd.Flags().Bool("raw", false, "Stream through the API instead of following a presigned URL") + filesDownloadCmd.Flags().Bool("ciphertext", false, "Write an encrypted file's raw envelope instead of decrypting it") filesCmd.AddCommand(filesListCmd, filesCheckCmd, filesUploadCmd, filesGetCmd, filesDownloadCmd) rootCmd.AddCommand(filesCmd) diff --git a/cmd/files_test.go b/cmd/files_test.go index 6f1df9f..e0b8701 100644 --- a/cmd/files_test.go +++ b/cmd/files_test.go @@ -5,10 +5,16 @@ package cmd import ( "bytes" + "io" + "net/http" + "net/http/httptest" "os" "path/filepath" "strings" "testing" + + "github.com/HarborMyNotes/harbor-cli/client" + "github.com/HarborMyNotes/harbor-cli/crypto" ) func TestHashFile(t *testing.T) { @@ -87,3 +93,207 @@ func TestMapFileError(t *testing.T) { } } } + +// readAll drains a reader in tests, failing rather than returning an error. +func readAll(t *testing.T, r io.Reader) []byte { + t.Helper() + b, err := io.ReadAll(r) + if err != nil { + t.Fatalf("reading: %v", err) + } + return b +} + +// TestDecryptDownloadPassesPlaintextThrough proves an ordinary blob is handed +// back byte-identical, including files shorter than the sniff window — reading +// the first 33 bytes must never truncate a 5-byte file. +func TestDecryptDownloadPassesPlaintextThrough(t *testing.T) { + lockedSession(t) + for _, body := range []string{"", "x", "hello", "just a normal file, not encrypted at all", strings.Repeat("z", 5000)} { + got, err := decryptDownload(nil, nil, strings.NewReader(body), false) + if err != nil { + t.Fatalf("decryptDownload(%d bytes): %v", len(body), err) + } + if out := string(readAll(t, got)); out != body { + t.Fatalf("plaintext altered: got %d bytes, want %d", len(out), len(body)) + } + } +} + +// TestDecryptDownloadDecryptsEnvelope proves an encrypted blob is unwrapped when +// the session is unlocked, which is the whole point of the transparent path. +func TestDecryptDownloadDecryptsEnvelope(t *testing.T) { + key := newMasterKey(t) + unlockedSession(t, key) + + original := []byte("the real file bytes, secret") + sealed, err := crypto.SealBytes(key, original) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + got, err := decryptDownload(nil, nil, bytes.NewReader(sealed), false) + if err != nil { + t.Fatalf("decryptDownload: %v", err) + } + if out := readAll(t, got); !bytes.Equal(out, original) { + t.Fatalf("decrypted = %q, want %q", out, original) + } +} + +// TestDecryptDownloadRefusesWhenLocked proves the CLI does NOT write ciphertext +// into a file the user thinks is their document, and that the refusal names both +// the env var and the escape hatch. +func TestDecryptDownloadRefusesWhenLocked(t *testing.T) { + key := newMasterKey(t) + sealed, err := crypto.SealBytes(key, []byte("secret")) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + lockedSession(t) + + got, err := decryptDownload(nil, nil, bytes.NewReader(sealed), false) + if err == nil { + t.Fatalf("expected a refusal, got %q", readAll(t, got)) + } + for _, want := range []string{"encrypted", "HARBOR_PASSPHRASE", "--ciphertext", "nothing was written"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal missing %q: %v", want, err) + } + } +} + +// TestDecryptDownloadCiphertextOptOut proves --ciphertext hands back the raw +// envelope untouched, for backups and moving bytes between machines. +func TestDecryptDownloadCiphertextOptOut(t *testing.T) { + key := newMasterKey(t) + sealed, err := crypto.SealBytes(key, []byte("secret")) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + lockedSession(t) + + got, err := decryptDownload(nil, nil, bytes.NewReader(sealed), true) + if err != nil { + t.Fatalf("decryptDownload(--ciphertext): %v", err) + } + if out := readAll(t, got); !bytes.Equal(out, sealed) { + t.Fatalf("--ciphertext altered the envelope: %d bytes, want %d", len(out), len(sealed)) + } +} + +// TestDecryptDownloadWrongKey proves a blob sealed under a different key fails +// closed with an explanation rather than writing garbage to disk. +func TestDecryptDownloadWrongKey(t *testing.T) { + sealed, err := crypto.SealBytes(newMasterKey(t), []byte("secret")) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + unlockedSession(t, make([]byte, 32)) // a different, all-zero key + + if _, err := decryptDownload(nil, nil, bytes.NewReader(sealed), false); err == nil { + t.Fatal("expected a decrypt failure") + } else if !strings.Contains(err.Error(), "nothing was written") { + t.Errorf("error should say nothing was written: %v", err) + } +} + +// TestFilesKeyRefusals proves the upload path refuses with actionable text +// rather than uploading a plaintext file stamped is_encrypted. +func TestFilesKeyRefusals(t *testing.T) { + lockedSession(t) + _, err := filesKey(nil, nil) + if err == nil { + t.Fatal("expected a refusal with no passphrase set") + } + for _, want := range []string{"HARBOR_PASSPHRASE", "nothing was uploaded", "in the clear"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal missing %q: %v", want, err) + } + } +} + +// TestUploadEncryptedSendsEnvelopeNotPlaintext is the regression guard for the +// bug this feature replaced: an upload that stamped the resource is_encrypted +// while putting the file on the server in the clear. It asserts against the +// actual multipart body on the wire, so reverting to a plaintext upload fails +// here even though every other test would still pass. +func TestUploadEncryptedSendsEnvelopeNotPlaintext(t *testing.T) { + const marker = "ATTACHMENT-PLAINTEXT-MARKER-12345" + + var body []byte + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ = io.ReadAll(r.Body) + if r.URL.Path != "/files/upload" { + t.Errorf("path = %s", r.URL.Path) + } + _, _ = w.Write([]byte(`{"hash":"abc","filename":"secret.txt","is_encrypted":true}`)) + })) + defer srv.Close() + + path := filepath.Join(t.TempDir(), "secret.txt") + if err := os.WriteFile(path, []byte(marker), 0644); err != nil { + t.Fatal(err) + } + key := newMasterKey(t) + if _, err := uploadEncrypted(client.NewClient(srv.URL, "at_test"), key, path, "", ""); err != nil { + t.Fatalf("uploadEncrypted: %v", err) + } + + if bytes.Contains(body, []byte(marker)) { + t.Fatal("the plaintext reached the server — the bytes were not encrypted before upload") + } + if !bytes.Contains(body, []byte("HRBC2")) { + t.Fatal("no HRBC2 envelope in the multipart body") + } + // Filename and MIME come from the ORIGINAL file, not from the ciphertext. + for _, want := range []string{"secret.txt", "text/plain", "is_encrypted"} { + if !bytes.Contains(body, []byte(want)) { + t.Errorf("multipart body missing %q", want) + } + } + + // And what was sent must decrypt back to the original file. + start := bytes.Index(body, []byte("HRBC2")) + sealed := body[start : start+crypto.BinaryEnvelopeMinBytes+len(marker)] + got, err := crypto.OpenBytes(key, sealed) + if err != nil { + t.Fatalf("the uploaded envelope does not decrypt: %v", err) + } + if string(got) != marker { + t.Fatalf("decrypted = %q, want %q", got, marker) + } +} + +// TestFilesKeyNoKeystore covers the other refusal branch: a passphrase is set +// but the account has no keys yet. +func TestFilesKeyNoKeystore(t *testing.T) { + resetSession() + t.Cleanup(resetSession) + t.Setenv("HARBOR_PASSPHRASE", "pw") + t.Setenv("HOME", t.TempDir()) + + sessionUnlockd, sessionKey, sessionErr = true, nil, errNoKeystore + _, err := filesKey(nil, nil) + if err == nil { + t.Fatal("expected a refusal when no keystore exists") + } + for _, want := range []string{"crypto setup", "nothing was uploaded"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal missing %q: %v", want, err) + } + } +} + +// TestFilesKeyHappyPath proves filesKey returns the unlocked key unchanged, so +// the refusal wrapper cannot break the working case. +func TestFilesKeyHappyPath(t *testing.T) { + key := newMasterKey(t) + unlockedSession(t, key) + got, err := filesKey(nil, nil) + if err != nil { + t.Fatalf("filesKey: %v", err) + } + if !bytes.Equal(got, key) { + t.Fatal("filesKey returned a different key") + } +} diff --git a/crypto/README.md b/crypto/README.md index 2acd730..3e3ed8a 100644 --- a/crypto/README.md +++ b/crypto/README.md @@ -73,13 +73,35 @@ contract): - An encrypted note may have an **empty title** (sent as `""`, not an envelope); `content` must always be a valid envelope. -### Attachments (binary envelope — not yet implemented in the CLI) +### Attachments (binary envelope) ``` "HRBC2"(5 ASCII bytes) ‖ iv(12) ‖ ciphertext ‖ tag(16) // raw bytes resources.hash = sha256(the whole binary envelope) ``` +- **AES-256-GCM** under the **master key** — the same key as note fields. There is + no per-note or per-resource key. +- **AAD: NONE.** Attachments are sealed with a nil/empty AAD. This is the + finalized rule, and it is what web, macOS/iOS, Android and Windows already do; + binding AAD here would make files written by the CLI unreadable by all of them. + Integrity is instead bound by the content address: the server stores + `hash = sha256(envelope)`, computed over the ciphertext. +- Because the hash covers ciphertext and every seal uses a fresh nonce, + **dedup only works across identical ciphertext** — in practice, never. Accepted. +- The **minimum envelope is 33 bytes** (5 + 12 + 16). Anything sniffing for the + magic must read at least that many bytes, or every input answers "not + encrypted"; that bug has shipped on another client before. Use + `crypto.BinaryEnvelopeMinBytes`. +- `filename` and `mime` stay **plaintext** on the resource record (an accepted + metadata leak, matching the other clients), and `size` is the **envelope** size, + not the plaintext size. +- ⚠️ A **string** envelope begins with the same 5-byte magic, so magic-sniffing + cannot distinguish the two. They never mix in practice — string envelopes live + in note fields, binary ones in blob bytes. + +Go: `crypto.SealBytes` / `crypto.OpenBytes` / `crypto.IsBinaryEnvelope`. + ## Writing an encrypted note Encrypted notes must be created with a **client-generated `id`** (sent in the diff --git a/crypto/crypto.go b/crypto/crypto.go index 11e73d7..e9efa7b 100644 --- a/crypto/crypto.go +++ b/crypto/crypto.go @@ -30,6 +30,7 @@ package crypto import ( + "bytes" "crypto/aes" "crypto/cipher" "crypto/rand" @@ -306,6 +307,87 @@ func IsEnvelope(s string) bool { return err == nil } +// BinaryEnvelopeMinBytes is the smallest possible HRBC2 binary envelope: the +// 5-byte magic, a 12-byte nonce and a 16-byte GCM tag, wrapping zero bytes of +// plaintext. +// +// Exported because sniffing fewer bytes than this makes IsBinaryEnvelope answer +// false for EVERY input, which silently turns "skip the ones already encrypted" +// into "skip all of them". That exact bug has shipped on another Harbor client +// before; read at least this many bytes before sniffing. +const BinaryEnvelopeMinBytes = len(EnvelopeVersion) + nonceLen + tagLen + +// SealBytes encrypts raw bytes into the HRBC2 BINARY envelope used for +// attachments: +// +// "HRBC2"(5 ASCII) ‖ iv(12) ‖ ciphertext ‖ tag(16) +// +// AES-256-GCM under the master key with a fresh random nonce, and — unlike the +// string envelope used for note fields — **no AAD**. That is not an oversight: +// 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 sha256 over this whole envelope. Adding AAD would make +// files written here unreadable everywhere else. See crypto/README.md. +func SealBytes(masterKey, plaintext []byte) ([]byte, error) { + nonce := make([]byte, nonceLen) + if _, err := rand.Read(nonce); err != nil { + return nil, fmt.Errorf("generating nonce: %w", err) + } + return sealBytesWithNonce(masterKey, plaintext, nonce) +} + +// sealBytesWithNonce is SealBytes with the nonce supplied rather than generated, +// so tests can reproduce the pinned binary known-answer vector byte for byte. +// Production code must always use SealBytes: reusing a nonce under the same key +// breaks AES-GCM catastrophically. +func sealBytesWithNonce(masterKey, plaintext, nonce []byte) ([]byte, error) { + if len(nonce) != nonceLen { + return nil, fmt.Errorf("nonce must be %d bytes, got %d", nonceLen, len(nonce)) + } + aead, err := newAEAD(masterKey) + if err != nil { + return nil, err + } + // Seal appends ciphertext‖tag to the destination, so starting from the magic + // plus the nonce builds the whole envelope in one allocation. + env := make([]byte, 0, BinaryEnvelopeMinBytes+len(plaintext)) + env = append(env, EnvelopeVersion...) + env = append(env, nonce...) + return aead.Seal(env, nonce, plaintext, nil), nil +} + +// OpenBytes reverses SealBytes, returning the original plaintext. It returns +// ErrNotEnvelope when the input is not structurally an HRBC2 binary envelope +// (so a caller can tell "this file was never encrypted" apart from "this file +// will not open"), and ErrDecrypt when authentication fails. +func OpenBytes(masterKey, envelope []byte) ([]byte, error) { + if !IsBinaryEnvelope(envelope) { + return nil, ErrNotEnvelope + } + aead, err := newAEAD(masterKey) + if err != nil { + return nil, err + } + magic := len(EnvelopeVersion) + nonce := envelope[magic : magic+nonceLen] + pt, err := aead.Open(nil, nonce, envelope[magic+nonceLen:], nil) + if err != nil { + return nil, ErrDecrypt + } + return pt, nil +} + +// IsBinaryEnvelope reports whether b begins an HRBC2 binary envelope and is long +// enough to be one. It never decrypts, so a false positive is possible in +// principle — arbitrary bytes starting with "HRBC2" — but the magic makes that +// vanishingly unlikely for real files, and OpenBytes fails closed regardless. +// +// b may be a prefix of the file rather than the whole of it, but it must be at +// least BinaryEnvelopeMinBytes long or this returns false for everything. +func IsBinaryEnvelope(b []byte) bool { + return len(b) >= BinaryEnvelopeMinBytes && bytes.HasPrefix(b, []byte(EnvelopeVersion)) +} + // splitEnvelope parses and validates an HRBC2 envelope's structure, returning the // decoded IV and ciphertext(‖tag). It enforces the same shape rules as the // server so anything the CLI considers an envelope will also pass server-side diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 07089fc..9e8967f 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -4,9 +4,11 @@ package crypto import ( + "bytes" "crypto/aes" "crypto/cipher" "encoding/base64" + "encoding/hex" "errors" "regexp" "strings" @@ -405,3 +407,180 @@ func TestSealedFieldMatchesServerShape(t *testing.T) { t.Fatalf("ct not ≥16 raw-url bytes: %q (%v)", parts[2], err) } } + +// binaryVector pins the HRBC2 BINARY envelope (attachments) as a hex literal: +// master key = the bytes 0x00..0x1f, nonce = the bytes 0x00..0x0b, plaintext +// "Secret file bytes", and NO AAD. It was computed with Python's cryptography +// AESGCM, independently of this package. +// +// Unlike the field envelope, no other Harbor client had published a binary +// known-answer vector — the CLI is minting the first one. It is derived purely +// from the format the other four already implement (magic ‖ iv ‖ ct ‖ tag, +// master key, no AAD), so it describes their behaviour rather than inventing a +// new rule; the value is here so a future client can check itself against it. +const ( + binaryVector = "4852424332000102030405060708090a0b1467b569a091e27de42df2abd3900c08f0b1a847dcafcd37cbc951802b90471172" + binaryVectorPlaintext = "Secret file bytes" +) + +// TestSealBytes_BinaryVector proves sealing the vector's parameters reproduces +// the pinned envelope byte for byte — magic, nonce placement, tag position and +// the absence of AAD all at once. Any of them changing breaks this. +func TestSealBytes_BinaryVector(t *testing.T) { + got, err := sealBytesWithNonce(crossClientKey(), []byte(binaryVectorPlaintext), crossClientNonce()) + if err != nil { + t.Fatalf("sealBytesWithNonce: %v", err) + } + if hex.EncodeToString(got) != binaryVector { + t.Fatalf("binary envelope does not match the pinned vector\n got: %s\nwant: %s", hex.EncodeToString(got), binaryVector) + } +} + +// TestOpenBytes_BinaryVector proves the other direction: the pinned envelope +// decrypts back to the original bytes, so a file written by another client is +// readable here. +func TestOpenBytes_BinaryVector(t *testing.T) { + env, err := hex.DecodeString(binaryVector) + if err != nil { + t.Fatalf("decoding the vector: %v", err) + } + got, err := OpenBytes(crossClientKey(), env) + if err != nil { + t.Fatalf("OpenBytes on the pinned binary vector: %v", err) + } + if string(got) != binaryVectorPlaintext { + t.Fatalf("plaintext = %q, want %q", got, binaryVectorPlaintext) + } +} + +// TestSealBytes_NoAAD proves attachments carry NO AAD, by opening our envelope +// with an independent decryptor that passes nil. If someone "hardens" SealBytes +// by binding AAD, every other Harbor client stops being able to read our files +// and this test says so. +func TestSealBytes_NoAAD(t *testing.T) { + key := crossClientKey() + env, err := SealBytes(key, []byte(binaryVectorPlaintext)) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + block, err := aes.NewCipher(key) + if err != nil { + t.Fatalf("aes: %v", err) + } + g, err := cipher.NewGCM(block) + if err != nil { + t.Fatalf("gcm: %v", err) + } + magic := len("HRBC2") + got, err := g.Open(nil, env[magic:magic+12], env[magic+12:], nil) + if err != nil { + t.Fatalf("a no-AAD reader could not open our envelope: %v (other clients would fail the same way)", err) + } + if string(got) != binaryVectorPlaintext { + t.Fatalf("plaintext = %q, want %q", got, binaryVectorPlaintext) + } +} + +// TestSealOpenBytesRoundTrip covers the sizes that break naive implementations: +// empty input, one byte, and a payload spanning several GCM blocks. It also +// pins that two seals of the same bytes differ, i.e. the nonce is fresh. +func TestSealOpenBytesRoundTrip(t *testing.T) { + key := crossClientKey() + for _, size := range []int{0, 1, 15, 16, 17, 4096} { + pt := make([]byte, size) + for i := range pt { + pt[i] = byte(i % 251) + } + env, err := SealBytes(key, pt) + if err != nil { + t.Fatalf("SealBytes(%d): %v", size, err) + } + if want := BinaryEnvelopeMinBytes + size; len(env) != want { + t.Fatalf("envelope for %d bytes is %d long, want %d", size, len(env), want) + } + if !IsBinaryEnvelope(env) { + t.Fatalf("SealBytes(%d) output does not sniff as an envelope", size) + } + if bytes.Contains(env, pt) && size > 16 { + t.Fatalf("SealBytes(%d) leaked the plaintext into the envelope", size) + } + got, err := OpenBytes(key, env) + if err != nil { + t.Fatalf("OpenBytes(%d): %v", size, err) + } + if !bytes.Equal(got, pt) { + t.Fatalf("round-trip mismatch at %d bytes", size) + } + } + + a, _ := SealBytes(key, []byte("same")) + b, _ := SealBytes(key, []byte("same")) + if bytes.Equal(a, b) { + t.Fatal("two seals of the same bytes are identical — the nonce is not fresh") + } +} + +// TestOpenBytes_Rejects proves OpenBytes fails closed and distinguishes "this +// was never encrypted" (ErrNotEnvelope) from "this will not open" (ErrDecrypt), +// which is what lets callers skip plaintext files instead of erroring on them. +func TestOpenBytes_Rejects(t *testing.T) { + key := crossClientKey() + env, err := SealBytes(key, []byte(binaryVectorPlaintext)) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + + for name, input := range map[string][]byte{ + "plain text": []byte("just a normal file, not encrypted at all"), + "empty": {}, + "magic but short": []byte("HRBC2" + strings.Repeat("x", 10)), + "wrong magic": append([]byte("HRBX9"), env[5:]...), + "exactly min-1": make([]byte, BinaryEnvelopeMinBytes-1), + } { + if _, err := OpenBytes(key, input); !errors.Is(err, ErrNotEnvelope) { + t.Errorf("OpenBytes(%s): err = %v, want ErrNotEnvelope", name, err) + } + } + + // A STRING envelope shares the same 5-byte magic, so it sniffs as binary and + // fails at authentication rather than at the structural check. Pinned so the + // overlap is a known, deliberate property rather than a surprise: the two + // never mix in practice (string envelopes live in note fields, binary ones in + // blob bytes), and every other client sniffs on the magic alone — narrowing + // it here would break the format we are matching. + strEnv := []byte("HRBC2.AAECAwQFBgcICQoL.FGe1aaCR4nXiNfKr04YcFISiyIPEgI0uXE_6tfgGWQw") + if _, err := OpenBytes(key, strEnv); !errors.Is(err, ErrDecrypt) { + t.Errorf("OpenBytes(string envelope): err = %v, want ErrDecrypt", err) + } + + // Structurally an envelope, but tampered: must be ErrDecrypt, not silence. + tampered := append([]byte(nil), env...) + tampered[len(tampered)-1] ^= 0xff + if _, err := OpenBytes(key, tampered); !errors.Is(err, ErrDecrypt) { + t.Errorf("OpenBytes(tampered): err = %v, want ErrDecrypt", err) + } + + wrongKey := make([]byte, 32) + if _, err := OpenBytes(wrongKey, env); !errors.Is(err, ErrDecrypt) { + t.Errorf("OpenBytes(wrong key): err = %v, want ErrDecrypt", err) + } +} + +// TestIsBinaryEnvelope_ShortPrefix pins the trap that has shipped on another +// Harbor client: sniffing fewer than BinaryEnvelopeMinBytes makes every input +// answer false, which turns "skip files already encrypted" into "skip all". +func TestIsBinaryEnvelope_ShortPrefix(t *testing.T) { + env, err := SealBytes(crossClientKey(), []byte(binaryVectorPlaintext)) + if err != nil { + t.Fatalf("SealBytes: %v", err) + } + if BinaryEnvelopeMinBytes != 33 { + t.Fatalf("BinaryEnvelopeMinBytes = %d, want 33 (5 magic + 12 iv + 16 tag)", BinaryEnvelopeMinBytes) + } + if IsBinaryEnvelope(env[:BinaryEnvelopeMinBytes-1]) { + t.Fatal("a too-short prefix sniffed as an envelope") + } + if !IsBinaryEnvelope(env[:BinaryEnvelopeMinBytes]) { + t.Fatal("a prefix of exactly BinaryEnvelopeMinBytes did not sniff as an envelope") + } +}