mathnmusic 131 days ago [-]
> Honestly when the Redis Lua engine was designed, it was not conceived with this security model of the customer VS the cloud provider in mind. The assumption kinda was that you can trust who pokes with your Redis server. So in general the Lua libraries were not scrutinized for security. The feeling back then was, if you have access to Redis API, anyway you can do far worse.

This is an interesting point. Cloud computing and managed/hosted services require a clear separation of what the host can do and what the customer (who's paying for the managed service) should be able to do.

Just today, our startup decided to use AWS Kinesis (as opposed to setting up Kafka ourselves), despite the vendor lock-in and closed-source nature of AWS components. :-/

zokier 131 days ago [-]
Yeah, these sorts of questions are typically something that goes under the umbrella of "multi-tenancy"; having proper access controls, isolation, accounting, logging, security model, etc, which incidentally also makes even trivial software explode with complexity. I haven't been paying attention, but I would have not thought Redis to be multi-tenant ready out of the box.
samatman 131 days ago [-]
Consensus in the community is that Lua(JIT) sandboxing must be done on the process level.

Even with the debug library stripped and other safeguards against (inner) evaluation taken, the trivial DOS of `while true do end` remains.

If that happens, you want it to live in its own process, or at least its own thread.

antirez 131 days ago [-]
Those kind of dos (while true do end), actually we handle quite well already, because we don't use LuaJIT but plain Lua that has hooks that allow Redis to detect such conditions. But in general, it's a "can of worms" indeed.
andrewmcwatters 131 days ago [-]
iirc and ime, that sort of provision in Lua only allows you keep running at a cost. You'll still encounter hitching because infinite loop detection is based on a detection of number of line executions.
antirez 131 days ago [-]
Yes, but in the case of Redis the fact that an user makes its own Redis server slower is not an issue basically. It is enough that we can detect this and kill the script.
cjcole 131 days ago [-]
Are your Lua changes public? I couldn't find a relevant repo. Thanks.
antirez 131 days ago [-]
Yes but there is no direct modification to the Lua code, we just install the hooks from the Redis source code using the Lua interpreter. So the implementation of that is directly inside the Redis source tree, scripting.c file.
aarongolliver 131 days ago [-]
If you have the time he has a great video here[0] that talks about redis+LUA

[0] https://www.youtube.com/watch?v=RaIwBagADVM

breakingcups 131 days ago [-]
I appreciate the (what feels like) honest and direct communication from Antirez very much. It's always such a breath of fresh air.
moby 131 days ago [-]
Heroku's updated their Redis fleet: https://blog.heroku.com/redis-vulnerability
ksec 131 days ago [-]
Somewhat off topic:

What Happened to the mRuby Scripting in Redis? I remember there were plans to make mRuby in Redis too. Given mRuby has had quite a bit of security audit in recent years that cost Shopify millions.

andrewmcwatters 131 days ago [-]
> To be fair, I think that the assumptions Lua makes about the stack are a bit too trivial, with the Lua library developer having to constantly check if there is enough space on the stack to push a new value.

What the fuck? This is almost never a concern for Lua C developers. If you're concerned with LUA_MAXCSTACK defaulted at 2048 and you're running out of space, you're doing something seriously wrong and need to reevaluate how you're using the Lua C API.

antirez 131 days ago [-]
Hello Andrew, unfortunately the max limit is hit only if you make sure to call the appropriate function that will reallocate the stack if needed. I think that it would be saner for Lua, by default, just to accept stack pushes and resize the stack if needed. I've the feeling that this would not be a huge performance issue because most of the times it's just a check and no reallocation is needed. As I wrote in the blog post, note that other languages do not force you to think at such low level when using their C API. In general Lua implementation is great, but the API exposing the stack, really poses just a barrier that I feel is kinda pointless compared to providing the classical argv/argc API of Python, Ruby, Tcl, ...
MattJ100 131 days ago [-]
The reasoning behind not automatically growing the stack was given by the Lua authors recently: http://lua-users.org/lists/lua-l/2018-04/msg00036.html
antirez 131 days ago [-]
Thanks for the relevant info.
ufo 131 days ago [-]
The stack-based API is the trick that allows Lua to have much saner memory management than the alternatives. Unlike Python, C code using the Lua API does not need to manually keep track of reference counts. Unlike Ruby, Lua's garbage collector is accurate and can be coded in pure and portable C.

I agree with you that needing to keep track of the stack size is a pain though. I also wish it would just grow on demand as needed. That said, when you are doing things "by hand" the default stack limit of 10 slots per function call frame is often enough. It could be worse :)

samatman 131 days ago [-]
Lua's minimalist design philosophy means everything is a tradeoff. If there's a way to write an allocator which can handle a growing stack, while adding only ~10k to the runtime, I think you could get buy-in on that.

Keeping the stack on the lua_State has clear advantages, such as allowing the library to be reentrant. Handling malloc and free in a single pass for each lua_State is simpler than alternatives and fits well-enough with existing practices.

samatman 131 days ago [-]
I am curious what kind of workload is prompting you to want this. I've only ever seen stack overflows from predictable recursive errors, which is generally a pretty shallow bug.

Second observation: LuaJIT might be the better platform to pursue this approach. Since the tracer is already on, stack overflows could potentially be another cold path that gets handled on spill; performance impact could be minimal, perhaps none if existing trace guards can be used.

andrewmcwatters 131 days ago [-]
No, it wouldn't be saner. You're completely ignoring memory allocation concerns. You should almost never reach 2048 values on the stack to begin with. It's intentionally arbitrarily high.

Reviewing the commit in question: you have a function that blindly accepts any number of arguments. ~2047 arguments in a function is not an acceptable design choice for Lua. I can't see any language where I'd design an API for users where they could insert an upwards of ~2047 arguments and say to myself that's a good idea.

Your misuse of the Lua C API here does not grant a special use case, nor does it justify entire language implementation changes.

Your number one concern for hardening starts at the stack? And further more you think it's a good idea to compile a custom version of Lua it to avoid stack issues instead of just writing sane bindings? You would really go out of your way to do this?

Maybe instead of saying there's a problem with X, Y or Z, ask yourself if you're doing something wrong first.

Allow developers to pass in a table of values to pack instead of an arbitrary amount of arguments, push the values by index onto the stack, work with them, then pop them off. How is this difficult? What makes your use case special? Why would you avoid conventional Lua use?

vararg (..., select, {...} convention) usage in Lua almost always involves the use of tables.

munificent 131 days ago [-]
Your aggressive, insulting communication style is making you less effective and undermining your own goals with communicating.
andrewmcwatters 131 days ago [-]
I'm fully aware of the tone I'm passing off: take it or leave it. I have no vested interest in this. I'm simply a gawking spectator. Learn to accept input from others who don't give it to you on a silver platter.

The otherwise timid and passive voice on HN is frequently an irritation to me. This is the author of Redis making an uninformed decision in both writing Lua bindings, and extending that ignorance to making changes to Lua to suit his ignorance. I don't care. What my tone is suggesting is how ludicrous this is and how it shouldn't be taken as a good suggestion.

This is a great example of someone who has a large exposure simply using something incorrectly and him having a large exposure doesn't make his technical observations of a language meaningful. In no way here can you leverage that to justify poor decision making.