feat(data_encryption): support AES-256 keys in the encryption keyring - #13756
feat(data_encryption): support AES-256 keys in the encryption keyring#13756AlinsRan wants to merge 2 commits into
Conversation
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>
membphis
left a comment
There was a problem hiding this comment.
[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.
ea542ca
|
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. items = { type = "string", minLength = 16, maxLength = 16 }
So the fix is now on both levels you suggested:
The reason CI was green is that all four tests called [P2] Rewritten. TEST 3 now encrypts with a keyring holding only the 16-byte key, then decrypts that ciphertext with Also updated |
Description
core.data_encryption.init_iv_tblonly ever built AES-128 ciphers:and the CLI config schema pinned every
apisix.data_encryption.keyringentry to exactly 16 characters, so a 32-byte key could not even get pastapisix 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_tbland an empty keyring makesencrypt()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
t/core/data_encryption.tfor the cipher selection, the keyring read fromconfig.yamland AES-128 → AES-256 rotation;t/cli/test_main.shfor the schema validation)conf/config.yaml.example, en/zhplugin-develop.md)t/core/data_encryption.tpasses; themake initschema cases were checked by hand and confirmed to fail without the schema change)