From 02402ec70b7a00c81b263fd4d2e924bd4cbd4f7b Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 5 Jul 2018 20:54:35 +0200 Subject: [PATCH] fix: additional checks for CORS XHR when dnslink is enabled Dnslink lookup is disabled by default, but if user opted-in to it, many websites were broken due to invalid gateway redirects. This change fixes regression introduced by: https://github.com/ipfs-shipyard/ipfs-companion/pull/511 and adds tests to guard against the problem in future. --- add-on/src/lib/dns-link.js | 22 +++++++--- add-on/src/lib/ipfs-request.js | 21 ++++++---- test/functional/lib/ipfs-request.test.js | 53 ++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 12 deletions(-) diff --git a/add-on/src/lib/dns-link.js b/add-on/src/lib/dns-link.js index f5b7a959c..0102ae716 100644 --- a/add-on/src/lib/dns-link.js +++ b/add-on/src/lib/dns-link.js @@ -39,21 +39,22 @@ module.exports = function createDnsLink (getState) { let dnslink = cache.get(fqdn) if (typeof dnslink === 'undefined') { try { - console.info('dnslink cache miss for: ' + fqdn) + console.info(`[ipfs-companion] dnslink cache miss for '${fqdn}', running DNS TXT lookup`) dnslink = dnsLink.readDnslinkFromTxtRecord(fqdn) if (dnslink) { cache.set(fqdn, dnslink) - console.info(`Resolved dnslink: '${fqdn}' -> '${dnslink}'`) + console.info(`[ipfs-companion] found dnslink: '${fqdn}' -> '${dnslink}'`) } else { cache.set(fqdn, false) - console.info(`Resolved NO dnslink for '${fqdn}'`) + console.info(`[ipfs-companion] found NO dnslink for '${fqdn}'`) } } catch (error) { - console.error(`Error in dnslinkLookupAndOptionalRedirect for '${fqdn}'`) + console.error(`[ipfs-companion] Error in dnslinkLookupAndOptionalRedirect for '${fqdn}'`) console.error(error) } } else { - console.info(`Resolved via cached dnslink: '${fqdn}' -> '${dnslink}'`) + // Most of the time we will hit cache, which makes below line is too noisy + // console.info(`[ipfs-companion] using cached dnslink: '${fqdn}' -> '${dnslink}'`) } return dnslink }, @@ -84,6 +85,17 @@ module.exports = function createDnsLink (getState) { } }, + validDnslinkRedirect (url) { + if (getState().dnslink && !url.pathname.startsWith('/ipfs/') && !url.pathname.startsWith('/ipns/')) { + const fqdn = url.hostname + const dnslink = dnsLink.cachedDnslinkLookup(fqdn) + if (dnslink) { + return true + } + } + return false + }, + redirectToIpnsPath (originalUrl) { // TODO: redirect to `ipns://` if hasNativeProtocolHandler === true const fqdn = originalUrl.hostname diff --git a/add-on/src/lib/ipfs-request.js b/add-on/src/lib/ipfs-request.js index 8b1312cab..e443aa1e8 100644 --- a/add-on/src/lib/ipfs-request.js +++ b/add-on/src/lib/ipfs-request.js @@ -43,11 +43,14 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) { } // Detect valid /ipfs/ and /ipns/ on any site if (ipfsPathValidator.publicIpfsOrIpnsResource(request.url) && isSafeToRedirect(request, runtime)) { - return redirectToGateway(request.url, state) + return redirectToGateway(request.url, state, dnsLink) } // Look for dnslink in TXT records of visited sites - if (state.dnslink && dnsLink.isDnslookupSafeForURL(request.url) && isSafeToRedirect(request, runtime)) { - return dnsLink.dnslinkLookupAndOptionalRedirect(request.url) + if (state.dnslink && dnsLink.isDnslookupSafeForURL(request.url)) { + const dnslinkRedirect = dnsLink.dnslinkLookupAndOptionalRedirect(request.url) + if (dnslinkRedirect && isSafeToRedirect(request, runtime)) { + return dnslinkRedirect + } } } }, @@ -99,7 +102,7 @@ function createRequestModifier (getState, dnsLink, ipfsPathValidator, runtime) { if (onHeadersReceivedRedirect.has(request.requestId)) { const state = getState() onHeadersReceivedRedirect.delete(request.requestId) - return redirectToGateway(request.url, state) + return redirectToGateway(request.url, state, dnsLink) } }, @@ -151,10 +154,14 @@ function postNormalizationSkip (state, request) { return false } -function redirectToGateway (requestUrl, state) { +function redirectToGateway (requestUrl, state, dnsLink) { // TODO: redirect to `ipfs://` if hasNativeProtocolHandler === true - const gwUrl = state.ipfsNodeType === 'embedded' ? state.pubGwURL : state.gwURL const url = new URL(requestUrl) + if (state.dnslink && dnsLink.validDnslinkRedirect(url)) { + // if no known path prefix, then it is a website with dnslink + return dnsLink.redirectToIpnsPath(url) + } + const gwUrl = state.ipfsNodeType === 'embedded' ? state.pubGwURL : state.gwURL url.protocol = gwUrl.protocol url.host = gwUrl.host url.port = gwUrl.port @@ -210,7 +217,7 @@ function normalizedRedirectingProtocolRequest (request, pubGwUrl) { path = path.replace(/^#dweb:\//i, '/') // dweb:/ipfs/Qm → /ipfs/Qm path = path.replace(/^#ipfs:\/\//i, '/ipfs/') // ipfs://Qm → /ipfs/Qm path = path.replace(/^#ipns:\/\//i, '/ipns/') // ipns://Qm → /ipns/Qm - console.log(`oldPath: '${oldPath}' new: '${path}'`) + // console.log(`oldPath: '${oldPath}' new: '${path}'`) if (oldPath !== path && IsIpfs.path(path)) { return { redirectUrl: urlAtPublicGw(path, pubGwUrl) } } diff --git a/test/functional/lib/ipfs-request.test.js b/test/functional/lib/ipfs-request.test.js index 0763da5d4..46a755927 100644 --- a/test/functional/lib/ipfs-request.test.js +++ b/test/functional/lib/ipfs-request.test.js @@ -432,6 +432,59 @@ describe('modifyRequest.onBeforeRequest', function () { expect(modifyRequest.onBeforeRequest(request)).to.equal(undefined) }) }) + + describe('request to FQDN with dnslink experiment enabled', function () { + let activeGateway + beforeEach(function () { + // pretend API is online and we can do dns lookups with it + state.dnslink = true + state.peerCount = 1 + // embedded node (js-ipfs) defaults to public gw + activeGateway = (state.ipfsNodeType === 'external' ? state.gwURLString : state.pubGwURLString) + }) + it('should be redirected to active gateway if dnslink exists', function () { + // stub the existence of valid dnslink + const fqdn = 'ipfs.git.sexy' + dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss') + // + const request = url2request('http://ipfs.git.sexy/index.html?argTest#hashTest') + expect(modifyRequest.onBeforeRequest(request).redirectUrl).to.equal(activeGateway + '/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + }) + it('should be redirected to active gateway if fetched from the same origin and redirect is enabled in non-Firefox', function () { + // stub the existence of valid dnslink + const fqdn = 'ipfs.git.sexy' + dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss') + // + runtime.isFirefox = false + const xhrRequest = {url: 'http://ipfs.git.sexy/index.html?argTest#hashTest', type: 'xmlhttprequest', initiator: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId()} + expect(modifyRequest.onBeforeRequest(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + }) + it('should be redirected to active gateway via late redirect if dnslink exists and XHR is cross-origin in Firefox', function () { + // stub the existence of valid dnslink + const fqdn = 'ipfs.git.sexy' + dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss') + // + // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + runtime.isFirefox = true + const xhrRequest = {url: 'http://ipfs.git.sexy/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId()} + // onBeforeRequest should not change anything, as it will trigger false-positive CORS error + expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) + // onHeadersReceived is after CORS validation happens, so its ok to cancel and redirect late + expect(modifyRequest.onHeadersReceived(xhrRequest).redirectUrl).to.equal(activeGateway + '/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + }) + it('should be left unouched if dnslink does not exist and XHR is cross-origin in Firefox', function () { + // stub no dnslink + const fqdn = 'youtube.com' + dnsLink.readDnslinkFromTxtRecord = sinon.stub().withArgs(fqdn).returns(undefined) + // Context for CORS XHR problems in Firefox: https://github.com/ipfs-shipyard/ipfs-companion/issues/436 + runtime.isFirefox = true + const xhrRequest = {url: 'https://youtube.com/index.html?argTest#hashTest', type: 'xmlhttprequest', originUrl: 'https://www.nasa.gov/foo.html', requestId: fakeRequestId()} + // onBeforeRequest should not change anything + expect(modifyRequest.onBeforeRequest(xhrRequest)).to.equal(undefined) + // onHeadersReceived should not change anything + expect(modifyRequest.onHeadersReceived(xhrRequest)).to.equal(undefined) + }) + }) }) })