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

Expose IPFS API as window.ipfs #330

Closed
7 tasks done
lidel opened this issue Dec 11, 2017 · 54 comments
Closed
7 tasks done

Expose IPFS API as window.ipfs #330

lidel opened this issue Dec 11, 2017 · 54 comments
Assignees
Labels
area/window-ipfs Issues related to IPFS API exposed on every page kind/enhancement A net-new feature or improvement to an existing feature status/in-progress In progress topic/security Work related to security
Milestone

Comments

@lidel
Copy link
Member

lidel commented Dec 11, 2017

There is an area of interest that can be summarized as..

..exposing a subset of IPFS APIs as window.ipfs on every webpage ✨

tl;dr If we have it, websites could detect that window.ipfs already exists and use it instead of spawning own js-ipfs node (saves resources, battery etc).

We had discussions in ipfs-inactive/js-ipfs-http-client#387 and #37, ipfs/in-web-browsers#9, ipfs/in-web-browsers#35 but the consensus at the time was that it is not safe without some kind of access-control (eg. via proposed API Tokens: ipfs/kubo#1532).

@victorbjelkholm suggested a simple solution to the lack of access controls in the API itself:
On the first use of window.ipfs user could be prompted with something like "Do you want to allow scripts at <website> to access API of your IPFS node?". The choice would be remembered eg. for the page til the end of browsing session. There could also be an additional "remember for this site" checkbox to make UX better for users who wants to make permission persist between browser restarts.

Tasks

  • Create a content script that creates a proxy object under window.ipfs of the page it is injected into
    • This object should expose a subset of IPFS APIs

      <victorbjelkholm> there is of course a lot more once there is a full ipfs node in the browser, but if we only have the simplest and most useful functions (add, cat, pubsub subscribe/publish, dag get/put), it suddenly becomes a lot simpler and useful quickly

    • Operations performed on window.ipfs should be proxied to the real IPFS API object present in webextension's Background page (requires robust (de)serialization, we don't want to mangle upload data etc)
    • First use of window.ipfs should trigger a dialog where user confirms that access to IPFS API is granted to requesting site (and decides if permission is for this session only or permanent)
  • Add a checkbox under "Preferences/Experiments" to control if content script is injected to every visited page, make it disabled by default initially
  • Write documentation, namely provide code snippet that enables apps to use the IPFS Companion IPFS node if it is available

Potential problems

  • Something to investigate and solve before we make window.ipfs enabled by default: make sure it is not possible for a website to approve itself 🙃 This may mean we can't inject dialog to the tab itself and the dialog needs to be displayed in a new Tab or via other means.
@lidel lidel added kind/enhancement A net-new feature or improvement to an existing feature topic/security Work related to security labels Dec 11, 2017
@lidel lidel added this to the 2018-Q1 milestone Dec 11, 2017
@victorb
Copy link
Member

victorb commented Dec 11, 2017

I've made two PoCs on this concept. One was separate and you can see it here: https://github.com/victorbjelkholm/js-ipfs-in-the-browser

The other one was a recent try to integrate that work into ipfs-companion and fix the stream issue in .cat, but didn't get really far yet: 6b0ac8b

suggested a simple solution to the lack of access controls in the API itself:
On the first use of window.ipfs user could be prompted with something like

I don't think that's good enough. I think (judging only five different methods) that the functions themselves should trigger the permission dialog (all with a "remember for this site" checkbox)

  • ipfs.id would trigger a "website would like to know your ID" prompt
  • ipfs.cat would trigger a "website would like to get content from the IPFS network via you or content in your local node" prompt
  • ipfs.pubsub.subscribe would trigger a "website would like to read messages from a pubsub channel via you"
  • ipfs.pubsub.publish would trigger a "website would like to send messages as you"

@victorb
Copy link
Member

victorb commented Dec 11, 2017

make sure it is not possible for a website to approve itself

Should not be possible if we store the state in the extensions storage.

This may mean we can't inject dialog to the tab itself and the dialog needs to be displayed in a new Tab or via other means.

We absolutely should not inject the dialog into the page itself, or via a new tab but rather use the popup that ipfs-companion provides. Take a look at how MetaMask deals with confirmation when you do a transaction.

@lidel
Copy link
Member Author

lidel commented Dec 11, 2017

@victorbjelkholm thanks, great points!

For future reference, metamask webextension:
https://github.com/MetaMask/metamask-extension/tree/master/app

@daviddias
Copy link
Member

@lidel can a quick experiment to let users access window.ipfs after enabling a flag on the settings pane? @joaosantos15 would really appreciate this.

