Recently I noticed a number of Dockerfiles I had broke, and in tracking it down it seems the recently file permission defaults have been changed (primarily around windows users).
I would purpose default file permissions when permissions are unknown (or too promiscuous if wanting to handle other bad actors) would be 644 or 755.
This is primarily talked about in issue #11047 and pull request #11148. I think the primary intent was to:
A) Make sure files added from the windows CLI are executable by default
B) Try to make defaults more secure rather than less (many Windows commands that grab permissions get 666 by default)
To this end the code changes resulted in a mask of 711 applied to windows files by default, and the execute bit being manually set.
With the permissions most commonly being 666 this results on 600 and with the manual setting of the execute bit 700.
This prevents files from being world readable by default.
Security is good, and I think this was in part argued as an improvement for security by default (as someone adding a private key will not have that key world writable). There was also an example given that if someone set Windows ACLS to prevent all other users from accessing that file, they may have the expectation that carries over when the file is added to a container.
The issues I purpose with this code is:
*) Container files are not (by default) accessible by other containers. Containers are generally expected not to be running multiple users inside one container (while fully possible I think is rarely done, and probably not often a use care for it). This means the only benefit restrictive read permissions have on something in a container has is of very limited scope (and only one use case I believe). That case is if the container itself is dropping to another user. Dropping to another user requires additional privileges and is not as secure as running the container as the user to start.
*) There is the obvious convenience factor this causes a problem for for example:
FROM busybox
RUN adduser -D bob
ADD test.conf /tmp/
USER bob
CMD cat /tmp/test.conf
no longer works with this change on windows, an additional chmod is required. In addition older AUFS drivers are not great on overlapping permissions/ownership (issue #6047).
*) Sensitive files are generally regarded as best practice NOT to be committed into containers. If this is the case, then the default benefit is reduced.
*) Net security is possibly degraded and not improved. While file permissions are tighter on added/copied files and this would seem better, i think it will harm a far more important security feature of running containers as another user. Most users by far run docker containers as root. This can be seen as many of the most popular dockerfiles in the hub do not work out of the box if run as non-root. While the security benefits of running a user (vs user dropping in the container) even with crap dropping can be debated, it is no doubt most secure not to run containers as root. This change will make even more Dockerfiles broken for running as non-root. I think this is the wrong direction the project would want to be moving.
*) I think the assumption that users expect Windows ACLS to be honored in file transfers/copies is not common. To start Windows ACLS are generally folder specific (files inherit from the folder, and often that folder from its parent). That means if I copy a file to even another folder on a Windows machine that another user has access to they can generally read the file. While this is certainly overridable (windows ACLS are certainly very granular). In addition, short of copying a file using SMB to another windows host or another drive, none of the file transfer apps I have ever used have preserved windows ACLS.
I think this is critical to settle before the next freeze. Projects should try to ever avoid changing defaults to be less secure, especially as drastic as this could be down the line. If permissions go out as they are now, so files added default to not being group/world readable users will expect that functionality going forward. If it ends up being decided later on this was the wrong choice, and its better to have files readable by default its very hard to make this transition as you could cause people relying on the previous security to now be exposed. Conversely while tightening security (if so desired) in the future may break apps (as it has here) it does not leave user data possibly vulnerable.
I apologize this is not a PR I have not dived into much of the docker code yet so do not know exactly where this is set for everything. I am not sure if PR #11148 touched everything or just effected archiving (as it primarily looks like the latter). I would suggest this extends to files fetched from the web using ADD or COPY (however as I think that is already in 0.50 I am not sure that would be something people would move for).
Recently I noticed a number of Dockerfiles I had broke, and in tracking it down it seems the recently file permission defaults have been changed (primarily around windows users).
I would purpose default file permissions when permissions are unknown (or too promiscuous if wanting to handle other bad actors) would be 644 or 755.
This is primarily talked about in issue #11047 and pull request #11148. I think the primary intent was to:
A) Make sure files added from the windows CLI are executable by default
B) Try to make defaults more secure rather than less (many Windows commands that grab permissions get 666 by default)
To this end the code changes resulted in a mask of 711 applied to windows files by default, and the execute bit being manually set.
With the permissions most commonly being 666 this results on 600 and with the manual setting of the execute bit 700.
This prevents files from being world readable by default.
Security is good, and I think this was in part argued as an improvement for security by default (as someone adding a private key will not have that key world writable). There was also an example given that if someone set Windows ACLS to prevent all other users from accessing that file, they may have the expectation that carries over when the file is added to a container.
The issues I purpose with this code is:
*) Container files are not (by default) accessible by other containers. Containers are generally expected not to be running multiple users inside one container (while fully possible I think is rarely done, and probably not often a use care for it). This means the only benefit restrictive read permissions have on something in a container has is of very limited scope (and only one use case I believe). That case is if the container itself is dropping to another user. Dropping to another user requires additional privileges and is not as secure as running the container as the user to start.
*) There is the obvious convenience factor this causes a problem for for example:
FROM busybox
RUN adduser -D bob
ADD test.conf /tmp/
USER bob
CMD cat /tmp/test.conf
no longer works with this change on windows, an additional chmod is required. In addition older AUFS drivers are not great on overlapping permissions/ownership (issue #6047).
*) Sensitive files are generally regarded as best practice NOT to be committed into containers. If this is the case, then the default benefit is reduced.
*) Net security is possibly degraded and not improved. While file permissions are tighter on added/copied files and this would seem better, i think it will harm a far more important security feature of running containers as another user. Most users by far run docker containers as root. This can be seen as many of the most popular dockerfiles in the hub do not work out of the box if run as non-root. While the security benefits of running a user (vs user dropping in the container) even with crap dropping can be debated, it is no doubt most secure not to run containers as root. This change will make even more Dockerfiles broken for running as non-root. I think this is the wrong direction the project would want to be moving.
*) I think the assumption that users expect Windows ACLS to be honored in file transfers/copies is not common. To start Windows ACLS are generally folder specific (files inherit from the folder, and often that folder from its parent). That means if I copy a file to even another folder on a Windows machine that another user has access to they can generally read the file. While this is certainly overridable (windows ACLS are certainly very granular). In addition, short of copying a file using SMB to another windows host or another drive, none of the file transfer apps I have ever used have preserved windows ACLS.
I think this is critical to settle before the next freeze. Projects should try to ever avoid changing defaults to be less secure, especially as drastic as this could be down the line. If permissions go out as they are now, so files added default to not being group/world readable users will expect that functionality going forward. If it ends up being decided later on this was the wrong choice, and its better to have files readable by default its very hard to make this transition as you could cause people relying on the previous security to now be exposed. Conversely while tightening security (if so desired) in the future may break apps (as it has here) it does not leave user data possibly vulnerable.
I apologize this is not a PR I have not dived into much of the docker code yet so do not know exactly where this is set for everything. I am not sure if PR #11148 touched everything or just effected archiving (as it primarily looks like the latter). I would suggest this extends to files fetched from the web using ADD or COPY (however as I think that is already in 0.50 I am not sure that would be something people would move for).