From 60cef7bcdce49f80532b4e55d931578fcc2c9eaa Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Fri, 23 Oct 2026 16:07:55 +0200 Subject: [PATCH] feat: return one uniform decrypt error from the org cipher and the instance encrypter for malformed, short, tampered or foreign-key ciphertext, and answer a send whose subject or body cannot be decrypted or whose workspace key cannot be loaded with EMAIL_FAILED and a fixed reason through failSend, logging only the failure class and never sending --- internal/app/cipher/decrypt.go | 16 ++-- internal/app/cipher/decrypt_test.go | 56 ++++++++++++++ internal/app/worker/event_send_email.go | 70 +++++++++++------ internal/app/worker/send_decrypt_test.go | 96 ++++++++++++++++++++++++ internal/pkg/encrypt/decrypt_test.go | 52 +++++++++++++ internal/pkg/encrypt/encrypter.go | 19 +++-- 6 files changed, 272 insertions(+), 37 deletions(-) create mode 100644 internal/app/cipher/decrypt_test.go create mode 100644 internal/app/worker/send_decrypt_test.go create mode 100644 internal/pkg/encrypt/decrypt_test.go diff --git a/internal/app/cipher/decrypt.go b/internal/app/cipher/decrypt.go index 49f069d32..cb86cf619 100644 --- a/internal/app/cipher/decrypt.go +++ b/internal/app/cipher/decrypt.go @@ -8,28 +8,32 @@ import ( "errors" ) +// ErrDecrypt is the one error Decrypt returns, whatever was wrong with the +// input: malformed, short, tampered or sealed under another key. +var ErrDecrypt = errors.New("cipher: value could not be decrypted") + func (c *Cipher) Decrypt(ctxc context.Context, s string) (string, error) { encSecret, err := base64.StdEncoding.DecodeString(s) if err != nil { - return "", err + return "", ErrDecrypt } block, err := aes.NewCipher(c.plainDEK) if err != nil { - return "", err + return "", ErrDecrypt } gcm, err := cipher.NewGCM(block) if err != nil { - return "", err + return "", ErrDecrypt } nonceSize := gcm.NonceSize() - if len(encSecret) < nonceSize { - return "", errors.New("invalid ciphertext") + if len(encSecret) < nonceSize+gcm.Overhead() { + return "", ErrDecrypt } nonce, ciphertext := encSecret[:nonceSize], encSecret[nonceSize:] plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) if err != nil { - return "", err + return "", ErrDecrypt } return string(plaintext), nil diff --git a/internal/app/cipher/decrypt_test.go b/internal/app/cipher/decrypt_test.go new file mode 100644 index 000000000..be3ea35fd --- /dev/null +++ b/internal/app/cipher/decrypt_test.go @@ -0,0 +1,56 @@ +package cipher + +import ( + "bytes" + "encoding/base64" + "errors" + "testing" + + "github.com/google/uuid" +) + +func TestDecryptRefusesEveryBadInputWithOneError(t *testing.T) { + key := bytes.Repeat([]byte{7}, 32) + c, err := NewStatic(key).Cipher(t.Context(), uuid.New()) + if err != nil { + t.Fatal(err) + } + sealed, err := c.Encrypt(t.Context(), "Quarterly pricing for Acme") + if err != nil { + t.Fatal(err) + } + raw, err := base64.StdEncoding.DecodeString(sealed) + if err != nil { + t.Fatal(err) + } + tampered := append([]byte(nil), raw...) + tampered[len(tampered)-1] ^= 1 + other, err := NewStatic(bytes.Repeat([]byte{9}, 32)).Cipher(t.Context(), uuid.New()) + if err != nil { + t.Fatal(err) + } + + cases := map[string]struct { + c *Cipher + input string + }{ + "malformed base64": {c, "not base64 at all!"}, + "empty": {c, ""}, + "nonce only": {c, base64.StdEncoding.EncodeToString(raw[:12])}, + "short tag": {c, base64.StdEncoding.EncodeToString(raw[:20])}, + "tampered": {c, base64.StdEncoding.EncodeToString(tampered)}, + "other key": {other, sealed}, + "bad key length": {&Cipher{plainDEK: []byte("short")}, sealed}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + plain, err := tc.c.Decrypt(t.Context(), tc.input) + if !errors.Is(err, ErrDecrypt) || err.Error() != ErrDecrypt.Error() || plain != "" { + t.Fatalf("Decrypt = %q, %v; want ErrDecrypt", plain, err) + } + }) + } + if plain, err := c.Decrypt(t.Context(), sealed); err != nil || plain != "Quarterly pricing for Acme" { + t.Fatalf("round trip = %q, %v", plain, err) + } +} diff --git a/internal/app/worker/event_send_email.go b/internal/app/worker/event_send_email.go index 50f4005e8..9a89c90dc 100644 --- a/internal/app/worker/event_send_email.go +++ b/internal/app/worker/event_send_email.go @@ -97,20 +97,16 @@ func (w *WorkerService) HandleSendEmail(ctx context.Context, sendEmail models.Se } transportGeneration := mail.TransportGeneration - // Decrypt subject - subject := sendEmail.Subject - if w.CipherService != nil { - c, cerr := w.CipherService.Cipher(ctx, sendEmail.OrgID) - if cerr == nil { - decSubject, cerr := c.Decrypt(ctx, sendEmail.Subject) - if cerr == nil { - subject = decSubject - } - } + subject, err := w.openSendField(ctx, sendEmail.OrgID, sendEmail.Subject) + if err != nil { + return w.failUnopenedSend(ctx, sendEmail, err) } // Fetch email body from S3 (attachment refs ride inside the emsg blob). body, err := w.fetchEmailBody(ctx, sendEmail.OrgID, sendEmail.BodyS3Key) + if errors.Is(err, errSendKeyUnavailable) || errors.Is(err, errSendUndecryptable) { + return w.failUnopenedSend(ctx, sendEmail, err) + } if err != nil { log.Error().Err(err).Str("s3_key", sendEmail.BodyS3Key).Msg("Failed to fetch email body from S3") return w.failSend(ctx, sendEmail, fmt.Sprintf("failed to fetch email body: %v", err), true) @@ -273,19 +269,11 @@ func (w *WorkerService) fetchEmailBody(ctx context.Context, orgID uuid.UUID, s3K bodyPlain := string(blob.PlainText) bodyHTML := string(blob.HTMLBody) - if w.CipherService != nil { - if c, cerr := w.CipherService.Cipher(ctx, orgID); cerr == nil { - if bodyPlain != "" { - if decPlain, decErr := c.Decrypt(ctx, bodyPlain); decErr == nil { - bodyPlain = decPlain - } - } - if bodyHTML != "" { - if decHTML, decErr := c.Decrypt(ctx, bodyHTML); decErr == nil { - bodyHTML = decHTML - } - } - } + if bodyPlain, err = w.openSendField(ctx, orgID, bodyPlain); err != nil { + return nil, err + } + if bodyHTML, err = w.openSendField(ctx, orgID, bodyHTML); err != nil { + return nil, err } return &sendBody{ @@ -370,6 +358,42 @@ func (w *WorkerService) failSend(ctx context.Context, sendEmail models.SendEmail return w.sendEmailFailure(sendEmail.TaskID, sendEmail.EmailID, nil, reason) } +// Fixed reasons for a send the worker could not open; they never carry the underlying error. +const ( + sendReasonKeyUnavailable = "The workspace encryption key could not be loaded, so nothing was sent." + sendReasonUndecryptable = "The message could not be decrypted, so nothing was sent." +) + +var ( + errSendKeyUnavailable = errors.New("send: workspace key unavailable") + errSendUndecryptable = errors.New("send: message could not be decrypted") +) + +// openSendField decrypts one sealed send field; an empty field stays empty. +func (w *WorkerService) openSendField(ctx context.Context, orgID uuid.UUID, sealed string) (string, error) { + if w.CipherService == nil || sealed == "" { + return sealed, nil + } + c, err := w.CipherService.Cipher(ctx, orgID) + if err != nil { + return "", errSendKeyUnavailable + } + plain, err := c.Decrypt(ctx, sealed) + if err != nil { + return "", errSendUndecryptable + } + return plain, nil +} + +// failUnopenedSend answers a send it could not open; a missing key is retried, unreadable ciphertext is not. +func (w *WorkerService) failUnopenedSend(ctx context.Context, sendEmail models.SendEmail, err error) error { + log.Error().Str("task_id", sendEmail.TaskID.String()).Str("class", err.Error()).Msg("Send not opened") + if errors.Is(err, errSendKeyUnavailable) { + return w.failSend(ctx, sendEmail, sendReasonKeyUnavailable, true) + } + return w.failSend(ctx, sendEmail, sendReasonUndecryptable, false) +} + // Account-level refusals additionally raise their typed mailbox event. func (w *WorkerService) sendMailboxError(taskID uuid.UUID, emailID uuid.UUID, mail *wmail.WMail, mailErr *errx.MailError) { // Determine the appropriate event type based on error diff --git a/internal/app/worker/send_decrypt_test.go b/internal/app/worker/send_decrypt_test.go new file mode 100644 index 000000000..a861569be --- /dev/null +++ b/internal/app/worker/send_decrypt_test.go @@ -0,0 +1,96 @@ +package worker + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "strings" + "testing" + + "github.com/google/uuid" + "golang.org/x/oauth2" + + "github.com/warmbly/warmbly/internal/app/cipher" + "github.com/warmbly/warmbly/internal/app/worker/wmail" + "github.com/warmbly/warmbly/internal/client/goog" + "github.com/warmbly/warmbly/internal/infrastructure/codec" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/emsg" +) + +type failingCipherService struct{} + +func (failingCipherService) Cipher(context.Context, uuid.UUID) (*cipher.Cipher, error) { + return nil, errors.New("key store unavailable") +} + +func TestSendThatCannotBeDecryptedFailsWithoutSending(t *testing.T) { + keys := cipher.NewStatic(bytes.Repeat([]byte{5}, 32)) + orgID := uuid.New() + c, err := keys.Cipher(t.Context(), orgID) + if err != nil { + t.Fatal(err) + } + seal := func(s string) string { + out, err := c.Encrypt(t.Context(), s) + if err != nil { + t.Fatal(err) + } + return out + } + goodSubject, goodBody := seal("Hello Ann"), seal("Body for Ann") + const garbage = "c2VjcmV0LWJ1dC1ub3QtY2lwaGVydGV4dA==" + + cases := map[string]struct { + keys cipher.CipherService + subject, body string + html string + reason string + }{ + "subject undecryptable": {keys, garbage, goodBody, "", sendReasonUndecryptable}, + "plain undecryptable": {keys, goodSubject, garbage, "", sendReasonUndecryptable}, + "html undecryptable": {keys, goodSubject, goodBody, garbage, sendReasonUndecryptable}, + "short ciphertext": {keys, goodSubject, "AAAA", "", sendReasonUndecryptable}, + "key unavailable": {failingCipherService{}, goodSubject, goodBody, "", sendReasonKeyUnavailable}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + rt := &dispatchNativeTransport{} + ctx := context.WithValue(t.Context(), oauth2.HTTPClient, &http.Client{Transport: rt}) + client := &goog.Client{Email: "sender@example.test"} + if err := client.InitWithSource(ctx, oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "fixture-token"})); err != nil { + t.Fatal(err) + } + bus := &filingBus{} + w := newLoadingWorker(&capturedEvents{}) + w.ID, w.Bus, w.Codec, w.CipherService = uuid.NewString(), bus, codec.NewJSON(), tc.keys + id := uuid.New() + w.mailManager.Emails[id] = &wmail.WMail{OrgID: &orgID, EmailType: models.InboxProviderGoogle, GoogleData: &wmail.GoogleData{Client: client}} + blob := &emsg.EmailBlob{PlainText: []byte(tc.body), HTMLBody: []byte(tc.html)} + data, err := blob.EncodeBinary() + if err != nil { + t.Fatal(err) + } + w.Storage = dispatchBodyStore{data: data} + command := models.SendEmail{TaskID: uuid.New(), EmailID: id, OrgID: orgID, To: []string{"recipient@example.test"}, Subject: tc.subject, BodyS3Key: "fixture"} + if err := w.HandleSendEmail(ctx, command); err != nil { + t.Fatalf("HandleSendEmail: %v", err) + } + if rt.calls.Load() != 0 { + t.Fatal("a message that could not be opened reached the provider") + } + if len(bus.events) != 1 || bus.events[0].Type != models.JobEventTypeEmailFailed { + t.Fatalf("events = %+v, want one EMAIL_FAILED", bus.events) + } + raw, err := json.Marshal(bus.events[0].Body) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(raw), tc.reason) || strings.Contains(string(raw), garbage) || strings.Contains(string(raw), "key store") { + t.Fatalf("failure = %s, want only %q", raw, tc.reason) + } + }) + } +} diff --git a/internal/pkg/encrypt/decrypt_test.go b/internal/pkg/encrypt/decrypt_test.go new file mode 100644 index 000000000..c2e0e1c13 --- /dev/null +++ b/internal/pkg/encrypt/decrypt_test.go @@ -0,0 +1,52 @@ +package encrypt + +import ( + "errors" + "strings" + "testing" +) + +func TestDecryptRefusesEveryBadInputWithOneError(t *testing.T) { + e, err := NewEncrypterFromHex(strings.Repeat("11", 32)) + if err != nil { + t.Fatal(err) + } + other, err := NewEncrypterFromHex(strings.Repeat("22", 32)) + if err != nil { + t.Fatal(err) + } + sealed, err := e.Encrypt("mailbox-password") + if err != nil { + t.Fatal(err) + } + last := sealed[len(sealed)-1:] + flipped := "0" + if last == "0" { + flipped = "1" + } + for name, input := range map[string]string{ + "not hex": "zz" + sealed[2:], + "odd hex": sealed[:len(sealed)-1], + "empty": "", + "short": sealed[:16], + "no tag": sealed[:24+8], + "tampered": sealed[:len(sealed)-1] + flipped, + "plaintext": "mailbox-password", + } { + t.Run(name, func(t *testing.T) { + plain, err := e.Decrypt(input) + if !errors.Is(err, ErrDecrypt) || err.Error() != ErrDecrypt.Error() || plain != "" { + t.Fatalf("Decrypt = %q, %v; want ErrDecrypt", plain, err) + } + }) + } + if _, err := other.Decrypt(sealed); !errors.Is(err, ErrDecrypt) { + t.Fatalf("other key: %v, want ErrDecrypt", err) + } + if _, err := e.DecryptEncrypted(&Encrypted{Nonce: "zz", Ciphertext: "00"}); !errors.Is(err, ErrDecrypt) { + t.Fatalf("DecryptEncrypted: %v, want ErrDecrypt", err) + } + if plain, err := e.Decrypt(sealed); err != nil || plain != "mailbox-password" { + t.Fatalf("round trip = %q, %v", plain, err) + } +} diff --git a/internal/pkg/encrypt/encrypter.go b/internal/pkg/encrypt/encrypter.go index 2ec9fc111..f92151ead 100644 --- a/internal/pkg/encrypt/encrypter.go +++ b/internal/pkg/encrypt/encrypter.go @@ -22,6 +22,9 @@ type Encrypted struct { // ErrUnreadable is a value shaped like ciphertext that no configured key opens. var ErrUnreadable = errors.New("sealed value does not open under any configured credential key") +// ErrDecrypt is the one error a decrypt returns, whatever was wrong with the input. +var ErrDecrypt = errors.New("encrypt: value could not be decrypted") + // Encrypter performs AES-GCM encryption with a fixed 32-byte key. type Encrypter struct { aead cipher.AEAD @@ -99,11 +102,11 @@ func (e *Encrypter) Encrypt(plaintext string) (string, error) { func (e *Encrypter) Decrypt(hexStr string) (string, error) { enc, err := ParseHex(hexStr) if err != nil { - return "", err + return "", ErrDecrypt } plain, err := e.DecryptEncrypted(enc) if err != nil { - return "", err + return "", ErrDecrypt } return string(plain), nil } @@ -129,14 +132,14 @@ func (e *Encrypter) EncryptBytes(plain []byte) (*Encrypted, error) { func (e *Encrypter) DecryptEncrypted(enc *Encrypted) ([]byte, error) { nonce, err := hex.DecodeString(enc.Nonce) if err != nil { - return nil, err + return nil, ErrDecrypt } ct, err := hex.DecodeString(enc.Ciphertext) if err != nil { - return nil, err + return nil, ErrDecrypt } if len(nonce) != e.aead.NonceSize() { - return nil, errors.New("invalid encryption nonce") + return nil, ErrDecrypt } plaintext, err := e.aead.Open(nil, nonce, ct, nil) if err == nil { @@ -147,7 +150,7 @@ func (e *Encrypter) DecryptEncrypted(enc *Encrypted) ([]byte, error) { return plaintext, nil } } - return nil, err + return nil, ErrDecrypt } // Reseal returns stored sealed under the primary key when only a read key opens it; changed is false when the primary already does. @@ -203,11 +206,11 @@ func (e *Encrypted) Hex() string { func ParseHex(hexStr string) (*Encrypted, error) { data, err := hex.DecodeString(hexStr) if err != nil { - return nil, err + return nil, ErrDecrypt } nonceSize := 12 // GCM standard nonce if len(data) < nonceSize { - return nil, errors.New("ciphertext too short") + return nil, ErrDecrypt } return &Encrypted{ Nonce: hex.EncodeToString(data[:nonceSize]),