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

Permissions on a module-level #171

Closed
rivertam opened this issue Jun 6, 2018 · 13 comments
Closed

Permissions on a module-level #171

rivertam opened this issue Jun 6, 2018 · 13 comments

Comments

@rivertam
Copy link

rivertam commented Jun 6, 2018

So I love the concept of privileged vs. unprivileged. During the talk at JSConf, Ryan recognizes that a linter should not be allowed to, for example, make network requests. I think this is a powerful concept, but I also think it's not quite powerful enough.

JS and in particular node/npm gets a bad rep these days for security. First of all, I think this is unwarranted generally speaking, as the kinds of things people point out are equally accurate in most package managers/languages/runtimes, if not more so. However, if deno is going to consider permissions to be a language/runtime level concern, I think it should go a step further.

For some context, I want to point to the article I've seen most on this topic. The concern in this particular article is a little different than the one I'm proposing deno attempt to solve because deno is backend and the article proposes a frontend security issue, but it's certainly related and if deno is going to end up with similar usecases to node, of course we will be using it on the front end as well. The issue is that a node module N dependencies deep can do arbitrary things like read files, read environment variables, make requests to servers, etc., and the end user won't necessarily ever know about it (and the middle package maintainers very well might not either).

Sandboxing on an application-level is a start, but I just don't think it goes far enough for practical needs. I think an amazing module/dependency system (which I think deno is trying to create) should be able to "sandbox" individual module/dependencies, and that this should even be the default behavior.

Unfortunately, barring switching back to require, all of the ideas I have either rely on using some kind of manifest such as package.json, a language-level change, some kind of string-related kludge, or mandatory dependency injection. In my opinion, the first two are the most reasonable (knowing absolutely nothing about deno internals).


The manifest solution is, of course, quite simple. You can just add permissions to modules that would cascade down. An example given using typical package.json structure:

{
  "dependencies": {
    "https://unpkg.com/somepackage@0.0.1/somepackage.min.js": {
      "permissions": {
         "http": true,
         "fs": true
       }
    },
    "https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js": {
      "permissions": {
          "*": true
       }
    }
  }
}

A language level solution might include a tweak to import such as import a from 'https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js' permit(http, fs) or a more Rusty solution which I kind of like better:

import a from 'https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js';

await a.initializeSafely();

// ...

%permit(http, fs) {
  await a.doSomethingDangerous();
}

The reason I like this solution better is because it now makes it even harder for package writers to hijack permissions just because they happen to be using them elsewhere. That is to say, now only doSomethingDangerous can make http requests, while initializeSafely remains safe. So the package writers can't use the permissions you give them just because you import them.

I think this is harder to enforce at runtime in general but especially in-browser, but I'd leave that to ebwckpa or whomever.

Arguably, this is also similar to what Haskell might do, or just similar to how async/await works with the implicit "pre-emption" permission; you need to yield power to it for it to use that power. If arbitrary commands could allow pre-emption, that would be inherently insecure as much of our code is written with the understanding that it will be executed synchronously.


A string related kludge is very similar to modifying import:

import a from 'https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js permit(http, fs)';

I don't love it but it would be relatively easy to implement on the surface level, I believe, as it's just a matter of resolution middleware.


Mandatory dependency injection would essentially disallow libraries from importing system APIs. This is kind of nice for all the reasons dependency injection is nice, and annoying for all the reasons dependency injection is annoying.

import makeA from 'https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js';
import fs from 'fs';
import http from 'http';

const a = makeA({ fs, http });

await a.doSomethingDangerous();

I could also see this being combined with an import modification:

import makeA from 'https://unpkg.com/highlytrustedpackage@1.2.3/ok.min.js' with fs=fs, http=http;

wherein the inner package would not be allowed to import 'fs' without the place where it's imported having explicitly defined this behavior. The issue here is a lack of flexibility (such as a lower ability to mock-in-place).

In an ideal world (and I do actually think this would be the best solution possible), deno would disallow importing system libraries from inside modules and allow grand overrides of inner module dependencies, with additional configuration allowed at runtime. This would enable the following usecases, almost all of which are more ergonomic if it's allowed at runtime:

  • Allowing inner modules to easily and ergonomically offer custom replacements
    • Bluebird instead of native promises, for example
  • Mocking during testing (think Jest, but available all the time as a first-class citizen)
  • Initialization parameters (no need to separately call mod.initialize({ /* ... */ }))

Very curious to hear what everyone's thoughts are.

@rivertam
Copy link
Author

rivertam commented Jun 6, 2018

I also want to apologize for bringing this up as I know it's perhaps not highest priority, but I feel like this is the kind of thing that's really hard to put in later than the very early stages. I think it's more helpful than harmful.

@balupton
Copy link
Contributor

balupton commented Jun 7, 2018

I could still add a bitcoin miner to my web server module that has local caching.

Security shouldn't be all or nothing.

Little Snitch already has me covered perfectly for network access. However, nothing has me covered against rm -Rf besides backups. Apple's efforts in Mojave seem a great solution to the issue for mac apps - when a privilege for a particular thing is requested bubble it up to the operator - it is their system after all, not the random module developers all trusting each others code and their own.

@rivertam
Copy link
Author

rivertam commented Jun 7, 2018

@balupton I'm not sure what you're saying, pretty much at all, to be honest.

I could still add a bitcoin miner to my web server module that has local caching.

Are you saying this is the case with the current (node/deno) system, or are you saying this with module-level permissions? In the latter case, I'd argue that I'm not recommending flawless security but still rather significant peace of mind. The issue isn't typically that request or express has a hidden bitcoin miner, but rather that is-array does, and it's used indirectly by 7 of your dependencies, all of which are written by Facebook, Airbnb, etc. but they didn't have the manpower to read through 200mb of minified code.

As for Apple's efforts, I don't use OSX but that sounds like app-level permissions, which Deno already offers, rather than module-level permissions, which I'm recommending.

@gillchristian
Copy link
Contributor

doing something similar to async/await seems like a good idea. As it would make very explicit what parts of the code are using different permissions.

One concern about that is, how can I know the code I imported is not using it? How do I give permission?

Here's my 2 cents:

// package https://foo.ts

// async like syntax to request fs permission
export fs function foo() {
  // writes to disk
}

// file.ts
import { foo } from 'https://foo.ts'

foo() // this shouldn't work

permit_fs foo() // await like syntax to grant fs permission

@Janpot
Copy link

Janpot commented Jun 7, 2018

@gillchristian If there's top-level await, maybe something like

import { allow } from 'deno';
const foo = await import(allow({
  write: true,
  net: true
}, 'https://foo.ts'));

Or, if decorators on import statements would be a thing

import { allow } from 'deno';
@allow({
  write: true,
  net: true
})
import foo from 'https://foo.ts';

@gillchristian
Copy link
Contributor

gillchristian commented Jun 7, 2018

@Janpot looks good for module level (which is the topic of the issue). My suggestion was function oriented, which is yet more granular.

From my snippet, what if that foo package did also export another function that does not need permission slip?

// package https://foo.ts

// async like syntax to request fs permission
export fs function foo() {
  // writes to disk
}

export function bar() {
  // no permissions needed for this one
}

One could rise many questions to a function level vs. module level permissions, in the same way module level vs. app level permissions does.

@ry
Copy link
Member

ry commented Jun 8, 2018

This would indeed be cool - but it seems very difficult to make this secure. I would not be surprised if traversing the object graph in some manner from an unprivileged module wouldn't result in a reference to privileged functions. The only security we can truly guarantee is at the VM boundary.

@rivertam
Copy link
Author

rivertam commented Jun 8, 2018

@ry I agree, but I don't know if that's necessarily a deal-breaker on this issue. My thinking would be that the functions could still exist, but we would disable them (no sending or receiving to Golang, or requires some kind of 'hard-coded'/transpiled security key. I've been thinking about ways to make this secure a lot and I've discounted pretty much everything I've seen or written so far for various reasons, but I'll continue brainstorming. I think this could be kind of a game-changer for module systems (the ease-of-use of npm-style systems with the peace of mind of C++), so it's pretty important to me.

@ry
Copy link
Member

ry commented Jun 8, 2018

@rivertam Maybe it's possible with V8's context? It might be worth discussing with some V8 people....

@mathiasbynens Sorry to summon you into a random issue - but is it possible to have secure module level permissions as described above?

@mathiasbynens
Copy link

Looping in @hashseed.

@hashseed
Copy link

hashseed commented Jun 8, 2018

I guess you could have a per native context granularity for permissions. I think there are just way too many holes if you have permissions per module. V8 has a way to perform access checks across native contexts, but not across modules.

I don't even know how you would define the semantics. What do you do if you call a function defined in a privileged module from an unprivileged module? What if you use eval to create new functions in the privileged module?

What I actually would feel safest about is if the security model follows that of the browser wrt site isolation. Modules with different privileges should run in different processes and isolates, and communicate via message passing. I think a model similar to Chrome's mojo would make some sense.

@lukejagodzinski
Copy link

lukejagodzinski commented Jun 8, 2018

In my opinion module level/file level/function level permissions is a little bit overkill. If you're going to import some file/package you should be responsible for checking what permissions it requires. I would propose different solution. Deno could allow checking permissions of a given file from the cli:

$ deno permissions https://unpkg.com/deno_testing@0.0.5/testing.ts

or even:

$ deno permissions 'import { test } from "https://unpkg.com/deno_testing@0.0.5/testing.ts"'

In the future, such functionality could be built into code editor.
Such command would result in something like:

https://unpkg.com/deno_testing@0.0.5/testing.ts - fs, http

or

import { test } from https://unpkg.com/deno_testing@0.0.5/testing.ts - fs

My thinking comes from the mobile application development. When you install some application you get prompted with all the permissions application requires. The same is true about current web. You can run whatever website but if it tries to access for example camera, you have to allow it to do so.

In Deno, it could be solved in the following way:

$ deno --allow-all - allow all modules to access everything
$ deno --allow-http - allow only http access
$ deno permissions - get list of all imported files and native modules that they're using

As @ry said on the JSConf:

Whenever you're designing a program there are things that you think might be cute to add in. You'll always regret those.

So just keep it simple :). But that's just my humble opinion :)

@ry
Copy link
Member

ry commented Jun 8, 2018

@mathiasbynens @hashseed Thank you.

@lukejagodzinski Agree - I like the idea of being prompted to gain access like the browser. The exact spelling of how this happens will have to be worked out.

Closing this issue. Permission model will remain as they are in the prototype.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants