vishesh92 opened a new pull request, #14453: URL: https://github.com/apache/cloudstack/pull/14453
### Description This PR fixes several problems in the KMS volume encryption that came with 4.23. **Volumes migrated to KMS don't start anymore** `migrateVolumesToKMS` wrapped the legacy passphrase as it is stored in the `passphrase` table, which is its base64 text. When the key is read back, `VolumeObject.getPassphrase()` base64-encodes the unwrapped key again, so the volume gets a different passphrase from the one it was encrypted with. The VM fails on the next start or attach. Migration now wraps the decoded passphrase. For keys already migrated on 4.23, a background task runs after the upgrade. It unwraps every existing key once and rewraps the ones that are in the old format. If a key is used by a volume created after the migration (e.g. a copy made by a storage migration on 4.23), that volume may have been encrypted with the wrong passphrase, so the task leaves the key alone and logs it. Migration also removes the old `passphrase` row once no volume uses it. The upgrade removes rows that were already left behind. **Wrapped keys removed while still in use** `VolumeDaoImpl.remove()` removed a volume's wrapped key unconditionally. After a volume migration the new volume shares the key with the source, so expunging the source left the migrated volume without a key. Expunging a volume that has snapshots did the same to the snapshots: they could be neither restored nor deleted. The DAO doesn't touch the key anymore. `KMSManager.deleteKMSWrappedKey()` and `VolumeObject.deletePassphrase()` keep the key (KMS or legacy passphrase) while another volume or a snapshot of the volume still needs it. The storage cleanup task releases it afterwards through `VolumeService.releaseKeysOfRemovedVolumes()`. On upgrade, keys removed this way are restored if their KMS key and KEK still exist. The ones that can't be recovered are logged. **Key wrapping in the PKCS#11 provider** The PKCS#11 provider wrapped keys with `AES/CBC/PKCS5Padding`, which detects neither a modified blob nor a wrong KEK. `unwrapKey()` also tried all KEK versions when the recorded one failed. So a wrong key could be returned without any error, and during a rotation it would be rewrapped under the new KEK and the real key lost. - The wrap algorithm is now set per HSM profile with the `wrap_algorithm` detail. The options are `AES/GCM/NoPadding` (the default), `AES/KWP/NoPadding` (needs Java 18 or later) and `AES/CBC/PKCS5Padding`. It can't be changed once the profile exists. - The upgrade sets existing PKCS#11 profiles to `AES/CBC/PKCS5Padding`, so nothing changes for them. To move existing keys over, create a profile with GCM or KWP and rotate the KMS keys onto it. - Each wrapped key records the algorithm it was wrapped with, in `kms_wrapped_key.wrap_algorithm`. - `unwrapKey()` uses the recorded KEK version first. It tries the other versions only for GCM/KWP keys, where a wrong KEK is detected. - Padding and tag failures give the same error message. - When a KMS key is created or rotated, the new KEK is tested with a wrap/unwrap round trip. An HSM that doesn't support the configured algorithm fails right there, and the KEK is deleted again. - `KMSProvider` got a `deleteKek(kekId, hsmProfileId)` variant for KEKs that don't have a version row (yet). Rotation now uses it when its DB update fails. Before, that KEK stayed in the HSM. - The `algorithm` shown for a KMS key is now the actual wrap algorithm, instead of always `AES/GCM/NoPadding`. **Other changes** - `deleteKMSKey` no longer deletes the KMS key if the HSM fails to delete one of its KEKs, so the KEK isn't left in the HSM without anything pointing to it. A KEK that is already gone is fine, e.g. one deleted after a rotation. The PKCS#11 provider now reports such a KEK as not found instead of a generic failure. - For a removed volume whose key no longer exists, `getPassphrase()` returns an empty passphrase instead of throwing, so its snapshots can still be deleted. - `kms.dek.size.bits` is now checked to be 128, 192 or 256. - The database provider is documented as meant for testing. Its default profile is renamed from "HSM Database Provider" to "Database Provider", and enabling it logs a warning. Schema changes: `kms_wrapped_key` gets `wrap_algorithm` and `format_check_pending`. For the release notes: volumes already migrated on 4.23.0.0 are repaired automatically after the upgrade, once their HSM profile is reachable. Existing PKCS#11 profiles keep using CBC until their keys are rotated onto a GCM/KWP profile. <!--- Describe your changes in DETAIL - And how has behaviour functionally changed. --> <!-- For new features, provide link to FS, dev ML discussion etc. --> <!-- In case of bug fix, the expected and actual behaviours, steps to reproduce. --> <!-- When "Fixes: #<id>" is specified, the issue/PR will automatically be closed when this PR gets merged --> <!-- For addressing multiple issues/PRs, use multiple "Fixes: #<id>" --> <!-- Fixes: # --> <!--- ******************************************************************************* --> <!--- NOTE: AUTOMATION USES THE DESCRIPTIONS TO SET LABELS AND PRODUCE DOCUMENTATION. --> <!--- PLEASE PUT AN 'X' in only **ONE** box --> <!--- ******************************************************************************* --> ### Types of changes - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] New feature (non-breaking change which adds functionality) - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] Enhancement (improves an existing feature and functionality) - [ ] Cleanup (Code refactoring and cleanup, that may add test cases) - [ ] Build/CI - [ ] Test (unit or integration test code) ### Feature/Enhancement Scale or Bug Severity #### Feature/Enhancement Scale - [ ] Major - [ ] Minor #### Bug Severity - [ ] BLOCKER - [ ] Critical - [ ] Major - [ ] Minor - [ ] Trivial ### Screenshots (if appropriate): ### How Has This Been Tested? <!-- Please describe in detail how you tested your changes. --> <!-- Include details of your testing environment, and the tests you ran to --> #### How did you try to break this feature and the system with this change? <!-- see how your change affects other areas of the code, etc. --> <!-- Please read the [CONTRIBUTING](https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md) document --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
