Move PassphraseVO to use String instead of byte[] to support Encrypt annotation #7302

weizhouapache · 2023-03-01T18:44:31Z

Description

This PR fixes #7299

This is proposed by @mlsorensen

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)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

…annotation

weizhouapache · 2023-03-01T18:44:54Z

@blueorangutan package

blueorangutan · 2023-03-01T18:46:04Z

@weizhouapache a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

sonarcloud · 2023-03-01T19:53:11Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

codecov · 2023-03-01T21:06:35Z

Codecov Report

Merging #7302 (82fec84) into main (da58a20) will increase coverage by 0.00%.
The diff coverage is 50.00%.

@@            Coverage Diff            @@
##               main    #7302   +/-   ##
=========================================
  Coverage     12.69%   12.69%           
- Complexity     8655     8657    +2     
=========================================
  Files          2716     2716           
  Lines        256117   256117           
  Branches      39927    39928    +1     
=========================================
+ Hits          32503    32513   +10     
+ Misses       219482   219469   -13     
- Partials       4132     4135    +3

Impacted Files	Coverage Δ
...stack/engine/orchestration/VolumeOrchestrator.java	`0.00% <0.00%> (ø)`
...ava/org/apache/cloudstack/secret/PassphraseVO.java	`52.38% <56.25%> (-0.26%)`	⬇️
...rg/apache/cloudstack/quota/QuotaStatementImpl.java	`40.26% <0.00%> (+3.98%)`	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

weizhouapache · 2023-03-01T21:27:30Z

@blueorangutan test rocky8 kvm-rocky8 keepEnv

blueorangutan · 2023-03-01T21:28:03Z

@weizhouapache a Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

mlsorensen · 2023-03-02T03:53:07Z

Thanks @weizhouapache - need to remember to confirm also that we included the change for passphrase column size larger than 64.

weizhouapache · 2023-03-02T07:22:07Z

Thanks @weizhouapache - need to remember to confirm also that we included the change for passphrase column size larger than 64.

@mlsorensen thanks for the reminder

Just checked it, the size has been changed to 255 in #7003 . it should be good

