-
Notifications
You must be signed in to change notification settings - Fork 863
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
updates to security guidelines and docker config #2669
Conversation
These ports are accessible to `localhost` by default. The address can be configured by following the [guide](https://github.com/pytorch/serve/blob/master/docs/configuration.md#configure-torchserve-listening-address-and-port) | ||
TorchServe does not prevent users from configuring the address to be `0.0.0.0`. Please be aware of the security risks if you use `0.0.0.0` | ||
2. TorchServe's Docker image is conigured to listen to `localhost` by [default](https://github.com/pytorch/serve/blob/master/docker/config.properties) | ||
|
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.
also mention that torchserve can run arbitrary python files so don't download mar files you don't trust from the internet
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.
done
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.
the updated language is good but please explicitly also mention that torchserve executes arbitrary python files when running a mar file, the risk is not just in downloading. There's a risk in downloading, unzipping and finally running
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.
Got it. So, how do they verify it? We should mention that too
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.
There is no generic way to verify if a piece of python code is safe, they key is that it needs to be code from a source they trust
Codecov Report
@@ Coverage Diff @@
## master #2669 +/- ##
=======================================
Coverage 71.34% 71.34%
=======================================
Files 85 85
Lines 3905 3905
Branches 58 58
=======================================
Hits 2786 2786
Misses 1115 1115
Partials 4 4 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
3. Be sure to validate the authenticity of the `.mar` file being used with TorchServe. | ||
1. A `.mar` file being downloaded from the internet from an untrusted source may have malicious code, compromising the integrity of your application | ||
2. TorchServe executes arbitrary python code packaged in the `mar` file. Make sure that you've either audited that the code you're using is safe and/or is from a source that you trust | ||
|
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.
archiver can support mar, tgz and folder. we can make it more generic at this section to let cx know they are responsible for the security of the code in model artifacts
Description
Update Security guidelines for using TorchServe
Fixes #(issue)
Type of change
Please delete options that are not relevant.
Feature/Issue validation/testing
Please describe the Unit or Integration tests that you ran to verify your changes and relevant result summary. Provide instructions so it can be reproduced.
Please also list any relevant details for your test configuration.
Test A
Logs for Test A
Test B
Logs for Test B
Checklist: