Restricted filesystem access with sel_ldr?

[JT Olds]

Hey folks,

I'm trying to execute some untrusted Python code safely, and I thought NaCl could help. I'm not in the context of a browser or anything.

I downloaded naclports and built Python from that repo. I can definitely run Python from NaCl:

[GCC 4.4.3 20141204 (Native Client r14191, Git Commit 7faaabb9f10e6dcae5f2b799da43e236e65cda95)] on nacl

Unfortunately, every invocation I can find with NaCl to start Python through sel_ldr passes the "-a" option, which is really confusing to me. I definitely don't want the "-a" option, but I do want to be able to provide the Python standard library to Python inside of NaCl.

With sel_ldr, how can I provide file tree additions to the virtual filesystem inside NaCl? I'd love to be able to point sel_ldr at some folder and make it its root (either via copying or chrooting or who knows). 

"-a" is labeled as not very safe (allows arbitrary filesystem access in addition to "other" syscalls), so I'd either rather not use it for untrusted code, or have my notions that it is unsafe disabused.

Thanks!

-JT

[JT Olds]

Or perhaps there's some other way of loading the Python standard library (and other files) into NaCl that I've missed?

 

[JF Bastien]

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

That would keep sel_ldr as-is for current uses, but make in pluggable by developers with usecases similar to yours.

The NaCl team hasn't implemented this, so I wouldn't count on one of us tackling this soon. Are you interested in discussing a design and then contributing patches? Our open source contribution process is pretty straightforward :-)

--
You received this message because you are subscribed to the Google Groups "Native-Client-Discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to native-client-di...@googlegroups.com.
To post to this group, send email to native-cli...@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-discuss.
For more options, visit https://groups.google.com/d/optout.

[JT Olds]

Yes! I am interested. Pretty swamped right now, but this would make a $dayjob project much more straightforward, so I'll be getting to it soon. I'll familiarize myself with the code, but in the meantime what should I know/look at first?

Also, LD_PRELOAD is fine of course, but reduces discoverability and configurability of the feature a bit - why not a command line option as well, out of curiosity?

You received this message because you are subscribed to a topic in the Google Groups "Native-Client-Discuss" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/native-client-discuss/5UFlj0e5fvw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to native-client-di...@googlegroups.com.

[JF Bastien]

On Fri, Jun 12, 2015 at 3:09 PM, JT Olds <jto...@xnet5.com> wrote:

Yes! I am interested. Pretty swamped right now, but this would make a $dayjob project much more straightforward, so I'll be getting to it soon. I'll familiarize myself with the code, but in the meantime what should I know/look at first?

Also, LD_PRELOAD is fine of course, but reduces discoverability and configurability of the feature a bit - why not a command line option as well, out of curiosity?

I suggest looking at the sel_ldr code, and then proposing a design on this mailing list. I have no preference for how this should work, besides being easily testable and secure. 

[Victor Khimenko]

On Fri, Jun 12, 2015 at 2:13 AM, JT Olds <ma...@jtolds.com> wrote:

Hey folks,

I'm trying to execute some untrusted Python code safely, and I thought NaCl could help. I'm not in the context of a browser or anything.

I downloaded naclports and built Python from that repo. I can definitely run Python from NaCl:

[GCC 4.4.3 20141204 (Native Client r14191, Git Commit 7faaabb9f10e6dcae5f2b799da43e236e65cda95)] on nacl

Unfortunately, every invocation I can find with NaCl to start Python through sel_ldr passes the "-a" option, which is really confusing to me. I definitely don't want the "-a" option, but I do want to be able to provide the Python standard library to Python inside of NaCl.

With sel_ldr, how can I provide file tree additions to the virtual filesystem inside NaCl?

There are no such facility, unfortunately. Fine-grained access is handled by browser's PPAPI plugin, not sel_ldr (actually latest versions of Chrome don't use sel_ldr at all).

 

I'd love to be able to point sel_ldr at some folder and make it its root (either via copying or chrooting or who knows). 

I'm afraid you'll need to change sel_ldr for such use and/or provide your own container process.

 

"-a" is labeled as not very safe (allows arbitrary filesystem access in addition to "other" syscalls), so I'd either rather not use it for untrusted code, or have my notions that it is unsafe disabused.

It's definitely not safe and is not intended for production use, only for development.

[JT Olds]

Okay! Finally starting to get to this. I take it by filesystem.c you meant sys_filename.c. 

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase? I'm afraid to check it out), and NaClSysOpen's job is solely for the sel_ldr commandline program and is not at all used in browser. Is that true? Just trying to figure out how wide the scope is of some of these files. 

Should I be posting these kinds of questions somewhere else?

Thanks again. 

[Mark Seaborn]

On 23 June 2015 at 15:29, JT Olds <jto...@xnet5.com> wrote:

On Fri, Jun 12, 2015 at 7:20 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

On Fri, Jun 12, 2015 at 3:09 PM, JT Olds <jto...@xnet5.com> wrote:

