From 370f21b11723196b67f0acebe85d45ca7fc7a34e Mon Sep 17 00:00:00 2001 From: Pierre Ossman Date: Thu, 7 Sep 2023 14:59:36 +0200 Subject: [PATCH] Correctly handle legacy security rejections The code comment of this code was entirely incorrect, but the commit message for 5671072 when it was added was correct. I.e. there is a result, but not a reason. Adjust the unit tests to make sure this doesn't regress again. --- core/rfb.js | 7 ------- tests/test.rfb.js | 27 ++++++++++++++++++++------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/core/rfb.js b/core/rfb.js index 477b30f5e..6bce48fd6 100644 --- a/core/rfb.js +++ b/core/rfb.js @@ -1966,13 +1966,6 @@ export default class RFB extends EventTargetMixin { } _handleSecurityResult() { - // There is no security choice, and hence no security result - // until RFB 3.7 - if (this._rfbVersion < 3.7) { - this._rfbInitState = 'ClientInitialisation'; - return true; - } - if (this._sock.rQwait('VNC auth response ', 4)) { return false; } const status = this._sock.rQshift32(); diff --git a/tests/test.rfb.js b/tests/test.rfb.js index d1313713e..fa7f14023 100644 --- a/tests/test.rfb.js +++ b/tests/test.rfb.js @@ -2238,23 +2238,36 @@ describe('Remote Frame Buffer Protocol Client', function () { }); describe('Legacy SecurityResult', function () { - beforeEach(function () { + it('should not include reason in securityfailure event for versions < 3.7', function () { client.addEventListener("credentialsrequired", () => { client.sendCredentials({ password: 'passwd' }); }); - sendVer('003.007\n', client); - client._sock._websocket._getSentData(); - sendSecurity(2, client); + const spy = sinon.spy(); + client.addEventListener("securityfailure", spy); + sendVer('003.006\n', client); + client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2])); const challenge = []; for (let i = 0; i < 16; i++) { challenge[i] = i; } client._sock._websocket._receiveData(new Uint8Array(challenge)); - client._sock._websocket._getSentData(); - clock.tick(); + + client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2])); + expect(spy).to.have.been.calledOnce; + expect(spy.args[0][0].detail.status).to.equal(2); + expect('reason' in spy.args[0][0].detail).to.be.false; }); - it('should not include reason in securityfailure event', function () { + it('should not include reason in securityfailure event for versions < 3.8', function () { + client.addEventListener("credentialsrequired", () => { + client.sendCredentials({ password: 'passwd' }); + }); const spy = sinon.spy(); client.addEventListener("securityfailure", spy); + sendVer('003.007\n', client); + sendSecurity(2, client); + const challenge = []; + for (let i = 0; i < 16; i++) { challenge[i] = i; } + client._sock._websocket._receiveData(new Uint8Array(challenge)); + client._sock._websocket._receiveData(new Uint8Array([0, 0, 0, 2])); expect(spy).to.have.been.calledOnce; expect(spy.args[0][0].detail.status).to.equal(2);