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/22 13:13:57 UTC

[GitHub] [cloudstack] weizhouapache commented on a diff in pull request #7015: Secure KVM VNC Console Access Using the CA Framework

weizhouapache commented on code in PR #7015:
URL: https://github.com/apache/cloudstack/pull/7015#discussion_r1055409023


##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -239,16 +273,349 @@ public byte[] encodePassword(byte[] challenge, String password) throws Exception
         return response;
     }
 
+    /**
+     * Decide the RFB protocol version with the VNC server
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#711protocolversion
+     */
+    protected String handshakeProtocolVersion(RemoteEndpoint clientRemote) throws IOException {
+        // Read protocol version
+        byte[] buf = new byte[12];
+        tunnelInputStream.readFully(buf);
+        String rfbProtocol = new String(buf);
+
+        // Server should use RFB protocol 3.x
+        if (!rfbProtocol.contains(RfbConstants.RFB_PROTOCOL_VERSION_MAJOR)) {

Review Comment:
   maybe there is no difference
   ```suggestion
           if (!rfbProtocol.startsWith(RfbConstants.RFB_PROTOCOL_VERSION_MAJOR)) {
   
   ```



##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -75,6 +97,10 @@ public void connectToWebSocket(String websocketUrl, Session session) throws URIS
         webSocketReverseProxy.connect();
     }
 
+    public boolean isVncOverTunnel() {
+        return this.tunnelSocket != null;
+    }
+

Review Comment:
   @nvazquez 
   would it be good to add a method like `isVncOverNioSocket ` ?
   it can be used to replace `if (!isVncOverTunnel)`



##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -239,16 +273,349 @@ public byte[] encodePassword(byte[] challenge, String password) throws Exception
         return response;
     }
 
