<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements.  See the NOTICE file
distributed with this work for additional information
regarding copyright ownership.  The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License.  You may obtain a copy of the License at

  http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied.  See the License for the
specific language governing permissions and limitations
under the License.
-->

# SIP: Authenticated encryption (AES-GCM) for app-encrypted fields

## [DRAFT — proposal for discussion]

This document is a draft proposal accompanying the code in this PR. It is
intended to seed the formal SIP discussion. The code here ships the
backward-compatible engine selection **and** the re-encryption migrator
(Phases 1–2 below); both are opt-in and change nothing for existing installs by
default. Flipping the default for fresh installs (Phase 3) remains future work.

## Motivation

Superset app-encrypts a number of sensitive fields before persisting them to
the metadata database, including:

- database connection passwords and `encrypted_extra` (`superset/models/core.py`),
- SSH tunnel credentials — password, private key, private-key password
  (`superset/databases/ssh_tunnel/models.py`),
- OAuth2 tokens and other secrets stored via `EncryptedType`.

These fields are encrypted with `sqlalchemy_utils.EncryptedType`, which
**defaults to `AesEngine` (AES-CBC)**. AES-CBC provides confidentiality but is
**unauthenticated**: it has no integrity tag. An attacker with write access to
the ciphertext (e.g. direct metadata-DB access, a backup, or a compromised
replica) can perform **bit-flipping / chosen-ciphertext manipulation** to
silently alter the decrypted plaintext of a secret without detection.

`AesGcmEngine` (AES-GCM) is authenticated encryption: tampering causes
decryption to fail loudly rather than yielding attacker-influenced plaintext.
Using authenticated encryption for secrets at rest is an ASVS L1 expectation
(11.3.2 / cryptography best practice).

`config.py` already documents that operators *can* switch to GCM by writing a
custom `AbstractEncryptedFieldAdapter`, but:

1. it is opt-in, undocumented as a security recommendation, and easy to miss;
2. there is **no migration path** — flipping the engine on a populated database
   makes every existing secret undecryptable, because GCM ciphertext is not
   format-compatible with CBC.

## Proposed change

A three-part change, delivered incrementally so existing deployments are never
broken:

### Phase 1 — engine selection (this PR)

- Add a `SQLALCHEMY_ENCRYPTED_FIELD_ENGINE` config (`"aes"` | `"aes-gcm"`),
  **defaulting to `"aes"`** (no behavior change for existing installs).
- Teach the default `SQLAlchemyUtilsAdapter` to honor it (an explicit `engine`
  kwarg still wins, so the migrator can pin an engine).
- This lets **new** deployments choose AES-GCM from day one with a one-line
  config, instead of writing a custom adapter.

### Phase 2 — CBC→GCM re-encryption migrator (this PR)

The existing `SecretsMigrator` (previously only used for `SECRET_KEY` rotation)
gains an **engine migration** mode that:

1. discovers every `EncryptedType` column (via `discover_encrypted_fields()`),
2. decrypts each value with the **source** engine (AES-CBC) under the current
   `SECRET_KEY`,
3. re-encrypts with the **target** engine (AES-GCM),
4. runs transactionally per the existing all-or-nothing semantics, and is
   idempotent per column (already-migrated values are skipped), so a run can be
   safely repeated or resumed.

Exposed via a new `--engine` option on the existing CLI command:
`superset re-encrypt-secrets --engine aes-gcm`, runnable by operators with a DB
backup in hand. The `SECRET_KEY` is unchanged; an engine change and a key
rotation can also be combined (pass `--previous_secret_key` as well).

### Phase 3 — flip the default for new installs

Once the migrator and docs are in place, change the default to `"aes-gcm"` for
**fresh** installs only (e.g. keyed off an empty metadata DB / documented in
`UPDATING.md`), keeping existing installs on `"aes"` until they run Phase 2.

## New or changed public interfaces

- New config: `SQLALCHEMY_ENCRYPTED_FIELD_ENGINE: Literal["aes", "aes-gcm"]`.
- New (Phase 2) CLI: `superset re-encrypt-secrets --engine <name>`.
- No schema changes; ciphertext format changes per migrated column.

## Migration plan and compatibility

- **Backward compatible by default.** Phase 1 changes nothing unless the
  operator opts in.
- Switching an existing deployment to `"aes-gcm"` **without** running the Phase
  2 migrator will make existing secrets undecryptable — this is called out in
  the config comment and must be in `UPDATING.md`.
- Recommended operator runbook: take a metadata-DB backup → run
  `re-encrypt-secrets --engine aes-gcm` → set
  `SQLALCHEMY_ENCRYPTED_FIELD_ENGINE = "aes-gcm"` → restart → re-run
  `re-encrypt-secrets --engine aes-gcm` once more to sweep up any secrets a live
  instance wrote as AES-CBC during the cutover window. The canonical, more
  detailed version of this runbook lives in `UPDATING.md`; this is a summary.
- `AesEngine` allows queryability over encrypted fields; AES-GCM does not.
  Any code that filters/queries on an encrypted column directly must be audited
  before Phase 3 (none is expected, but it must be verified).

## Rejected alternatives

- **Flip the default immediately.** Rejected: bricks every existing
  deployment's secrets with no migration path.
- **Document-only (custom adapter).** Status quo; high friction and no
  migration tooling — most operators will never do it.

## Open questions

- GCM→CBC rollback (for operators who need queryability) already works via the
  same command (`re-encrypt-secrets --engine aes`), since the migrator is
  engine-symmetric. Should rollback be documented as a supported path or
  discouraged?
- The migrator already supports a concurrent `SECRET_KEY` rotation + engine
  change in a single pass (pass `--previous_secret_key` alongside `--engine`).
  Is that combination worth calling out in the operator docs, or kept advanced?