@@ -214,7 +214,7 @@ BEGIN
-- Add passphrase table
CREATE TABLE IF NOT EXISTS `cloud`.`passphrase` (
    `id` bigint unsigned NOT NULL auto_increment,
    `passphrase` varchar(64) DEFAULT NULL,
    `passphrase` varchar(255) DEFAULT NULL,
    PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

DaanHoogland

clgtm

weizhouapache · 2023-03-02T09:42:25Z

Tested OK with fresh installation with rocky8

create some VMs with encrypted root/data disk. passphrase are encrypted

+----+------------------------------------------------------------------------------------------------------------------------------+
| id | passphrase                                                                                                                   |
+----+------------------------------------------------------------------------------------------------------------------------------+
|  1 | ZoP8LIZr8cpzq3GDpWQVf9axTeGjKTyK9DePfAGx3pObQdI23Nno1jUZTUUF5cBklP2tYDU6tHIzEBfyVf+zSxbc9k+VTpjrti+6gtU6VpF/U9LLFMou7ba8F5g= |
|  2 | SRg5yIavPJ+G/vORKyidCHnJWif3m0JVuDK5s6C91ZmyPOjywUAHGVySNMtuPbL9Y6l+bnKe5qihljHajWiAySY3YCkyJP7efUdYRlFwsA1yBJg6tPdG3q5/X1M= |
|  3 | kqKeTg5Eh0QRp18CrZbYy6n4Pf7iM7goTlsxC6jRjAZ2KR5+qn7gzRHYGi+uK/5lGBlYmJwRSz+YpKkS8vfERgsMHNePpq2o0OUl10O4AaZfPmwVFvIopLv8UTQ= |
|  4 | 0+fOG2mF6iCv2469BdObBtTIQcHwI2/6byV/3JUflUb5qG7EhhGMMXn0uFNCLZuhJX4eNYAf0/Z+V0XjcMcB7Vd32b05SmDaPaJCwHLnW3hb0HPgViBwRtTbOaU= |
|  5 | xMBTPMKzVlG+FPixCBhfTjy/meAzaw7gLMkQU7j0N11XF6zgWl+8mF+VxkozKmjZ+EcEnFEwkQeNHRkmOXRQhgORgv0ijaoPAsJsRoSnp83gCkCaIlYRou4A7k8= |
|  6 | 74DWaFuC/xquwEcGlBkoC+HcOeJpFHzQYUSbEWDgthKPhTaTaaZi9t+yY+4K3uvSbS+IL+imMmkvM68UVp5dxgXrqdw7mjJ1IylXzGSm7e1BZy4/kDANLPvjSqo= |
|  7 | tITq2tqD6SGKcrT6B4jmFTgANLQ8Wt+MXiCoWAPE+pOlBPCxsJqcbGbS4TPYWh7ukLeKWYN/svPI3QlPsshapduIgAuN3fQJ63II0pe23OAFfIXIkSQCsA8O048= |
|  8 | SJtJS6co7cYaSrd89h6fxcOOkJrfrPPUUhXn04sOZDbTlZlSg2+ZZU60zjfos6VowQ0F/W/GguoiZ+ZpR4yJ8NH2Y4oxcS6BOQRsSW4UEqnirjKlrEYeHwZNV0A= |
|  9 | ipkNdyzrxA127EAnV8tjMnx22Uzqy9BPNQeu+bQkp0//pQH9Guohm1XVzzUPAvqOtI4NGZkFiyRH3XmCOvg7rYLVR1cFdyf5khOvEu5pBFm2FRqGupJQchMz3wg= |
+----+------------------------------------------------------------------------------------------------------------------------------+
9 rows in set (0.01 sec)

migrate database

[root@pr7302-t6251-kvm-rocky8-mgmt1 ~]# cloudstack-migrate-databases -m password -d password -e password2 -n password3 -v V2
Started database migration at Thu Mar 02 09:26:02 UTC 2023
Parsing db.properties file
DB Secret key provided matched the key in db.properties
INFO: Migrate properties with DB encryptor version: V2
Migrating db.properties..
Migrating db.properties Done.
Migrating server.properties..
Skipping server.properties as password.encryption.type is null
Begin Data migration
Initialised Encryptors
WARN  [c.c.u.c.EncryptionSecretKeyChecker] (main:null) (logid:) Encryption already enabled, is check() called twice?
INFO  [c.c.u.d.T.Transaction] (main:null) (logid:) Is Data Base High Availiability enabled? Ans : false
Begin migrate config values
End migrate config values
Begin migrate host details
End migrate host details
Begin migrate cluster details
End migrate cluster details
Begin migrate image store details
End migrate image store details
Begin migrate storage pool details
End migrate storage pool details
Begin migrate storage pool details for ScaleIO
End migrate storage pool details for ScaleIO
Begin migrate user vm details
End migrate user vm details
Begin migrate user vm deploy_as_is details
End migrate user vm deploy_as_is details
Begin migrate image store url if protocol is cifs
End migrate image store url if protocol is cifs
Begin migrate storage pool path if pool type is SMB
End migrate storage pool path if pool type is SMB
Skipped table sslcerts as there is no data in the table 
Skipped table vpn_users as there is no data in the table 
Skipped table account_details as there is no data in the table 
Skipped table domain_details as there is no data in the table 
Skipped table s2s_customer_gateway as there is no data in the table 
Skipped table virtual_supervisor_module as there is no data in the table 
Skipped table ucs_manager as there is no data in the table 
Begin migrate table vm_instance field vnc_password 
Done migrating database field vm_instance.vnc_password 
Skipped table external_stratosphere_ssp_credentials as there is no data in the table 
Skipped table vmware_data_center as there is no data in the table 
Skipped table keystore as there is no data in the table 
Begin migrate table passphrase field passphrase 
Done migrating database field passphrase.passphrase 
Skipped table remote_access_vpn as there is no data in the table 
Begin migrate table storage_pool field user_info 
Done migrating database field storage_pool.user_info 
Begin migrate table user field secret_key 
Done migrating database field user.secret_key 
Skipped table oobm as there is no data in the table 
End Data migration
Successfully updated secret key(s)
Finished database migration at Thu Mar 02 09:26:11 UTC 2023

passphrase are encrypted

mysql> select * from passphrase;
+----+------------------------------------------------------------------------------------------------------------------------------+
| id | passphrase                                                                                                                   |
+----+------------------------------------------------------------------------------------------------------------------------------+
|  1 | TRpfx+HdRPzMv3XpDY1lFRCpLmz/dliM/yTZULWetC2t6sqbVwbOVpNE9RTpR8Yt/ZMTp8eaWMdmeAEIfj5xudc36DzMnZrJd7gsqidpK5QuuTkaTwTY7gZsTOk= |
|  2 | uR5XG6b/Tb79EfzUjUdU8Wl2QdO8lW8xDNWeQc3MwrcKAcSAtsmBG1P1aEwuAxa2BFywa7CLMj1lOzNdp/SBQ31bP9DxTBPShA3riWWgfESiplgAmihJy7JNn0M= |
|  3 | /ek8oFBFaiCSP2taFKz9D3uZYwtiYbYTzPe8opr/IrULYjmyIX7dXNfG0fx817S56kgDVVmSjOh2s9r3LrJNobj9v4ofN6J9Nm5FO0N+41eBSiifziHBCavYvJY= |
|  4 | wg+g5DcyZNVIr+bfggSV57k7moksuHM2xexy9YSJnBSoPuO2vK7TgKkr68AmDaYi5XPmGRL3/Up5Ty6j3dg6w29HwGp0JOh/CGzEtsN94w9xitvsYazoBSlDdUw= |
|  5 | jO+C7R67yRB3gQM6Ix4AaFlny8uvueCKzMDOqbWL5Amm9A1vbrGwhVOm8uL45TNbBZRaeD2UwxLMuNA27/DPyM6HL46I/wnB4vSOWubs/U/L3rq0+ZpwjupU6Qk= |
|  6 | dywEzT1O5Sb4GWb9yCmy0HDk5wNaYphWC4YKTJYGPYqTMllajGWisC2ST9wU4E095DDCo1RM5EglCPh03EqRI6BqKP49f20szGelilo8lA+A4DjuPC5yWwRM4Dk= |
|  7 | oq0dkTYS6AfKS9SPF0uOVMvphmMUcQ/vrKHG3MgCQ3rLw0oiOUU31ALxa6A4XSeQxpm6PoklvDt8isiQF33pKLRmiKE8Zxk0njsNA/epFd7xd8x8YcJ9X6K0rZI= |
|  8 | WSWWJ0QMwWFX18tE09vjXxYzOcHpENpOeiKwsZfELXOhGVe4ZYKsmCHHWXYhwaN16L+Y0ye79VD0Xjhy9tl2Qb2KwC0v4mKnFGnGz22w+V6HTWsethJ6GuK874o= |
|  9 | 3KR4q3xzWiwS6zNdP2m2UiMPutYCe13SdRawh5PHoF7CrVLte2Ws/NuONgtROv6xwsmeaDUO80bGv/BxOhnu2zPVS/xq7oAqIJbaJH+CrxFFMII/YhVB1Qds4IE= |
+----+------------------------------------------------------------------------------------------------------------------------------+
9 rows in set (0.00 sec)

Stop vm, start it on same host: works. Both root/data disks are detected.
Stop vm, start it on another host: works. Both root/data disks are detected.
create new vm with root/data disk. Both root/data disks are detected.

passphrase is encrypted

mysql> select * from passphrase where id > 9;
+----+------------------------------------------------------------------------------------------------------------------------------+
| id | passphrase                                                                                                                   |
+----+------------------------------------------------------------------------------------------------------------------------------+
| 10 | tEscVLBCJAohTRyM57Y+z/k7uPEJubcC44xzN3e5C63IqTxDnNJV2EtJFJCws0QY1DcZW63nLczDzQymao8d5JfN6dgQCHCInzRS/OixZXKHqbw1YvtgNDxlGmE= |
| 11 | YuDccp76kDpxqhF07Kft9MH5M+aB3D3MhiOOUhnb+qAW2Gf/MxHI/dEfwraLV+ElvfY8oiJILQmxv7AcuQVotUSCH4XcujBTpUqzdo8zEv1z4Eq9DyCvOj4gqCs= |
+----+------------------------------------------------------------------------------------------------------------------------------+
2 rows in set (0.00 sec)

borisstoyanov · 2023-03-02T11:14:55Z

@blueorangutan test matrix

blueorangutan · 2023-03-02T11:16:03Z

@borisstoyanov a Trillian-Jenkins matrix job (centos7 mgmt + xenserver71, rocky8 mgmt + vmware67u3, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2023-03-02T11:20:55Z

Trillian test result (tid-6248)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 47994 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7302-t6248-kvm-rocky8.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

vladimirpetrov

LGTM based on manual testing. I tested the passphrase length, decrypting it and basic CRUD operations - all looks fine.

blueorangutan · 2023-03-02T22:14:54Z

Trillian test result (tid-6252)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38214 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7302-t6252-xenserver-71.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2023-03-02T22:19:21Z

Trillian test result (tid-6254)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38360 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7302-t6254-kvm-centos7.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

blueorangutan · 2023-03-03T00:06:38Z

Trillian test result (tid-6253)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 44858 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7302-t6253-vmware-67u3.zip
Smoke tests completed. 108 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

rohityadavcloud · 2023-03-03T08:58:23Z

Is this ready for merging or needs further review/testing - @borisstoyanov @vladimirpetrov @weizhouapache ?

Move PassphraseVO to use String instead of byte[] to support Encrypt …

82fec84

…annotation

boring-cyborg bot added component:kvm component:orchestration labels Mar 1, 2023

weizhouapache mentioned this pull request Mar 1, 2023

engine/schema: remove Encrypt annotation from passphrase #7300

Closed

12 tasks

DaanHoogland approved these changes Mar 2, 2023

View reviewed changes

DaanHoogland added this to the 4.18.0.0 milestone Mar 2, 2023

weizhouapache marked this pull request as ready for review March 2, 2023 09:42

borisstoyanov added the status:needs-testing label Mar 2, 2023

vladimirpetrov approved these changes Mar 2, 2023

View reviewed changes

rohityadavcloud approved these changes Mar 3, 2023

View reviewed changes

DaanHoogland merged commit 8592de9 into apache:main Mar 3, 2023

weizhouapache deleted the 4.18-fix-passphrase-unencrypted branch June 23, 2023 13:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move PassphraseVO to use String instead of byte[] to support Encrypt annotation #7302

Move PassphraseVO to use String instead of byte[] to support Encrypt annotation #7302

weizhouapache commented Mar 1, 2023

weizhouapache commented Mar 1, 2023

blueorangutan commented Mar 1, 2023

sonarcloud bot commented Mar 1, 2023

codecov bot commented Mar 1, 2023 •

edited

weizhouapache commented Mar 1, 2023

blueorangutan commented Mar 1, 2023

mlsorensen commented Mar 2, 2023

weizhouapache commented Mar 2, 2023

DaanHoogland left a comment

weizhouapache commented Mar 2, 2023

borisstoyanov commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

vladimirpetrov left a comment

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 3, 2023

rohityadavcloud commented Mar 3, 2023

Move PassphraseVO to use String instead of byte[] to support Encrypt annotation #7302

Move PassphraseVO to use String instead of byte[] to support Encrypt annotation #7302

Conversation

weizhouapache commented Mar 1, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

weizhouapache commented Mar 1, 2023

blueorangutan commented Mar 1, 2023

sonarcloud bot commented Mar 1, 2023

codecov bot commented Mar 1, 2023 • edited

Codecov Report

weizhouapache commented Mar 1, 2023

blueorangutan commented Mar 1, 2023

mlsorensen commented Mar 2, 2023

weizhouapache commented Mar 2, 2023

DaanHoogland left a comment

Choose a reason for hiding this comment

weizhouapache commented Mar 2, 2023

borisstoyanov commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

vladimirpetrov left a comment

Choose a reason for hiding this comment

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 2, 2023

blueorangutan commented Mar 3, 2023

rohityadavcloud commented Mar 3, 2023

codecov bot commented Mar 1, 2023 •

edited