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

Investigate reintroducing sandbox support #114

Open
TysonAndre opened this Issue Aug 17, 2017 · 3 comments

Comments

Projects
None yet
1 participant
@TysonAndre
Copy link
Member

TysonAndre commented Aug 17, 2017

This was disabled earlier on for the following reasons:

  • Unclear documentation about supported php 5 versions (It's available in php 5.6)
  • Earlier focus on method and function manipulation.

Restore code, Check if it's viable to port this to php 7. Aim to reintroduce runkit_lint first.

@TysonAndre

This comment has been minimized.

Copy link
Member Author

TysonAndre commented Aug 26, 2017

runkit_lint is still broken. There are a lot of C static variables in files and various state in the interpreter, I'm not sure which of those is affected.
And the sandbox code hasn't been ported from php5.

  • The original code was probably an use of the thread switching API that PHP authors won't support, which isn't really that documented.
    Modifying a currently running thread might start breaking other things in PHP7.
  • Need to double check if runkit_lint is supposed to be aware of classes that were already declared. I don't think so

It might be possible to implement runkit_lint by switching to a different thread, initializing that thread (PHP init), performing the import and checking for compiler errors, shutting down the new thread, then reporting those errors/exceptions from that thread

  • I'm not sure which SAPI to use. This may require copying code from an SAPI, e.g. CLI.

Another possibility is to try to fork a different process(sharing memory) and communicate with that, but that might not work on windows.

@TysonAndre

This comment has been minimized.

Copy link
Member Author

TysonAndre commented Aug 26, 2017

https://github.com/runkit7/runkit7/tree/wip-restore-sandbox contains work on that, and a large number of debugging statements that should be removed.

TysonAndre added a commit that referenced this issue Oct 1, 2017

Disable options for sandbox support, keeping incomplete work
This is closer to compatibility with php 7, but nothing is working.

Related to #114

TysonAndre added a commit that referenced this issue Oct 1, 2017

work on restoring runkit_lint. Currently broken
This leaves in debugging code, which will need to be removed
when implementation is finished.

Disable options for sandbox support, keeping incomplete work

This is closer to compatibility with php 7, but nothing is working.

Related to #114

Don't attempt compilation of sandbox for windows builds.

Only build master branch

TysonAndre added a commit that referenced this issue Oct 1, 2017

work on restoring runkit_lint. Currently broken
This leaves in debugging code, which will need to be removed
when implementation is finished.

Disable options for sandbox support, keeping incomplete work

This is closer to compatibility with php 7, but nothing is working.

Related to #114

Don't attempt compilation of sandbox for windows builds.

Only build master branch

fix valgrind builds
@TysonAndre

This comment has been minimized.

Copy link
Member Author

TysonAndre commented Feb 10, 2019

https://github.com/krakjoe/sandbox is a potential alternative.

It may also be useful as a reference for implementing this feature in runkit7, but may force this to rely on pthread.

  • Earlier work was blocked by not being able to switch between thread context objects - At the time, it looks like there was probably some global state that I couldn't identify or feasibly modify, and documentation was sparse.
  • I haven't tested https://github.com/krakjoe/sandbox yet. It supports php 7.1+
  • krakjoe/sandbox depends on <pthread.h> (actual threading)

Maybe prefixing the code to be evaluated in the sandbox with <?php if (some_true_cond) { return or throw; }?>rest of code would be a substitute for runkit_lint($code)

  • (check for ParseError or CompileError )
  • I wouldn't count on it.
  • pthreads requires linux/unix

https://blog.krakjoe.ninja/2019/01/boxes-of-sand.html mentions that the original approach to sandbox support may be a dead end, and clarifies some things:

Runkit "switched" between the parent and the sandbox context to execute code as if it were in another thread, but there was never actually another thread, so this extremely old documentation is very misleading. For the sake of nostalgia, I don't intend to fix the documentation, it has been that way for 13 years, and the feature is all but dead, so it's not affecting anyone anymore.

The only people who care about TSRM/ZTS in PHP are the windows people, and myself. The windows people need it for Apache, and I need it for pthreads, what's more I recognise the value in the abilities that TSRM provides. So when PHP 7 quietly improved the performance of ZTS with improvements to TSRM, nobody really noticed, I've even heard people say TSRM is going to be removed. It isn't.

Those improvements broke the ability to abuse TSRM as Runkit did, for many boring reasons, runkits sandbox cannot work as it did before, it's not possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment