-
Notifications
You must be signed in to change notification settings - Fork 180
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
Slurm Exposed REST API Detector #496
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @lanced00m, thank you for your contribution!
I'm reviewing your plugin and I confirm it works correctly. I left a few comments on things that could be improved. Also, please update the first message with the link of the new testbed PR (google/security-testbeds#98)
This detector checks for exposed slurm REST API daemon by running an arbitrary command. | ||
|
||
Ref: | ||
|
||
- https://slurm.schedmd.com/rest.html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please clarify here that the vulnerable configuration involves a reverse proxy to the API that does not check authentication and that the default REST API is secure.
Reference:
...rc/main/java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java
Outdated
Show resolved
Hide resolved
...java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java
Outdated
Show resolved
Hide resolved
...java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java
Outdated
Show resolved
Hide resolved
...java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java
Outdated
Show resolved
Hide resolved
...java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java
Outdated
Show resolved
Hide resolved
...java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Savio Sisco <[email protected]>
Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonVuLnDetectorTest.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java Co-authored-by: Savio Sisco <[email protected]>
…oogle/tsunami/plugins/detectors/rce/SlurmExposedRestApiDaemonDetector.java Co-authored-by: Savio Sisco <[email protected]>
…se) and TCS availability check"
@lokiuox, thank you for your comprehensive review! All issues are solved now, so you can check the PR again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @lanced00m, here's just a few more minor things to fix, then the plugin's good to go.
..._api/src/main/java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDetector.java
Outdated
Show resolved
Hide resolved
..._api/src/main/java/com/google/tsunami/plugins/detectors/rce/SlurmExposedRestApiDetector.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Approved
@maoning we can merge this and google/security-testbeds#98
Reviewer: Savio, Doyensec
Plugin: Slurm Exposed Rest API Detector
Drawbacks: None.
#423
google/security-testbeds#98
I wrote this plugin based on this which has been merged recently with minimal changes as many as possible.