You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2022/12/27 12:43:59 UTC
[GitHub] [cloudstack] GutoVeronezi commented on a diff in pull request #7015: Secure KVM VNC Console Access Using the CA Framework
GutoVeronezi commented on code in PR #7015:
URL: https://github.com/apache/cloudstack/pull/7015#discussion_r1057350343
##########
python/lib/cloudutils/serviceConfig.py:
##########
@@ -630,6 +630,12 @@ def config(self):
cfo.addEntry("user", "\"root\"")
cfo.addEntry("group", "\"root\"")
cfo.addEntry("vnc_listen", "\"0.0.0.0\"")
+ if self.syscfg.env.secure:
+ cfo.addEntry("vnc_tls", "1")
+ cfo.addEntry("vnc_tls_x509_verify", "1")
+ cfo.addEntry("vnc_tls_x509_cert_dir", "\"/etc/pki/libvirt-vnc\"")
+ else:
+ cfo.addEntry("vnc_tls", "0")
Review Comment:
This code is repeated 3 times. We could extract to a function.
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncSecurity.java:
##########
@@ -0,0 +1,43 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.consoleproxy.vnc.security;
+
+import com.cloud.consoleproxy.vnc.RfbConstants;
+import com.cloud.consoleproxy.vnc.network.NioSocketHandler;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+public interface VncSecurity {
+
+ static List<VncSecurity> getSecurityStack(int securityType, String vmPassword, String host, int port) {
+ switch (securityType) {
+ case RfbConstants.NO_AUTH: return Collections.singletonList(new NoneVncSecurity());
+ case RfbConstants.VNC_AUTH: return Collections.singletonList(new VncAuthSecurity(vmPassword));
+
+ // Do not add VEncrypt type = 19 but its supported subtypes
+ case RfbConstants.V_ENCRYPT_X509_VNC:
+ return Arrays.asList(new VncTLSSecurity(host, port), new VncAuthSecurity(vmPassword));
+ default: throw new CloudRuntimeException("Unsupported security type " + securityType);
Review Comment:
```suggestion
default:
throw new CloudRuntimeException("Unsupported security type " + securityType);
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -235,8 +262,61 @@ public byte[] encodePassword(byte[] challenge, String password) throws Exception
Cipher cipher = Cipher.getInstance("DES/ECB/NoPadding");
cipher.init(Cipher.ENCRYPT_MODE, secretKey);
- byte[] response = cipher.doFinal(challenge);
- return response;
+ return cipher.doFinal(challenge);
+ }
+
+ private void agreeVEncryptVersion() throws IOException {
+ int majorVEncryptVersion = nioSocketConnection.readUnsignedInteger(8);
+ int minorVEncryptVersion = nioSocketConnection.readUnsignedInteger(8);
+ int vEncryptVersion = (majorVEncryptVersion << 8) | minorVEncryptVersion;
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("VEncrypt version offered by the server: " + vEncryptVersion);
+ }
+ nioSocketConnection.writeUnsignedInteger(8, majorVEncryptVersion);
+ if (vEncryptVersion >= 0x0002) {
Review Comment:
```suggestion
if (vEncryptVersion >= 2) {
```
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -235,8 +262,61 @@ public byte[] encodePassword(byte[] challenge, String password) throws Exception
Cipher cipher = Cipher.getInstance("DES/ECB/NoPadding");
cipher.init(Cipher.ENCRYPT_MODE, secretKey);
- byte[] response = cipher.doFinal(challenge);
- return response;
+ return cipher.doFinal(challenge);
+ }
+
+ private void agreeVEncryptVersion() throws IOException {
+ int majorVEncryptVersion = nioSocketConnection.readUnsignedInteger(8);
+ int minorVEncryptVersion = nioSocketConnection.readUnsignedInteger(8);
+ int vEncryptVersion = (majorVEncryptVersion << 8) | minorVEncryptVersion;
+ if (s_logger.isDebugEnabled()) {
+ s_logger.debug("VEncrypt version offered by the server: " + vEncryptVersion);
+ }
+ nioSocketConnection.writeUnsignedInteger(8, majorVEncryptVersion);
+ if (vEncryptVersion >= 0x0002) {
+ nioSocketConnection.writeUnsignedInteger(8, 2);
+ nioSocketConnection.flushWriteBuffer();
+ } else {
+ nioSocketConnection.writeUnsignedInteger(8, 0);
+ nioSocketConnection.flushWriteBuffer();
+ throw new CloudRuntimeException("Server reported an unsupported VeNCrypt version");
+ }
+ int ack = nioSocketConnection.readUnsignedInteger(8);
+ if (ack != 0) {
+ throw new IOException("The VNC server did not agree on the VEncrypt version");
+ }
+ }
+
+ private int selectVEncryptSubtype() {
+ int numberOfSubtypes = nioSocketConnection.readUnsignedInteger(8);
+ if (numberOfSubtypes <= 0) {
+ throw new CloudRuntimeException("The server reported no VeNCrypt sub-types");
+ }
+ for (int i = 0; i < numberOfSubtypes; i++) {
+ nioSocketConnection.waitForBytesAvailableForReading(4);
+ int subtype = nioSocketConnection.readUnsignedInteger(32);
+ if (subtype == RfbConstants.V_ENCRYPT_X509_VNC) {
+ if (s_logger.isDebugEnabled()) {
+ s_logger.info("Selected VEncrypt subtype " + subtype);
+ }
Review Comment:
We intend to log it in `info` only if debug is enabled here?
##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/security/VncSecurity.java:
##########
@@ -0,0 +1,43 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package com.cloud.consoleproxy.vnc.security;
+
+import com.cloud.consoleproxy.vnc.RfbConstants;
+import com.cloud.consoleproxy.vnc.network.NioSocketHandler;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+public interface VncSecurity {
+
+ static List<VncSecurity> getSecurityStack(int securityType, String vmPassword, String host, int port) {
+ switch (securityType) {
+ case RfbConstants.NO_AUTH: return Collections.singletonList(new NoneVncSecurity());
+ case RfbConstants.VNC_AUTH: return Collections.singletonList(new VncAuthSecurity(vmPassword));
Review Comment:
```suggestion
case RfbConstants.NO_AUTH:
return Collections.singletonList(new NoneVncSecurity());
case RfbConstants.VNC_AUTH:
return Collections.singletonList(new VncAuthSecurity(vmPassword));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org