Persist IP addresses related to VM access via CPVM

[bernardodemarco]

Description

This PR proposes to persist IP addresses related to VM access via CPVM. Specifically, it stores the IP address of the client accessing a VM and the IP address of the console endpoint creator. To achieve this, the cloud.console_session table was extended with two new columns: client_address, which stores the client's IP address, and console_endpoint_creator_address, which captures the IP address of the console endpoint creator.

These IP addresses were already being captured for logging and validation purposes. The console endpoint creator's IP is captured here, at the start of the CreateConsoleEndpointCmd execution:

cloudstack/api/src/main/java/org/apache/cloudstack/api/command/user/consoleproxy/CreateConsoleEndpointCmd.java

Lines 65 to 68 in 47a6b70

	@Override
	public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, NetworkRuleConflictException {
	String clientAddress = getClientAddress();
	ConsoleEndpoint endpoint = consoleManager.generateConsoleEndpoint(vmId, extraSecurityToken, clientAddress);

The client address is captured by calling session.getRemoteAddress().getAddress().getHostAddress() in this method, that gets executed when the client connects to the console:

cloudstack/services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVNCHandler.java

Lines 157 to 160 in 47a6b70

	private boolean checkSessionSourceIp(final Session session, final String sourceIP) throws IOException {
	// Verify source IP
	String sessionSourceIP = session.getRemoteAddress().getAddress().getHostAddress();
	logger.info("Get websocket connection request from remote IP : " + sessionSourceIP);

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

Screenshots (if appropriate):

How Has This Been Tested?

1. I manually added the client_address and console_endpoint_creator_address columns to the cloud.console_session table.
2. I generated a VM console endpoint.
3. I executed the following query in order to validate whether the stored IP was correct.

SELECT * FROM `cloud`.`console_session` ORDER BY created DESC LIMIT 1;

id	uuid	created	account_id	user_id	instance_id	host_id	acquired	removed	client_address	console_endpoint_creator_address
2	77f08cea-8068-471f-9464-44810e1ef545	2024-08-06 17:57:05	2	2	14	1	NULL	NULL	NULL	192.168.122.1

As can be noticed, the console_endpoint_creator_address column was populated accordingly, whereas the client_adress was still empty since the VM had not been accessed yet.

4. I accessed the VM via CPVM.
5. I executed the following query in order to validate whether the client stored IP was correct.

SELECT * FROM `cloud`.`console_session` ORDER BY created DESC LIMIT 1;

id	uuid	created	account_id	user_id	instance_id	host_id	acquired	removed	client_address	console_endpoint_creator_address
2	77f08cea-8068-471f-9464-44810e1ef545	2024-08-06 17:57:05	2	2	14	1	NULL	NULL	192.168.122.1	192.168.122.1

As can be noticed, the IP address of the client that accessed the VM through CPVM was persisted correctly.

[bernardodemarco]

@blueorangutan package

[blueorangutan]

@bernardodemarco a [SL] 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.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 32 lines in your changes missing coverage. Please review.

Project coverage is 15.77%. Comparing base (501d8c1) to head (1ed976f).

Files with missing lines	Patch %	Lines
...a/src/main/java/com/cloud/vm/ConsoleSessionVO.java	0.00%	12 Missing ⚠️
.../agent/api/ConsoleAccessAuthenticationCommand.java	0.00%	8 Missing ⚠️
...udstack/consoleproxy/ConsoleAccessManagerImpl.java	0.00%	5 Missing ⚠️
...nt/resource/consoleproxy/ConsoleProxyResource.java	0.00%	2 Missing ⚠️
...n/java/com/cloud/vm/dao/ConsoleSessionDaoImpl.java	0.00%	2 Missing ⚠️
...ain/java/com/cloud/consoleproxy/AgentHookBase.java	0.00%	2 Missing ⚠️
...main/java/com/cloud/consoleproxy/ConsoleProxy.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9534      +/-   ##
============================================
- Coverage     15.77%   15.77%   -0.01%     
  Complexity    12538    12538              
============================================
  Files          5621     5621              
  Lines        491556   491578      +22     
  Branches      60227    62625    +2398     
============================================
- Hits          77562    77561       -1     
- Misses       405537   405559      +22     
- Partials       8457     8458       +1     

Flag	Coverage Δ
uitests	4.05% <ø> (ø)
unittests	16.59% <0.00%> (-0.01%)	⬇️

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

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

[blueorangutan]

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

[github-actions]

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

[DaanHoogland]

@bernardodemarco , I understand what and how, but have a hard time to get the why of this PR. Can you expand on that please?

[bernardodemarco]

@bernardodemarco , I understand what and how, but have a hard time to get the why of this PR. Can you expand on that please?

The main idea behind the proposed enhancement is to help operators better identify who accessed a virtual machine console.

The cloud.console_session table already stores data relating to who (account_id and user_id) accessed a given virtual machine (instance_id), as well as when that happened (created and removed). By adding the client_address and console_endpoint_creator_address fields, it'll be easier to identify who accessed the console, which can be extremely helpful for security and auditing purposes.

[DaanHoogland]

@bernardodemarco , I understand what and how, but have a hard time to get the why of this PR. Can you expand on that please?

The main idea behind the proposed enhancement is to help operators better identify who accessed a virtual machine console.

The cloud.console_session table already stores data relating to who (account_id and user_id) accessed a given virtual machine (instance_id), as well as when that happened (created and removed). By adding the client_address and console_endpoint_creator_address fields, it'll be easier to identify who accessed the console, which can be extremely helpful for security and auditing purposes.

thanks, makes sense.

[DaanHoogland]

@nvazquez can you have a look at this one?

[github-actions]

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

[nvazquez]

Code LGTM

[sureshanaparti]

clgtm

[DaanHoogland]

@blueorangutan package

[blueorangutan]

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

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

[github-actions]

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

[DaanHoogland]

@blueorangutan package

[blueorangutan]

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

[weizhouapache]

client_address is always NULL in my testing.
can you check ? @bernardodemarco cc @DaanHoogland

[blueorangutan]

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

[DaanHoogland]

@blueorangutan test keepEnv

[blueorangutan]

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

[bernardodemarco]

client_address is always NULL in my testing.

I tested the PR again, following the steps provided on the description, and the IP addresses were stored correctly.

It's important to remember that if there were any running CPVMs prior to the environment update, then they would need to be recreated in order to test if the client_address column is being populated correctly.

Furthermore, it's interesting taking a look at the CPVM logs. The following logs were generated during my tests, which indicates that the client IP 192.168.122.1 was captured correctly.

2024-09-10T17:19:38,330 INFO  [cloud.consoleproxy.ConsoleProxyNoVNCHandler] (qtp2090537387-40:[]) Verifying session
source IP 192.168.122.1 from WebSocket connection request.

2024-09-10T17:19:38,331 DEBUG [cloud.consoleproxy.ConsoleProxyNoVNCHandler] (qtp2090537387-40:[]) Session source IP
192.168.122.1 has been verified successfully.

@weizhouapache, if the problem continues happening, could you please describe your tests steps and share the CPVM logs?

[weizhouapache]

client_address is always NULL in my testing.

I tested the PR again, following the steps provided on the description, and the IP addresses were stored correctly.

It's important to remember that if there were any running CPVMs prior to the environment update, then they would need to be recreated in order to test if the client_address column is being populated correctly.

Furthermore, it's interesting taking a look at the CPVM logs. The following logs were generated during my tests, which indicates that the client IP 192.168.122.1 was captured correctly.

2024-09-10T17:19:38,330 INFO  [cloud.consoleproxy.ConsoleProxyNoVNCHandler] (qtp2090537387-40:[]) Verifying session
source IP 192.168.122.1 from WebSocket connection request.

2024-09-10T17:19:38,331 DEBUG [cloud.consoleproxy.ConsoleProxyNoVNCHandler] (qtp2090537387-40:[]) Session source IP
192.168.122.1 has been verified successfully.

@weizhouapache, if the problem continues happening, could you please describe your tests steps and share the CPVM logs?

Thanks @bernardodemarco
It works after patching the cpvm

[blueorangutan]

[SF] Trillian test result (tid-11441)
Environment: kvm-ol8 (x2), Advanced Networking with Mgmt server ol8
Total time taken: 50973 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9534-t11441-kvm-ol8.zip
Smoke tests completed. 139 look OK, 2 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	97.32	test_vm_life_cycle.py
test_02_migrate_VM_with_two_data_disks	Error	50.60	test_vm_life_cycle.py
test_08_migrate_vm	Error	0.08	test_vm_life_cycle.py
test_01_migrate_vm_strict_tags_success	Error	0.23	test_vm_strict_host_tags.py
test_02_migrate_vm_strict_tags_failure	Error	0.23	test_vm_strict_host_tags.py

[DaanHoogland]

@JoaoJandre , it looks like this is ready. should we merge it before 4.20?

[JoaoJandre]

@JoaoJandre , it looks like this is ready. should we merge it before 4.20?

I think we should leave this for 4.21. We're only accepting bugfixes currently.