Skip to content

console proxy: use AeadBase64Encryptor instead of AES/CBC/PKCS5Padding#7237

Merged
DaanHoogland merged 1 commit intoapache:4.18from
weizhouapache:4.18-replace-cpvm-encryption
Jul 5, 2023
Merged

console proxy: use AeadBase64Encryptor instead of AES/CBC/PKCS5Padding#7237
DaanHoogland merged 1 commit intoapache:4.18from
weizhouapache:4.18-replace-cpvm-encryption

Conversation

@weizhouapache
Copy link
Copy Markdown
Member

@weizhouapache weizhouapache commented Feb 15, 2023 •
edited
Loading

Description

This PR fixes #4694

key will be used as password of AeadBase64Encryptor
iv will be used as aad of AeadBase64Encryptor

This need testing, at least:

  • patch systemvm (cpvm)
  • recreate systenvm (destroy and then recreate automatically)

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

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@weizhouapache
console proxy: use AeadBase64Encryptor instead of AES/CBC/PKCS5Padding
e1484d7 
This fixes apache#4694

key will be used as password of AeadBase64Encryptor
iv will be used as aad of AeadBase64Encryptor
@weizhouapache weizhouapache added this to the 4.18.1.0 milestone Feb 15, 2023
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 15, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

29.4% 29.4% Coverage
32.0% 32.0% Duplication

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 15, 2023 •
edited
Loading

Codecov Report

Merging #7237 (e1484d7) into main (597a803) will increase coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##               main    #7237   +/-   ##
=========================================
  Coverage     12.67%   12.67%           
  Complexity     8639     8639           
=========================================
  Files          2716     2716           
  Lines        256112   256070   -42     
  Branches      39926    39926           
=========================================
  Hits          32456    32456           
+ Misses       219528   219486   -42     
  Partials       4128     4128
Impacted Files Coverage Δ
...ud/servlet/ConsoleProxyPasswordBasedEncryptor.java 0.00% <0.00%> (ø)

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

yadvr
yadvr approved these changes Feb 24, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@yadvr yadvr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, is this critical for 4.18.0.0 @weizhouapache cc @DaanHoogland ?

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Feb 24, 2023

LGTM, is this critical for 4.18.0.0 @weizhouapache cc @DaanHoogland ?

I think we can wait until 4.18.1.0.
currently this has not been reviewed and tested

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Feb 24, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 24, 2023

@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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 24, 2023

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5643

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Feb 24, 2023

@blueorangutan test keepEnv

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 24, 2023

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Feb 24, 2023

Trillian test result (tid-6227)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38410 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7237-t6227-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

@yadvr yadvr requested review from DaanHoogland and nvazquez May 8, 2023 07:15
@yadvr yadvr changed the base branch from main to 4.18 May 8, 2023 07:15
@yadvr
Copy link
Copy Markdown
Member

yadvr commented May 8, 2023

@weizhouapache cc @nvazquez @DaanHoogland @shwstppr - is it still relevant, do we need any testing to merge this or can this be merged now?

@yadvr yadvr requested a review from shwstppr May 8, 2023 07:16
@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented May 9, 2023

@weizhouapache cc @nvazquez @DaanHoogland @shwstppr - is it still relevant, do we need any testing to merge this or can this be merged now?

code looks good but I have not tested this. I think this was a user request but don't recall exactly, @weizhouapache can you enlighten us?

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented May 9, 2023

@weizhouapache cc @nvazquez @DaanHoogland @shwstppr - is it still relevant, do we need any testing to merge this or can this be merged now?

code looks good but I have not tested this. I think this was a user request but don't recall exactly, @weizhouapache can you enlighten us?

Sure @DaanHoogland
See #4694 (also mentioned in the description of this PR)

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Jun 1, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 1, 2023

@weizhouapache a [LL] 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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 1, 2023

