PBM-1791: Add client-side encryption for backups#1341
Draft
jouir wants to merge 1 commit into
Draft
Conversation
|
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds client-side, streaming encryption of backup payloads so plaintext doesn’t leave the node, and wires encryption metadata/config through backup, PITR oplog handling, and restore paths.
Changes:
- Introduces
pbm/encryptstreaming AEAD format (AES-256-GCM, XChaCha20-Poly1305) with Argon2id-derived keys and framing. - Threads encryption type + passphrase resolution through upload/download, backup metadata, oplog chunks, and restore flows.
- Adds CLI/config/e2e coverage for encryption (including passphrase redaction in config output).
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/sdk.go | Exposes encryption type/consts and adds encryption options to SDK backup option structs. |
| pbm/storage/storage.go | Extends upload pipeline to include encryption stage. |
| pbm/snapshot/dump.go | Encrypts logical dump streams on upload and decrypts on download. |
| pbm/slicer/slicer.go | Propagates encryption metadata into PITR oplog chunk handling and uploads encrypted chunks. |
| pbm/restore/selective.go | Adds decrypt step before decompress in selective configsvr restores. |
| pbm/restore/restore.go | Adds decrypt step for PITR oplog chunk replay. |
| pbm/restore/physical.go | Adds decrypt step for physical file restore and reuses resolved passphrase. |
| pbm/restore/logical.go | Adds decrypt step for snapshot restore paths and propagates encryption metadata into oplog chunks. |
| pbm/restore/logical_cfgsvr_full.go | Adds decrypt step for full configsvr restore paths. |
| pbm/oplog/chunk.go | Stores encryption type on oplog chunk metadata. |
| pbm/encrypt/encrypt.go | New streaming encryption/decryption implementation. |
| pbm/encrypt/encrypt_test.go | Unit tests for stream format correctness, tamper detection, and deadlock avoidance. |
| pbm/defs/defs.go | Adds DefaultEncryption. |
| pbm/ctrl/cmd.go | Adds encryption field to backup command payload. |
| pbm/config/config.go | Adds encryption config fields, passphrase resolution, and redaction for config output. |
| pbm/backup/types.go | Persists encryption type in backup metadata. |
| pbm/backup/physical.go | Encrypts physical backup data uploads (excluding filelist). |
| pbm/backup/logical.go | Encrypts logical dump and single-namespace archive uploads. |
| pbm/backup/backup.go | Carries encryption setting into initial backup metadata. |
| go.mod | Promotes golang.org/x/crypto to a direct dependency. |
| e2e-tests/pkg/tests/sharded/test_encryption.go | Adds sharded e2e tests validating encrypted objects and wrong-passphrase failure. |
| e2e-tests/docker/conf/*.yaml | Adds MinIO encryption configs (AES/XChaCha + wrong-passphrase). |
| e2e-tests/cmd/pbm-test/run.go | Wires new encryption e2e scenarios into test runner. |
| e2e-tests/cmd/ensure-oplog/main.go | Encrypts ensured oplog chunk uploads when configured. |
| cmd/pbm/main.go | Adds pbm backup --encryption flag and enum validation. |
| cmd/pbm/backup.go | Threads CLI encryption choice into backup command. |
| cmd/pbm-speed-test/speedt.go | Updates speed test upload call with encryption args (none). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+187
to
+193
| header := make([]byte, fixedHeaderLen) | ||
| if _, err := io.ReadFull(r, header); err != nil { | ||
| return nil, errors.Wrap(err, "read header") | ||
| } | ||
| if [4]byte(header[:versionOffset]) != magic { | ||
| return nil, errors.New("not a PBM encrypted stream") | ||
| } |
Comment on lines
+331
to
+346
| func (d *aeadReader) Read(p []byte) (int, error) { | ||
| for len(d.buf) == 0 { | ||
| if d.eof { | ||
| return 0, io.EOF | ||
| } | ||
| if err := d.readFrame(); err != nil { | ||
| return 0, err | ||
| } | ||
| } | ||
| n := copy(p, d.buf) | ||
| d.buf = d.buf[n:] | ||
| return n, nil | ||
| } | ||
|
|
||
| func (d *aeadReader) Close() error { return nil } | ||
|
|
Comment on lines
+224
to
+229
| return &aeadReader{r: r, aead: aead, noncePrefix: noncePrefix, header: header}, nil | ||
| case EncryptionTypeNone: | ||
| fallthrough | ||
| default: | ||
| return io.NopCloser(r), nil | ||
| } |
Comment on lines
+103
to
+111
| // EncryptionPassphrase returns the passphrase used to derive the encryption | ||
| // key, resolved from the environment, passphrase file or config, or an empty | ||
| // string when encryption is not configured | ||
| func (c *Config) EncryptionPassphrase() (string, error) { | ||
| if c == nil || c.Backup == nil { | ||
| return "", nil | ||
| } | ||
| return c.Backup.resolvePassphrase() | ||
| } |
Comment on lines
263
to
268
| go func() { | ||
| n, rwErr.read = src.WriteTo(w) | ||
| rwErr.compress = w.Close() | ||
| rwErr.encrypt = encw.Close() | ||
| pw.Close() | ||
| }() |
Comment on lines
37
to
+55
| @@ -44,12 +49,17 @@ | |||
| atomic.AddInt64(&uploadSize, rc.n) | |||
| }() | |||
|
|
|||
| w, err := compress.Compress(pw, compression, compressionLevel) | |||
| encw, err := encrypt.Encrypt(pw, encryption, passphrase) | |||
| if err != nil { | |||
| return nil, errors.Wrapf(err, "create encryptor: %q", ns) | |||
| } | |||
Comment on lines
89
to
103
| r, err := download(ns) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "download: %q", ns) | ||
| } | ||
|
|
||
| if ns == archive.MetaFile { | ||
| return r, nil | ||
| } | ||
|
|
||
| r, err = encrypt.Decrypt(r, encryption, passphrase) | ||
| if err != nil { | ||
| return nil, errors.Wrapf(err, "create decryptor: %q", ns) | ||
| } | ||
| r, err = compress.Decompress(r, compression) | ||
| return r, errors.Wrapf(err, "create decompressor: %q", ns) |
Member
|
Hey @jouir, thanks a lot for your PR. We'll review it at some point, but first, we need to discuss the feature within the team. We already prioritized your Jira ticket. I appreciate your patience! |
Encrypt backup data on the client before it reaches storage so plaintext never leaves the node. A new encrypt package provides a streaming AEAD format (AES-256-GCM and XChaCha20-Poly1305) with an Argon2id-derived key, wired through the backup, oplog, and restore paths. Configured via the encryption fields in the backup configuration and the --encryption flag on `pbm backup`. The passphrase is redacted from config output. Default is none. Backup metadata and the physical-backup filelist are not yet encrypted and are left for follow-up commits.
Author
I have rebased my branch to resolve the conflicts. No worries, I'll wait. |
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.
Encrypt backup data on the client before it reaches storage so plaintext never leaves the node. A new encrypt package provides a streaming AEAD format (AES-256-GCM and XChaCha20-Poly1305) with an Argon2id-derived key, wired through the backup, oplog, and restore paths. Configured via the encryption fields in the backup configuration and the --encryption flag on
pbm backup. The passphrase is redacted from config output. Default is none.Backup metadata and the physical-backup filelist are not yet encrypted and are left for follow-up commits.