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

lua_sandbox requires patched dependencies #142

Closed
sathieu opened this issue Jun 14, 2016 · 11 comments
Closed

lua_sandbox requires patched dependencies #142

sathieu opened this issue Jun 14, 2016 · 11 comments

Comments

@sathieu
Copy link
Contributor

sathieu commented Jun 14, 2016

Lua_sandbox currently requires:

This currently blocks the creation of a proper Debian package.

@sathieu
Copy link
Contributor Author

sathieu commented Jun 14, 2016

@trink: What is the plan about this? Do you plan to send patches upstream?

@trink
Copy link
Contributor

trink commented Jun 15, 2016

No the changes are specific to the sandbox. I haven't had any packaging issues on Debian or Redhat. The sandbox version of Lua successfully co-exists with the real lua package and there are no issues with the shared Lua module libraries since they are installed under luasandbox.

@trink trink closed this as completed Jun 15, 2016
@rhertzog
Copy link
Contributor

@sathieu and I are Debian packagers. Debian generally frowns upon maintaining multiple copies of the same software. When a security issue is found in lua, we would have to update lua and also the patched version used by luasandbox. Thus it would be best if the features needed by luasandbox could be merged in lua upstream.

Also download of code at build time is not allowed, so we have to work around your build system. Maybe you could use git submodule to directly include the required external deps?

@trink
Copy link
Contributor

trink commented Jun 15, 2016

Interesting. To your first point I would love if it was the 'same' software unfortunately it is not and never will be so I am stuck maintaining the forks (the sandbox integrity cannot be achieved without the changes so it should be thought of as a different piece of software)

To the second point a git submodule is still a download from an external source. The current build system fetches code from a git hub repo (pinned to a SHA) and is functionally equivalent. Also, I recently moved to pulling down a tarball of the lpeg library since the Lua project has no official repository (the many copies people have created on github are out of date and creating yet another seemed pointless). The md5sum of the downloaded bits are verified before they are used. The current build process is 100% reproducible and reasonably safe. With that being said, I am open to any suggestions that can improve the process since going forward more external code will be integrated.

@rhertzog
Copy link
Contributor

Ok so it would be nice to have a way to generate a tarball ("make dist"?) with all the external deps included and on top of which we could build lua_sandbox without any modification. Is this something that you could do? It would be even better if those tarballs could be uploaded as release tarball on github. (BTW last official release dates back to 2014...)

For the first point about the patched lua, I understand what you mean but I would have hoped that the features you need to ensure integrity of the sandbox could be merged in the upstream lua in a non-intrusive way (i.e. something that could be enabled/disabled dynamically). But I have zero knowledge here so I might be off the track.

@trink
Copy link
Contributor

trink commented Jun 24, 2016

That is flipped the externals are not dependencies (with the exception of lua). So would you prefer if there was just a lua_sandbox core and all the modules were separate packages e.g., lsb_circular_buffer.deb, lsb_cjson.deb etc... that depend on the core (as opposed to building a bundle like it is now?)

@rhertzog
Copy link
Contributor

@trink Yes that would certainly be better.

@trink
Copy link
Contributor

trink commented Jul 12, 2016

The modified lua code is now included in this repo and built into libluasandbox (libluasb is gone) and the package name is luasandbox dropping the 'core' since a component build is no longer necessary.

Also all extensions (modules and sandboxes) have been moved to https://github.com/mozilla-services/lua_sandbox_extensions/ and produce individual packages for each extension (luasandbox-extension name) dependent on the luasandbox package (plus any extension specific dependencies e.g. openssl).

@sathieu
Copy link
Contributor Author

sathieu commented Jul 12, 2016

@trink Thanks.

@sathieu
Copy link
Contributor Author

sathieu commented Aug 19, 2016

@trink I prefer to have the patched lua in its own repository (i.e like trink/lua-cjson from lua_sandbox_extensions). This will ease packaging from our side (i.e copy the vanilla lua-5.1 packaging, then modify). This will also ease security fixes application (if any). Can you revert this part of 856593a?

In the longterm, we want to use vanilla lua if possible. Would it be possible to forward those patches for lua-5.3?

Same for lua-cjson.

Also can you confirm that vanilla lua_struct package is used now (and not your fork)?

FYI we have setup a team to package hindsight and its dependencies in Debian (transitively Ubuntu). Current status report is at: http://lists.alioth.debian.org/pipermail/pkg-heka-maint/Week-of-Mon-20160815/000000.html

Thanks

@trink
Copy link
Contributor

trink commented Aug 19, 2016

Your original suggestion of removing all external dependencies from the core library has worked quite well. The changes for the lib, os, and math modules will never be accepted as upstream changes so the sandbox will have to be thought of as sharing a compatible core API with Lua 5.1 but not actually Lua 5.1 (there is no plan to move to Lua 5.3). This change allowed the two sandbox libraries to be combined into one reducing some confusion and complexity. As for forking an unofficial repo to create a new distribution, it buys us nothing; if the official 5.1 distribution changes is relatively easy to create a patch and apply it to the lua directory in this repo. In the past the official version was pulled down and patched but managing a committed patch file was even more tedious.

As stated before the lua-cjson changes are sandbox specific and not applicable to the upstream distribution so a fork of the official repo will be maintained.

Yes lua-struct fetches the official distribution.

Thanks for your work and feedback; the packaging and distribution has greatly improved.

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

3 participants