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

restricted edit-run #2687

Open
hjoliver opened this issue Jun 4, 2018 · 9 comments
Open

restricted edit-run #2687

hjoliver opened this issue Jun 4, 2018 · 9 comments
Milestone

Comments

@hjoliver
Copy link
Member

hjoliver commented Jun 4, 2018

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.

@hjoliver hjoliver added this to the later milestone Jun 4, 2018
@matthewrmshin
Copy link
Contributor

Some quick thoughts:

  • I guess this would affect broadcast just as well?
  • I wonder how we should present the interface to edit-run. At the moment, we give the user the freedom to edit the generated job file. I guess we should present them with a suite.rc runtime configuration fragment with the relevant combined settings instead?

@hjoliver
Copy link
Member Author

hjoliver commented Jun 5, 2018

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
Copy link
Member Author

hjoliver commented Jun 5, 2018

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
Copy link
Contributor

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

@hjoliver
Copy link
Member Author

hjoliver commented Jun 5, 2018

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

@matthewrmshin matthewrmshin modified the milestones: later, cylc-8.0.0 Aug 28, 2019
@oliver-sanders
Copy link
Member

oliver-sanders commented Jun 9, 2020

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
Copy link
Member Author

hjoliver commented Jun 9, 2020

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
Copy link
Contributor

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
Copy link
Member

oliver-sanders commented Jun 10, 2020

(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:

  • The broadcast of *script items.
  • The broadcast of [environment] items which contain subshell calls (actually quite easy).
  • Trigger edit.

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

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