From 18c98b75f51e68cd2646f208bb13c5eae9d83da0 Mon Sep 17 00:00:00 2001 From: Marcin Rataj Date: Thu, 3 Jan 2019 14:17:29 +0100 Subject: [PATCH] fix(privacy): remove ACL whitelist for window.ipfs This change means no command can be run without explicit approval from the user. Rationale can be found in https://github.com/ipfs-shipyard/ipfs-companion/pull/619#issuecomment-450900594 --- add-on/src/lib/ipfs-proxy/acl-whitelist.json | 25 ------------------- add-on/src/lib/ipfs-proxy/enable-command.js | 14 +++++------ add-on/src/lib/ipfs-proxy/pre-acl.js | 17 ++----------- .../lib/ipfs-proxy/enable-command.test.js | 20 --------------- .../functional/lib/ipfs-proxy/pre-acl.test.js | 22 +--------------- 5 files changed, 9 insertions(+), 89 deletions(-) delete mode 100644 add-on/src/lib/ipfs-proxy/acl-whitelist.json diff --git a/add-on/src/lib/ipfs-proxy/acl-whitelist.json b/add-on/src/lib/ipfs-proxy/acl-whitelist.json deleted file mode 100644 index 17ac05ae5..000000000 --- a/add-on/src/lib/ipfs-proxy/acl-whitelist.json +++ /dev/null @@ -1,25 +0,0 @@ -[ - "block.get", - "block.stat", - "dag.get", - "dag.tree", - "dht.get", - "dht.findprovs", - "dht.findpeer", - "dht.query", - "files.cat", - "files.catPullStream", - "files.get", - "files.getReadableStream", - "files.getPullStream", - "object.get", - "object.data", - "object.links", - "object.stat", - "pubsub.subscribe", - "pubsub.unsubscribe", - "pubsub.peers", - "swarm.peers", - "swarm.addrs", - "swarm.localAddrs" -] diff --git a/add-on/src/lib/ipfs-proxy/enable-command.js b/add-on/src/lib/ipfs-proxy/enable-command.js index 110c9b300..918ba50bd 100644 --- a/add-on/src/lib/ipfs-proxy/enable-command.js +++ b/add-on/src/lib/ipfs-proxy/enable-command.js @@ -1,5 +1,5 @@ const { inCommandWhitelist, createCommandWhitelistError } = require('./pre-command') -const { inNoAclPromptWhitelist, createProxyAclError } = require('./pre-acl') +const { createProxyAclError } = require('./pre-acl') // Artificial API command responsible for backend orchestration // during window.ipfs.enable() @@ -26,13 +26,11 @@ function createEnableCommand (getIpfs, getState, getScope, accessControl, reques } // Get the current access flag to decide if it should be added // to the list of permissions to be prompted about in the next step - if (!inNoAclPromptWhitelist(command)) { - let access = await accessControl.getAccess(scope, command) - if (!access) { - missingAcls.push(command) - } else if (access.allow !== true) { - deniedAcls.push(command) - } + let access = await accessControl.getAccess(scope, command) + if (!access) { + missingAcls.push(command) + } else if (access.allow !== true) { + deniedAcls.push(command) } } // Fail fast if user already denied any of requested permissions diff --git a/add-on/src/lib/ipfs-proxy/pre-acl.js b/add-on/src/lib/ipfs-proxy/pre-acl.js index febb53aaf..b8b2a7612 100644 --- a/add-on/src/lib/ipfs-proxy/pre-acl.js +++ b/add-on/src/lib/ipfs-proxy/pre-acl.js @@ -1,7 +1,3 @@ -// This are the functions that DO NOT require an allow/deny decision by the user. -// All other IPFS functions require authorization. -const ACL_WHITELIST = Object.freeze(require('./acl-whitelist.json')) - // Creates a "pre" function that is called prior to calling a real function // on the IPFS instance. It will throw if access is denied, and ask the user if // no access decision has been made yet. @@ -10,9 +6,6 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc // Check if all access to the IPFS node is disabled if (!getState().ipfsProxy) throw new Error('User disabled access to API proxy in IPFS Companion') - // No need to verify access if permission is on the whitelist - if (inNoAclPromptWhitelist(permission)) return args - const scope = await getScope() const access = await getAccessWithPrompt(accessControl, requestAccess, scope, permission) @@ -24,10 +17,6 @@ function createPreAcl (permission, getState, getScope, accessControl, requestAcc } } -function inNoAclPromptWhitelist (permission) { - return ACL_WHITELIST.includes(permission) -} - async function getAccessWithPrompt (accessControl, requestAccess, scope, permission) { let access = await accessControl.getAccess(scope, permission) if (!access) { @@ -37,7 +26,7 @@ async function getAccessWithPrompt (accessControl, requestAccess, scope, permiss return access } -// Standardized error thrown when a command is not on the ACL_WHITELIST +// Standardized error thrown when a command access is denied // TODO: return errors following conventions from https://github.com/ipfs/js-ipfs/pull/1746 function createProxyAclError (scope, permission) { const err = new Error(`User denied access to selected commands over IPFS proxy: ${permission}`) @@ -65,7 +54,5 @@ function createProxyAclError (scope, permission) { module.exports = { createPreAcl, - createProxyAclError, - inNoAclPromptWhitelist, - ACL_WHITELIST + createProxyAclError } diff --git a/test/functional/lib/ipfs-proxy/enable-command.test.js b/test/functional/lib/ipfs-proxy/enable-command.test.js index be2aa7857..5791986b5 100644 --- a/test/functional/lib/ipfs-proxy/enable-command.test.js +++ b/test/functional/lib/ipfs-proxy/enable-command.test.js @@ -10,7 +10,6 @@ const Sinon = require('sinon') const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control') const createEnableCommand = require('../../../../add-on/src/lib/ipfs-proxy/enable-command') const createRequestAccess = require('../../../../add-on/src/lib/ipfs-proxy/request-access') -const { ACL_WHITELIST } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl') describe('lib/ipfs-proxy/enable-command', () => { before(() => { @@ -71,25 +70,6 @@ describe('lib/ipfs-proxy/enable-command', () => { expect(requestAccess.called).to.equal(false) }) - it('should allow access without prompt if permission is on ACL whitelist', async () => { - const getState = () => ({ ipfsProxy: true }) - const accessControl = new AccessControl(new Storage()) - const getScope = () => 'https://3.foo.tld/path/' - const getIpfs = () => {} - const requestAccess = async () => { throw new Error('Requested access for whitelist permission') } - const enable = createEnableCommand(getIpfs, getState, getScope, accessControl, requestAccess) - - let error - - try { - await Promise.all(ACL_WHITELIST.map(permission => enable({ commands: [permission] }))) - } catch (err) { - error = err - } - - expect(error).to.equal(undefined) - }) - it('should request access if no grant exists', async () => { const getState = () => ({ ipfsProxy: true }) const accessControl = new AccessControl(new Storage()) diff --git a/test/functional/lib/ipfs-proxy/pre-acl.test.js b/test/functional/lib/ipfs-proxy/pre-acl.test.js index c2151a8f6..4f5814029 100644 --- a/test/functional/lib/ipfs-proxy/pre-acl.test.js +++ b/test/functional/lib/ipfs-proxy/pre-acl.test.js @@ -5,7 +5,7 @@ const { URL } = require('url') const Storage = require('mem-storage-area/Storage') const Sinon = require('sinon') const AccessControl = require('../../../../add-on/src/lib/ipfs-proxy/access-control') -const { createPreAcl, ACL_WHITELIST } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl') +const { createPreAcl } = require('../../../../add-on/src/lib/ipfs-proxy/pre-acl') describe('lib/ipfs-proxy/pre-acl', () => { before(() => { @@ -31,26 +31,6 @@ describe('lib/ipfs-proxy/pre-acl', () => { expect(() => { if (error) throw error }).to.throw('User disabled access to API proxy in IPFS Companion') }) - it('should allow access if permission is on whitelist', async () => { - const getState = () => ({ ipfsProxy: true }) - const accessControl = new AccessControl(new Storage()) - const getScope = () => 'https://ipfs.io/' - const requestAccess = async () => { throw new Error('Requested access for whitelist permission') } - - let error - - try { - await Promise.all(ACL_WHITELIST.map(permission => { - const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess) - return preAcl() - })) - } catch (err) { - error = err - } - - expect(error).to.equal(undefined) - }) - it('should request access if no grant exists', async () => { const getState = () => ({ ipfsProxy: true }) const accessControl = new AccessControl(new Storage())