Yes! I am interested. Pretty swamped right now, but this would make a $dayjob project much more straightforward, so I'll be getting to it soon. I'll familiarize myself with the code, but in the meantime what should I know/look at first?

Also, LD_PRELOAD is fine of course, but reduces discoverability and configurability of the feature a bit - why not a command line option as well, out of curiosity?

I suggest looking at the sel_ldr code, and then proposing a design on this mailing list. I have no preference for how this should work, besides being easily testable and secure. 

Okay! Finally starting to get to this. I take it by filesystem.c you meant sys_filename.c. 

Yes.

 

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase?

No, the same set of syscalls is registered whether NaCl is running inside Chromium or in standalone sel_ldr.  That is why NaClSysOpen checks the NaClAclBypassChecks flag -- because the open() syscall is always registered, but it disables itself by default in Chromium and in sel_ldr.

I actually intend to change that so that syscalls are registered conditionally.  The idea would be to make it easier to switch out sys_filename.c, at runtime, for another implementation that implements restricted file access.

 

I'm afraid to check it out), and NaClSysOpen's job is solely for the sel_ldr commandline program and is not at all used in browser. Is that true? Just trying to figure out how wide the scope is of some of these files. 

Should I be posting these kinds of questions somewhere else?

native-client-discuss is fine, though another option is to use native-client-dev, which is for discussing development of NaCl itself (https://groups.google.com/forum/#!forum/native-client-dev).

Cheers,

Mark

[JT Olds]

On Tue, Jun 23, 2015 at 5:10 PM Mark Seaborn <msea...@chromium.org> wrote:

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase?

No, the same set of syscalls is registered whether NaCl is running inside Chromium or in standalone sel_ldr.  That is why NaClSysOpen checks the NaClAclBypassChecks flag -- because the open() syscall is always registered, but it disables itself by default in Chromium and in sel_ldr.

Huh! Okay, so that's a bit confusing. You're saying that there's no working file open syscall in browser mode? How does https://naclports.storage.googleapis.com/builds/pepper_41/trunk-176-g9b9e342/publish/devenv/pnacl/app/bash.html work?

I actually intend to change that so that syscalls are registered conditionally.  The idea would be to make it easier to switch out sys_filename.c, at runtime, for another implementation that implements restricted file access.

Oh! Yeah, I'm trying to add configuration for an implementation that restricts file access myself. Definitely don't want to step on any toes.

 

[Mark Seaborn]

On 23 June 2015 at 17:11, JT Olds <jto...@xnet5.com> wrote:

On Tue, Jun 23, 2015 at 5:10 PM Mark Seaborn <msea...@chromium.org> wrote:

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase?

No, the same set of syscalls is registered whether NaCl is running inside Chromium or in standalone sel_ldr.  That is why NaClSysOpen checks the NaClAclBypassChecks flag -- because the open() syscall is always registered, but it disables itself by default in Chromium and in sel_ldr.

Huh! Okay, so that's a bit confusing. You're saying that there's no working file open syscall in browser mode? How does https://naclports.storage.googleapis.com/builds/pepper_41/trunk-176-g9b9e342/publish/devenv/pnacl/app/bash.html work?

That uses the "nacl_io" library, which overrides open() and implements it in user code.

 

I actually intend to change that so that syscalls are registered conditionally.  The idea would be to make it easier to switch out sys_filename.c, at runtime, for another implementation that implements restricted file access.

Oh! Yeah, I'm trying to add configuration for an implementation that restricts file access myself. Definitely don't want to step on any toes.

Don't worry, you're not stepping on any toes!

Cheers,

Mark

[JT Olds]

On Tue, Jun 23, 2015 at 6:17 PM Mark Seaborn <msea...@chromium.org> wrote:

On 23 June 2015 at 17:11, JT Olds <jto...@xnet5.com> wrote:

On Tue, Jun 23, 2015 at 5:10 PM Mark Seaborn <msea...@chromium.org> wrote:

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase?

No, the same set of syscalls is registered whether NaCl is running inside Chromium or in standalone sel_ldr.  That is why NaClSysOpen checks the NaClAclBypassChecks flag -- because the open() syscall is always registered, but it disables itself by default in Chromium and in sel_ldr.

Huh! Okay, so that's a bit confusing. You're saying that there's no working file open syscall in browser mode? How does https://naclports.storage.googleapis.com/builds/pepper_41/trunk-176-g9b9e342/publish/devenv/pnacl/app/bash.html work?

That uses the "nacl_io" library, which overrides open() and implements it in user code.

Ohhh interesting, okay. My initial goal was to run the naclports Python standalone without passing the "-a" option (so it can access the stdlib files but is otherwise safe). Is it possible to use nacl_io without a browser and sel_ldr for this purpose?

[Victor Khimenko]

No. Yes. Maybe. Confused yet? You should be.

nacl_io does not implements open() from the thin air, it emulates it on top of PPAPI and sel_ldr does not provide PPAPI. Which is "no" part of the answer.

However there are exist another wrapper for sel_ldr called sel_universal which implements some (but not all!) PPAPI interfaces. That's "yes" part of the answer.

However sel_universal is not something we actively support: it was important part of the infrastructure back when NaCl was young, but today NaCl tests are run mostly in browser. And AFAIK sel_universal was never part of the SDK, it only used by NaCl itself. Perhaps it's good idea to extend sel_universal instead? I'm not sure - thus "maybe" part of the answer.

Mark should know more about sel_universal's future plans...

[Mark Seaborn]

On 24 June 2015 at 01:18, Victor Khimenko <kh...@chromium.org> wrote:

On Wed, Jun 24, 2015 at 5:10 AM, JT Olds <jto...@xnet5.com> wrote:

On Tue, Jun 23, 2015 at 6:17 PM Mark Seaborn <msea...@chromium.org> wrote:

On 23 June 2015 at 17:11, JT Olds <jto...@xnet5.com> wrote:

On Tue, Jun 23, 2015 at 5:10 PM Mark Seaborn <msea...@chromium.org> wrote:

I'm a bit puzzled about the Native Client codebase - I only see the NACL_sys_open (for example) syscall registered once via grepping (in service_runtime/nacl_syscall_list.c) in the entire codebase, and it's registered to sys_filename.c's NaClSysOpen, which checks if NaClAclBypassChecks is set (the -a option to sel_ldr) and bails if it isn't. This must mean nexes running in Chromium have a different syscall registration table in some other codebase (maybe the Chromium codebase?

No, the same set of syscalls is registered whether NaCl is running inside Chromium or in standalone sel_ldr.  That is why NaClSysOpen checks the NaClAclBypassChecks flag -- because the open() syscall is always registered, but it disables itself by default in Chromium and in sel_ldr.

Huh! Okay, so that's a bit confusing. You're saying that there's no working file open syscall in browser mode? How does https://naclports.storage.googleapis.com/builds/pepper_41/trunk-176-g9b9e342/publish/devenv/pnacl/app/bash.html work?

That uses the "nacl_io" library, which overrides open() and implements it in user code.

Ohhh interesting, okay. My initial goal was to run the naclports Python standalone without passing the "-a" option (so it can access the stdlib files but is otherwise safe). Is it possible to use nacl_io without a browser and sel_ldr for this purpose?

No. Yes. Maybe. Confused yet? You should be.

nacl_io does not implements open() from the thin air, it emulates it on top of PPAPI and sel_ldr does not provide PPAPI. Which is "no" part of the answer.

However there are exist another wrapper for sel_ldr called sel_universal which implements some (but not all!) PPAPI interfaces. That's "yes" part of the answer.

sel_universal no longer has that support for PPAPI.  That was removed a long time ago.  (It was based on the SRPC-based PPAPI proxy.)  The only remaining use for sel_universal is for testing the sandboxed PNaCl translator.

Cheers,

Mark

[JT Olds]

On Fri, Jun 12, 2015 at 1:47 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

Working on a patch to add this, but I have some questions about loading shared object portability. 

Would a patch make it through review that added dlopen/dlsym calls directly, or will this break Windows builds? Would it be better to try and add a platform-specific layer, or only conditionally compile in the pluggable filesystem filter module on POSIX?

-JT

[JF Bastien]

Could you provide more details of how you suggest doing this? Maybe in a Google doc, which would make it easier to get a clear picture without reading this entire thread.

To address your question, I think it would be desirable to have sel_ldr support this functionality on Linux/Windows/OSX, since it's essentially just opening a shared library and calling a C function. LoadLibrary works in a similar way as dlopen in that respect (more complex usecases are different). 

[JT Olds]

On Wed, Jun 24, 2015 at 10:29 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

On Wed, Jun 24, 2015 at 9:21 AM, JT Olds <jto...@xnet5.com> wrote:

On Fri, Jun 12, 2015 at 1:47 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

Working on a patch to add this, but I have some questions about loading shared object portability. 

Would a patch make it through review that added dlopen/dlsym calls directly, or will this break Windows builds? Would it be better to try and add a platform-specific layer, or only conditionally compile in the pluggable filesystem filter module on POSIX?

Could you provide more details of how you suggest doing this? Maybe in a Google doc, which would make it easier to get a clear picture without reading this entire thread.

Good idea. Here you go! 

https://docs.google.com/document/d/1O9c1TEDzQdey5ENFM8UN4ELDKIye9hC9JxOl8FFPs0Y/edit?usp=sharing 

I'm not usually a C developer, nor have I ever written a dlopen plugin system before, so I probably have no idea what I'm doing from a design perspective. So please take a very critical look at this - definitely interested in learning.

-JT

[Mark Seaborn]

As Victor replied, nacl_io usually uses PPAPI to use the browser-provided filesystem APIs.  I think nacl_io also has some support for in-memory filesystems, which might or might not be useful for your purposes.  In principle that should work outside of the browser without PPAPI being present, though I haven't checked whether that is the case.

Cheers,

Mark

[Mark Seaborn]

Are you interested in having a restricted file access mode in the stock version of sel_ldr?  If so, I don't think we need dynamically-loadable plugins for that.  If the functionality is fairly simple and generic, I would be happy to put the functionality directly into the NaCl codebase and statically link it into sel_ldr, and add some new command line options to sel_ldr.

Would that work for you?  What command line options would you want for granting access to files?  Would the functionality you want be fairly general-purpose, or do you envisage needing some app-specific rules?

So, the simplest approach would currently be:

• Extend sys_filename.c to check filenames against a whitelist, when the whitelist is enabled.
• Add command line options to sel_main.c for setting up a file/directory whitelist.

Otherwise, if you want to have more complex rules for open() etc., I would recommend building a custom version of sel_ldr, similar to how sel_ldr_seccomp is built.  sel_ldr_seccomp is a version of sel_ldr that uses a Seccomp-BPF-based outer sandbox on Linux for extra security (see src/trusted/seccomp_bpf/).  It uses a hook for enabling the Seccomp-BPF sandbox after startup.  We could add similar hooks for replacing syscall implementations.  When we build sel_ldr_seccomp, we're basically treating the NaCl runtime as a library that can be embedded.

Cheers,

Mark

[JT Olds]

On Wed, Jun 24, 2015 at 3:05 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 10:22, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 10:29 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

On Wed, Jun 24, 2015 at 9:21 AM, JT Olds <jto...@xnet5.com> wrote:

On Fri, Jun 12, 2015 at 1:47 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

Working on a patch to add this, but I have some questions about loading shared object portability. 

Would a patch make it through review that added dlopen/dlsym calls directly, or will this break Windows builds? Would it be better to try and add a platform-specific layer, or only conditionally compile in the pluggable filesystem filter module on POSIX?

Could you provide more details of how you suggest doing this? Maybe in a Google doc, which would make it easier to get a clear picture without reading this entire thread.

Good idea. Here you go! 

https://docs.google.com/document/d/1O9c1TEDzQdey5ENFM8UN4ELDKIye9hC9JxOl8FFPs0Y/edit?usp=sharing 

I'm not usually a C developer, nor have I ever written a dlopen plugin system before, so I probably have no idea what I'm doing from a design perspective. So please take a very critical look at this - definitely interested in learning.

Are you interested in having a restricted file access mode in the stock version of sel_ldr?  If so, I don't think we need dynamically-loadable plugins for that.  If the functionality is fairly simple and generic, I would be happy to put the functionality directly into the NaCl codebase and statically link it into sel_ldr, and add some new command line options to sel_ldr.

Oh alright, we can do that too. The plugin approach was suggested by JF Bastien, and it seemed pretty reasonable to me.

Would that work for you?  What command line options would you want for granting access to files?  Would the functionality you want be fairly general-purpose, or do you envisage needing some app-specific rules?

I think we could get by with some simple options. I would envision something like

  -d <path>   If provided, allows restricted filesystem access as if <path> was the new root folder. Does not allow access to files outside of <path>. 

It seems like it would be nice to have further configurability about which subfolders are read only and which subfolders are read/write. So, just thinking while typing, perhaps `-d` by default causes everything to be read only, and you can supply zero or more subfolders to be read/write with another flag?

 

Otherwise, if you want to have more complex rules for open() etc., I would recommend building a custom version of sel_ldr, similar to how sel_ldr_seccomp is built.  sel_ldr_seccomp is a version of sel_ldr that uses a Seccomp-BPF-based outer sandbox on Linux for extra security (see src/trusted/seccomp_bpf/).  It uses a hook for enabling the Seccomp-BPF sandbox after startup.  We could add similar hooks for replacing syscall implementations.  When we build sel_ldr_seccomp, we're basically treating the NaCl runtime as a library that can be embedded.

Yeah I don't currently need complication like that.

[JT Olds]

On Wed, Jun 24, 2015 at 3:27 PM Petr Hosek <pho...@google.com> wrote:

I have already prepared a design document for the restricted filesystem access. Please feel free to comment on the document if there is anything you feel is missing.

https://docs.google.com/a/chromium.org/document/d/1q4LNB0hdA0H7vYmYuClRICZHYZoxXX9LPKjnSf2Kqng/edit?usp=sharing

We currently don't have any timeline for when this'll be implemented. Is this something you would like to work on?

Requested access. And sure! I would like to see a feature like this soon, and I'd be happy to implement it.

-JT

[JT Olds]

On Wed, Jun 24, 2015 at 3:52 PM Petr Hosek <pho...@google.com> wrote:

It should be accessible now.

Looks like I don't have comment access, but I have two comments so I'll comment here.

First, I think taking the lead from other systems (Docker) for the UX is a great impulse, but in this case it will unnecessarily complicate the implementation to have to keep track of what is essentially a virtual file system with path mappings and mount points. Simply using an interface similar to chroot (instead of Docker's -v), in conjunction with users using bind mounts where appropriate, will give 100% of the functionality with a much, much simpler implementation (add a path prefix to all cleaned paths). I earlier proposed letting sel_ldr control read/write access, but frankly that isn't strictly necessary, as the filesystem could do that securely just fine.

Second, I totally get wanting to reduce the attack surface area of code that will be running in Chrome, but here we're talking about a single if in each syscall. That might be too much, but adding a check (like we already do with the NaclBypassAclChecks global) takes waaaay less code than adding a whole slew of new syscall registration tables and implementations. Overall, adding these new implementations seems like it will cause *more* auditing work, instead of less, just simply due to how simple this feature could be added directly.

If we vastly simplified the configuration UX (to be more like chroot, maaaybe with a whitelist of subpaths that get read/write, and everything else is read only) so that the implementation was simpler, and we made it so this didn't take essentially creating a whole new syscall registration table, then I'd be wholeheartedly on board.

-JT

[Sam Clegg]

As somebody else who is looking for similar features for a different
project I agree that the simpler approach makes more sense to me.
I'm looking for something that works like a read-only chroot.

cheers,
sam

[Mark Seaborn]

On 24 June 2015 at 14:14, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 3:05 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 10:22, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 10:29 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

On Wed, Jun 24, 2015 at 9:21 AM, JT Olds <jto...@xnet5.com> wrote:

On Fri, Jun 12, 2015 at 1:47 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

Working on a patch to add this, but I have some questions about loading shared object portability. 

Would a patch make it through review that added dlopen/dlsym calls directly, or will this break Windows builds? Would it be better to try and add a platform-specific layer, or only conditionally compile in the pluggable filesystem filter module on POSIX?

Could you provide more details of how you suggest doing this? Maybe in a Google doc, which would make it easier to get a clear picture without reading this entire thread.

Good idea. Here you go! 

https://docs.google.com/document/d/1O9c1TEDzQdey5ENFM8UN4ELDKIye9hC9JxOl8FFPs0Y/edit?usp=sharing 

I'm not usually a C developer, nor have I ever written a dlopen plugin system before, so I probably have no idea what I'm doing from a design perspective. So please take a very critical look at this - definitely interested in learning.

Are you interested in having a restricted file access mode in the stock version of sel_ldr?  If so, I don't think we need dynamically-loadable plugins for that.  If the functionality is fairly simple and generic, I would be happy to put the functionality directly into the NaCl codebase and statically link it into sel_ldr, and add some new command line options to sel_ldr.

Oh alright, we can do that too. The plugin approach was suggested by JF Bastien, and it seemed pretty reasonable to me.

I talked to JF, and I think he thought I wanted dynamically-loadable extensions, though that wasn't actually the case. :-)  If you have dynamically-loadable extensions, it raises questions such as what interface sel_ldr exports to the extension, and whether you try to make that interface a stable ABI.  Having dynamically-loadable extensions is strictly more complicated than statically-linkable extensions, and I'd rather make sure we can do the latter well first.

 

Would that work for you?  What command line options would you want for granting access to files?  Would the functionality you want be fairly general-purpose, or do you envisage needing some app-specific rules?

I think we could get by with some simple options. I would envision something like

  -d <path>   If provided, allows restricted filesystem access as if <path> was the new root folder. Does not allow access to files outside of <path>. 

That sounds good to me.  I'd be happy to review a change that implements that.

sel_ldr already has an option called "-d", so you'd have to pick another option name.  You could potentially use a long option (i.e. like "--foo") -- sel_main.c uses getopt_long(), though currently only on Linux.

Do you have any requirements about symlinks?  Symlinks pose some awkward problems, so in a first version it would be easiest to disallow creating symlinks.

 

It seems like it would be nice to have further configurability about which subfolders are read only and which subfolders are read/write. So, just thinking while typing, perhaps `-d` by default causes everything to be read only, and you can supply zero or more subfolders to be read/write with another flag?

That sounds OK too.

Cheers,

Mark

[JT Olds]

On Wed, Jun 24, 2015 at 5:13 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 14:14, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 3:05 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 10:22, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 10:29 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

On Wed, Jun 24, 2015 at 9:21 AM, JT Olds <jto...@xnet5.com> wrote:

On Fri, Jun 12, 2015 at 1:47 AM 'JF Bastien' via Native-Client-Discuss <native-cli...@googlegroups.com> wrote:

This is a somewhat-frequent request, which nobody has taken the time to tackle. The solution the NaCl team had discussed for this was to change sel_ldr to default to its current filesystem.c's implementation, or (if provided on the command-line) use a .so which overrides this behavior with your own filters.

Working on a patch to add this, but I have some questions about loading shared object portability. 

Would a patch make it through review that added dlopen/dlsym calls directly, or will this break Windows builds? Would it be better to try and add a platform-specific layer, or only conditionally compile in the pluggable filesystem filter module on POSIX?

Could you provide more details of how you suggest doing this? Maybe in a Google doc, which would make it easier to get a clear picture without reading this entire thread.

Good idea. Here you go! 

https://docs.google.com/document/d/1O9c1TEDzQdey5ENFM8UN4ELDKIye9hC9JxOl8FFPs0Y/edit?usp=sharing 

I'm not usually a C developer, nor have I ever written a dlopen plugin system before, so I probably have no idea what I'm doing from a design perspective. So please take a very critical look at this - definitely interested in learning.

Are you interested in having a restricted file access mode in the stock version of sel_ldr?  If so, I don't think we need dynamically-loadable plugins for that.  If the functionality is fairly simple and generic, I would be happy to put the functionality directly into the NaCl codebase and statically link it into sel_ldr, and add some new command line options to sel_ldr.

Oh alright, we can do that too. The plugin approach was suggested by JF Bastien, and it seemed pretty reasonable to me.

I talked to JF, and I think he thought I wanted dynamically-loadable extensions, though that wasn't actually the case. :-)  If you have dynamically-loadable extensions, it raises questions such as what interface sel_ldr exports to the extension, and whether you try to make that interface a stable ABI.  Having dynamically-loadable extensions is strictly more complicated than statically-linkable extensions, and I'd rather make sure we can do the latter well first.

 

Would that work for you?  What command line options would you want for granting access to files?  Would the functionality you want be fairly general-purpose, or do you envisage needing some app-specific rules?

I think we could get by with some simple options. I would envision something like

  -d <path>   If provided, allows restricted filesystem access as if <path> was the new root folder. Does not allow access to files outside of <path>. 

That sounds good to me.  I'd be happy to review a change that implements that.

sel_ldr already has an option called "-d", so you'd have to pick another option name.  You could potentially use a long option (i.e. like "--foo") -- sel_main.c uses getopt_long(), though currently only on Linux.

Oh sure, right, I'm not tied to a particular flag title; I'm not sure I really liked -d anyway. I'll try to pick a good one.

 

Do you have any requirements about symlinks?  Symlinks pose some awkward problems, so in a first version it would be easiest to disallow creating symlinks.

At least in the case of running Python with its standard library, symlinks would probably be useful (symlinks are often used for Python installations, though true, not necessary). Thoughts on making symlink support an additional option? I don't want to add too many options lest this start to get less straightforward than just "add a path prefix to cleaned paths when requested".

It seems like it would be nice to have further configurability about which subfolders are read only and which subfolders are read/write. So, just thinking while typing, perhaps `-d` by default causes everything to be read only, and you can supply zero or more subfolders to be read/write with another flag?

That sounds OK too.

Actually, I think I've brought myself around to concluding we should just have one flag that does read/write, and if the user wants read-only, they should just make the folder read only for the user sel_ldr is running as.

Thanks for all the feedback!

 

Cheers,

Mark

--
You received this message because you are subscribed to a topic in the Google Groups "Native-Client-Discuss" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/native-client-discuss/5UFlj0e5fvw/unsubscribe.
To unsubscribe from this group and all its topics, send an email to native-client-di...@googlegroups.com.

[Mark Seaborn]

On 24 June 2015 at 15:40, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 3:52 PM Petr Hosek <pho...@google.com> wrote:

It should be accessible now.

Looks like I don't have comment access, but I have two comments so I'll comment here.

First, I think taking the lead from other systems (Docker) for the UX is a great impulse, but in this case it will unnecessarily complicate the implementation to have to keep track of what is essentially a virtual file system with path mappings and mount points. Simply using an interface similar to chroot (instead of Docker's -v), in conjunction with users using bind mounts where appropriate,

What is the difference between the bind mounts that you're proposing and the Docker-like "--volume" option that Petr was proposing?  They sound like the same thing to me.

Anyhow, I would agree that copying any existing system (such as Docker) is not a particular requirement.

 

will give 100% of the functionality with a much, much simpler implementation (add a path prefix to all cleaned paths). I earlier proposed letting sel_ldr control read/write access, but frankly that isn't strictly necessary, as the filesystem could do that securely just fine.

Second, I totally get wanting to reduce the attack surface area of code that will be running in Chrome, but here we're talking about a single if in each syscall. That might be too much, but adding a check (like we already do with the NaclBypassAclChecks global) takes waaaay less code than adding a whole slew of new syscall registration tables and implementations. Overall, adding these new implementations seems like it will cause *more* auditing work, instead of less, just simply due to how simple this feature could be added directly.

I agree that refactoring how syscalls are registered is not a requirement for adding this feature.  Adding conditionals into sys_filename.c is OK.

Cheers,

Mark

[JT Olds]

On Wed, Jun 24, 2015 at 5:30 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 15:40, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 3:52 PM Petr Hosek <pho...@google.com> wrote:

It should be accessible now.

Looks like I don't have comment access, but I have two comments so I'll comment here.

First, I think taking the lead from other systems (Docker) for the UX is a great impulse, but in this case it will unnecessarily complicate the implementation to have to keep track of what is essentially a virtual file system with path mappings and mount points. Simply using an interface similar to chroot (instead of Docker's -v), in conjunction with users using bind mounts where appropriate,

What is the difference between the bind mounts that you're proposing and the Docker-like "--volume" option that Petr was proposing?  They sound like the same thing to me.

Totally equivalent in functionality. The difference is we don't write any code for bind mounts. I'm proposing we just do a chroot-type feature, and if someone wants arbitrary mappings, they do bind mounts themselves, unrelated to sel_ldr. sel_ldr would do no bind mounting of its own. You'd have to use your own operating system tools (such as bind mounts) to put together whatever folder structure you wanted before even running sel_ldr.

One other comment from before:

>  If you have dynamically-loadable extensions, it raises questions such as what interface sel_ldr exports to the extension, and whether you try to make that interface a stable ABI.  Having dynamically-loadable extensions is strictly more complicated than statically-linkable extensions, and I'd rather make sure we can do the latter well first.

If we do ever get there, I attempted to address ABI and forwards compatibility in https://docs.google.com/document/d/1O9c1TEDzQdey5ENFM8UN4ELDKIye9hC9JxOl8FFPs0Y/edit

 

[Petr Hosek]

I have already prepared a design document for the restricted filesystem access. Please feel free to comment on the document if there is anything you feel is missing.

https://docs.google.com/a/chromium.org/document/d/1q4LNB0hdA0H7vYmYuClRICZHYZoxXX9LPKjnSf2Kqng/edit?usp=sharing

We currently don't have any timeline for when this'll be implemented. Is this something you would like to work on?

[Petr Hosek]

It should be accessible now.

[Petr Hosek]

Yes, I already have a design proposal for adding this functionality and there is an issue for tracking the progress. I've started implementing the functionality a few weeks ago, but haven't made much progress as I had to handle some other issues with higher priority. I hope to get a prototype ready within next few weeks; I recommend subscribing to the issue to be notified once I make some progress.

On Thursday, June 11, 2015 at 4:13:41 PM UTC-7, JT Olds wrote:

Hey folks,

I'm trying to execute some untrusted Python code safely, and I thought NaCl could help. I'm not in the context of a browser or anything.

I downloaded naclports and built Python from that repo. I can definitely run Python from NaCl:

[GCC 4.4.3 20141204 (Native Client r14191, Git Commit 7faaabb9f10e6dcae5f2b799da43e236e65cda95)] on nacl

Unfortunately, every invocation I can find with NaCl to start Python through sel_ldr passes the "-a" option, which is really confusing to me. I definitely don't want the "-a" option, but I do want to be able to provide the Python standard library to Python inside of NaCl.

With sel_ldr, how can I provide file tree additions to the virtual filesystem inside NaCl? I'd love to be able to point sel_ldr at some folder and make it its root (either via copying or chrooting or who knows). 

"-a" is labeled as not very safe (allows arbitrary filesystem access in addition to "other" syscalls), so I'd either rather not use it for untrusted code, or have my notions that it is unsafe disabused.

Thanks!

-JT

[JT Olds]

Hey Petr!

We talked a bunch about your proposal already on this thread. Did you have thoughts there?

-JT

--

[JT Olds]

On Wed, Jun 24, 2015 at 5:13 PM Mark Seaborn <msea...@chromium.org> wrote:

Do you have any requirements about symlinks?  Symlinks pose some awkward problems, so in a first version it would be easiest to disallow creating symlinks.

Whoops, so you're absolutely right, symlinks pose a bunch of problems, and it's not just the creation of them. I was assuming that it would be easy for existing symlinks to be resolved relative to the new untrusted root location, but whoops, the host syscalls are often resolving the symlinks for me, and those resolutions will be relative to the true (and trusted) root (if I pass through a stat call, without doing lstats myself, for instance).

So that's probably what you meant, and yeah I guess we'll just say that for v1, symlinks look like real files, resolved to whatever they point to outside of the untrusted root, and you can't create your own symlinks, and you have to be careful when you put symlinks inside of your untrusted root.

Not quite what I was expecting, but certainly much easier to implement that way.

Thoughts?

[JT Olds]

I suppose this also means lstat should be implemented using the host stat call, instead of host lstat.

[JT Olds]

K, pushed a draft up that is not complete (left some TODO(jtolds) comments), but I'm very interested in feedback on the overall approach. 

https://codereview.chromium.org/1211173002/

I'll continue to wrap this up.

-JT

[Mark Seaborn]

On 24 June 2015 at 16:33, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 5:30 PM Mark Seaborn <msea...@chromium.org> wrote:

On 24 June 2015 at 15:40, JT Olds <jto...@xnet5.com> wrote:

On Wed, Jun 24, 2015 at 3:52 PM Petr Hosek <pho...@google.com> wrote:

It should be accessible now.

Looks like I don't have comment access, but I have two comments so I'll comment here.

First, I think taking the lead from other systems (Docker) for the UX is a great impulse, but in this case it will unnecessarily complicate the implementation to have to keep track of what is essentially a virtual file system with path mappings and mount points. Simply using an interface similar to chroot (instead of Docker's -v), in conjunction with users using bind mounts where appropriate,

What is the difference between the bind mounts that you're proposing and the Docker-like "--volume" option that Petr was proposing?  They sound like the same thing to me.

Totally equivalent in functionality. The difference is we don't write any code for bind mounts. I'm proposing we just do a chroot-type feature, and if someone wants arbitrary mappings, they do bind mounts themselves, unrelated to sel_ldr. sel_ldr would do no bind mounting of its own. You'd have to use your own operating system tools (such as bind mounts) to put together whatever folder structure you wanted before even running sel_ldr.

Ah, I see what you mean -- thanks for clarifying.  I didn't realise that by "bind mount" you meant Linux's bind mount facility.

Cheers,

Mark

[dsa...@gmail.com]

The new "restricted filesystem" feature (-m flag) is great, thank you all for designing and adding it.

I have a difficulty caused by the decision to always allow modifications, relying on the user to make the folder read-only if they want to restrict writing to it. The issue is that "chmod" is still allowed. If there is just a regular user (not superuser) creating the folder and running sel_ldr, the untrusted code will run as the owner of the files and will be able to chmod and modify them.

Any suggestions on how to restrict untrusted code from writing?

[JT Olds]

On Thu, May 26, 2016, 1:07 AM <dsa...@gmail.com> wrote:

The new "restricted filesystem" feature (-m flag) is great, thank you all for designing and adding it.

I have a difficulty caused by the decision to always allow modifications, relying on the user to make the folder read-only if they want to restrict writing to it. The issue is that "chmod" is still allowed. If there is just a regular user (not superuser) creating the folder and running sel_ldr, the untrusted code will run as the owner of the files and will be able to chmod and modify them.

Any suggestions on how to restrict untrusted code from writing?

I take it that it's not possible to change the owner of the files to some other user before invoking sel_ldr? If you have root to some degree, you could either change the owner of the files or the user running sel_ldr to not match. 

You could maybe even make a small setuid binary to do so, or even just make sel_ldr setuid for some untrusted user.

Does that work? Or does your context not even have that flexibility? Certainly seems like we could disable chmod.

[dsa...@gmail.com]

No, my use case is for the plain old unprivileged user who has no root to any degree, but wants to give partial read access to untrusted code.

It's possible that disabling chmod is sufficient, but it would be better yet if a user could give read-only access to untrusted code without limiting his own access. I.e. I may have regular directories with files that I work with outside the sandbox, and untrusted code may read them, but isn't trusted to modify.

I guess I am hoping for a "--read-only" option, which restricts the mode for opening files, and blocks all fs-modifying syscalls (chmod, symlink, mkdir, etc.) with EACCESS error.

Also, loosely related: the current support for symlinks is a bit broken (ok, technically there is just no support, but in reality, they sometimes work but not always quite right). Would it make sense to fully resolve symlinks when translating paths (i.e. like realpath does) to make symlinks fully work, and be fully restricted to the -m directory?

Dmitry

[dsa...@gmail.com]

I am continuing to think about more flexible mounting in the sandbox, and symlink support. The interface I propose is:

   -m /host-path[:/virt-path:flags] Where flags may be "ro" for read-only or "rw" for read-write. When the optional part is omitted, it's interpreted as "/host-path:/:rw", just like existing -m usage. Multiple -m options may be given.

(This is inspired by docker's -v option, which was mentioned earlier on this thread.)

I worry that all of this is overkill, so I seek your input.

I have an implementation in mind, and a good chunk written. As described in filesystem_access.txt, it would translate all virtual paths to canonicalize them AND resolve symlinks (interpreting them within the virtual filesystem). The fully resolved absolute virtual path will then be translated to the host path using the longest matching -m option, which will also determine whether modifying operations are allowed on this path (if "ro" flag was specified, various syscalls would return EACCESS for such paths).

I have convinced myself (and would like to convince others) that the performance hit of resolving symlinks isn't terrible, since symlinks need to be resolved either way, whether by us or by the OS.

I see one serious roadblock to finishing and presenting an implementation. There is a race condition that opens a security hole: no matter how well we translate paths to be absolute and symlink-free, right before we get to, say, the underlying open() syscall, another thread could come in and move paths around, introducing a symlink into our *resolved* path that would cause the open() to reach outside of the mounted directories.

Today, the protections against this are: symlink() call is disallowed, and user is warned not to have any symlinks in the mounted directory. (This is mentioned in some of today's implementation comments.)

A *proper* solution seems to require locking of paths from the time they are seen during translation to the completion of the filesystem operation, where any rename() or symlink() calls have to wait if any lock affects them.

This seems too involved, so I'm seeking input on whether this is needed by anyone else, and whether anyone has simpler ideas. For my narrow purposes, what would be sufficient is a read-only mount (that prohibits all modifying syscalls) with full symlink following (even if they point outside). I feel that that's not compatible with the current approach, so I was hoping for something more general that could be committed upstream (rather than maintaining my own fork).

Thanks all!

Dmitry