@joaosantos15, perhaps you can grab @victorbjelkholm PoC and submit a PR to station?

@olizilla
Copy link
Member

@diasdavid there is some very early wip for that here #333

It's building on the ideas in https://github.com/victorbjelkholm/js-ipfs-in-the-browser
adding window.ipfs as a proxy to a ipfs instance running in ipfs-companion. Only ipfs.id() implemented so far... there are a lot of hurdles to jump through to make it work.

@lidel
Copy link
Member Author

lidel commented Dec 13, 2017

Just like @diasdavid suggested, we can work on PR #333 as an experiment that is disabled by default (another checkbox in Preferences 🙃 ). I feel that as long it is an "opt-in experiment" we can ship it as a crude PoC, even without authorization GUI (although it may not be the hardest part, as noted in #333 (review)).

@daviddias
Copy link
Member

Thanks @lidel @olizilla. Yes, shipping it as an opt-in would enable @joaosantos15 use case for now, that would help a ton :)

@olizilla
Copy link
Member

@diasdavid @joaosantos15 out of interest, what is the use case?

@daviddias
Copy link
Member

glad you asked :) it is for @joaosantos15 thesis -> ipfs/notes#268 & https://github.com/inesc-id/hypercerts

@olizilla
Copy link
Member

@diasdavid given all the limitations of proxy-ing across to an ipfs in the webext background page, would it be of any use if simply provide js-ipfs as a constructor atwindow.IPFS in the meantime?

We can do both, it's just that a useable window.ipfs proxy is a little way off yet. So let pages construct their own jsipfs instance until we can proxy through to a shared one?

@joaosantos15
Copy link

Hello, sorry it has taken me this long to reply, and thank you for the help! :)
@olizilla, essentially what I need it to connect my web extension to the go-ipfs daemon (both running on the same machine). What you suggested,

provide js-ipfs as a constructor atwindow.IPFS in the meantime?

would make that work, right?

Thanks!

@lidel
Copy link
Member Author

lidel commented Dec 14, 2017

@joaosantos15 ah, if you are developing a webextension, be sure to see our other proposal:
"Provide API for other browser extensions". Perhaps it is a better match?

Let us know which approach is better for you (runtime.onMessageExternal vs window.ipfs) and we can prioritize accordingly.

@joaosantos15
Copy link

Right now my extension is pretty simple, it only injects some js to change the layout of the webpage, I'd prefer window.ipfs.

@olizilla
Copy link
Member

@joaosantos15 to add and get files with an ifps daemon you'll want window.ipfs set up as a proxy to an ipfs-api client hosted in the background page in ipfs-companion. The work in #333, when it's ready, will make that possible.

@victorb
Copy link
Member

victorb commented Dec 14, 2017

@joaosantos15 you could already today use js-ipfs-api easily to build what you need. The Ports or window.ipfs would just be a optimization to minimize dependencies, but what you need is already possible today.

If you check for window.ipfs or a ping message (interface to be defined) over a Port, before actually using it, once we have this feature it should work automagically.

Example:

if (window.ipfs) {
  // do whatever
} {
  // logic for loading js-ipfs-api, require/import async
  node = new ipfs()
  node.once('ready', () => { /* do whatever */ })
}

With this example, your application will work today and once window.ipfs or the Port is available, you'll automatically start using it.

@joaosantos15
Copy link

@victorbjelkholm that's great. I'll go ahead and do it that way and when #333 is ready I'll switch to that. Thanks for the help, everyone 😄

@alanshaw alanshaw self-assigned this Dec 15, 2017
@alanshaw
Copy link
Member

alanshaw commented Dec 15, 2017

Hey, some progress here:
https://youtu.be/t1ldUp_mjDk

  • window.ipfs property!
    • also adds window.Buffer if not there
    • also adds window.Ipfs
  • proxies to running js-ipfs in the extension
    • video demonstrates shared repo across tabs
  • Buffers sent across postMessage

WIP PR for companion here: #333
IPFS postMessage proxy here: https://github.com/tableflip/ipfs-postmsg-proxy

@alanshaw
Copy link
Member

alanshaw commented Jan 4, 2018

Ok so, some more news.

As of v0.0.12, ipfs-postmsg-proxy has feature parity with js-ipfs in the browser, according to the interface-ipfs-core tests, barring a few small caveats.

