Skip to content

feat(data_encryption): support AES-256 keys in the encryption keyring - #13756

Open
AlinsRan wants to merge 2 commits into
apache:masterfrom
AlinsRan:feat/data-encryption-aes-256
Open

feat(data_encryption): support AES-256 keys in the encryption keyring#13756
AlinsRan wants to merge 2 commits into
apache:masterfrom
AlinsRan:feat/data-encryption-aes-256

Conversation

@AlinsRan

@AlinsRan AlinsRan commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

core.data_encryption.init_iv_tbl only ever built AES-128 ciphers:

tbl.insert(iv_tbl, assert(aes:new(iv, nil, aes.cipher(128, "cbc"), {iv = iv})))

and the CLI config schema pinned every apisix.data_encryption.keyring entry to exactly 16 characters, so a 32-byte key could not even get past apisix start.

This selects the cipher by key length: a 16-byte key → AES-128, a 32-byte key → AES-256. The schema accepts those two lengths and keeps rejecting everything else, because an unsupported key is dropped by init_iv_tbl and an empty keyring makes encrypt() store the value in clear text. A dropped key is also logged as an error, for the paths that do not go through the CLI schema.

Both lengths can be mixed in one keyring, so an existing AES-128 keyring can be rotated to AES-256 without losing access to the already encrypted data.

Which issue(s) this PR fixes

Lets operators use a stronger AES-256 keyring for encrypt_fields / SSL key encryption.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change (t/core/data_encryption.t for the cipher selection, the keyring read from config.yaml and AES-128 → AES-256 rotation; t/cli/test_main.sh for the schema validation)
  • I have updated the documentation accordingly (conf/config.yaml.example, en/zh plugin-develop.md)
  • I have verified locally (t/core/data_encryption.t passes; the make init schema cases were checked by hand and confirmed to fail without the schema change)

init_iv_tbl only ever built AES-128 ciphers, so a 32-byte data_encryption
keyring key could not be used for AES-256. A 16-byte key now selects
AES-128 and a 32-byte key selects AES-256; keys of any other length are
skipped instead of being forced through aes.cipher(128, ...), which
previously errored via the assert.

Adds t/core/data_encryption.t covering the 16-byte, 32-byte, mixed-keyring
and unsupported-length cases.

Signed-off-by: AlinsRan <alinsran@apache.org>
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Jul 29, 2026
nic-6443
nic-6443 previously approved these changes Aug 3, 2026

@membphis membphis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Invalid key lengths can silently disable data encryption

init_iv_tbl now skips every key whose length is not 16 or 32 bytes. If all configured keys are invalid, iv_tbl stays empty, and the existing empty-keyring branches return plaintext unchanged. This turns a configuration error into a fail-open path for encrypted plugin fields and SSL private keys. Please reject unsupported key lengths during schema or initialization validation and fail startup or configuration loading instead of treating this as an absent keyring.

[P2] The mixed-keyring test does not exercise legacy ciphertext fallback

The test encrypts with the first AES-256 entry and then decrypts with the same entry; the trailing AES-128 key is never used. Please create ciphertext with the legacy 16-byte key only, then decrypt it using {new_32_byte_key, old_16_byte_key} to prove existing AES-128 data remains readable during key rotation.

…g path

A 32 byte key never reached init_iv_tbl: the CLI config schema pins every
keyring entry to exactly 16 characters, so `apisix start` died before the
AES-256 branch could run. Allow 16 and 32 character keys there, and keep
rejecting every other length -- an unsupported key is dropped by
init_iv_tbl, and a keyring that ends up empty makes encrypt() store the
value in clear text.

Also report a dropped key in the error log, so the same mistake is visible
on the paths that do not go through the CLI schema.

The tests only called init_iv_tbl directly, which is why this was missed:
add a t/cli case that runs the config through the schema, a block that
reads the keyring from config.yaml, and rewrite the mixed keyring test so
it actually decrypts AES-128 data with an AES-256 first key.
@AlinsRan
AlinsRan dismissed stale reviews from nic-6443 and shreemaan-abhishek via ea542ca August 4, 2026 08:17
@AlinsRan

AlinsRan commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Both addressed in ea542ca, and the first one turned out to hide a bigger problem.

[P1] You are right that skipping is the wrong handling, and while fixing it I found the feature did not work at all. apisix/cli/schema.lua pins every keyring entry to exactly 16 characters:

items = { type = "string", minLength = 16, maxLength = 16 }

apisix start runs schema.validate() and util.die()s on failure, so a 32-byte key in config.yaml never reached init_iv_tbl — APISIX just refused to start. My PR description claiming no schema change was needed was wrong.

So the fix is now on both levels you suggested:

  • schema: a keyring entry must be 16 or 32 characters, anything else fails startup;
  • init_iv_tbl: an unsupported length is logged as an error instead of being dropped silently, for the paths that do not go through the CLI schema.

The reason CI was green is that all four tests called init_iv_tbl directly and bypassed config validation. Added t/cli/test_main.sh cases that run the keyring through the schema (32-byte accepted, 20-byte rejected) plus a block that reads the keyring from config.yaml. Verified the new t/cli case is discriminating: with the old schema the 32-byte keyring fails with failed to validate config: ... property "keyring" validation failed.

[P2] Rewritten. TEST 3 now encrypts with a keyring holding only the 16-byte key, then decrypts that ciphertext with {32_byte_key, 16_byte_key} and checks new writes go through the AES-256 key.

Also updated conf/config.yaml.example and the en/zh plugin-develop.md, which still documented the keyring as 16 characters / AES-128 only.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants