From 374b8736ba1bd048d48b27d21da122e969a8c9f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miha=20Ple=C5=A1ko?= Date: Mon, 26 Mar 2018 14:05:37 +0200 Subject: [PATCH] Add support for non-binary WebMKS websocket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now, 'binary' protocol for websockets was assumed. But VMware vCloud console access only works via legacy 'uint8utf8' protocol. To make it even more interesting, vCloud's JavaScript SDK actually thinks it supports 'binary', but in fact it doesn't. Well 'binary' protocol works when connecting to vCenter (i.e. `Vmware::InfraManager`), but not for vCloud (`Vmware::CloudManager`). With this commit we implement websocket proxy client for 'uint8utf8' protocol and remove 'binary' from list of offered protocols when 'uint8utf8' is used to make sure JavaScript SDK doesn't overestimate itself (and then fail). Signed-off-by: Miha Pleško --- lib/websocket_proxy_uint8utf8.rb | 42 ++++++++++++ lib/websocket_server.rb | 3 +- lib/websocket_webmks_uint8utf8.rb | 31 +++++++++ spec/lib/websocket_proxy_spec.rb | 108 ++++++++++++++++++++++++++++-- 4 files changed, 179 insertions(+), 5 deletions(-) create mode 100644 lib/websocket_proxy_uint8utf8.rb create mode 100644 lib/websocket_webmks_uint8utf8.rb diff --git a/lib/websocket_proxy_uint8utf8.rb b/lib/websocket_proxy_uint8utf8.rb new file mode 100644 index 000000000000..fbc79335446e --- /dev/null +++ b/lib/websocket_proxy_uint8utf8.rb @@ -0,0 +1,42 @@ +class WebsocketProxyUint8utf8 < WebsocketProxy + def initialize(env, console, logger) + @env = env + @id = SecureRandom.uuid + @console = console + @logger = logger + + secure = Rack::Request.new(env).ssl? + scheme = secure ? 'wss:' : 'ws:' + @url = scheme + '//' + env['HTTP_HOST'] + env['REQUEST_URI'] + @driver = WebSocket::Driver.rack(self, :protocols => %w(uint8utf8)) + + begin + # Hijack the socket from the Rack middleware + @ws = env['rack.hijack'].call + # Set up the socket client for the proxy + @sock = TCPSocket.open(@console.host_name, @console.port) + @right = WebsocketWebmksUint8utf8.new(@sock, @console) + rescue => ex + @logger.error(ex) + @error = true + end + + @driver.on(:open) { @console.update(:opened => true) } + @driver.on(:message) { |msg| @right.issue(msg.data) } + @driver.on(:close) { cleanup } + end + + def transmit(sockets, is_ws) + # Do not read when the other end is not ready for writing + return unless is_ws ? sockets.include?(@ws) : sockets.include?(@sock) + + if is_ws + data = @ws.recv_nonblock(64.kilobytes) + @driver.parse(data) + else + @right.fetch(64.kilobytes) do |data| + @driver.frame(data) + end + end + end +end diff --git a/lib/websocket_server.rb b/lib/websocket_server.rb index 094f15917a5e..32d41e80b32f 100644 --- a/lib/websocket_server.rb +++ b/lib/websocket_server.rb @@ -62,7 +62,8 @@ def healthy? def init_proxy(env, url) console = SystemConsole.find_by!(:url_secret => url) - proxy = WebsocketProxy.new(env, console, logger) + proxy_klass = (console && console.protocol.to_s.end_with?('uint8utf8')) ? WebsocketProxyUint8utf8 : WebsocketProxy + proxy = proxy_klass.new(env, console, logger) return proxy.cleanup if proxy.error logger.info("Starting websocket proxy for VM #{console.vm_id}") proxy.start diff --git a/lib/websocket_webmks_uint8utf8.rb b/lib/websocket_webmks_uint8utf8.rb new file mode 100644 index 000000000000..9a9639ddd251 --- /dev/null +++ b/lib/websocket_webmks_uint8utf8.rb @@ -0,0 +1,31 @@ +class WebsocketWebmksUint8utf8 < WebsocketSSLSocket + attr_accessor :url + + def initialize(socket, model) + super(socket, model) + @url = URI::Generic.build(:scheme => 'wss', + :host => @model.host_name, + :port => @model.port, + :path => @model.url).to_s + + @driver = WebSocket::Driver.client(self, :protocols => ['uint8utf8']) + @driver.on(:close) { socket.close unless socket.closed? } + @driver.start + end + + def fetch(length) + # WebSocket::Driver requires an event handler that should be registered only once + @driver.on(:message) { |msg| yield(msg.data) } if @driver.listeners(:message).length.zero? + + data = @ssl.sysread(length) + @driver.parse(data) + end + + def issue(data) + @driver.frame(data) + end + + def write(data) + @ssl.syswrite(data) + end +end diff --git a/spec/lib/websocket_proxy_spec.rb b/spec/lib/websocket_proxy_spec.rb index a63098b07ce7..1c6112bf87e8 100644 --- a/spec/lib/websocket_proxy_spec.rb +++ b/spec/lib/websocket_proxy_spec.rb @@ -8,9 +8,97 @@ subject { described_class.new(env, console, logger) } describe '#initialize' do + before do + allow(TCPSocket).to receive(:open) # prevent real sockets from opening + end + it 'sets the URL' do expect(subject.url).to eq("ws://#{host}#{uri}") end + + describe 'based on console type' do + before do + [WebsocketSocket, WebsocketWebmks, WebsocketWebmksUint8utf8].each do |k| + allow(k).to receive(:new).and_return(k.allocate) + end + end + + let(:proxy) { described_class.new(env, FactoryGirl.create(:system_console, :protocol => console_type), logger) } + let(:proto) { proxy.instance_variable_get(:@driver).instance_variable_get(:@options)[:protocols] } + let(:adapter) { proxy.instance_variable_get(:@right) } + let(:on_message) { proxy.instance_variable_get(:@driver).listeners(:message).first } + let(:right) { double('right socket') } + + context 'when vnc' do + let(:console_type) { 'vnc' } + + it 'uses binary protocol' do + expect(proto).to eq(['binary']) + end + + it 'uses WebsocketSocket adapter' do + expect(adapter).to be_an_instance_of(WebsocketSocket) + end + + it 'decodes message' do + assert_message_transformation('BANANA'.unpack('C*'), 'BANANA') + end + end + + context 'when spice' do + let(:console_type) { 'spice' } + + it 'uses binary protocol' do + expect(proto).to eq(['binary']) + end + + it 'uses WebsocketSocket adapter' do + expect(adapter).to be_an_instance_of(WebsocketSocket) + end + + it 'decodes message' do + assert_message_transformation('BANANA'.unpack('C*'), 'BANANA') + end + end + + context 'when webmks' do + let(:console_type) { 'webmks' } + + it 'uses binary protocol' do + expect(proto).to eq(['binary']) + end + + it 'uses WebsocketWebmks adapter' do + expect(adapter).to be_an_instance_of(WebsocketWebmks) + end + + it 'decodes message' do + assert_message_transformation('BANANA'.unpack('C*'), 'BANANA') + end + end + + context 'when webmks-uint8utf8' do + let(:console_type) { 'webmks-uint8utf8' } + + it 'uses uint8utf8 protocol' do + expect(proto).to eq(['uint8utf8']) + end + + it 'uses WebsocketWebmksUint8utf8 adapter' do + expect(adapter).to be_an_instance_of(WebsocketWebmksUint8utf8) + end + + it 'does not decode message' do + assert_message_transformation('BANANA', 'BANANA') + end + end + end + end + + def assert_message_transformation(input, output) + proxy.instance_variable_set(:@right, right) + expect(right).to receive(:issue).with(output) + on_message.call(double('message', :data => input)) end describe '#cleanup' do @@ -67,11 +155,23 @@ context 'socket to websocket' do let(:is_ws) { false } - it 'reads from the socket and sends the result to the driver' do - expect(sock).to receive(:recv_nonblock).and_return(123) - expect(driver).to receive(:binary).with(123) + context 'binary' do + it 'reads from the socket and sends the result to the driver' do + expect(sock).to receive(:recv_nonblock).and_return(123) + expect(driver).to receive(:binary).with(123) + + subject.transmit([sock], is_ws) + end + end + + context 'non-binary' do + it 'reads from the socket and sends the result to the driver' do + allow(subject).to receive(:binary?).and_return(false) + expect(sock).to receive(:recv_nonblock).and_return(123) + expect(driver).to receive(:frame).with(123) - subject.transmit([sock], is_ws) + subject.transmit([sock], is_ws) + end end end end