-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Secure KVM VNC Console Access Using the CA Framework #7015
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Secure KVM VNC Console Access Using the CA Framework #7015
Conversation
@blueorangutan package |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 5053 |
Codecov Report
@@ Coverage Diff @@
## main #7015 +/- ##
============================================
+ Coverage 11.77% 11.78% +0.01%
- Complexity 7662 7665 +3
============================================
Files 2503 2505 +2
Lines 245958 246029 +71
Branches 38374 38382 +8
============================================
+ Hits 28953 28986 +33
- Misses 213240 213272 +32
- Partials 3765 3771 +6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clggtm, just some remarks on structure and logging.
needs extensive testing though, might be good to have in 4.18
...console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/ConsoleProxyNoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
.../console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/network/SSLEngineManager.java
Outdated
Show resolved
Hide resolved
if (manager == null) { | ||
if (socketHandler.readUnsignedInteger(8) == 0) { | ||
int result = socketHandler.readUnsignedInteger(32); | ||
String reason; | ||
if (result == RfbConstants.VNC_AUTH_FAILED || result == RfbConstants.VNC_AUTH_TOO_MANY) { | ||
reason = socketHandler.readString(); | ||
} else { | ||
reason = "Authentication failure (protocol error)"; | ||
} | ||
throw new CloudRuntimeException(reason); | ||
} | ||
setParam(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move to a handleErrorState() type of method?
@blueorangutan package |
@nvazquez 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. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 5057 |
@nvazquez please go through these. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
overall code LGTM
left few minor comments
great job @nvazquez !
I'd learn these codes when I have time :-D
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java
Outdated
Show resolved
Hide resolved
@@ -471,22 +474,31 @@ const UI = { | |||
clearTimeout(UI.statusTimeout); | |||
|
|||
switch (statusType) { | |||
case 'encrypted': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to re-apply these changes when upgrade novnc.
not a big issue, just need to pay a bit more attention
@blueorangutan package |
@nvazquez 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 package |
@nvazquez 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 Quality Gate failed. |
Packaging result: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 5340 |
@blueorangutan package |
@nvazquez 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. |
Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5347 |
@borisstoyanov , you approved based on manual testing, am i right? |
@blueorangutan test |
@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Trillian test result (tid-5929)
|
Merging this based on review, testing and smoketests. cc @DaanHoogland |
Description
This PR allows securing the console access through CloudStack to the virtual machines running on KVM. The secure access is achieved through the generated certificates for the CA Framework in CloudStack, that provides mutual TLS connections between agents. These certificates are used to also secure the connection between the console proxies and the VNC ports for VM console access.
This feature is only supported on the KVM hypervisor
Design Document: https://cwiki.apache.org/confluence/display/CLOUDSTACK/Secure+KVM+VNC+connection+using+the+CA+framework
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
Tested on KVM environment, enabling TLS on VNC