diff --git a/docs/content/docs/development/configuration.mdx b/docs/content/docs/development/configuration.mdx index ae904d511..e0a01c6bc 100644 --- a/docs/content/docs/development/configuration.mdx +++ b/docs/content/docs/development/configuration.mdx @@ -56,7 +56,7 @@ Five values protect the whole instance. Compose ships a working default for each | `INTERNAL_API_TOKEN` | any random string | The backend's `/api/v1/internal/` routes, which workers and the tracking service authenticate against | yes | | `SECRET_KEY_BASE` | 64 characters or more | Phoenix session signing in the realtime service | yes | | `KMS_LOCAL_MASTER_KEY` | base64, exactly 32 bytes | The root key that seals every per-organization data key | yes | -| `CREDENTIALS_ENCRYPTION_KEY` | exactly 64 hex characters | Mailbox SMTP and IMAP credentials at rest | yes | +| `CREDENTIALS_ENCRYPTION_KEY` | exactly 64 hex characters | Mailbox credentials at rest: SMTP and IMAP passwords, and Gmail and Outlook OAuth access and refresh tokens | yes | Generate real ones before anyone else can reach the instance: @@ -82,7 +82,7 @@ Values that must be identical across services, because each service reads its ow | `AUTH_SECRET`, seen by realtime as `JWT_SECRET` | backend, realtime | The dashboard loads but never goes live: the websocket rejects every token | | `INTERNAL_API_TOKEN`, seen by workers as `ENCRYPTED_KEYS_WORKER_TOKEN` | backend, worker, tracking | Workers cannot fetch decryption keys and tracking cannot resolve click tickets. Both fail closed with `401` | | `KMS_LOCAL_MASTER_KEY` | backend, consumer, worker | Sealed data keys cannot be opened, so mailbox credentials stop decrypting | -| `CREDENTIALS_ENCRYPTION_KEY` | backend, worker | Stored SMTP and IMAP passwords stop decrypting | +| `CREDENTIALS_ENCRYPTION_KEY` | backend, worker | Stored SMTP and IMAP passwords and OAuth tokens stop decrypting, so no mailbox can send or sync | The secret check runs in the backend. The consumer and the workers start happily on a published default, so an instance can look healthy while one process is using a key anyone can read from GitHub. The `secret_published_default` check on [Instance health](/development/instance-health/#secret_published_default) is what catches it. diff --git a/docs/content/docs/development/deployment-guide.mdx b/docs/content/docs/development/deployment-guide.mdx index ba9d8e5fe..4c1668249 100644 --- a/docs/content/docs/development/deployment-guide.mdx +++ b/docs/content/docs/development/deployment-guide.mdx @@ -141,7 +141,7 @@ EOF | `INTERNAL_API_TOKEN` | any random string | The backend's internal API, which workers and the tracking service authenticate against | | `SECRET_KEY_BASE` | 64+ chars | Phoenix session signing in the realtime service | | `KMS_LOCAL_MASTER_KEY` | base64, exactly 32 bytes | The root key that seals every per-organization data key | -| `CREDENTIALS_ENCRYPTION_KEY` | exactly 64 hex chars | Mailbox SMTP and IMAP credentials at rest | +| `CREDENTIALS_ENCRYPTION_KEY` | exactly 64 hex chars | Mailbox credentials at rest: SMTP and IMAP passwords, and Gmail and Outlook OAuth access and refresh tokens | `APP_ENV=prod` requires no cloud account. Error reporting and GeoIP lookups are used when configured and skipped with a logged note when they are not. @@ -204,7 +204,7 @@ These are the ones that break things quietly when they drift, because each servi | `AUTH_SECRET`, seen by realtime as `JWT_SECRET` | backend, realtime | The dashboard loads but never goes live: the websocket rejects every token | | `INTERNAL_API_TOKEN`, seen by workers as `ENCRYPTED_KEYS_WORKER_TOKEN` | backend, worker, tracking | Workers cannot fetch decryption keys and tracking cannot resolve click tickets. Both fail closed with `401` | | `KMS_LOCAL_MASTER_KEY` | backend, consumer, worker | Sealed data keys cannot be opened. Mailbox credentials stop decrypting | -| `CREDENTIALS_ENCRYPTION_KEY` | backend, worker | Stored SMTP and IMAP passwords stop decrypting | +| `CREDENTIALS_ENCRYPTION_KEY` | backend, worker | Stored SMTP and IMAP passwords and OAuth tokens stop decrypting, so no mailbox can send or sync | | `EVENTBUS_PROVIDER` and `NATS_URL` | every service | Producers and consumers land on different buses and work silently disappears | | `CODEC_PROVIDER` | every service | Events are written in one format and read in another | | `PUBSUB_ENABLED` | backend, consumer, realtime | Realtime events are published to a transport nobody is listening on | diff --git a/internal/repository/credential_seal_test.go b/internal/repository/credential_seal_test.go new file mode 100644 index 000000000..04a176753 --- /dev/null +++ b/internal/repository/credential_seal_test.go @@ -0,0 +1,84 @@ +package repository + +import ( + "strings" + "testing" + + "github.com/warmbly/warmbly/internal/pkg/encrypt" +) + +func testRepo(t *testing.T) *emailRepository { + t.Helper() + enc, err := encrypt.NewEncrypterFromHex(strings.Repeat("ab", 32)) + if err != nil { + t.Fatalf("build encrypter: %v", err) + } + return &emailRepository{Encrypt: enc} +} + +func TestSealCredentialRoundTrip(t *testing.T) { + r := testRepo(t) + + for _, plain := range []string{ + "ya29.a0ARGnu0-fake-google-access-token", + "1//0gFAKE_google_refresh_token", + "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.fake.graph-token", + "", + } { + sealed, err := r.sealCredential(plain) + if err != nil { + t.Fatalf("seal %q: %v", plain, err) + } + if plain != "" && sealed == plain { + t.Fatalf("seal %q returned the plaintext unchanged", plain) + } + + opened, legacy, err := r.openCredential(sealed) + if err != nil { + t.Fatalf("open %q: %v", plain, err) + } + if legacy { + t.Errorf("open %q reported a sealed value as legacy plaintext", plain) + } + if opened != plain { + t.Errorf("round trip of %q returned %q", plain, opened) + } + } +} + +// Rows written before OAuth tokens were sealed must still be readable, and must +// be reported as legacy so the caller re-seals them. +func TestOpenCredentialDetectsLegacyPlaintext(t *testing.T) { + r := testRepo(t) + + for _, stored := range []string{ + "ya29.a0ARGnu0-fake-google-access-token", + "1//0gFAKE_google_refresh_token", + "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.fake.graph-token", + "seed-fake-access-token", + } { + opened, legacy, err := r.openCredential(stored) + if err != nil { + t.Fatalf("open %q: %v", stored, err) + } + if !legacy { + t.Errorf("open %q was not flagged as legacy plaintext", stored) + } + if opened != stored { + t.Errorf("open %q returned %q, want the stored value verbatim", stored, opened) + } + } +} + +// Without CREDENTIALS_ENCRYPTION_KEY the repository must fail closed rather +// than fall back to writing provider tokens in the clear. +func TestSealCredentialFailsClosedWithoutEncrypter(t *testing.T) { + r := &emailRepository{} + + if _, err := r.sealCredential("ya29.token"); err == nil { + t.Fatal("sealCredential succeeded with no encrypter configured") + } + if _, _, err := r.openCredential("ya29.token"); err == nil { + t.Fatal("openCredential succeeded with no encrypter configured") + } +} diff --git a/internal/repository/pg_email.go b/internal/repository/pg_email.go index e3a622e52..d1aacb262 100644 --- a/internal/repository/pg_email.go +++ b/internal/repository/pg_email.go @@ -123,6 +123,32 @@ func NewEmailRepostory(db *db.DB, enc *encrypt.Encrypter) EmailRepository { // without CREDENTIALS_ENCRYPTION_KEY configured. var errNoCredentialEncrypter = errors.New("credential encrypter not configured (set CREDENTIALS_ENCRYPTION_KEY)") +// sealCredential encrypts a credential for storage. It fails closed: without an +// encrypter the caller must abort rather than fall back to a plaintext write. +func (r *emailRepository) sealCredential(plain string) (string, error) { + if r.Encrypt == nil { + return "", errNoCredentialEncrypter + } + return r.Encrypt.Encrypt(plain) +} + +// openCredential returns the plaintext behind a stored credential, reporting +// whether the row was still in the pre-sealing plaintext format. +// +// Rows written before OAuth tokens were sealed hold the provider token verbatim +// ("ya29...", "1//0g...", a Graph JWT), none of which is valid hex, so a +// ParseHex/AEAD failure identifies a legacy row rather than corruption. Callers +// re-seal on the spot so the plaintext window closes on first read. +func (r *emailRepository) openCredential(stored string) (string, bool, error) { + if r.Encrypt == nil { + return "", false, errNoCredentialEncrypter + } + if plain, err := r.Encrypt.Decrypt(stored); err == nil { + return plain, false, nil + } + return stored, true, nil +} + func (r *emailRepository) ExistsForUser(ctx context.Context, userID, email string) (bool, *errx.Error) { var exists bool query := `SELECT EXISTS(SELECT 1 FROM email_accounts WHERE user_id = $1 AND email = $2)` @@ -209,6 +235,19 @@ func (r *emailRepository) NewOauthAccount(ctx context.Context, userID string, da return nil, errx.InternalError() } + // Seal before opening the transaction so a misconfigured encrypter aborts + // the connect instead of writing provider tokens in the clear. + encAccessToken, encErr := r.sealCredential(data.AccessToken) + if encErr != nil { + db.CaptureError(encErr, "", nil, "encrypt-access-token") + return nil, errx.InternalError() + } + encRefreshToken, encErr := r.sealCredential(data.RefreshToken) + if encErr != nil { + db.CaptureError(encErr, "", nil, "encrypt-refresh-token") + return nil, errx.InternalError() + } + tx, err := r.DB.Begin(ctx) if err != nil { db.CaptureError(err, "", nil, "begin") @@ -261,8 +300,8 @@ func (r *emailRepository) NewOauthAccount(ctx context.Context, userID string, da params = []any{ id, - data.AccessToken, - data.RefreshToken, + encAccessToken, + encRefreshToken, data.ExpiresAt, } @@ -272,8 +311,9 @@ func (r *emailRepository) NewOauthAccount(ctx context.Context, userID string, da params..., ) if err != nil { - db.CaptureError(err, query, params, "exec") - errx.InternalError() + // params holds the sealed tokens; keep them out of the error report. + db.CaptureError(err, query, nil, "exec") + return nil, errx.InternalError() } if err := tx.Commit(ctx); err != nil { @@ -1369,18 +1409,20 @@ func (r *emailRepository) GetOAuthCredentials(ctx context.Context, emailAccountI return nil, errx.InternalError() } - // Decrypt tokens - var xerr error - decryptedAccessToken, xerr := r.Encrypt.Decrypt(accessToken) + // Decrypt tokens, tolerating rows written before OAuth tokens were sealed. + decryptedAccessToken, accessLegacy, xerr := r.openCredential(accessToken) if xerr != nil { sentry.CaptureException(xerr) return nil, errx.InternalError() } - decryptedRefreshToken, xerr := r.Encrypt.Decrypt(refreshToken) + decryptedRefreshToken, refreshLegacy, xerr := r.openCredential(refreshToken) if xerr != nil { sentry.CaptureException(xerr) return nil, errx.InternalError() } + if accessLegacy || refreshLegacy { + r.resealOAuthCredentials(ctx, emailAccountID, decryptedAccessToken, decryptedRefreshToken) + } return &OAuthCredentials{ AccessToken: decryptedAccessToken, @@ -1389,6 +1431,33 @@ func (r *emailRepository) GetOAuthCredentials(ctx context.Context, emailAccountI }, nil } +// resealOAuthCredentials rewrites a pre-sealing row in its encrypted form. It +// is the migration path for tokens stored before sealing existed: there is no +// SQL-only migration for them because the key lives in the application, so the +// upgrade happens on first read instead. Best-effort by design, a failure here +// must not break the read that triggered it; the next read retries. +func (r *emailRepository) resealOAuthCredentials(ctx context.Context, emailAccountID uuid.UUID, accessToken, refreshToken string) { + encAccessToken, err := r.sealCredential(accessToken) + if err != nil { + sentry.CaptureException(err) + return + } + encRefreshToken, err := r.sealCredential(refreshToken) + if err != nil { + sentry.CaptureException(err) + return + } + + query := ` + UPDATE email_accounts_oauth + SET access_token = $1, refresh_token = $2 + WHERE email_account_id = $3 + ` + if _, err := r.DB.Exec(ctx, query, encAccessToken, encRefreshToken, emailAccountID); err != nil { + db.CaptureError(err, query, nil, "reseal-oauth-credentials") + } +} + // GetWorkerID retrieves the worker ID assigned to an email account func (r *emailRepository) GetWorkerID(ctx context.Context, emailAccountID uuid.UUID) (*uuid.UUID, *errx.Error) { query := `SELECT worker_id FROM email_accounts WHERE id = $1` diff --git a/internal/repository/pg_email_auth.go b/internal/repository/pg_email_auth.go index e9a2428ab..83d2ecf27 100644 --- a/internal/repository/pg_email_auth.go +++ b/internal/repository/pg_email_auth.go @@ -150,6 +150,20 @@ func (r *emailRepository) RevokeOauth(ctx context.Context, id string) *errx.Erro } func (r *emailRepository) RefreshBoxToken(ctx context.Context, id uuid.UUID, accessToken, refreshToken string, expiresAt time.Time) error { + // Refreshed tokens land here from the worker's OnTokenRefresh callback and + // must be sealed on the same terms as the ones written at connect time, + // otherwise the first refresh silently reverts the row to plaintext. + encAccessToken, err := r.sealCredential(accessToken) + if err != nil { + db.CaptureError(err, "", nil, "encrypt-access-token") + return err + } + encRefreshToken, err := r.sealCredential(refreshToken) + if err != nil { + db.CaptureError(err, "", nil, "encrypt-refresh-token") + return err + } + query := ` UPDATE email_accounts_oauth SET access_token = $1, refresh_token = $2, expires_at = $3 @@ -157,19 +171,19 @@ func (r *emailRepository) RefreshBoxToken(ctx context.Context, id uuid.UUID, acc ` params := []any{ - accessToken, - refreshToken, + encAccessToken, + encRefreshToken, expiresAt, id, } - _, err := r.DB.Exec( + if _, err := r.DB.Exec( ctx, query, params..., - ) - if err != nil { - db.CaptureError(err, query, params, "exec") + ); err != nil { + // params holds the sealed tokens; keep them out of the error report. + db.CaptureError(err, query, nil, "exec") return err } return nil