+    /**
+     * Decide the RFB protocol version with the VNC server
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#711protocolversion
+     */
+    protected String handshakeProtocolVersion(RemoteEndpoint clientRemote) throws IOException {
+        // Read protocol version
+        byte[] buf = new byte[12];
+        tunnelInputStream.readFully(buf);
+        String rfbProtocol = new String(buf);
+
+        // Server should use RFB protocol 3.x
+        if (!rfbProtocol.contains(RfbConstants.RFB_PROTOCOL_VERSION_MAJOR)) {
+            s_logger.error("Cannot handshake with VNC server. Unsupported protocol version: \"" + rfbProtocol + "\".");
+            throw new RuntimeException(
+                    "Cannot handshake with VNC server. Unsupported protocol version: \"" + rfbProtocol + "\".");
+        }
+        tunnelOutputStream.write(buf);
+        return RfbConstants.RFB_PROTOCOL_VERSION + "\n";
+    }
+
+    /**
+     * Agree on the security type with the VNC server
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#712security
+     * @return list of the security types to be processed
+     */
+    protected List<VncSecurity> handshakeSecurityTypes(RemoteEndpoint clientRemote, String vmPassword,
+                                                       String host, int port) throws IOException {
+        int securityType = selectFromTheServerOfferedSecurityTypes();
+
+        // Inform the server about our decision
+        this.tunnelOutputStream.writeByte(securityType);
+
+        byte[] numberTypesToClient = new byte[] { 1, (byte) securityType };
+        clientRemote.sendBytes(ByteBuffer.wrap(numberTypesToClient, 0, 2));
+
+        if (securityType == RfbConstants.V_ENCRYPT) {
+            securityType = getVEncryptSecuritySubtype();
+        }
+        return VncSecurity.getSecurityStack(securityType, vmPassword, host, port);
+    }
+
+    /**
+     * Obtain the VEncrypt subtype from the VNC server
+     *
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#724vencrypt
+     */
+    protected int getVEncryptSecuritySubtype() throws IOException {
+        int majorVEncryptVersion = socketConnection.readUnsignedInteger(8);
+        int minorVEncryptVersion = socketConnection.readUnsignedInteger(8);
+        int vEncryptVersion = (majorVEncryptVersion << 8) | minorVEncryptVersion;
+        s_logger.debug("VEncrypt version: " + vEncryptVersion);
+        socketConnection.writeUnsignedInteger(8, majorVEncryptVersion);
+        if (vEncryptVersion >= 0x0002) {
+            socketConnection.writeUnsignedInteger(8, 2);
+            socketConnection.flushWriteBuffer();
+        } else {
+            socketConnection.writeUnsignedInteger(8, 0);
+            socketConnection.flushWriteBuffer();
+            throw new CloudRuntimeException("Server reported an unsupported VeNCrypt version");
+        }
+        int ack = socketConnection.readUnsignedInteger(8);
+        if (ack != 0) {
+            throw new IOException("The VNC server did not agree on the VEncrypt version");
+        }
+
+        int numberOfSubtypes = socketConnection.readUnsignedInteger(8);
+        if (numberOfSubtypes <= 0) {
+            throw new CloudRuntimeException("The server reported no VeNCrypt sub-types");
+        }
+        int selectedSubtype = 0;
+        for (int i = 0; i < numberOfSubtypes; i++) {
+            while (!socketConnection.checkIfBytesAreAvailableForReading(4)) {
+                s_logger.trace("Waiting for vEncrypt subtype");
+            }
+            int subtype = socketConnection.readUnsignedInteger(32);
+            if (subtype == RfbConstants.V_ENCRYPT_X509_VNC) {
+                selectedSubtype = subtype;
+                break;
+            }
+        }
+
+        s_logger.info("Selected VEncrypt subtype " + selectedSubtype);
+        socketConnection.writeUnsignedInteger(32, selectedSubtype);
+        socketConnection.flushWriteBuffer();
+
+        return selectedSubtype;
+    }
+
+    private int selectFromTheServerOfferedSecurityTypes() throws IOException {
+        int numberOfSecurityTypes = tunnelInputStream.readByte();
+        if (numberOfSecurityTypes == 0) {
+            int reasonLength = tunnelInputStream.readInt();
+            byte[] reasonBuffer = new byte[reasonLength];
+            tunnelInputStream.readFully(reasonBuffer);
+            String reason = new String(reasonBuffer);
+            String errMsg = "No security type provided by the VNC server, reason: " + reason;
+            s_logger.error(errMsg);
+            throw new IOException(errMsg);
+        }
+
+        for (int i = 0; i < numberOfSecurityTypes; i++) {
+            int securityType = tunnelInputStream.readByte();
+            if (securityType != 0 && VncSecurity.supportedSecurityTypes.contains(securityType)) {
+                s_logger.info("Selected the security type: " + securityType);
+                return securityType;
+            }
+        }
+        throw new IOException("Could not select a supported or valid security type from the offered by the server");
+    }
+
+    /**
+     * VNC authentication.
+     */
+    public void processSecurityResult(String password)

Review Comment:
   it seems this method is not used in any other class.