It's cool firstly because we're able to use interface-ipfs-core tests to ensure the proxy is working well and secondly because there's two types of tests:

  1. Node.js tests - where we use mock window objects and postMessage APIs that use realistic-structured-clone to simulate what happens in a browser when objects get passed between windows
  2. Browser tests - because our Node.js simulation is good, but it's not the real thing and there are differences between the two environments that we can't fake or mock. The browser tests setup a document with an iframe (where js-ipfs runs) to run the tests in an actual real life real environment ftw

I've opened issues to deal with the caveats:
ipfs-inactive/ipfs-postmsg-proxy#2
ipfs-inactive/ipfs-postmsg-proxy#3
ipfs-inactive/ipfs-postmsg-proxy#4

However my priority now is to implement ipfs-inactive/ipfs-postmsg-proxy#5 which will allows us to add in the per method access control for ipfs companion.

I'll then do that in #333, and also implement a preference to enable/disable adding ipfs to the window

...and then, a first version can be shipped 🎉

@alanshaw
Copy link
Member

alanshaw commented Jan 8, 2018

Is this what we'd like for access control?:

  • Access control is fine grained, it requires permission to be granted to each IPFS function prior to usage, with option to allow access to all
  • Granted permissions are scoped to the origin (site) on which they are granted and persist until the browser process exits. When granting permission, the user is given the option for the grant to be "remembered" for a given origin so that it persists across browser restarts
  • Settings allow the user to revoke "remembered" privileges given to origins

@whyrusleeping
Copy link
Member

Thinking through this a bit, on the permissions model, It would be really nice to be able to limit which directories in the files api that pages have access to. Maybe give them /apps/DOMAIN/ or something?

@whyrusleeping
Copy link
Member

@alanshaw more like, restricting calls like ipfs files cp, ipfs files read, ipfs files ls etc to operating on paths under: /apps/example.com/ or so.

@victorb
Copy link
Member

victorb commented Feb 21, 2018

We need to scope things based on not just the origin but the path as well. So an IPFS application would be scoped to /apps/ipfs.io/ipfs/Qmasd/. It's actually quite a nice model, and could also open up ways of applications sharing data via common directories, kind of how KBFS (from Keybase) works. /keybase/private/victor is mine, /keybase/private/victor+why is mine + why's.

Edit, although, my first example will lead to some troubles when apps not using IPNS gets updated, as the data cannot be migrated in that case.

@alanshaw
Copy link
Member

I’m not sure I’m understanding you both properly...the permissions are scoped to origin + pathname.

If you allow to /apps/example.com then anything under it will get access, but /apps/another.com will not.

@whyrusleeping
Copy link
Member

@alanshaw I'm talking about paths within the ipfs files api.

@alanshaw
Copy link
Member

Ah, got ya. I didn't pick up that you meant the actual paths of files stored in IPFS.

@victorbjelkholm has suggested disabling all access to files.* until we can implement this. Which would allow us to ship this feature.

Thanks for taking the time to evaluate this @whyrusleeping - do you have any other concerns? I'm also wondering if there's more people that we should get to review this also...?

@alanshaw
Copy link
Member

So just to flesh this out a little - we're talking about the MFS specific functions of ipfs.files.

When accessing MFS functions via window.ipfs your changes are limited to directories with the same scope as your application and only if the user grants permission.

e.g. if you application is running at https://example.com/mycoolapp then you can only access files and directories in or under /https/example.com/mycoolapp

To distinguish this scope from general access permissions scope we'll call it mfs-scope.

  • files.cp - both source and destination path must be below mfs-scope
  • files.mkdir - path must be below mfs-scope
  • files.stat - path must be mfs-scope or below
  • files.rm - path must be below mfs-scope
  • files.read - path must be below mfs-scope
  • files.write - path must be below mfs-scope
  • files.mv - both source and destination path must be below mfs-scope
  • files.flush - path must be below mfs-scope
  • files.ls - path must be mfs-scope or below

For the beta release I'll temporarily disable access to the above functions until I can implement the scope rules.

@alanshaw
Copy link
Member

Is it worth namespacing access to companion? e.g. /companion/https/example.com/mycoolapp

@lidel
Copy link
Member Author

lidel commented Feb 22, 2018

I think it is a good idea, having https and https in root dir looks like a bad UX.
Perhaps /dapps/https/example.com/mycoolapp would be a better name? or /app-data/.. ?

@alanshaw
Copy link
Member

alanshaw commented Mar 2, 2018

I've been mulling this over and had a thought - should mfs-scope be transparent? So the app would read and write to /*, but that would be internally mapped onto /app-data/https/example.com/*?

I think that would be cool, but I don't know if there's any repercussions to doing this that I've perhaps not considered.

@lidel
Copy link
Member Author

lidel commented Mar 2, 2018

MFS is local and it should be fine for different apps to see different roots.
Transparent sandboxing of ipfs.files would be just a syntactic sugar, but makes developer's life more pleasant.

Question: is it possible to escape the sandbox via ../ or something?

@alanshaw
Copy link
Member

BREAKING NEWS ✨ PR is here ->> #431
Demo is here => https://youtu.be/KvgxAafphPg

@alanshaw
Copy link
Member

alanshaw commented Mar 27, 2018

@whyrusleeping @victorbjelkholm something that came up today was sharing data between different versions of an application.

The MFS scoping does a good job of stopping applications writing all over your stuff and each other's stuff but it means that if I have an app at ipfs.io/ipfs/Qm0 and I update it to ipfs.io/ipfs/Qm1 then ipfs.io/ipfs/Qm1 no longer has access to the files saved by ipfs.io/ipfs/Qm0, even though they are the same app.

It's been suggested that IPNS is the answer to this, but I wondered what your thoughts were?

We also discussed an alternative to MFS scoping - instead ask the user for permission to read/write to a directory (and sub directories).

@victorb
Copy link
Member

victorb commented Mar 27, 2018

Hmm.... Thinking out loud, might contain traces of mistakes.

The same problem appears with general application updates. Let's say you deployed application with hash ABC (which in MFS would be scoped appropriately), and now you have an update. Unless you have a mechanism for telling the user about the new hash (IPNS/application pubsub updates [?]/other), the user will be stuck at that version and that might be fine.

Instead, by using IPNS you'd have MFS scoped properly to the publishing key instead, where both application updates and MFS scoping would work correctly.

Asking for permissions is interesting though, because that also gives the application developer a way of requesting access to another applications data, for interop between apps. But there is also the risk of collision between apps (since the application developer and not the user is requesting access to a path [or maybe not? Maybe the user can change it to their liking ala Android with sdcards?])

Also, regarding requesting access, the user would have to keep track of which application has access to which path, as otherwise you could accidentally give access to the wrong directory, that might not be a great idea.

I'm leaning towards having things as transparent as possible (let IPFS applications be locked to /ipfs/asd and IPNS applications be locked to /ipns/asd) but I also see value in offering easy ways of doing data sharing between applications. I'm thorn.

@lidel
Copy link
Member Author

lidel commented Mar 27, 2018

On Pushing Security Decisions Onto Users

Maybe this will help:

Let's take a look at Origin-based security context that isolates local storage, cookies etc in the regular web. Note that there is no GUI for opting out of Origin-based isolation per website. We have low-level CORS dance just so that one page can access API of the other. But no GUI for escaping Origin sandbox. That is for a good reason.

Here comes my main fear: as soon as we introduce UI for apps to ask user for access outside of its default security perimeter, we will end up with Windows Vista UAC-like hell of nagging dialogs. Of course every app will want access to the root of your ipfs.files. Should it? IPFS introduces interesting space for innovation, as we can have zero-cost copies of the same data across multiple sandboxes.

I feel we should aim in the other direction: remove friction (dialogs) for every sandboxed API method. Look closely at various methods and decide which ones are safe to be executed without any ACL prompt.
Otherwise users will just learn to press "alllow" without reading, which defeats the purpose of ACLs.

@lidel
Copy link
Member Author

lidel commented Mar 27, 2018

MFS Chroot/Sandboxing Just Landed

PR #431 just got merged. This means your app's MFS root directory is based on the origin and path where your application is running.

Just for the record, I was playing with running this on various sites:

ipfs.files.write('/test-from-' + window.location.host, Buffer.from('ehlo'), { create: true }).then(() => console.log('done'))

and sandboxing of ipfs.files works as expected 👌

@lidel
Copy link
Member Author

lidel commented Apr 13, 2018

Closing this issue: IPFS Companion v2.2.0 shipped with window.ipfs enabled by default.

Remaining work related to refining security and UX constraints for window.ipfs will be tracked in #454.

The window.ipfs label can be used for tracking all issues related to this endeavor.

@lidel lidel closed this as completed Apr 13, 2018
@lidel lidel added the area/window-ipfs Issues related to IPFS API exposed on every page label May 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/window-ipfs Issues related to IPFS API exposed on every page kind/enhancement A net-new feature or improvement to an existing feature status/in-progress In progress topic/security Work related to security
Projects
None yet
Development

No branches or pull requests

9 participants