maskedPaths and readonlyPaths: skip non-existent paths #982
Conversation
alban
changed the title
|
While I understand why |
|
@cyphar runc is not ignoring any of them. I tries to setup the ro or mask, but if the destination does not exist, then there is nothing for it mask or ro so it returns nil in this case. I think the runc functionality is correct but the spec change with the wording |
|
I'm not sure that that this is correct though -- it depends what a user expects and what people want to use masking for. If we're told to mask a non-existent path then we arguably should still mask it (to be fair, we won't be able to figure out if we should put a file or a directory over the path). The concern would be that the path isn't immediately available, but we want to make sure that the container process can never access that path. Of course, on the other hand, runc does not traditionally modify |
|
I'm not sure that would work, we mask an non-existent path and then someone mounts over the top later, it's not masked. |
|
We cannot always create an empty file or directory when the destination is non-existent: in the example of I think the current runc can only masking paths it knows about at the time the container is set up. Subsequent mounts cannot be protected with
Any other suggestion for the word |
runc ignores unexistent paths in maskedPaths and readonlyPaths. That's useful for blocking /proc/latency_stats (default in buildah) because this path is not existing on all kernels. In this case, no error should be generated. Other errors should be generated. For example, using readonlyPaths on a unbindable path fails and this error must be generated, otherwise the path would silently stay read-write. Signed-off-by: Alban Crequy <alban@kinvolk.io>
alban
force-pushed
the
alban/missing-paths
branch
from
2d8d6d9 to
c5e0999
Compare
alban
changed the title
|
should this be a MUST or SHOULD? |
|
If anything it should be SHOULD, but I'm still not a fan of this... |
| @@ -635,6 +635,7 @@ The following parameters can be specified to set up seccomp: | |||
|
|
|||
| **`maskedPaths`** (array of strings, OPTIONAL) will mask over the provided paths inside the container so that they cannot be read. | |||
| The values MUST be absolute paths in the [container namespace](glossary.md#container_namespace). | |||
| Unexistent paths MUST be skipped without generating an error. | |||
There was a problem hiding this comment.
"Unexistent" is not a word. Either "inexistent" or "nonexistent".
And this is best a SHOULD, not a MUST.
| @@ -648,6 +649,7 @@ The following parameters can be specified to set up seccomp: | |||
|
|
|||
| **`readonlyPaths`** (array of strings, OPTIONAL) will set the provided paths as readonly inside the container. | |||
| The values MUST be absolute paths in the [container namespace](glossary.md#container-namespace). | |||
| Unexistent paths MUST be skipped without generating an error. | |||
runc ignores non-existent paths in maskedPaths and readonlyPaths. That's useful for blocking /proc/latency_stats (default in buildah) because this path is not existing on all kernels.
In this case, no error should be generated. Other errors should be generated. For example, using readonlyPaths on a unbindable path fails and this error must be generated, otherwise the path would silently stay read-write.
Signed-off-by: Alban Crequy alban@kinvolk.io