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.

Sign up for GitHub

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

Jump to bottom

Make non-privileged user owner of setup scripts #216

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Make non-privileged user owner of setup scripts #216

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docker/api/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ RUN mkdir -p /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/
Copy link

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?

Copy link
Collaborator Author

@lunkwill42 lunkwill42 Nov 23, 2021 •
edited
Loading

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...

Copy link
Member

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.

Copy link
Collaborator Author

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

Copy link
Collaborator Author

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.

RUN cd /opt/cnaas && chmod g+x cnaas-setup.sh createca.sh exec-pre-app.sh nosetests.sh

# Copy cnaas configuration files
COPY --chown=www-data:www-data config/api.yml config/db_config.yml config/plugins.yml config/repository.yml /etc/cnaas-nms/
Expand Down