Packaging result [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6064

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 12, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 12, 2023

@rohityadavcloud a [LL] Jenkins job has been kicked to build packages. It will be bundled with

SystemVM template(s). I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 12, 2023

Packaging result [LL]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 6125

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Jun 12, 2023

@blueorangutan package

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Jun 12, 2023

we need to test cpvm patch/recreation manually

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jun 12, 2023

we need to test cpvm patch/recreation manually

Can you add what you expect to the description of this PR, @weizhouapache ?

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jun 12, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 12, 2023

@DaanHoogland a [LL] 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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 12, 2023

Packaging result [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6129

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jun 12, 2023

@blueorangutan test keepEnv

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 12, 2023

@DaanHoogland a [SF] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 13, 2023

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

Test Result Time (s) Test File
test_01_migrate_VM_and_root_volume Error 77.62 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 50.32 test_vm_life_cycle.py
test_06_download_detached_volume Error 298.83 test_volumes.py
test_13_migrate_volume_and_change_offering Error 122.00 test_volumes.py
ContextSuite context=TestIpv6Vpc>:setup Error 0.00 test_vpc_ipv6.py
ContextSuite context=TestVPCRedundancy>:setup Error 0.00 test_vpc_redundant.py
ContextSuite context=TestVPCNics>:setup Error 0.00 test_vpc_router_nics.py
ContextSuite context=TestRVPCSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVPCSite2SiteVPNMultipleOptions>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcRemoteAccessVpn>:setup Error 0.00 test_vpc_vpn.py
ContextSuite context=TestVpcSite2SiteVpn>:setup Error 0.00 test_vpc_vpn.py
test_disable_oobm_ha_state_ineligible Error 0.35 test_hostha_kvm.py

@DaanHoogland DaanHoogland self-assigned this Jun 22, 2023
@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jun 22, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 22, 2023

@DaanHoogland a [SF] 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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 22, 2023

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

@weizhouapache
Copy link
Copy Markdown
Member Author

weizhouapache commented Jun 22, 2023

@blueorangutan test

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 22, 2023

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jun 23, 2023

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

Test Result Time (s) Test File
test_01_migrate_VM_and_root_volume Error 77.86 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 53.59 test_vm_life_cycle.py

DaanHoogland
DaanHoogland approved these changes Jun 23, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLGTM and did

  • patch systemvm (cpvm)

  • recreate systenvm (destroy and then recreate automatically)

no errors returned, but I am not sure my tests where extensive enough.

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Jun 27, 2023 •
edited
Loading

Testing LGTM @DaanHoogland - did you manually test console against most supported hypervisors (KVM EL/Ubuntu, XCP-ng, VMware 6.7, 7.0)
@weizhouapache - thnx could you review if that's enough to merge this or we need additional review, testing.

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jul 3, 2023

@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 3, 2023

@DaanHoogland a [SF] 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.

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 3, 2023

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

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jul 4, 2023

@blueorangutan test matrix keepEnv

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 4, 2023

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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 4, 2023

[SF] Trillian test result (tid-6957)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37198 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7237-t6957-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
Copy link
Copy Markdown

blueorangutan commented Jul 4, 2023

[SF] Trillian test result (tid-6958)
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server r8
Total time taken: 43478 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7237-t6958-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

@blueorangutan
Copy link
Copy Markdown

blueorangutan commented Jul 4, 2023

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

Test Result Time (s) Test File
test_01_migrate_VM_and_root_volume Error 79.61 test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks Error 53.27 test_vm_life_cycle.py

@DaanHoogland
Copy link
Copy Markdown
Contributor

DaanHoogland commented Jul 5, 2023

Testing LGTM @DaanHoogland - did you manually test console against most supported hypervisors (KVM EL/Ubuntu, XCP-ng, VMware 6.7, 7.0)

I tried kvm el, vmware and xen and these work.

@DaanHoogland DaanHoogland merged commit 3e04779 into apache:4.18 Jul 5, 2023
DaanHoogland added a commit that referenced this pull request Jul 5, 2023
@DaanHoogland
Merge release branch 4.18 to main
02be2cd 
* 4.18:
  cleanup removal of physical network (#7630)
  console proxy: use AeadBase64Encryptor instead of AES/CBC/PKCS5Padding (#7237)
@weizhouapache weizhouapache deleted the 4.18-replace-cpvm-encryption branch July 24, 2023 06:44
@weizhouapache weizhouapache mentioned this pull request Jul 28, 2023
Closed
@DaanHoogland DaanHoogland removed their assignment Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yadvr yadvr yadvr approved these changes

@DaanHoogland DaanHoogland DaanHoogland approved these changes

@nvazquez nvazquez Awaiting requested review from nvazquez

@shwstppr shwstppr Awaiting requested review from shwstppr

Assignees

No one assigned

Labels

component:console-proxy

Projects

None yet

Milestone

4.18.1.0

Development

Successfully merging this pull request may close these issues.

Usage of "AES/CBC/PKCS5Padding" is insecure

4 participants

@weizhouapache @blueorangutan @yadvr @DaanHoogland