allow rodsuser to run a rule string

[d-w-moore]

The command:

irule -r irods_rule_engine_plugin-python-instance python_rule null  null

should succeed if executed by a rodsuser.
In this context, the command line argument python_rule should be taken as the identifier of a python function (a rule) defined in the global namespace of /etc/irods/core.py.
Currently this command only succeeds for a rodsadmin. A rodsuser gets the errorSYS_NO_API_PRIV.

[korydraughn]

The following code is the reason why only rodsadmin users can use irule to invoke rules in core.py.

irods_rule_engine_plugin_python/irods_rule_engine_plugin-python.cxx

Lines 623 to 626 in 0a07fb9

	if ( ( client_user_authflag < REMOTE_PRIV_USER_AUTH ) || ( proxy_user_authflag < REMOTE_PRIV_USER_AUTH ) ) {
	rodsLog( LOG_DEBUG, "Insufficient privileges to run irule in Python rule engine plugin" );
	return ERROR( SYS_NO_API_PRIV, "Insufficient privileges to run irule in Python rule engine plugin" );
	}

If Boost.Python gives us a way to fetch all rules/functions inside of core.py, then we can use that to determine whether the rule text sent by the client should be executed.

[stsnel]

We've also been discussing this issue in the context of migrating completely from the legacy rule language RE plugin to the Python RE plugin. I was wondering if it would be a good solution to pass the rule text to a wrapper template in Python for checking and execution, rather than checking the rule text in the rule engine itself.

Provided that the wrapper template is configurable, this would also give system administrators a way to customize logic regarding what code can be run by rodsusers.

[korydraughn]

Can you provide an example demonstrating how this would look/work?

[stsnel]

Please find a code fragment that illustrates what I have in mind below.

In this example, the rule engine would insert the rule text with escaped quotes at the code = line at line 12. The current rule text in the code variable is just a random example. The code in the wrapper_main function is the actual wrapper. It checks that the rule text has only approved/safe Python code using the ast module before executing it. The code below the wrapper_main function is just some boilerplate / dummy code so that the example can run as a standalone script.

#!/usr/bin/env python3
#
# Example of how a Python wrapper could be used to check
# rule code for any insecure operations and execute it if
# approved.

import ast

# This should be the rule text the user wants to execute. This is just
# a random example from a Yoda rule that we might want to run as a
# non-rodsadmin user.
code = \
"""def main(rule_args, callback, rei):
    tests = global_vars["*tests"].strip('"')
    result = callback.rule_run_integration_tests(tests, "")
    callback.writeLine("stdout", result["arguments"][1])
"""


def wrapper_main(code, rule_args, callback, rei):
    # Wrapper that verifies whether the rule text is safe to run
    # and runs it.

    try:
        parsed_code = ast.parse(code)
    except SyntaxError:
        callback.writeLine("stdout", "Error: wrapper could not parse rule code.")
        exit(1)

    # Refuse to run it if there are any non-approved nodes in the abstract syntax tree
    for node in ast.walk(parsed_code):
        if isinstance(node, (ast.alias,
                             ast.arg,
                             ast.arguments,
                             ast.Assign,
                             ast.Attribute,
                             ast.Constant,
                             ast.Expr,
                             ast.Load,
                             ast.Module,
                             ast.Name,
                             ast.Pass,
                             ast.Store,
                             ast.Subscript
                             )):
            continue
        elif isinstance(node, ast.FunctionDef):
            arguments = [argument.arg for argument in node.args.args]
            if node.name == "main" and arguments == ['rule_args', 'callback', 'rei']:
                continue
        elif isinstance(node, ast.Call):
            function_name = None
            try:
                if "value" in vars(node.func):
                    function_name =  node.func.value.id
                else:
                    function_name = node.func.id
            except AttributeError:
                if isinstance(node.func.value, ast.Subscript):
                    function_name = node.func.value.value.id
            if function_name in ("global_vars", "callback"):
                continue

        callback.writeLine("stdout", "Refused AST node " + ast.dump(node))
        exit(1)

    # Run the rule code in a namespace
    exec(code, namespace)
    result = namespace["main"](None, callback, None)

# This dummy code is just here to enable executing the code standalone.
# It needs to be removed when running it in the Python rule engine.
class DummyCallback:
    def writeLine(self, output, message):
        print("Dummy writeline: " + message)

    def rule_run_integration_tests(self, a, b):
        print("Dummy run integration tests ...")
        return { "arguments": ["dummy response", "dummy response"] }

global namespace
namespace = { "global_vars": { "*tests": "foo" }}

# Invoke the wrapper code
wrapper_main(code, None, DummyCallback(),None)

For a production version of such a wrapper, it would be better to check the code using string functions, rather than trying to parse untrusted Python code, because of security risks related to parsing untrusted code.

[korydraughn]

I see. That would be part of the PREP's implementation. I guess performance wouldn't be an issue because this would only apply to the irule code path.

For a production version of such a wrapper, it would be better to check the code using string functions ...

What do you mean by string functions?

I think I know what you're referring to, but I don't want to assume anything.

[stsnel]

For a production version of such a wrapper, it would be better to check the code using string functions ...

What do you mean by string functions?

I meant using functions such as re.search, startswith, endswith and split to ensure that the rule text consists of safe commands. Something along the lines of:

import re
if re.match("^\s*callback\.\w+\([\w,\"]+\)$", line):
    # Insert more checks here
    pass  # Function calls to callback object are considered safe

[korydraughn]

Got it. We'll keep that in mind for when we start looking at this.

[luijs]

What we currently do is that we have a function in core.re that calls a function in core.py. This way we can let a user call a rule that we control as admins of the server.

[trel]

What we currently do is that we have a function in core.re that calls a function in core.py. This way we can let a user call a rule that we control as admins of the server.

Yes, this is probably the safest route and does not involve parsing any user-supplied Python code.