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

@@ -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

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)
+	}
+}

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

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)
+			}
+		})
+	}
+}

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)
+	}
+}

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]),