##########
systemvm/agent/noVNC/app/ui.js:
##########
@@ -471,22 +474,31 @@ const UI = {
         clearTimeout(UI.statusTimeout);
 
         switch (statusType) {
+            case 'encrypted':

Review Comment:
   we need to re-apply these changes when upgrade novnc.
   not a big issue, just need to pay a bit more attention



##########
services/console-proxy/server/src/main/java/com/cloud/consoleproxy/vnc/NoVncClient.java:
##########
@@ -239,16 +273,349 @@ public byte[] encodePassword(byte[] challenge, String password) throws Exception
         return response;
     }
 
+    /**
+     * Decide the RFB protocol version with the VNC server
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#711protocolversion
+     */
+    protected String handshakeProtocolVersion(RemoteEndpoint clientRemote) throws IOException {
+        // Read protocol version
+        byte[] buf = new byte[12];
+        tunnelInputStream.readFully(buf);
+        String rfbProtocol = new String(buf);
+
+        // Server should use RFB protocol 3.x
+        if (!rfbProtocol.contains(RfbConstants.RFB_PROTOCOL_VERSION_MAJOR)) {
+            s_logger.error("Cannot handshake with VNC server. Unsupported protocol version: \"" + rfbProtocol + "\".");
+            throw new RuntimeException(
+                    "Cannot handshake with VNC server. Unsupported protocol version: \"" + rfbProtocol + "\".");
+        }
+        tunnelOutputStream.write(buf);
+        return RfbConstants.RFB_PROTOCOL_VERSION + "\n";
+    }
+
+    /**
+     * Agree on the security type with the VNC server
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#712security
+     * @return list of the security types to be processed
+     */
+    protected List<VncSecurity> handshakeSecurityTypes(RemoteEndpoint clientRemote, String vmPassword,
+                                                       String host, int port) throws IOException {
+        int securityType = selectFromTheServerOfferedSecurityTypes();
+
+        // Inform the server about our decision
+        this.tunnelOutputStream.writeByte(securityType);
+
+        byte[] numberTypesToClient = new byte[] { 1, (byte) securityType };
+        clientRemote.sendBytes(ByteBuffer.wrap(numberTypesToClient, 0, 2));
+
+        if (securityType == RfbConstants.V_ENCRYPT) {
+            securityType = getVEncryptSecuritySubtype();
+        }
+        return VncSecurity.getSecurityStack(securityType, vmPassword, host, port);
+    }
+
+    /**
+     * Obtain the VEncrypt subtype from the VNC server
+     *
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#724vencrypt
+     */
+    protected int getVEncryptSecuritySubtype() throws IOException {
+        int majorVEncryptVersion = socketConnection.readUnsignedInteger(8);
+        int minorVEncryptVersion = socketConnection.readUnsignedInteger(8);
+        int vEncryptVersion = (majorVEncryptVersion << 8) | minorVEncryptVersion;
+        s_logger.debug("VEncrypt version: " + vEncryptVersion);
+        socketConnection.writeUnsignedInteger(8, majorVEncryptVersion);
+        if (vEncryptVersion >= 0x0002) {
+            socketConnection.writeUnsignedInteger(8, 2);
+            socketConnection.flushWriteBuffer();
+        } else {
+            socketConnection.writeUnsignedInteger(8, 0);
+            socketConnection.flushWriteBuffer();
+            throw new CloudRuntimeException("Server reported an unsupported VeNCrypt version");
+        }
+        int ack = socketConnection.readUnsignedInteger(8);
+        if (ack != 0) {
+            throw new IOException("The VNC server did not agree on the VEncrypt version");
+        }
+
+        int numberOfSubtypes = socketConnection.readUnsignedInteger(8);
+        if (numberOfSubtypes <= 0) {
+            throw new CloudRuntimeException("The server reported no VeNCrypt sub-types");
+        }
+        int selectedSubtype = 0;
+        for (int i = 0; i < numberOfSubtypes; i++) {
+            while (!socketConnection.checkIfBytesAreAvailableForReading(4)) {
+                s_logger.trace("Waiting for vEncrypt subtype");
+            }
+            int subtype = socketConnection.readUnsignedInteger(32);
+            if (subtype == RfbConstants.V_ENCRYPT_X509_VNC) {
+                selectedSubtype = subtype;
+                break;
+            }
+        }
+
+        s_logger.info("Selected VEncrypt subtype " + selectedSubtype);
+        socketConnection.writeUnsignedInteger(32, selectedSubtype);
+        socketConnection.flushWriteBuffer();
+
+        return selectedSubtype;
+    }
+
+    private int selectFromTheServerOfferedSecurityTypes() throws IOException {
+        int numberOfSecurityTypes = tunnelInputStream.readByte();
+        if (numberOfSecurityTypes == 0) {
+            int reasonLength = tunnelInputStream.readInt();
+            byte[] reasonBuffer = new byte[reasonLength];
+            tunnelInputStream.readFully(reasonBuffer);
+            String reason = new String(reasonBuffer);
+            String errMsg = "No security type provided by the VNC server, reason: " + reason;
+            s_logger.error(errMsg);
+            throw new IOException(errMsg);
+        }
+
+        for (int i = 0; i < numberOfSecurityTypes; i++) {
+            int securityType = tunnelInputStream.readByte();
+            if (securityType != 0 && VncSecurity.supportedSecurityTypes.contains(securityType)) {
+                s_logger.info("Selected the security type: " + securityType);
+                return securityType;
+            }
+        }
+        throw new IOException("Could not select a supported or valid security type from the offered by the server");
+    }
+
+    /**
+     * VNC authentication.
+     */
+    public void processSecurityResult(String password)
+            throws IOException {
+        // Read security result
+        int authResult = this.tunnelInputStream.readInt();
+
+        switch (authResult) {
+            case RfbConstants.VNC_AUTH_OK: {
+                // Nothing to do
+                break;
+            }
+
+            case RfbConstants.VNC_AUTH_TOO_MANY:
+                s_logger.error("Connection to VNC server failed: too many wrong attempts.");
+                throw new RuntimeException("Connection to VNC server failed: too many wrong attempts.");
+
+            case RfbConstants.VNC_AUTH_FAILED:
+                s_logger.error("Connection to VNC server failed: wrong password.");
+                throw new RuntimeException("Connection to VNC server failed: wrong password.");
+
+            default:
+                s_logger.error("Connection to VNC server failed, reason code: " + authResult);
+                throw new RuntimeException("Connection to VNC server failed, reason code: " + authResult);
+        }
+    }
+
     public int read(byte[] b) throws IOException {
-        return is.read(b);
+        return tunnelInputStream.read(b);
     }
 
     public void write(byte[] b) throws IOException {
         if (isVncOverWebSocketConnection()) {
             proxyMsgOverWebSocketConnection(ByteBuffer.wrap(b));
+        } else if (!isVncOverTunnel()) {
+            this.socketConnection.writeBytes(b, 0, b.length);
+        } else {
+            tunnelOutputStream.write(b);
+        }
+    }
+
+    public void writeFrame(Frame frame) {
+        byte[] data = new byte[frame.getPayloadLength()];
+        frame.getPayload().get(data);
+
+        if (securityPhaseCompleted) {
+            socketConnection.writeBytes(ByteBuffer.wrap(data), data.length);
+            socketConnection.flushWriteBuffer();
+            if (writerLeft == null) {
+                writerLeft = 3;
+                setWaitForNoVnc(false);
+            } else if (writerLeft > 0) {
+                writerLeft--;
+            }
+        } else {
+            socketConnection.writeBytes(data, 0, data.length);
+            if (flushAfterReceivingNoVNCData) {
+                socketConnection.flushWriteBuffer();
+                flushAfterReceivingNoVNCData = false;
+            }
+        }
+
+        if (!securityPhaseCompleted || (writerLeft != null && writerLeft == 0)) {
+            setWaitForNoVnc(false);
+        }
+    }
+
+    /**
+     * Starts the handshake with the VNC server - ProtocolVersion
+     *
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#711protocolversion
+     */
+    public ByteBuffer handshakeProtocolVersion() {
+        ByteBuffer verStr = ByteBuffer.allocate(12);
+        int majorVersion;
+        int minorVersion;
+
+        s_logger.debug("Reading RFB protocol version");
+
+        socketConnection.readBytes(verStr, 12);
+
+        if ((new String(verStr.array())).matches("RFB \\d{3}\\.\\d{3}\\n")) {
+            majorVersion = Integer.parseInt((new String(verStr.array())).substring(4,7));
+            minorVersion = Integer.parseInt((new String(verStr.array())).substring(8,11));
+        } else {
+            throw new CloudRuntimeException("Reading version failed: not an RFB server?");
+        }
+
+        s_logger.info("Server supports RFB protocol version " + majorVersion + "." + minorVersion);
+
+        verStr.clear();
+        verStr.put(String.format("RFB %03d.%03d\n", majorVersion, minorVersion).getBytes()).flip();
+
+        s_logger.info("Using RFB protocol version " + majorVersion + "." + minorVersion);
+        setWaitForNoVnc(true);
+        return verStr;
+    }
+
+    /**
+     * Once the protocol version has been decided, the server and client must agree on the type
+     * of security to be used on the connection.
+     *
+     * Reference: https://github.com/rfbproto/rfbproto/blob/master/rfbproto.rst#712security
+     */
+    public int handshakeSecurityType() {
+        while (isWaitForNoVnc()) {
+            s_logger.trace("Waiting for noVNC msg received");
+        }
+        s_logger.debug("Processing security types message");
+
+        int secType = RfbConstants.CONNECTION_FAILED;
+
+        List<Integer> secTypes = Arrays.asList(1, 2, 19, 261);
+
+        while (!socketConnection.checkIfBytesAreAvailableForReading(1)) {
+            s_logger.trace("Waiting for inStream to be ready");
+        }
+        int nServerSecTypes = socketConnection.readUnsignedInteger(8);
+        if (nServerSecTypes == 0) {
+            throw new CloudRuntimeException("No security types provided by the server");
+        }
+
+        Iterator<Integer> j;
+        for (int i = 0; i < nServerSecTypes; i++) {
+            int serverSecType = socketConnection.readUnsignedInteger(8);
+            s_logger.debug("Server offers security type " + serverSecType);
+
+            /*
+             * Use the first type sent by server which matches client's type.
+             * It means server's order specifies priority.
+             */
+            if (secType == RfbConstants.CONNECTION_FAILED) {
+                for (j = secTypes.iterator(); j.hasNext(); ) {
+                    int refType = (Integer) j.next();
+                    if (refType == serverSecType) {
+                        secType = refType;
+                        break;
+                    }
+                }
+            }
+        }
+        this.flushAfterReceivingNoVNCData = true;
+        setWaitForNoVnc(true);
+        return secType;
+    }
+
+    private final Object lock = new Object();
+    public void setWaitForNoVnc(boolean val) {
+        synchronized (lock) {
+            this.waitForNoVnc = val;
+        }
+    }
+
+    public boolean isWaitForNoVnc() {
+        synchronized (lock) {
+            return this.waitForNoVnc;
+        }
+    }
+
+    private boolean waitForNoVnc = false;
+
+    public void processSecurityResultMsg(int secType) {
+        s_logger.info("Processing security result message");
+        int result;
+
+        if (secType == RfbConstants.NO_AUTH) {
+            result = RfbConstants.VNC_AUTH_OK;
         } else {
-            os.write(b);
+            while (!socketConnection.checkIfBytesAreAvailableForReading(1)) {
+                s_logger.trace("Waiting for inStream");
+            }
+            result = socketConnection.readUnsignedInteger(32);
+        }
+
+        switch (result) {
+            case RfbConstants.VNC_AUTH_OK:
+                s_logger.info("Security completed");

Review Comment:
   these message are a bit simple.
   maybe can reuse the message in unused method `processSecurityResult`
   
   ```
               case RfbConstants.VNC_AUTH_TOO_MANY:
                   s_logger.error("Connection to VNC server failed: too many wrong attempts.");
                   throw new RuntimeException("Connection to VNC server failed: too many wrong attempts.");
   
               case RfbConstants.VNC_AUTH_FAILED:
                   s_logger.error("Connection to VNC server failed: wrong password.");
                   throw new RuntimeException("Connection to VNC server failed: wrong password.");
   
               default:
                   s_logger.error("Connection to VNC server failed, reason code: " + authResult);
                   throw new RuntimeException("Connection to VNC server failed, reason code: " + authResult);
           }
   ```



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