[vishesh92]

Description

Fixes a set of problems in the KMS feature shipped in 4.23.0.0, plus an upgrade path for 4.23 databases.

Volumes migrated with migrateVolumesToKMS could not be opened. Migration wrapped the base64 text of the legacy passphrase. VolumeObject.getPassphrase() base64-encodes the unwrapped bytes again, so qemu got a different passphrase than the one the LUKS header was created with (Invalid password, cannot unlock any keyslot). Migration now wraps the 48 decoded bytes, and the old passphrase row is deleted in the same transaction. Blobs already migrated on 4.23 are repaired by a background task after the upgrade (see Upgrade).

Keys were removed while still needed.

• VolumeDaoImpl.remove() removed the wrapped key. Volume migration (duplicateVolumeOnAnotherStorage) gives the new volume the same key, so expunging the source broke the migrated volume.
• A snapshot of an expunged volume needs that volume's key (SnapshotObjectTO unwraps it eagerly), but expunging released the key anyway. The legacy passphrase had the same problem, since deletePassphrase() hard-deleted the row.

Now remove() leaves the key alone, and deleteKMSWrappedKey / deletePassphrase keep it while another live volume or a non-destroyed snapshot still needs it. The storage GC calls VolumeService.releaseKeysOfRemovedVolumes() to release such keys once nothing needs them anymore. For a removed volume whose key is already gone, getPassphrase() returns an empty passphrase, so its snapshots can still be deleted.

Wrap algorithm. PKCS#11 wrapped keys were AES-CBC with no authentication, and kms_keys.algorithm was hardcoded to GCM even for PKCS#11 keys.

• PKCS#11 HSM profiles get a wrap_algorithm detail: AES/GCM/NoPadding (default), AES/KWP/NoPadding (needs Java 18+ on the management server) or AES/CBC/PKCS5Padding (4.23's format). It can't be changed after the profile is created, and each wrapped key records the algorithm it was written with in kms_wrapped_key.wrap_algorithm.
• createKMSKey and rotateKMSKey wrap and unwrap a random test key right after creating the KEK and refuse the key (and delete the KEK again) if the HSM can't do the profile's algorithm.
• KMSKey.algorithm now comes from the provider.
• Unwrap tries the recorded KEK version first. It falls back to other versions only for GCM/KWP blobs, because with CBC a wrong KEK can "succeed" and produce a wrong key. Unwrap failures give one generic error.
• DEKs of PKCS#11 profiles come from the token's random generator (C_GenerateRandom), with the JVM's SecureRandom as fallback for tokens without one. kms.dek.size.bits is validated.
• A session whose operation failed is closed instead of going back to the pool. bouncyhsm leaves a failed C_Decrypt active, which made every later operation on that session fail with CKR_OPERATION_ACTIVE.

Smaller fixes

• deleteKMSKey no longer removes the DB row when deleting a KEK from the HSM fails (a KEK that is already gone is skipped). deleteKey now reports KEK_NOT_FOUND.
• getPassphrase() no longer copies the key into an immutable String.
• The database provider is documented as testing/simulator only, and the seeded profile is renamed from HSM Database Provider to Database Provider.
• token_label is gone from the UI. The provider wrote tokenLabel= into the SunPKCS11 config, which SunPKCS11 doesn't have (any JDK 17-25), so profiles using it never connected. A profile needs slot or slot_list_index.

Upgrade (4.23.0.0 -> 24.0.0)

No HSM access is needed during the upgrade.

