[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 47.49455% with 241 lines in your changes missing coverage. Please review.
✅ Project coverage is 20.48%. Comparing base (c76c01e) to head (efb1c68).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...udstack/kms/provider/pkcs11/PKCS11HSMProvider.java	15.09%	89 Missing and 1 partial ⚠️
...java/org/apache/cloudstack/kms/KMSManagerImpl.java	77.21%	46 Missing and 8 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 ⚠️
...ava/org/apache/cloudstack/kms/KMSWrappedKeyVO.java	50.00%	6 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     20.45%   20.48%   +0.02%     
- Complexity    21417    21457      +40     
============================================
  Files          6487     6487              
  Lines        589329   589676     +347     
  Branches      72246    72282      +36     
============================================
+ Hits         120554   120796     +242     
- Misses       455642   455741      +99     
- Partials      13133    13139       +6     

Flag	Coverage Δ
uitests	3.70% <ø> (ø)
unittests	21.79% <47.49%> (+0.03%)	⬆️

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