-
Notifications
You must be signed in to change notification settings - Fork 16
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
Make non-privileged user owner of setup scripts #216
base: develop
Are you sure you want to change the base?
Make non-privileged user owner of setup scripts #216
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #216 +/- ##
========================================
Coverage 59.38% 59.38%
========================================
Files 61 61
Lines 6180 6180
========================================
Hits 3670 3670
Misses 2510 2510 Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@@ -52,7 +52,7 @@ RUN mkdir -p /opt/cnaas/templates /opt/cnaas/settings /opt/cnaas/venv \ | |||
&& chown www-data:www-data /opt/cnaas/templates /opt/cnaas/settings /opt/cnaas/venv | |||
|
|||
# Copy cnaas scripts | |||
COPY --chown=root:www-data cnaas-setup.sh createca.sh exec-pre-app.sh nosetests.sh /opt/cnaas/ |
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.
I wonder why this one line has a different owner. Originally a thinko?
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.
Probably. There are even more small issues with the Dockerfile that will only be triggered on a Linux host using a strict umask, so more PRs may come...
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 basic principle is to let www-data have write access to as absolutely little as possible, best case www-data only has write access to settings and templates and not much else. Scripts and source-code does not need to be modified at runtime so www-data should not be owner or have write access. The reason we have the www-data user and don't run everything as root is so we can separate privileges like this. If everything runs as www-data and everything is owned by www-data it's the same as everything is run by root and everything is owned by root, which we got some complaints about is not a secure solution.
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.
Then the best course of action is to ensure a chmod is run to give the correct privilege bits to the file, imho
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.
I've force-pushed changes in line with the last comment.
If host system is running with a restrictive umask, these scripts may be copied with a mode of 700. Owned by root, they would not be executable by the www-data user inside a container.
23f89c2
to
2025808
Compare
If the host system is running with a restrictive umask (which mine does), these scripts may be copied with a mode of
700
. Owned by root, they would not be executable by the www-data user inside a container.