Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: scope mfs calls to an application path #431

Merged
merged 9 commits into from
Mar 27, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion add-on/src/lib/ipfs-proxy/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const browser = require('webextension-polyfill')
const { createProxyServer, closeProxyServer } = require('ipfs-postmsg-proxy')
const AccessControl = require('./access-control')
const createPreAcl = require('./pre-acl')
const createPreMfsScope = require('./pre-mfs-scope')
const createRequestAccess = require('./request-access')

// Creates an object that manages the "server side" of the IPFS proxy
Expand All @@ -31,7 +32,10 @@ function createIpfsProxy (getIpfs, getState) {
removeListener: (_, handler) => port.onMessage.removeListener(handler),
postMessage: (data) => port.postMessage(data),
getMessageData: (d) => d,
pre: (fnName) => createPreAcl(getState, accessControl, getScope, fnName, requestAccess)
pre: (fnName) => [
createPreAcl(fnName, getState, getScope, accessControl, requestAccess),
createPreMfsScope(fnName, getScope, getIpfs)
]
})

const close = () => {
Expand Down
20 changes: 1 addition & 19 deletions add-on/src/lib/ipfs-proxy/pre-acl.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,32 +2,14 @@
// All other IPFS functions require authorization.
const ACL_WHITELIST = Object.freeze(require('./acl-whitelist.json'))

// TEMPORARY blacklist of MFS functions that are automatically denied access
// https://github.com/ipfs-shipyard/ipfs-companion/issues/330#issuecomment-367651787
const MFS_BLACKLIST = Object.freeze([
'files.cp',
'files.mkdir',
'files.stat',
'files.rm',
'files.read',
'files.write',
'files.mv',
'files.flush',
'files.ls'
])

// 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.
function createPreAcl (getState, accessControl, getScope, permission, requestAccess) {
function createPreAcl (permission, getState, getScope, accessControl, requestAccess) {
return async (...args) => {
// Check if all access to the IPFS node is disabled
if (!getState().ipfsProxy) throw new Error('User disabled access to IPFS')

if (MFS_BLACKLIST.includes(permission)) {
throw new Error('MFS functions are temporarily disabled')
}

// No need to verify access if permission is on the whitelist
if (ACL_WHITELIST.includes(permission)) return args

Expand Down
103 changes: 103 additions & 0 deletions add-on/src/lib/ipfs-proxy/pre-mfs-scope.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
const Path = require('path')
const DEFAULT_ROOT_PATH = '/dapps'

// Creates a "pre" function that is called prior to calling a real function
// on the IPFS instance. It modifies the arguments to MFS functions to scope
// file access to a directory designated to the web page
function createPreMfsScope (fnName, getScope, getIpfs, rootPath = DEFAULT_ROOT_PATH) {
return MfsPre[fnName] ? MfsPre[fnName](getScope, getIpfs, rootPath) : null
}

module.exports = createPreMfsScope

const MfsPre = {
Copy link
Member

@lidel lidel Mar 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question; how will it behave when a new API ipfs.files.foo is added ?
Will it not work, or will it work with no path scoping?
If the latter is the case, we should explicitly block executions of new methods in scoped ipfs.files API to avoid surprises in future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I mean, you'd need to update the ipfs-postmsg-proxy dependency to allow the new method to be used in the first place, but no it wouldn't be scoped automatically unless you did some work here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, being dependent on ipfs-postmsg-proxy makes this safe enough for now.
Unless its trivial to add, I propose we add additional safeguards in separate PR.

'files.cp': createSrcDestPre,
'files.mkdir': createSrcPre,
'files.stat': createSrcPre,
'files.rm' (getScope, getIpfs, rootPath) {
const srcPre = createSrcPre(getScope, getIpfs, rootPath)
// Do not allow rm app root
// Need to explicitly deny because it's ok to rm -rf /a/path that's not /
return (...args) => {
if (isRoot(args[0])) throw new Error('cannot delete root')
return srcPre(...args)
}
},
'files.read': createSrcPre,
'files.write' (getScope, getIpfs, rootPath) {
const srcPre = createSrcPre(getScope, getIpfs, rootPath)
// Do not allow write to app root
// Need to explicitly deny because app path might not exist yet
return (...args) => {
if (isRoot(args[0])) throw new Error('/ was not a file')
return srcPre(...args)
}
},
'files.mv': createSrcDestPre,
'files.flush': createOptionalSrcPre,
'files.ls': createOptionalSrcPre
}

// Scope a src/dest tuple to the app path
function createSrcDestPre (getScope, getIpfs, rootPath) {
return async (...args) => {
const appPath = await getAppPath(getScope, getIpfs, rootPath)
args[0][0] = Path.join(appPath, safePath(args[0][0]))
args[0][1] = Path.join(appPath, safePath(args[0][1]))
return args
}
}

// Scope a src path to the app path
function createSrcPre (getScope, getIpfs, rootPath) {
return async (...args) => {
const appPath = await getAppPath(getScope, getIpfs, rootPath)
args[0] = Path.join(appPath, safePath(args[0]))
return args
}
}

// Scope an optional src path to the app path
function createOptionalSrcPre (getScope, getIpfs, rootPath) {
return async (...args) => {
const appPath = await getAppPath(getScope, getIpfs, rootPath)

if (Object.prototype.toString.call(args[0]) === '[object String]') {
args[0] = Path.join(appPath, safePath(args[0]))
} else {
switch (args.length) {
case 0: return [appPath] // e.g. ipfs.files.ls()
case 1: return [appPath, args[0]] // e.g. ipfs.files.ls(options)
case 2: return [appPath, args[1]] // e.g. ipfs.files.ls(null, options)
default: throw new Error('Unexpected number of arguments')
}
}
return args
}
}

// Get the app path (create if not exists) for a scope, prefixed with rootPath
const getAppPath = async (getScope, getIpfs, rootPath) => {
const appPath = rootPath + scopeToPath(await getScope())
await getIpfs().files.mkdir(appPath, { parents: true })
return appPath
}

// Turn http://ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn
// into /http/ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn
const scopeToPath = (scope) => {
return ('/' + scope)
.replace(/\/\//g, '/')
.split('/')
// Special case for protocol in scope, remove : from the end
.map((seg, i) => i === 1 && seg.endsWith(':') ? seg.slice(0, -1) : seg)
.map(encodeURIComponent)
.join('/')
}

// Make a path "safe" by resolving any directory traversal segments relative to
// '/'. Allows us to then prefix the app path without worrying about the user
// breaking out of their jail.
const safePath = (path) => Path.resolve('/', path)

const isRoot = (path) => Path.resolve('/', path) === '/'
18 changes: 18 additions & 0 deletions docs/window.ipfs.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- [Do I need to confirm every API call?](#do-i-need-to-confirm-every-api-call)
- [Can I disable this for now?](#can-i-disable-this-for-now)
- [How are permissions scoped?](#how-are-permissions-scoped)
- [Are mutable file system (MFS) files sandboxed to a directory?](#are-mutable-file-system-mfs-files-sandboxed-to-a-directory)

## Background

Expand Down Expand Up @@ -160,3 +161,20 @@ e.g.
* `https://domain.com/`
* `https://domain.com/files`
* etc.

## Are mutable file system (MFS) files sandboxed to a directory?

Yes. To avoid conflicts, each app gets it's own MFS directory where it can store files. When using MFS functions (see [docs](https://github.com/ipfs/interface-ipfs-core/blob/master/SPEC/FILES.md#mutable-file-system)) this directory will be automatically added to paths you pass. Your app's MFS directory is based on the **origin and path** where your application is running.

e.g.

* `files.write` to `/myfile.txt` on `https://domain.com/`
* writes to `/dapps/https/domain.com/myfile.txt`
* `files.write` to `/path/to/myfile.txt` on `https://domain.com/feature`
* writes to `/dapps/https/domain.com/feature/path/to/myfile.txt`
* `files.read` from `/feature/path/to/myfile.txt` on `https://domain.com/`
* reads from `/dapps/https/domain.com/feature/path/to/myfile.txt`
* `files.stat` to `/` on `https://domain.com/feature`
* stats `/dapps/https/domain.com/feature`
* `files.read` from `/../myfile.txt` on `https://domain.com/feature`
* reads from `/dapps/https/domain.com/feature/myfile.txt` (no traverse above your app's root)
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
"ipfs": "0.28.2",
"ipfs-api": "18.2.0",
"ipfs-css": "0.2.0",
"ipfs-postmsg-proxy": "2.8.4",
"ipfs-postmsg-proxy": "2.10.0",
"is-ipfs": "0.3.2",
"is-svg": "3.0.0",
"lru_map": "0.3.3",
Expand Down
12 changes: 6 additions & 6 deletions test/functional/lib/ipfs-proxy/pre-acl.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {
const getScope = () => 'https://ipfs.io/'
const permission = 'files.add'

const preAcl = createPreAcl(getState, accessControl, getScope, permission)
const preAcl = createPreAcl(permission, getState, getScope, accessControl)

let error

Expand All @@ -42,7 +42,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {

try {
await Promise.all(ACL_WHITELIST.map(permission => {
const preAcl = createPreAcl(getState, accessControl, getScope, permission, requestAccess)
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)
return preAcl()
}))
} catch (err) {
Expand All @@ -58,7 +58,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {
const getScope = () => 'https://ipfs.io/'
const permission = 'files.add'
const requestAccess = Sinon.spy(async () => ({ allow: true }))
const preAcl = createPreAcl(getState, accessControl, getScope, permission, requestAccess)
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)

await preAcl()

Expand All @@ -71,7 +71,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {
const getScope = () => 'https://ipfs.io/'
const permission = 'files.add'
const requestAccess = Sinon.spy(async () => ({ allow: false }))
const preAcl = createPreAcl(getState, accessControl, getScope, permission, requestAccess)
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)

let error

Expand All @@ -91,7 +91,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {
const getScope = () => 'https://ipfs.io/'
const permission = 'files.add'
const requestAccess = Sinon.spy(async () => ({ allow: false }))
const preAcl = createPreAcl(getState, accessControl, getScope, permission, requestAccess)
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)

let error

Expand Down Expand Up @@ -123,7 +123,7 @@ describe('lib/ipfs-proxy/pre-acl', () => {
const getScope = () => 'https://ipfs.io/'
const permission = 'files.add'
const requestAccess = Sinon.spy(async () => ({ allow: true }))
const preAcl = createPreAcl(getState, accessControl, getScope, permission, requestAccess)
const preAcl = createPreAcl(permission, getState, getScope, accessControl, requestAccess)

await preAcl()
expect(requestAccess.callCount).to.equal(1)
Expand Down
Loading