From 7d9a1f2bfda8220fcbac56ccc91be52f186b9770 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Fri, 6 Oct 2017 11:03:30 +0300 Subject: [PATCH] feat: proper /ipns/ validation This commit removes false-positive redirects for paths that start with /ipns/{ipnsRoot} by following these steps: 1. is-ipfs test (may produce false-positives) 2. remove false-positives by checking if ipnsRoot is: - a valid CID (we check this first as its faster/cheaper) - or FQDN with a valid dnslin in DNS TXT record (expensive, but we reuse caching mechanism from dnslink experiment) This means we now _automagically_ detect valid IPFS resources on any website as long as path starts with /ipfs/ or /ipns/, removing problems described in https://github.com/ipfs/ipfs-companion/issues/16#issuecomment-331121999 This commit also closes #69 -- initial load is suspended until dnslink is read via API, then it is cached so that all subsequent requests are very fast. --- add-on/manifest.json | 6 +- add-on/src/lib/common.js | 148 ++++++++++++++------------- test/unit/01-onBeforeRequest.test.js | 45 +++++++- 3 files changed, 123 insertions(+), 76 deletions(-) diff --git a/add-on/manifest.json b/add-on/manifest.json index a93cf6f75..f0ed8dc30 100644 --- a/add-on/manifest.json +++ b/add-on/manifest.json @@ -2,7 +2,7 @@ "manifest_version": 2, "name": "IPFS Companion", "short_name": "IPFS Companion", - "version" : "2.0.10", + "version" : "2.0.11", "description": "Browser extension that simplifies access to IPFS resources", "homepage_url": "https://github.com/ipfs/ipfs-companion", @@ -59,8 +59,8 @@ "protocol_handlers": [ { - "protocol": "web+fs", - "name": "IPFS Add-On: *FS protocol handler", + "protocol": "web+dweb", + "name": "IPFS Add-On: DWEB protocol handler", "uriTemplate": "https://ipfs.io/%s" }, { diff --git a/add-on/src/lib/common.js b/add-on/src/lib/common.js index 17fb1c018..187035918 100644 --- a/add-on/src/lib/common.js +++ b/add-on/src/lib/common.js @@ -28,7 +28,7 @@ function initIpfsApi (ipfsApiUrl) { return window.IpfsApi({host: url.hostname, port: url.port, procotol: url.protocol}) } -async function initStates (options) { +function initStates (options) { state.redirect = options.useCustomGateway state.apiURL = new URL(options.ipfsApiUrl) state.apiURLString = state.apiURL.toString() @@ -54,8 +54,30 @@ function registerListeners () { // REDIRECT // =================================================================== -function publicIpfsResource (url) { - return window.IsIpfs.url(url) && !url.startsWith(state.gwURLString) && !url.startsWith(state.apiURLString) +function publicIpfsOrIpnsResource (url) { + // first, exclude gateway and api, otherwise we have infinite loop + if (!url.startsWith(state.gwURLString) && !url.startsWith(state.apiURLString)) { + // /ipfs/ is easy to validate, we just check if CID is correct and return if true + if (window.IsIpfs.ipfsUrl(url)) { + return true + } + // /ipns/ requires multiple stages/branches, as it can be FQDN with dnslink or CID + if (window.IsIpfs.ipnsUrl(url)) { + const ipnsRoot = new URL(url).pathname.match(/^\/ipns\/([^/]+)/)[1] + // console.log('=====> IPNS root', ipnsRoot) + // first check if root is a regular CID + if (window.IsIpfs.cid(ipnsRoot)) { + // console.log('=====> IPNS is a valid CID', ipnsRoot) + return true + } + if (isDnslookupSafe(url) && cachedDnslinkLookup(ipnsRoot)) { + // console.log('=====> IPNS for FQDN with valid dnslink: ', ipnsRoot) + return true + } + } + } + // everything else is not ipfs-related + return false } function redirectToCustomGateway (requestUrl) { @@ -107,13 +129,13 @@ function onBeforeRequest (request) { // handle redirects to custom gateway if (state.redirect) { - // IPFS resources - if (publicIpfsResource(request.url)) { + // Detect valid /ipfs/ and /ipns/ on any site + if (publicIpfsOrIpnsResource(request.url)) { return redirectToCustomGateway(request.url) } // Look for dnslink in TXT records of visited sites - if (isDnslookupEnabled(request)) { - return dnslinkLookup(request) + if (state.dnslink && isDnslookupSafe(request.url)) { + return dnslinkLookupAndOptionalRedirect(request.url) } } } @@ -174,54 +196,43 @@ function normalizedUnhandledIpfsProtocol (request) { // DNSLINK // =================================================================== -function isDnslookupEnabled (request) { - return state.dnslink && - state.peerCount > 0 && - request.url.startsWith('http') && - !request.url.startsWith(state.apiURLString) && - !request.url.startsWith(state.gwURLString) +function isDnslookupSafe (requestUrl) { + return state.peerCount > 0 && + requestUrl.startsWith('http') && + !requestUrl.startsWith(state.apiURLString) && + !requestUrl.startsWith(state.gwURLString) } -function dnslinkLookup (request) { - // TODO: benchmark and improve performance - const requestUrl = new URL(request.url) - const fqdn = requestUrl.hostname - let dnslink = state.dnslinkCache.get(fqdn) - if (typeof dnslink === 'undefined') { - // fetching fresh dnslink is expensive, so we switch to async - console.log('dnslink cache miss for: ' + fqdn) - /* According to https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onBeforeRequest - * "From Firefox 52 onwards, instead of returning BlockingResponse, the listener can return a Promise - * which is resolved with a BlockingResponse. This enables the listener to process the request asynchronously." - * - * Seems that this does not work yet, and even tho promise is executed, request is not blocked but resolves to regular URL. - * TODO: This should be revisited after Firefox 52 is released. If does not work by then, we need to fill a bug. - */ - return asyncDnslookupResponse(fqdn, requestUrl) - } +function dnslinkLookupAndOptionalRedirect (requestUrl) { + const url = new URL(requestUrl) + const fqdn = url.hostname + const dnslink = cachedDnslinkLookup(fqdn) if (dnslink) { - console.log('SYNC resolving to Cached dnslink redirect:' + fqdn) - return redirectToDnslinkPath(requestUrl, dnslink) + return redirectToDnslinkPath(url, dnslink) } } -async function asyncDnslookupResponse (fqdn, requestUrl) { - try { - const dnslink = await readDnslinkTxtRecordFromApi(fqdn) - if (dnslink) { - state.dnslinkCache.set(fqdn, dnslink) - console.log('ASYNC Resolved dnslink for:' + fqdn + ' is: ' + dnslink) - return redirectToDnslinkPath(requestUrl, dnslink) - } else { - state.dnslinkCache.set(fqdn, false) - console.log('ASYNC NO dnslink for:' + fqdn) - return {} +function cachedDnslinkLookup (fqdn) { + let dnslink = state.dnslinkCache.get(fqdn) + if (typeof dnslink === 'undefined') { + try { + console.log('dnslink cache miss for: ' + fqdn) + dnslink = readDnslinkFromTxtRecord(fqdn) + if (dnslink) { + state.dnslinkCache.set(fqdn, dnslink) + console.log(`Resolved dnslink: '${fqdn}' -> '${dnslink}'`) + } else { + state.dnslinkCache.set(fqdn, false) + console.log(`Resolved NO dnslink for '${fqdn}'`) + } + } catch (error) { + console.error(`Error in dnslinkLookupAndOptionalRedirect for '${fqdn}'`) + console.error(error) } - } catch (error) { - console.error(`ASYNC Error in asyncDnslookupResponse for '${fqdn}': ${error}`) - console.error(error) - return {} + } else { + console.log(`Resolved via cached dnslink: '${fqdn}' -> '${dnslink}'`) } + return dnslink } function redirectToDnslinkPath (url, dnslink) { @@ -232,31 +243,30 @@ function redirectToDnslinkPath (url, dnslink) { return { redirectUrl: url.toString() } } -function readDnslinkTxtRecordFromApi (fqdn) { +function readDnslinkFromTxtRecord (fqdn) { // js-ipfs-api does not provide method for fetching this // TODO: revisit after https://github.com/ipfs/js-ipfs-api/issues/501 is addressed - return new Promise((resolve, reject) => { - const apiCall = state.apiURLString + '/api/v0/dns/' + fqdn - const xhr = new XMLHttpRequest() // older XHR API us used because window.fetch appends Origin which causes error 403 in go-ipfs - xhr.open('GET', apiCall) - xhr.setRequestHeader('Accept', 'application/json') - xhr.onload = function () { - if (this.status === 200) { - const dnslink = JSON.parse(xhr.responseText).Path - resolve(dnslink) - } else if (this.status === 500) { - // go-ipfs returns 500 if host has no dnslink - // TODO: find/fill an upstream bug to make this more intuitive - resolve(false) - } else { - reject(new Error(xhr.statusText)) - } - } - xhr.onerror = function () { - reject(new Error(xhr.statusText)) + const apiCall = state.apiURLString + '/api/v0/dns/' + fqdn + const xhr = new XMLHttpRequest() // older XHR API us used because window.fetch appends Origin which causes error 403 in go-ipfs + // synchronous mode with small timeout + // (it is okay, because we do it only once, then it is cached and read via cachedDnslinkLookup) + xhr.open('GET', apiCall, false) + xhr.setRequestHeader('Accept', 'application/json') + xhr.send(null) + if (xhr.status === 200) { + const dnslink = JSON.parse(xhr.responseText).Path + // console.log('readDnslinkFromTxtRecord', readDnslinkFromTxtRecord) + if (!window.IsIpfs.path(dnslink)) { + throw new Error(`dnslink for '${fqdn}' is not a valid IPFS path: '${dnslink}'`) } - xhr.send() - }) + return dnslink + } else if (xhr.status === 500) { + // go-ipfs returns 500 if host has no dnslink + // TODO: find/fill an upstream bug to make this more intuitive + return false + } else { + throw new Error(xhr.statusText) + } } // RUNTIME MESSAGES (one-off messaging) diff --git a/test/unit/01-onBeforeRequest.test.js b/test/unit/01-onBeforeRequest.test.js index 0f4f9e9e2..fda73ad61 100644 --- a/test/unit/01-onBeforeRequest.test.js +++ b/test/unit/01-onBeforeRequest.test.js @@ -1,6 +1,7 @@ 'use strict' /* eslint-env webextensions, mocha */ -/* globals sinon, optionDefaults, should, state, onBeforeRequest */ +// eslint-disable-next-line no-unused-vars +/* globals sinon, initStates, optionDefaults, should, state, onBeforeRequest, readDnslinkFromTxtRecord */ var sandbox @@ -13,7 +14,11 @@ describe('onBeforeRequest', function () { browser.flush() sandbox = sinon.sandbox.create() browser.storage.local.get.returns(Promise.resolve(optionDefaults)) + // reset states + initStates(optionDefaults) + // stub default state for most of tests // redirect by default -- makes test code shorter + state.peerCount = 1 state.redirect = true state.catchUnhandledProtocols = true state.gwURLString = 'http://127.0.0.1:8080' @@ -34,18 +39,50 @@ describe('onBeforeRequest', function () { const request = url2request('https://ipfs.io/ipfs/QmbWqxBEKC3P8tqsKc98xmWNzrzDtRLMiMPL8wBuTGsMnR?argTest#hashTest') should.not.exist(onBeforeRequest(request)) }) + it('should be left untouched if CID is invalid', function () { + const request = url2request('https://ipfs.io/ipfs/notacid?argTest#hashTest') + should.not.exist(onBeforeRequest(request)) + }) }) describe('request for a path matching /ipns/{path}', function () { - it('should be served from custom gateway if redirect is enabled', function () { - const request = url2request('https://ipfs.io/ipns/ipfs.io/index.html?argTest#hashTest') - onBeforeRequest(request).redirectUrl.should.equal('http://127.0.0.1:8080/ipns/ipfs.io/index.html?argTest#hashTest') + it('should be served from custom gateway if {path} points to a FQDN with existing dnslink', function () { + const request = url2request('https://ipfs.io/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + // stub the existence of valid dnslink + const fqdn = 'ipfs.git.sexy' + // eslint-disable-next-line no-global-assign + readDnslinkFromTxtRecord = sandbox.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss') + // pretend API is online and we can do dns lookups with it + state.peerCount = 1 + onBeforeRequest(request).redirectUrl.should.equal('http://127.0.0.1:8080/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + }) + it('should be served from custom gateway if {path} starts with a valid CID', function () { + const request = url2request('https://ipfs.io/ipns/QmSWnBwMKZ28tcgMFdihD8XS7p6QzdRSGf71cCybaETSsU/index.html?argTest#hashTest') + // eslint-disable-next-line no-global-assign + readDnslinkFromTxtRecord = sandbox.stub().returns(false) + onBeforeRequest(request).redirectUrl.should.equal('http://127.0.0.1:8080/ipns/QmSWnBwMKZ28tcgMFdihD8XS7p6QzdRSGf71cCybaETSsU/index.html?argTest#hashTest') }) it('should be left untouched if redirect is disabled', function () { state.redirect = false const request = url2request('https://ipfs.io/ipns/ipfs.io?argTest#hashTest') should.not.exist(onBeforeRequest(request)) }) + it('should be left untouched if FQDN is not a real domain nor a valid CID', function () { + const request = url2request('https://ipfs.io/ipns/notafqdnorcid?argTest#hashTest') + // eslint-disable-next-line no-global-assign + readDnslinkFromTxtRecord = sandbox.stub().returns(false) + should.not.exist(onBeforeRequest(request)) + }) + it('should be left untouched if {path} points to a FQDN but API is offline', function () { + const request = url2request('https://ipfs.io/ipns/ipfs.git.sexy/index.html?argTest#hashTest') + // stub the existence of valid dnslink in dnslink cache + const fqdn = 'ipfs.git.sexy' + // eslint-disable-next-line no-global-assign + readDnslinkFromTxtRecord = sandbox.stub().withArgs(fqdn).returns('/ipfs/Qmazvovg6Sic3m9igZMKoAPjkiVZsvbWWc8ZvgjjK1qMss') + // pretend API is offline and we can do dns lookups with it + state.peerCount = 0 + should.not.exist(onBeforeRequest(request)) + }) }) describe('request made via "web+" handler from manifest.json/protocol_handlers', function () {