Re: [Monotone-devel] [Patch] mtn automate lua

[Thomas Keller]

Stephen Leake schrieb:
>> So I'd like to include this in mainline (tests and documentation will be
>> added, of course) if other people find this useful as well and are ok
>> with the implementation.
> 
> The code looks simple and straight-forward (except see one comment
> below). Although I can appreciate that it took a while to get it
> working!

Thanks ;)

>> I'm still a bit undecided when it comes to security constraints, like
>> should a couple of hooks be disabled (i.e. should I maintain a list of
>> "supported" hooks) or should I just leave this out into the wild as is?
> 
> As I understand the current security model, there are two layers. If
> there is a network connection, that connection has its own
> authentication/validation method, which we can assume works. Then
> there is a key required for write access to the database.

The problem is that if you do networking with automate stdio (like f.e.
Thomas Moschny's json server) there is no such security model at all,
i.e. netsync permissions are simply skipped / not used. You can still
put arbitrary contents over this interface into the database, via the
put_file, put_revision and cert commands.

>> However, this might be a completly different cup of tea if we open / use
>> the automation interface for networking - of course we do not want to
>> allow anybody even with write access to a database being able to
>> retrieve any kind of the local password. 
> 
> That's a good point. In terms of the two layers above, this adds a
> third layer, requiring protection of "local data". Not really another
> layer, just covers different data.

Yes, one could speak of "local data" here, but it might not always be
easy to draw the line here. Are uncommitted changes in a local workspace
("local data") already something which we want to protect from being
read by others? (Sure, it might not make sense in first place to manage
a workspace via stdio over a network, but what do I know about weird use
cases of our users?!)

>> Maybe we then need to introduce some kind of marking for each
>> automate command, like "secure_for_local_use" vs.
>> "secure_for_foreign_use" and check if automate stdio is connected to
>> a socket? 
> 
> I think that makes sense. Although I'm not sure it's easy to tell what
> automate stdio is connected to. We could probably set a global state
> variable in the top level main procedure, where it is clear what stdio
> is connected to.

I haven't seens Thomas Moschny's code yet - I thought we could get away
with an additional (internal) option for the automate stdio call.

>> +function hook_wrapper(func_name, ...)
>> +    -- evaluate each single string argument to resolve types
>> +    -- like nil's, table's and others - the select('#', ...) syntax is
>> +    -- borrowed from http://lua-users.org/wiki/StoringNilsInTables to
>> +    -- let this code properly work for nil arguments as well
>> +    local args = {n=select('#',...), ...}
>> +    for i=2,args.n do
>> +        args[i] = assert(loadstring("return " .. args[i]))()
> 
> It might be good to specify a more informative message in `assert' here.
> I think this is where syntax and "not defined" problems in the
> arguments will be detected, and a generic "assertion failed" will not
> be much help. Printing out the actual argument that caused the failure
> might be enough.
> 
> In the tests, include some that have erroneous arguments, to show what
> kind of error messages you get.

Thanks for the pointer, I found a problem with the argument evaluation
code (apparently this whole part was skipped due to a stupid bug of
mine), so this has been fixed and wrong arguments are now messaged to
the user:

$ ./mtn au lua echo 3 "{[1]=;}"
mtn: warning: [string "<std hooks>"]:1195: argument {[1]=;} could not be
evaluated
mtn: misuse: lua call 'echo' failed

There is another special failure: String arguments need to be given with
extra quotes, so

$ ./mtn au lua echo "'foo'" or
$ ./mtn au lua echo '"foo"'

works, while a simple

$ ./mtn au lua echo "foo" or
$ ./mtn au lua echo 'foo'

does not, since "foo" is evaluated as unknown global variable in these
cases and thus gets "silently" set to nil.

This is handled differently:

mtn: warning: [string "<std hooks>"]:1197: argument foo was evaluated to nil
mtn: misuse: lua call 'echo' failed

All these changes are part of nvm.experiment.automate-lua
(7c9aa18564dc4826aae41f1e9fab25614757ad4c).

Thomas.

-- 
GPG-Key 0x160D1092 | address@hidden | http://thomaskeller.biz
Please note that according to the EU law on data retention, information
on every electronic information exchange might be retained for a period
of six months or longer: http://www.vorratsdatenspeicherung.de/?lang=en

signature.asc
Description: OpenPGP digital signature