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

Sandboxed server review #8

Merged
merged 6 commits into from Nov 29, 2014

Conversation

cauthmann
Copy link
Contributor

This is an attempt to "sandbox" RInside in a separate process. This should prevent buggy or malicious R code from interfering with your application - which is always useful, especially when the R code is user-submitted.
The exposed client API provides similar functionality to RInside, with slightly different syntax. Custom data types can be supported (assuming they implement serialization methods in addition to the Rcpp wrappers), and the R code can call back into provided C++ functions.

I don't think it's possible or wise to tightly integrate this code into RInside; hence it lives in a separate directory in the examples.

This code hasn't seen more than two eyes yet, so please review and comment!

What you will need to run this:

  • RInside installed with RINSIDE_CALLBACKS (it's disabled by default, see inst/include/RInsideConfig.h)
  • Rcpp 0.11.3 (or later)
  • A C++11 compatible compiler
  • A POSIX-like system that supports sockets, signals, fork and others. I have only tested this on linux. Feedback from other POSIX systems or even windows is appreciated.
cd inst/examples/sandboxed_server/
make
# in one terminal, run
./example_server
# in a different terminal, run
./example_client

I've split the code into five separate commits for review; I suggest reading them commit by commit. If this gets accepted, I'll be happy to provide a different history for the actual pulling.

Currently, I've only implemented a separate standalone server that listen()s for clients. For my needs, this increases security (running the server as a different user) and improves performance (R is initialized only once, then fork()ed for each client).
I've chosen the abstraction in a way that should make it easy to switch to a solution where the application spawns a new process and communicates over a shared pipe - which is more suitable for a desktop application. I'm not currently interested in implementing that though.

@eddelbuettel
Copy link
Owner

About to leave town for a few days of workshopping so may not get to it "soon".

Couple of really quick first comments:

  • Keeping it in examples/ is a good idea, it won't break anything
  • On the other hand, "nobody sees it" that way. Just for argument's sake, should it be a separate project? "RProccessInside" or "RInside2" or something? Or move up into RInside?
  • All my code (or at least the code that matters) is GPL-2. If you stick it in here, I am not sure your BSD headers matter. It may just became GPL-2 as the whole aggregate does anyway as all of R, Rcpp and RInside are GPL-2 so why not make matters simpler and convert this to GPL-2 as well?

All that said, I do of course love a nice and well put-together pull request. And as it does not harm, I'm happy to take it. I was just wondering how to best give it more exposure...

…er passing, making conversion errors non-fatal

remove MIT licenses
@eddelbuettel
Copy link
Owner

Sorry. I had missed the update four days ago. I think technically speaking you are supposed to specify a license next to the copyright. I'll merge this in now.

eddelbuettel added a commit that referenced this pull request Nov 29, 2014
@eddelbuettel eddelbuettel merged commit d757a50 into eddelbuettel:master Nov 29, 2014
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

Successfully merging this pull request may close these issues.

None yet

2 participants