• kms_wrapped_key gets wrap_algorithm (existing PKCS#11 keys are set to CBC, database-provider keys to GCM) and format_check_pending (set to 1 on all existing rows).
• Existing PKCS#11 profiles get wrap_algorithm=AES/CBC/PKCS5Padding and their kms_keys.algorithm is corrected, so nothing changes for them. To move to GCM or KWP, create a profile with the new algorithm and run rotateKMSKey onto it. The existing rewrap worker converts the blobs.
• Upgrade42300to2400 restores wrapped keys that 4.23 removed while a live volume or non-destroyed snapshot still needed them, as long as the KMS key and KEK still exist. For the others it logs a WARN with the wrapped key and volume UUIDs.
• Passphrase rows that no volume references are deleted.
• After startup a background task (KMSKeyFormatRepair, shares the kms.rewrap.worker lock) checks every flagged key once, using the unwrapped length and format. Keys that 4.23 migrated wrongly are rewrapped with the raw bytes, and an event is added for the volume. A key whose volume was created after the key is not touched: that is a copy made by a storage migration on 4.23, which was encrypted with the wrong passphrase, so rewrapping would make it unreadable. It logs a WARN and leaves it to the admin. A key that can't be unwrapped (HSM down) stays flagged and is retried. The task stops when nothing is flagged.

Things to know

• GCM needs a PKCS#11 library that handles it through SunPKCS11. SoftHSM before 2.7.0 aborts the whole JVM at the first GCM operation on EL8/9/10 (an empty-AAD memcpy that trips _GLIBCXX_ASSERTIONS; Ubuntu's 2.6.1 is fine). bouncyhsm's client library rejects the GCM parameters JDK 17 sends first, so GCM there needs Java 21; the wrap test reports it cleanly on 17. Use CBC for such HSMs.
• KWP through SunPKCS11 needs the token to allow it with C_Encrypt. openCryptoki 3.26 does; SoftHSM 2.7.0 and bouncyhsm don't, and the wrap test refuses the key.
• Not part of this PR: offline storage migration of an encrypted volume on NFS/local storage fails because LibvirtStorageAdaptor.copyPhysicalDisk ignores the passphrases. That is not KMS-specific and goes back to 4.18.

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?

How did you try to break this feature and the system with this change?

[vishesh92]

@blueorangutan package

[blueorangutan]

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[codecov]

Codecov Report

❌ Patch coverage is 46.86192% with 254 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.75%. Comparing base (6019b89) to head (ee42536).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...udstack/kms/provider/pkcs11/PKCS11HSMProvider.java	12.90%	107 Missing and 1 partial ⚠️
...java/org/apache/cloudstack/kms/KMSManagerImpl.java	78.00%	46 Missing and 9 partials ⚠️
...main/java/com/cloud/storage/dao/VolumeDaoImpl.java	0.00%	39 Missing ⚠️
...java/com/cloud/upgrade/dao/Upgrade42300to2400.java	0.00%	21 Missing ⚠️
...e/cloudstack/storage/volume/VolumeServiceImpl.java	0.00%	16 Missing ⚠️
...pache/cloudstack/kms/dao/KMSWrappedKeyDaoImpl.java	0.00%	8 Missing ⚠️
...ain/java/com/cloud/storage/StorageManagerImpl.java	0.00%	5 Missing ⚠️
...apache/cloudstack/storage/volume/VolumeObject.java	92.85%	0 Missing and 1 partial ⚠️
...e/cloudstack/kms/provider/DatabaseKMSProvider.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              main   #14453       +/-   ##
============================================
+ Coverage     3.86%   20.75%   +16.88%     
- Complexity       0    20808    +20808     
============================================
  Files          494     6049     +5555     
  Lines        42875   548123   +505248     
  Branches      8136    73211    +65075     
============================================
+ Hits          1657   113743   +112086     
- Misses       40986   421059   +380073     
- Partials       232    13321    +13089     

Flag	Coverage Δ
uitests	3.86% <ø> (ø)
unittests	22.18% <46.86%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19644

[vishesh92]

@blueorangutan package

[blueorangutan]

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19656

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19659

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[vishesh92]

@blueorangutan test

[blueorangutan]

@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian test result (tid-17197)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 61162 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14453-t17197-kvm-ol8.zip
Smoke tests completed. 156 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_12_deploy_vm_with_root_disk_encryption	Error	7.00	test_kms_lifecycle.py

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[vishesh92]

@blueorangutan package

[blueorangutan]

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✖️ el8 ✖️ el9 ✔️ debian ✔️ suse15. SL-JID 19767

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19889

[vishesh92]

@blueorangutan test

[blueorangutan]

@vishesh92 a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian test result (tid-17482)
Environment: kvm-ol8 (x2), zone: Advanced Networking with Mgmt server ol8
Total time taken: 58612 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr14453-t17482-kvm-ol8.zip
Smoke tests completed. 157 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[Copilot]

🟡 Changes recommended

Key-format repair can make historical snapshots unreadable and hold the cluster-wide rewrap lock indefinitely if HSM encryption stalls.

5 open issues

• Guard historical volumes with non-destroyed snapshots · New
• Timeout the repair wrap call to avoid holding the cluster lock · New
• Preserve missing-KEK errors for CBC unwrap fallback · New
• Handle token RNG failures and wipe partial buffers · New
• Run HSM probe and cleanup through the timeout mechanism · New

_{Review effort: Balanced}

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[Copilot]

🟡 Changes recommended

The repair guard overlooks expunged volumes with retained snapshots, allowing key rewrites that make those snapshots unreadable, while HSM operations also have unresolved timeout and recovery gaps.

7 open issues

• Include expunged volumes in the repair safety check · New
• Timeout the repair wrap call to avoid holding the cluster lock
• Guard historical volumes with non-destroyed snapshots
• Handle PKCS#11 RNG failures through the recovery path · New
• Run HSM probe and cleanup through the timeout mechanism
• Handle token RNG failures and wipe partial buffers
• Preserve missing-KEK errors for CBC unwrap fallback

_{Review effort: Balanced}

[vishesh92]

@blueorangutan package

[blueorangutan]

@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with no SystemVM templates. I'll keep you posted as I make progress.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues

• Collectors is referenced here; ensure java.util.stream.Collectors is imported in this file.… · New
• provider.getWrapAlgorithm(hsmProfileId) is called with the raw command parameter, but the method… · New
• wrapAlgorithm is non-nullable at the ORM level, but it has no default initialization in the… · New
• Consider short-circuiting when volumeIds is empty to avoid an unnecessary DAO call (and to avoid… · New
• deleteUserKMSKey is declared to throw KMSException, but this path throws… · New

7 resolved since last review

• Include expunged volumes in the repair safety check
• Timeout the repair wrap call to avoid holding the cluster lock
• Guard historical volumes with non-destroyed snapshots
• Handle PKCS#11 RNG failures through the recovery path
• Run HSM probe and cleanup through the timeout mechanism
• Handle token RNG failures and wipe partial buffers
• Preserve missing-KEK errors for CBC unwrap fallback

_{Review effort: Lite}

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 20014