Problem/Motivation

The 2.x line of FuzzyConfigKeyProvider::encrypt() / decrypt() has three cryptographic issues that together weaken the encryption-at-rest guarantee this provider is meant to give:

  1. Wrong key derivation. The code does base64_decode(Settings::get('hash_salt')). hash_salt is not base64-encoded — Drupal's installer happens to generate a URL-safe-base64-like string, but operators routinely override it with arbitrary content (env var, secrets manager, custom installer). base64_decode() on non-base64 input silently drops invalid characters and returns garbage of unpredictable length. openssl_encrypt() with AES-256-CBC silently accepts any key length, so encryption "succeeds" either way but with much reduced (sometimes negligible) entropy.
  2. No authentication. AES-256-CBC by itself is unauthenticated. An attacker with write access to config storage — which is exactly the threat model where you would bother with a key provider — can mutate ciphertext, and openssl_decrypt() will return corrupted plaintext rather than failing. For a provider whose explicit purpose is storing sensitive key material safely, this is the wrong primitive.
  3. No format versioning. Stored values are bare base64(base64(ciphertext) :: raw_iv) with no algorithm or version marker, so once issues (1) and (2) are fixed, existing values become unmigratable: there is no way to tell old format from new at decryption time.

The implementation is a port of a 2014 bhoover.com tutorial (link in the comment block) which assumed a pre-base64'd encryption key; the assumption did not get stripped when the code was adapted.

Steps to reproduce

  1. Set $settings['hash_salt'] = 'plain-text-secret-not-base64'; in settings.php.
  2. Create a key using the Fuzzy configuration provider, give it any value.
  3. Read the stored configuration.key_value — encrypted, but with an AES key derived from base64_decode('plain-text-secret-not-base64') which silently drops the non-base64 characters and returns something much shorter than 32 bytes.
  4. Tamper with the ciphertext bytes in storage, save the config, read the key value back — decryption returns garbled plaintext rather than reporting tampering.

Proposed resolution

Ships as a 2.1.0 minor release. Public API is unchanged and the migration is fully automatic, so a major bump is not warranted under Drupal contrib convention.

Switch to AES-256-GCM with proper key derivation. AES-GCM is authenticated (any tampering causes openssl_decrypt to return FALSE). The encryption key is hash('sha256', hash_salt, true) — a stable 32-byte key regardless of whether hash_salt is base64-shaped or arbitrary text.

Introduce a versioned storage format, starting at v1. The legacy stored values from 2.0.x have no version marker at all (this is the bug that prevents safe migration in the first place), so the new format is the first explicitly-versioned one — labelled v1:

  • v1 format (new writes from 2.1.0 onward) — literal v1: followed by base64(iv || ciphertext || gcm_tag). AES-256-GCM, key = SHA-256(hash_salt).
  • Legacy format (read-only, 2.0.x writes) — the unversioned CBC blob, recognised by the absence of a vN: prefix. Decrypted with the old AES-256-CBC + base64_decode key-derivation path, so sites upgrading from 2.0.x keep reading their stored keys until the post-update migration runs. 2.1.x never writes the legacy format. The read path is expected to be removed in a future major.

The prefix is unambiguous: a legacy-format stored value is a pure base64 string (alphabet [A-Za-z0-9+/=]), which cannot contain the : character at any position. So v1: as a prefix cannot collide with legacy data.

Upgrade path for existing sites

Two layers, no operator intervention required for the default flow:

  1. Automatic migration via hook_post_update_NAME(). On drush updb after upgrading to 2.1.0, a post-update walks every key.key.* config entity whose provider is fuzzy_config, reads the stored value through the legacy fallback decrypt path (still works against the existing base64_decode(hash_salt) key derivation), and re-saves through the v1 encrypt path. Idempotent — entities already in v1 format are skipped. Per-key failures are logged via Drupal::logger('fuzzy_key_provider') and the post-update continues, so one corrupted entity does not block the entire upgrade.
  2. Re-runnable Drush command: drush fuzzy_key_provider:re-encrypt performs the same walk on demand, for sites that want to verify or re-run after the post-update (and for the follow-up rotation work — see Related below). Idempotent.

After drush updb, all stored keys end up in v1 format with no further action. The legacy read path remains in 2.1.x for sites that defer updb or roll back.

Remaining tasks

  • Replace encrypt() / decrypt() with the v1 implementation, dispatching on the v1: prefix.
  • Keep a private decryptLegacy() path that handles unversioned stored values exactly as today.
  • Add fuzzy_key_provider.post_update.php with a one-shot legacy-to-v1 migration.
  • Add Drush command fuzzy_key_provider:re-encrypt sharing the same walker as the post-update.
  • Add kernel test coverage: v1 round-trip, legacy-fixture read, tampering detection on v1, post-update migration end-to-end.
  • Update the class docblock (the existing one has a stray comma and an odd storage_method = mid-line break — clean up at the same time).

Related

A follow-up issue will add a fallback decryption path that tries previous hash_salt values (operator-supplied via settings.php), so that rotating hash_salt on a production site does not silently brick all stored keys. That work depends on this issue's v1 format — the GCM auth tag is what makes "try previous salts" safe (wrong-salt attempts fail loudly with GCM, silently with CBC). Linking once filed.

API changes

None at the public-method level. New post-update hook and Drush command. Storage shape unchanged on disk (still a single configuration.key_value string).

Data model changes

Stored-value format changes for new writes and for the post-update migration; legacy unversioned values continue to be readable in 2.1.x via the fallback path until drush updb migrates them. No config schema changes. No database changes.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Issue summary: View changes

mably’s picture

Status: Active » Needs review
mably’s picture

MR !6 is open against 2.x and the pipeline is green (cspell, phpcs, phpstan, phpunit all pass).

What landed:

  • AES-256-CBC → AES-256-GCM with hash('sha256', hash_salt, true) key derivation.
  • Versioned stored format with the literal v1: prefix; legacy (no-prefix) values from 2.0.x stay readable via a decryptLegacy() fallback.
  • One-shot hook_post_update_NAME on drush updb walks every fuzzy_config key and re-encrypts to v1. Backed by a static KeyMigrator helper (no service registration for one-shot upgrade code).
  • Kernel test coverage: v1 round-trip, IV randomness, GCM tampering detection, legacy fallback reads, end-to-end migration.

The Drush fuzzy_key_provider:re-encrypt command was deferred to the rotation follow-up at #3590540, where the on-demand entry point earns its keep alongside the previous-salts decryption fallback. Targeting a 2.1.0 release — public API is unchanged and the migration is fully automatic, so a major bump was not warranted under Drupal contrib convention.

  • mably committed e26cddef on 2.x
    fix: #3590533 Replace AES-256-CBC with authenticated AES-256-GCM and fix...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.