From 53c9bb1012bbd7e1372e623ecbfb62b905e78b43 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Tue, 25 May 2021 15:08:03 +1000 Subject: [PATCH] add some tests for 421 handling at destination --- ts/session/snode_api/onions.ts | 60 ++-- .../session/unit/onion/OnionErrors_test.ts | 309 ++++++++++-------- 2 files changed, 212 insertions(+), 157 deletions(-) diff --git a/ts/session/snode_api/onions.ts b/ts/session/snode_api/onions.ts index b0dcd93b5..cbc3773da 100644 --- a/ts/session/snode_api/onions.ts +++ b/ts/session/snode_api/onions.ts @@ -202,11 +202,7 @@ async function process421Error( lsrpcEd25519Key?: string ) { if (statusCode === 421) { - if (!lsrpcEd25519Key || !associatedWith) { - throw new Error('status 421 without a final destination or no associatedWith makes no sense'); - } - window?.log?.info(`Invalidating swarm for ${associatedWith}`); - await handle421InvalidSwarm(lsrpcEd25519Key, body, associatedWith); + await handle421InvalidSwarm({ snodeEd25519: lsrpcEd25519Key, body, associatedWith }); } } @@ -291,12 +287,8 @@ async function processAnyOtherErrorAtDestination( destinationEd25519: string, associatedWith?: string ) { - // this test checks for on error in your path. + // this test checks for error at the destination. if ( - // response.status === 502 || - // response.status === 503 || - // response.status === 504 || - // response.status === 404 || status !== 400 && status !== 406 && // handled in process406Error status !== 421 // handled in process421Error @@ -356,7 +348,14 @@ const debug = false; * Only exported for testing purpose */ export async function decodeOnionResult(symmetricKey: ArrayBuffer, ciphertext: string) { - const ciphertextBuffer = fromBase64ToArrayBuffer(ciphertext); + let parsedCiphertext = ciphertext; + try { + const jsonRes = JSON.parse(ciphertext); + parsedCiphertext = jsonRes.result; + } catch (e) { + // just try to get a json object from what is inside (for PN requests), if it fails, continue () + } + const ciphertextBuffer = fromBase64ToArrayBuffer(parsedCiphertext); const plaintextBuffer = await window.libloki.crypto.DecryptAESGCM(symmetricKey, ciphertextBuffer); @@ -409,18 +408,12 @@ export async function processOnionResponse({ let plaintext; let ciphertextBuffer; - try { - const jsonRes = JSON.parse(ciphertext); - ciphertext = jsonRes.result; - } catch (e) { - // just try to get a json object from what is inside (for PN requests), if it fails, continue () - } try { const decoded = await exports.decodeOnionResult(symmetricKey, ciphertext); + plaintext = decoded.plaintext; ciphertextBuffer = decoded.ciphertextBuffer; } catch (e) { - console.warn(e); window?.log?.error('[path] lokiRpc::processingOnionResponse - decode error', e); window?.log?.error( '[path] lokiRpc::processingOnionResponse - symmetricKey', @@ -450,7 +443,7 @@ export async function processOnionResponse({ const status = jsonRes.status_code || jsonRes.status; await processOnionRequestErrorAtDestination({ statusCode: status, - body: jsonRes?.body, // this is really important. + body: jsonRes?.body, // this is really important. the `.body`. the .body should be a string. for isntance for nodeNotFound but is most likely a dict (Record)) destinationEd25519: lsrpcEd25519Key, associatedWith, }); @@ -494,26 +487,35 @@ function isSnodeResponse(arg: any): arg is SnodeResponse { * @param body the new swarm not parsed. If an error happens while parsing this we will drop the snode. * @param associatedWith the specific publickey associated with this call */ -async function handle421InvalidSwarm(snodeEd25519: string, body: string, associatedWith?: string) { - // The snode isn't associated with the given public key anymore - // this does not make much sense to have a 421 without a publicKey set. - if (!associatedWith) { - window?.log?.warn('Got a 421 without an associatedWith publickey'); - return; +async function handle421InvalidSwarm({ + body, + snodeEd25519, + associatedWith, +}: { + body: string; + snodeEd25519?: string; + associatedWith?: string; +}) { + if (!snodeEd25519 || !associatedWith) { + // The snode isn't associated with the given public key anymore + // this does not make much sense to have a 421 without a publicKey set. + throw new Error('status 421 without a final destination or no associatedWith makes no sense'); } + window?.log?.info(`Invalidating swarm for ${associatedWith}`); + const exceptionMessage = '421 handled. Retry this request with a new targetNode'; try { - const json = JSON.parse(body); + const parsedBody = JSON.parse(body); // The snode isn't associated with the given public key anymore - if (json.snodes?.length) { + if (parsedBody?.snodes?.length) { // the snode gave us the new swarm. Save it for the next retry window?.log?.warn( 'Wrong swarm, now looking at snodes', - json.snodes.map((s: any) => s.pubkey_ed25519) + parsedBody.snodes.map((s: any) => s.pubkey_ed25519) ); - await updateSwarmFor(associatedWith, json.snodes); + await updateSwarmFor(associatedWith, parsedBody.snodes); throw new pRetry.AbortError(exceptionMessage); } // remove this node from the swarm of this pubkey diff --git a/ts/test/session/unit/onion/OnionErrors_test.ts b/ts/test/session/unit/onion/OnionErrors_test.ts index 409c2b2e3..0c6ded493 100644 --- a/ts/test/session/unit/onion/OnionErrors_test.ts +++ b/ts/test/session/unit/onion/OnionErrors_test.ts @@ -14,24 +14,35 @@ import { processOnionResponse } from '../../../../session/snode_api/onions'; import AbortController from 'abort-controller'; import * as Data from '../../../../../ts/data/data'; import { Snode } from '../../../../session/snode_api/snodePool'; -import { fromArrayBufferToBase64 } from '../../../../session/utils/String'; chai.use(chaiAsPromised as any); chai.should(); const { expect } = chai; -const getFakeResponse = (statusCode?: number, body?: string) => { +const getFakeResponseOnPath = (statusCode?: number, body?: string) => { return { status: statusCode || 0, text: async () => body || '', }; }; +const getFakeResponseOnDestination = (statusCode?: number, body?: string) => { + return { + status: 200 || 0, + text: async () => { + return JSON.stringify({ status: statusCode, body: body || '' }); + }, + }; +}; + // tslint:disable-next-line: max-func-body-length describe('OnionPathsErrors', () => { // Initialize new stubbed cache const sandbox = sinon.createSandbox(); + let updateSwarmSpy: sinon.SinonStub; + let dropSnodeFromSwarmSpy: sinon.SinonSpy; + let dropSnodeFromSnodePool: sinon.SinonSpy; // tslint:disable-next-line: one-variable-per-declaration let guardPubkeys: Array, otherNodesPubkeys: Array, @@ -43,7 +54,7 @@ describe('OnionPathsErrors', () => { otherNodesArray: Array, fakeSnodePool: Array, associatedWith: string, - fakeSwarmForAssocatedWith: Array; + fakeSwarmForAssociatedWith: Array; const fakeIP = '8.8.8.8'; let fakePortCurrent = 20000; @@ -81,15 +92,31 @@ describe('OnionPathsErrors', () => { fakeSnodePool = [...guardNodesArray, ...otherNodesArray]; associatedWith = TestUtils.generateFakePubKey().key; - fakeSwarmForAssocatedWith = otherNodesPubkeys.slice(0, 6); + fakeSwarmForAssociatedWith = otherNodesPubkeys.slice(0, 6); // Utils Stubs sandbox.stub(OnionPaths, 'selectGuardNodes').resolves(guardNodesArray); sandbox.stub(SNodeAPI.SNodeAPI, 'getSnodePoolFromSnode').resolves(guardNodesArray); TestUtils.stubData('getGuardNodes').resolves([guard1ed, guard2ed, guard3ed]); TestUtils.stubWindow('getSeedNodeList', () => ['seednode1']); sandbox.stub(SNodeAPI.SnodePool, 'refreshRandomPoolDetail').resolves(fakeSnodePool); + sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssociatedWith); + + updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); + dropSnodeFromSnodePool = sandbox.spy(SNodeAPI.SnodePool, 'dropSnodeFromSnodePool'); + dropSnodeFromSwarmSpy = sandbox.spy(SNodeAPI.SnodePool, 'dropSnodeFromSwarmIfNeeded'); OnionPaths.clearTestOnionPath(); + + TestUtils.stubWindow('libloki', { + crypto: { + DecryptAESGCM: async (s: any, e: string) => e, + } as any, + }); + sandbox + .stub(SNodeAPI.Onions, 'decodeOnionResult') + .callsFake((_symkey: ArrayBuffer, plaintext: string) => + Promise.resolve({ plaintext, ciphertextBuffer: new Uint8Array() }) + ); }); afterEach(() => { @@ -103,7 +130,7 @@ describe('OnionPathsErrors', () => { abortController.abort(); try { await processOnionResponse({ - response: getFakeResponse(), + response: getFakeResponseOnPath(), symmetricKey: new Uint8Array(), guardNode: guardSnode1, abortSignal: abortController.signal, @@ -116,168 +143,194 @@ describe('OnionPathsErrors', () => { } }); - it('throws an non retryable error we get a 406 status code', async () => { + it('does not throw if we get 200 on path and destination', async () => { try { await processOnionResponse({ - response: getFakeResponse(406), + response: getFakeResponseOnDestination(200), symmetricKey: new Uint8Array(), guardNode: guardSnode1, }); - throw new Error('Error expected'); + throw new Error('Did not throw'); } catch (e) { - expect(e.message).to.equal('You clock is out of sync with the network. Check your clock.'); - // this makes sure that this call would not be retried - expect(e.name).to.equal('AbortError'); + expect(e.message).to.equal('Did not throw'); } }); - describe('processOnionResponse - 421', () => { - it('throws a retryable error if we get a 421 status code without a new swarm', async () => { - sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssocatedWith); - const updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); - const targetNode = otherNodesPubkeys[0]; + it('does not throw if we get 200 on path but no status code on destination', async () => { + try { + await processOnionResponse({ + response: getFakeResponseOnDestination(), + symmetricKey: new Uint8Array(), + guardNode: guardSnode1, + }); + throw new Error('Did not throw'); + } catch (e) { + expect(e.message).to.equal('Did not throw'); + } + }); + describe('processOnionResponse - 406', () => { + it('throws an non retryable error we get a 406 on path', async () => { try { await processOnionResponse({ - response: getFakeResponse(421), + response: getFakeResponseOnPath(406), symmetricKey: new Uint8Array(), guardNode: guardSnode1, - lsrpcEd25519Key: targetNode, - - associatedWith, }); throw new Error('Error expected'); } catch (e) { - expect(e.message).to.equal('Bad Path handled. Retry this request. Status: 421'); + expect(e.message).to.equal( + 'You clock is out of sync with the network. Check your clock.' + ); + // this makes sure that this call would not be retried + expect(e.name).to.equal('AbortError'); } - expect(updateSwarmSpy.callCount).to.eq(1); - // if we don't get a new swarm in the returned json, we drop the target node considering it is a bad snode - expect(updateSwarmSpy.args[0][1]).to.deep.eq( - fakeSwarmForAssocatedWith.filter(m => m !== targetNode) - ); }); - - it('throws a retryable error we get a 421 status code with a new swarm', async () => { - sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssocatedWith); - const updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); - const targetNode = otherNodesPubkeys[0]; - - const resultExpected: Array = [ - otherNodesArray[4], - otherNodesArray[5], - otherNodesArray[6], - ]; + it('throws an non retryable error we get a 406 on destination', async () => { try { await processOnionResponse({ - response: getFakeResponse(421, JSON.stringify({ snodes: resultExpected })), + response: getFakeResponseOnDestination(406), symmetricKey: new Uint8Array(), guardNode: guardSnode1, - lsrpcEd25519Key: targetNode, - associatedWith, }); throw new Error('Error expected'); } catch (e) { - expect(e.message).to.equal('Bad Path handled. Retry this request. Status: 421'); + expect(e.message).to.equal( + 'You clock is out of sync with the network. Check your clock.' + ); + // this makes sure that this call would not be retried + expect(e.name).to.equal('AbortError'); } - expect(updateSwarmSpy.callCount).to.eq(1); - // we got 3 snode in the results, this is our new swarm for this associated with pubkey - expect(updateSwarmSpy.args[0][1]).to.deep.eq(resultExpected.map(m => m.pubkey_ed25519)); }); + }); - it('throws a retryable error we get a 421 status code with invalid json body', async () => { - sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssocatedWith); - const updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); - const targetNode = otherNodesPubkeys[0]; + describe('processOnionResponse - 421', () => { + describe('processOnionResponse - 421 - on path', () => { + it('throws a retryable error if we get a 421 status code without a new swarm', async () => { + const targetNode = otherNodesPubkeys[0]; - try { - await processOnionResponse({ - response: getFakeResponse(421, 'THIS IS SOME INVALID JSON'), - symmetricKey: new Uint8Array(), - guardNode: guardSnode1, - lsrpcEd25519Key: targetNode, + try { + await processOnionResponse({ + response: getFakeResponseOnPath(421), + symmetricKey: new Uint8Array(), + guardNode: guardSnode1, + lsrpcEd25519Key: targetNode, - associatedWith, - }); - throw new Error('Error expected'); - } catch (e) { - expect(e.message).to.equal('Bad Path handled. Retry this request. Status: 421'); - } - expect(updateSwarmSpy.callCount).to.eq(1); - // we got 3 snode in the results, this is our new swarm for this associated with pubkey - expect(updateSwarmSpy.args[0][1]).to.deep.eq( - fakeSwarmForAssocatedWith.filter(m => m !== targetNode) - ); + associatedWith, + }); + throw new Error('Error expected'); + } catch (e) { + expect(e.message).to.equal('421 handled. Retry this request with a new targetNode'); + } + expect(updateSwarmSpy.callCount).to.eq(1); + // if we don't get a new swarm in the returned json, we drop the target node considering it is a bad snode + expect(updateSwarmSpy.args[0][1]).to.deep.eq( + fakeSwarmForAssociatedWith.filter(m => m !== targetNode) + ); + + // now we make sure that this bad snode was dropped from this pubkey's swarm + expect(dropSnodeFromSwarmSpy.callCount).to.eq(1); + expect(dropSnodeFromSwarmSpy.firstCall.args[0]).to.eq(associatedWith); + expect(dropSnodeFromSwarmSpy.firstCall.args[1]).to.eq(targetNode); + + // this node failed only once. it should not be dropped yet from the snodepool + expect(dropSnodeFromSnodePool.callCount).to.eq(0); + }); }); - it('throws a retryable error we get a 421 status code inside the content of the json', async () => { - sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssocatedWith); - const updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); - const targetNode = otherNodesPubkeys[0]; - const json = JSON.stringify({ status: 421 }); + describe('processOnionResponse - 421 - on destination', () => { + it('throws a retryable error we get a 421 status code with a new swarm', async () => { + const targetNode = otherNodesPubkeys[0]; + + const resultExpected: Array = [ + otherNodesArray[4], + otherNodesArray[5], + otherNodesArray[6], + ]; + try { + await processOnionResponse({ + response: getFakeResponseOnDestination( + 421, + JSON.stringify({ snodes: resultExpected }) + ), + symmetricKey: new Uint8Array(), + guardNode: guardSnode1, + lsrpcEd25519Key: targetNode, + associatedWith, + }); + throw new Error('Error expected'); + } catch (e) { + expect(e.message).to.equal('421 handled. Retry this request with a new targetNode'); + } + expect(updateSwarmSpy.callCount).to.eq(1); + // we got 3 snode in the results, this is our new swarm for this associated with pubkey + expect(updateSwarmSpy.args[0][1]).to.deep.eq(resultExpected.map(m => m.pubkey_ed25519)); - TestUtils.stubWindow('libloki', { - crypto: { - DecryptAESGCM: async (s: any, e: string) => e, - } as any, + // we got a new swarm for this pubkey. so it's OK that dropSnodeFromSwarm was not called for this pubkey + + // this node failed only once. it should not be dropped yet from the snodepool + expect(dropSnodeFromSnodePool.callCount).to.eq(0); }); - sandbox - .stub(SNodeAPI.Onions, 'decodeOnionResult') - .resolves({ plaintext: json, ciphertextBuffer: new Uint8Array() }); - try { - await processOnionResponse({ - response: getFakeResponse(200, fromArrayBufferToBase64(Buffer.from(json))), - symmetricKey: new Uint8Array(), - guardNode: guardSnode1, - lsrpcEd25519Key: targetNode, - associatedWith, - }); - throw new Error('Error expected'); - } catch (e) { - expect(e.message).to.equal('Bad Path handled. Retry this request. Status: 421'); - } - expect(updateSwarmSpy.callCount).to.eq(1); - // 421 without swarm included means drop the target node only - expect(updateSwarmSpy.args[0][1]).to.deep.eq( - fakeSwarmForAssocatedWith.filter(m => m !== targetNode) - ); - }); + it('throws a retryable error we get a 421 status code with invalid json body', async () => { + const targetNode = otherNodesPubkeys[0]; + + try { + await processOnionResponse({ + response: getFakeResponseOnDestination(421, 'THIS IS SOME INVALID JSON'), + symmetricKey: new Uint8Array(), + guardNode: guardSnode1, + lsrpcEd25519Key: targetNode, - it('throws a retryable error we get a 421 status code inside the content of the json', async () => { - sandbox.stub(Data, 'getSwarmNodesForPubkey').resolves(fakeSwarmForAssocatedWith); - const updateSwarmSpy = sandbox.stub(Data, 'updateSwarmNodesForPubkey').resolves(); - const targetNode = otherNodesPubkeys[0]; - const resultExpected: Array = [ - otherNodesArray[4], - otherNodesArray[5], - otherNodesArray[6], - ]; - const json = JSON.stringify({ status: 421, snodes: resultExpected }); - - TestUtils.stubWindow('libloki', { - crypto: { - DecryptAESGCM: async (s: any, e: string) => e, - } as any, + associatedWith, + }); + throw new Error('Error expected'); + } catch (e) { + expect(e.message).to.equal('421 handled. Retry this request with a new targetNode'); + } + expect(updateSwarmSpy.callCount).to.eq(1); + // we have an invalid json content. just remove the targetNode from the list + expect(updateSwarmSpy.args[0][1]).to.deep.eq( + fakeSwarmForAssociatedWith.filter(m => m !== targetNode) + ); + // now we make sure that this bad snode was dropped from this pubkey's swarm + expect(dropSnodeFromSwarmSpy.callCount).to.eq(1); + expect(dropSnodeFromSwarmSpy.firstCall.args[0]).to.eq(associatedWith); + expect(dropSnodeFromSwarmSpy.firstCall.args[1]).to.eq(targetNode); + // this node failed only once. it should not be dropped yet from the snodepool + expect(dropSnodeFromSnodePool.callCount).to.eq(0); }); - sandbox - .stub(SNodeAPI.Onions, 'decodeOnionResult') - .resolves({ plaintext: json, ciphertextBuffer: new Uint8Array() }); - try { - await processOnionResponse({ - response: getFakeResponse(200, json), - symmetricKey: new Uint8Array(), - guardNode: guardSnode1, - lsrpcEd25519Key: targetNode, - associatedWith, - }); - throw new Error('Error expected'); - } catch (e) { - expect(e.message).to.equal('Bad Path handled. Retry this request. Status: 421'); - } - expect(updateSwarmSpy.callCount).to.eq(1); - // 421 without swarm included means drop the target node only - expect(updateSwarmSpy.args[0][1]).to.deep.eq(resultExpected.map(m => m.pubkey_ed25519)); + it('throws a retryable on destination 421 without new swarm ', async () => { + const targetNode = otherNodesPubkeys[0]; + const json = JSON.stringify({ status: 421 }); + + try { + await processOnionResponse({ + response: getFakeResponseOnDestination(421, json), + symmetricKey: new Uint8Array(), + guardNode: guardSnode1, + lsrpcEd25519Key: targetNode, + associatedWith, + }); + throw new Error('Error expected'); + } catch (e) { + expect(e.message).to.equal('421 handled. Retry this request with a new targetNode'); + } + expect(updateSwarmSpy.callCount).to.eq(1); + // 421 without swarm included means drop the target node only + expect(updateSwarmSpy.args[0][1]).to.deep.eq( + fakeSwarmForAssociatedWith.filter(m => m !== targetNode) + ); + + // now we make sure that this bad snode was dropped from this pubkey's swarm + expect(dropSnodeFromSwarmSpy.callCount).to.eq(1); + expect(dropSnodeFromSwarmSpy.firstCall.args[0]).to.eq(associatedWith); + expect(dropSnodeFromSwarmSpy.firstCall.args[1]).to.eq(targetNode); + + // this node failed only once. it should not be dropped yet from the snodepool + expect(dropSnodeFromSnodePool.callCount).to.eq(0); + }); }); }); });