shell: make registered services secure by default #2877
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: Shell services registered by flux_shell_service_register() are "open to all" by default, and require a call to shell_svc_allowed() in each msg handler in order to secure the service against multi-user access. However, this design requires repetitive calls in every service message handler, and makes it more likely that plugins install insecure services when this call is forgotten. Furthermore, shell_svc_allowed() is not even exported publicly in the shell.h API, so it is impossible to create secure services via external shell plugins. Internally wrap all message handlers installed by shell_svc_register() and call shell_svc_allowed() so that all services are secure by default. If a use case ever arises that requires multiuser access to a shell service, then a separate api call can be created to export an insecure service, so that the security of the service is explicit. Fixes flux-framework#2876
Since shell services registered by flux_shell_service_register() are now secure by default, remove redundant calls to shell_svc_allowed() in message handlers in the input and output services.
Add test to ensure proctable shell service is not accessible to all users in t2610-job-shell-mpir.t.
Codecov Report
@@ Coverage Diff @@
## master #2877 +/- ##
==========================================
+ Coverage 80.98% 81.04% +0.05%
==========================================
Files 250 250
Lines 39352 39368 +16
==========================================
+ Hits 31871 31905 +34
+ Misses 7481 7463 -18
|
garlick
approved these changes
Mar 29, 2020
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wrap all message handlers registered with
flux_shell_service_register(3)and checkshell_svc_allowedbefore calling the real msg handler. This makes shell services accessible only to the shell user (and instance owner) by default, instead of having services "open to all" when first registered. This should greatly reduce the potential for accidental security vulnerabilities introduced by shell plugins.If ever there is a need for a shell service that is open to all users, we can add a new shell.h API call which explicitly registers a multiuser accessible service.