Skip to content

Commit

Permalink
Add support for non-binary WebMKS websocket
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
miha-plesko committed Mar 29, 2018
1 parent 1d647ba commit 374b873
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 5 deletions.
42 changes: 42 additions & 0 deletions lib/websocket_proxy_uint8utf8.rb
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion lib/websocket_server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
31 changes: 31 additions & 0 deletions lib/websocket_webmks_uint8utf8.rb
Original file line number Diff line number Diff line change
@@ -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
108 changes: 104 additions & 4 deletions spec/lib/websocket_proxy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 374b873

Please sign in to comment.