cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
329 stars 93 forks source link

restricted edit-run #2687

Open hjoliver opened 6 years ago

hjoliver commented 6 years ago

Depends on better authentication and authorization - #1901

Production suite operators sometimes need to change certain job parameters on the fly before re-running a task, but letting them edit the full job script (and hence the ability to run arbitrary code on the job host) may be seen as a major risk.

We should support a restricted edit run, where (depending on authorization) only variables identified as "editable" in the suite definition are presented to the user, with appropriate metadata, for editing.

matthewrmshin commented 6 years ago

Some quick thoughts:

hjoliver commented 6 years ago

Broadcast ... yes, I suspect that those who insist on these kinds of restrictions are not aware of broadcast yet, but unlike edit runs, the automatic operation of a suite may depend on automatic broadcast. I guess that's fine so long as our yet-to-be-designed authorization system can distinguish suite tasks from users.

hjoliver commented 6 years ago

Interface: it's probably reasonable to assume that this edit-ability applies only to environment variables, so presenting a list of VAR=VALUE n a temporary text file (via CLI) or in web form of some kind (new GUI) would do it.

matthewrmshin commented 6 years ago

Job directives (including execution time limit) are probably the other most edited settings.

hjoliver commented 6 years ago

Good point - job directives definitely need to be in the mix.

oliver-sanders commented 4 years ago

In the new UI it will be possible to issue broadcasts via the right-click menu (ideally with auto-completion) which might be sufficient to close this?

hjoliver commented 4 years ago

I don't see how that solves the "restricted" bit of this? Unless we can restrict what broadcasts are accepted/rejected by a task based on authorization settings. (I must admit I'm not convinced this a a desirable thing to support, anyway ... it sounds like something security people would insist on - which is what motivated this issue originally, not mentioning any names - but no one would actually use in reality?).

matthewrmshin commented 4 years ago

Just chipping in.

The problem is that broadcast and edit run are ultimate logic injection.

It does not matter how good your UI or authorisation policies are - if a user is able to modify the job script with a change to e.g. an environment value setting that can call a sub-shell, then no security model can help you.

The best you can do is to have a schema for your broadcast or edit-run to restrict changes to given settings, and each setting will have to have a list of acceptable values. The server will have to reject anything that does not fit the schema - and suite designers will need to be educated in order to design suites with the correct schema for good security.

oliver-sanders commented 4 years ago

(having now properly read the issue)

The best you can do is to have a schema for your broadcast or edit-run to restrict changes to given setting

I think we've effectively got that in the form of rose optional configurations.

The problem is that broadcast and edit run are ultimate logic injection.

We could potentially add an authorisation layer which blocks:

Are there any ways users can inject logic without having to go via the filesystem I've missed out?

I would say that horrible cases like this are fundamental security flaws in the users code rather than the Cylc framework:

[runtime]
    [[task]]
        script = $PY myscript.py $CYLC_TASK_CYCLE_POINT
        [[[environment]]]
            PY = python3.7
            # PY = python3.8