diff --git a/lib/connect.js b/lib/connect.js index d347a26b..8a3f0db8 100644 --- a/lib/connect.js +++ b/lib/connect.js @@ -11,7 +11,7 @@ const Holepuncher = require('./holepuncher') const Sleeper = require('./sleeper') const { clearRelayTimeout, - closeRelayConnection, + confirmDirectUpgrade, destroyRelayConnection } = require('./relay-connection') const { FIREWALL, ERROR } = require('./constants') @@ -102,6 +102,7 @@ module.exports = function connect(dht, publicKey, opts = {}) { relaySocket: null, relayClient: null, relayPaired: false, + validUpgrade: true, relayKeepAlive: opts.relayKeepAlive || 5000 } @@ -135,9 +136,11 @@ module.exports = function connect(dht, publicKey, opts = {}) { // Check if the traffic originated from the socket on which we're expecting relay traffic. If so, // we haven't hole punched yet and the other side is just sending us traffic through the relay. if (c.relaySocket && isRelay(c.relaySocket, socket, port, host)) { + c.validUpgrade = false return false } + c.validUpgrade = true if (c.onsocket) { c.onsocket(socket, port, host) } else { @@ -496,12 +499,9 @@ async function connectThroughNode(c, address, socket) { if (c.rawStream === null) return // Already hole punched if (c.rawStream.connected) { - // Once this fires, changeRemote has moved the raw stream onto the direct path. - c.rawStream.once('remote-changed', () => closeRelayConnection(c)) - - const remoteChanging = c.rawStream.changeRemote(socket, c.connect.payload.udx.id, port, host) - - if (remoteChanging) remoteChanging.catch(safetyCatch) + const rawStream = c.rawStream + const remoteChanging = rawStream.changeRemote(socket, c.connect.payload.udx.id, port, host) + confirmDirectUpgrade(c, rawStream, remoteChanging) } else { // cache the relay addrs for a future reconnect, we prefer the remote one so they // can give us the correct ones from their pov diff --git a/lib/relay-connection.js b/lib/relay-connection.js index 58b81301..6758067b 100644 --- a/lib/relay-connection.js +++ b/lib/relay-connection.js @@ -1,5 +1,9 @@ +const b4a = require('b4a') +const safetyCatch = require('safety-catch') + exports.clearRelayTimeout = clearRelayTimeout exports.closeRelayConnection = closeRelayConnection +exports.confirmDirectUpgrade = confirmDirectUpgrade exports.destroyRelayConnection = destroyRelayConnection function clearRelayTimeout(connectionOrHandshake) { @@ -13,6 +17,42 @@ function closeRelayConnection(connectionOrHandshake) { if (socket) socket.end() } +function confirmDirectUpgrade(connectionOrHandshake, rawStream, remoteChanging) { + const cleanup = () => { + rawStream.off('data', ondirect) + rawStream.off('message', ondirect) + rawStream.off('close', cleanup) + } + + function ondirect() { + if (!connectionOrHandshake.validUpgrade) { + // reset, aka assume from direct + connectionOrHandshake.validUpgrade = true + return + } + + cleanup() + closeRelayConnection(connectionOrHandshake) + } + + const confirm = () => { + rawStream.on('data', ondirect) + rawStream.on('message', ondirect) + rawStream.once('close', cleanup) + + // Use a raw UDX message to make the peer observe the direct path without + // writing application data into the secret stream. + if (rawStream.trySend) rawStream.trySend(b4a.alloc(0)) + } + + if (!remoteChanging) { + confirm() + return + } + + remoteChanging.then(confirm).catch(safetyCatch) +} + function destroyRelayConnection(connectionOrHandshake) { const socket = resetRelayConnection(connectionOrHandshake) diff --git a/lib/server.js b/lib/server.js index e4bfa37a..de945bd0 100644 --- a/lib/server.js +++ b/lib/server.js @@ -11,7 +11,7 @@ const SecurePayload = require('./secure-payload') const Holepuncher = require('./holepuncher') const { clearRelayTimeout, - closeRelayConnection, + confirmDirectUpgrade, destroyRelayConnection } = require('./relay-connection') const { ALREADY_LISTENING, NODE_DESTROYED, KEYPAIR_ALREADY_USED } = require('./errors') @@ -235,7 +235,8 @@ module.exports = class Server extends EventEmitter { relayToken: null, relaySocket: null, relayClient: null, - relayPaired: false + relayPaired: false, + validUpgrade: true } this._holepunches[id] = hs @@ -291,10 +292,12 @@ module.exports = class Server extends EventEmitter { // Check if the traffic originated from the socket on which we're expecting relay traffic. If so, // we haven't hole punched yet and the other side is just sending us traffic through the relay. if (hs.relaySocket && isRelay(hs.relaySocket, socket, port, host)) { + hs.validUpgrade = false return false } hs.onsocket(socket, port, host) + hs.validUpgrade = true return false } }) @@ -328,12 +331,9 @@ module.exports = class Server extends EventEmitter { hs.rawStream.removeListener('close', onrawstreamclose) if (hs.rawStream.connected) { - // Once this fires, changeRemote has moved the raw stream onto the direct path. - hs.rawStream.once('remote-changed', () => closeRelayConnection(hs)) - - const remoteChanging = hs.rawStream.changeRemote(socket, remotePayload.udx.id, port, host) - - if (remoteChanging) remoteChanging.catch(safetyCatch) + const rawStream = hs.rawStream + const remoteChanging = rawStream.changeRemote(socket, remotePayload.udx.id, port, host) + confirmDirectUpgrade(hs, rawStream, remoteChanging) } else { hs.rawStream.connect(socket, remotePayload.udx.id, port, host) hs.encryptedSocket = this.createSecretStream(false, hs.rawStream, { @@ -664,7 +664,10 @@ module.exports = class Server extends EventEmitter { .on('close', () => destroyRelayConnection(hs)) .connect(socket, remoteId, remotePort, remoteHost) - hs.encryptedSocket = this.createSecretStream(false, hs.rawStream, { handshake: h }) + hs.encryptedSocket = this.createSecretStream(false, hs.rawStream, { + handshake: h, + keepAlive: this.dht.connectionKeepAlive + }) this.onconnection(hs.encryptedSocket) }) diff --git a/test/relaying.js b/test/relaying.js index f977347f..377c7abb 100644 --- a/test/relaying.js +++ b/test/relaying.js @@ -1,6 +1,7 @@ const test = require('brittle') const { once } = require('events') const RelayServer = require('blind-relay').Server +const NoiseSecretStream = require('@hyperswarm/secret-stream') const Holepuncher = require('../lib/holepuncher') const { swarm, createDHT, endAndCloseSocket } = require('./helpers') @@ -447,11 +448,18 @@ test('relay connections through node, client and server side', async function (t const { bootstrap } = await swarm(t) const a = createDHT({ bootstrap, quickFirewall: false, ephemeral: true }) - const b = createDHT({ bootstrap, quickFirewall: false, ephemeral: true }) + const b = createDHT({ + bootstrap, + quickFirewall: false, + ephemeral: true, + connectionKeepAlive: 12345 + }) const c = createDHT({ bootstrap, quickFirewall: false, ephemeral: true }) const lc = t.test('socket lifecycle') lc.plan(5) + const relayKeepAlive = t.test('relay keepalive') + relayKeepAlive.plan(1) const testRelay = t.test('relay server') testRelay.plan(2) // One each for the initiator and the follower const testRelayInitiator = t.test('relay initiator') @@ -486,7 +494,18 @@ test('relay connections through node, client and server side', async function (t { holepunch: false, // To ensure it relies only on relaying shareLocalAddress: false, // To help ensure it relies only on relaying (otherwise it can connect directly over LAN, without even trying to holepunch) - relayThrough: aServer.publicKey + relayThrough: aServer.publicKey, + createSecretStream(isInitiator, rawStream, opts) { + if (!isInitiator) { + relayKeepAlive.is( + opts.keepAlive, + 12345, + 'server relayed stream inherits connectionKeepAlive' + ) + } + + return new NoiseSecretStream(isInitiator, rawStream, opts) + } }, function (socket) { lc.pass('server socket opened') @@ -515,6 +534,7 @@ test('relay connections through node, client and server side', async function (t .end('hello world') await lc + await relayKeepAlive await c.destroy() await b.destroy() @@ -522,115 +542,149 @@ test('relay connections through node, client and server side', async function (t }) test('relay connection upgrades to direct connection', async function (t) { - const { bootstrap } = await swarm(t) - - const relayNode = createDHT({ bootstrap }) - const serverNode = createDHT({ bootstrap, quickFirewall: false, ephemeral: true }) - const clientNode = createDHT({ bootstrap, quickFirewall: false, ephemeral: true }) - - const resumePunching = pausePunching(t, [serverNode, clientNode]) - - const relayServer = new RelayServer({ - createStream(opts) { - return relayNode.createRawStream({ ...opts, framed: true }) - } - }) - - t.teardown(() => relayServer.close()) - - const relaySockets = [] - let resolveRelaySockets = null - const relaySocketsOpened = new Promise((resolve) => { - resolveRelaySockets = resolve - }) - - const relayTransportServer = relayNode.createServer(function (socket) { - relaySockets.push(socket) - // Wait until both client and server have opened their relay transport sockets. - if (relaySockets.length === 2) resolveRelaySockets(relaySockets) - - const session = relayServer.accept(socket, { id: socket.remotePublicKey }) - session.on('error', (err) => t.comment(err.message)) - }) + for (const opts of [ + { name: 'default keepalive' }, + { name: 'without keepalive', connectionKeepAlive: false, confirmWithAppData: true }, + { name: 'idle without keepalive', connectionKeepAlive: false } + ]) { + t.comment(opts.name) + const { bootstrap } = await swarm(t) + + const relayNode = createDHT({ bootstrap }) + const serverNode = createDHT({ + bootstrap, + quickFirewall: false, + ephemeral: true, + connectionKeepAlive: opts.connectionKeepAlive + }) + const clientNode = createDHT({ + bootstrap, + quickFirewall: false, + ephemeral: true, + connectionKeepAlive: opts.connectionKeepAlive + }) - await relayTransportServer.listen() + const resumePunching = pausePunching(t, [serverNode, clientNode]) - let resolveServerSocket = null - const serverSocketOpened = new Promise((resolve) => { - resolveServerSocket = resolve - }) + const relayServer = new RelayServer({ + createStream(opts) { + return relayNode.createRawStream({ ...opts, framed: true }) + } + }) - const appServer = serverNode.createServer( - { - relayThrough: relayTransportServer.publicKey, - shareLocalAddress: false - }, - function (socket) { - resolveServerSocket(socket) + t.teardown(() => relayServer.close()) - socket.on('data', (data) => socket.write(data)) - socket.on('end', () => socket.end()) - } - ) + const relaySockets = [] + let resolveRelaySockets = null + const relaySocketsOpened = new Promise((resolve) => { + resolveRelaySockets = resolve + }) - await appServer.listen() + const relayTransportServer = relayNode.createServer(function (socket) { + relaySockets.push(socket) + // Wait until both client and server have opened their relay transport sockets. + if (relaySockets.length === 2) resolveRelaySockets(relaySockets) - const clientSocket = clientNode.connect(appServer.publicKey, { - fastOpen: false, - localConnection: false - }) + const session = relayServer.accept(socket, { id: socket.remotePublicKey }) + session.on('error', (err) => t.comment(err.message)) + }) - const [serverSocket] = await Promise.all([serverSocketOpened, once(clientSocket, 'open')]) - await relaySocketsOpened + await relayTransportServer.listen() - t.is(relaySockets.length, 2, 'both peers opened relay transport sockets') + let resolveServerSocket = null + const serverSocketOpened = new Promise((resolve) => { + resolveServerSocket = resolve + }) - t.not( - serverSocket.rawStream.remotePort, - clientSocket.rawStream.localPort, - 'server starts on the relayed stream' - ) - t.not( - clientSocket.rawStream.remotePort, - serverSocket.rawStream.localPort, - 'client starts on the relayed stream' - ) + const appServer = serverNode.createServer( + { + relayThrough: relayTransportServer.publicKey, + shareLocalAddress: false + }, + function (socket) { + resolveServerSocket(socket) - // The relayed connection should already be usable before the direct path wins. - const beforeUpgrade = once(clientSocket, 'data') - clientSocket.write(Buffer.from('before upgrade')) - t.alike((await beforeUpgrade)[0], Buffer.from('before upgrade'), 'relay path carries data') + socket.on('data', (data) => socket.write(data)) + socket.on('end', () => socket.end()) + } + ) - const clientUpgraded = once(clientSocket.rawStream, 'remote-changed') - const serverUpgraded = once(serverSocket.rawStream, 'remote-changed') - const relaySocketsClosed = relaySockets.map((socket) => once(socket, 'close')) + await appServer.listen() - resumePunching() - await Promise.all([clientUpgraded, serverUpgraded]) - await Promise.all(relaySocketsClosed) - t.pass('relay transport sockets close after direct upgrade') + const clientSocket = clientNode.connect(appServer.publicKey, { + fastOpen: false, + localConnection: false + }) - t.is( - serverSocket.rawStream.remotePort, - clientSocket.rawStream.localPort, - 'server switches to the client address' - ) - t.is( - clientSocket.rawStream.remotePort, - serverSocket.rawStream.localPort, - 'client switches to the server address' - ) + const [serverSocket] = await Promise.all([serverSocketOpened, once(clientSocket, 'open')]) + await relaySocketsOpened + + t.is(relaySockets.length, 2, 'both peers opened relay transport sockets') + + t.not( + serverSocket.rawStream.remotePort, + clientSocket.rawStream.localPort, + 'server starts on the relayed stream' + ) + t.not( + clientSocket.rawStream.remotePort, + serverSocket.rawStream.localPort, + 'client starts on the relayed stream' + ) + + // The relayed connection should already be usable before the direct path wins. + const beforeUpgrade = once(clientSocket, 'data') + clientSocket.write(Buffer.from('before upgrade')) + t.alike((await beforeUpgrade)[0], Buffer.from('before upgrade'), 'relay path carries data') + + const clientUpgraded = once(clientSocket.rawStream, 'remote-changed') + const serverUpgraded = once(serverSocket.rawStream, 'remote-changed') + const relaySocketsClosed = relaySockets.map((socket) => once(socket, 'close')) + + resumePunching() + + if (opts.confirmWithAppData) { + await clientUpgraded + + t.ok( + relaySockets.every((socket) => !socket.destroyed), + 'relay stays open until direct traffic confirms the upgrade' + ) + + // Without keepalive, the passive upgrade is confirmed by the next app write. + const appData = once(clientSocket, 'data') + clientSocket.write(Buffer.from('after upgrade')) + t.alike((await appData)[0], Buffer.from('after upgrade'), 'app data confirms direct path') + } - const afterUpgrade = once(clientSocket, 'data') - clientSocket.write(Buffer.from('after upgrade')) - t.alike((await afterUpgrade)[0], Buffer.from('after upgrade'), 'direct path carries data') + await Promise.all([clientUpgraded, serverUpgraded]) + await Promise.all(relaySocketsClosed) + t.pass('relay transport sockets close after direct upgrade') + + t.is( + serverSocket.rawStream.remotePort, + clientSocket.rawStream.localPort, + 'server switches to the client address' + ) + t.is( + clientSocket.rawStream.remotePort, + serverSocket.rawStream.localPort, + 'client switches to the server address' + ) + + if (!opts.confirmWithAppData) { + const afterUpgrade = once(clientSocket, 'data') + clientSocket.write(Buffer.from('after upgrade')) + t.alike((await afterUpgrade)[0], Buffer.from('after upgrade'), 'direct path carries data') + } - await endAndCloseSocket(clientSocket) - if (!serverSocket.destroyed) await once(serverSocket, 'close') + await endAndCloseSocket(clientSocket) + if (!serverSocket.destroyed) await once(serverSocket, 'close') - await relayNode.destroy() - await serverNode.destroy() - await clientNode.destroy() + await relayNode.destroy() + await serverNode.destroy() + await clientNode.destroy() + } }) test.skip('relay several connections through node with pool', async function (t) {