schema/config-linux: add length limit for idmappings#689
schema/config-linux: add length limit for idmappings#689Mashimiao wants to merge 1 commit intoopencontainers:masterfrom
Conversation
According to spec, there is a limit of 5 mappings which is the Linux kernel hard limit. Signed-off-by: Ma Shimiao <mashimiao.fnst@cn.fujitsu.com>
4342fe4 to
96ba58a
Compare
|
I am not sure if we want to add a check like this that is too tied to the kernel limit. The kernel could change the limit in the future. We can let the runtimes check or fail if the limit is exceeded. That said, I wouldn't oppose if other maintainers think that this is okay to add. |
|
@mrunalp i've been against the validation of values that are based on the host system. I think its a bad idea, sets us up for issues and hotfixes down the road when the underlying hosts change and our values are no longer correct, and the simple fact that this does not belong in the spec because they host specific values that can differ. You cannot enforce these things using static validation because we are not the ones setting the rules/limits for these values. |
|
On Fri, Feb 17, 2017 at 10:17:25AM -0800, Mrunal Patel wrote:
I am not sure if we want to add a check like this that is too tied
to the kernel limit. The kernel could change the limit in the
future.
The limit for unprivileged namespaces is one [1]:
4. One of the following two cases applies:
* Either the writing process has the CAP_SETUID (CAP_SETGID)
capability in the parent user namespace.
…
* Or otherwise all of the following restrictions apply:
+ The data written to uid_map (gid_map) must consist of a
single line that maps the writing process's effective user
ID (group ID) in the parent user namespace to a user ID
(group ID) in the user namespace.
so it's already going to be trickier than the current spec wording and
the check here. But I'm ok with the spec echoing the current kernel
limit (and needing a bump if/when the kernel adjusts their rules), and
I'm also ok with the spec leaving this up to the kernel. Whichever
way we go, the current spec wording [2]:
There is a limit of 5 mappings which is the Linux kernel hard limit.
should be updated so the spec position is clear. Either enforce the
kernel's most-generous limit:
There MUST NOT be more than five entries in either mapping.
Or make it clear that it's being left up to the kernel:
Note that the kernel [places some restrictions on valid
mappings][1].
[1]: http://man7.org/linux/man-pages/man7/user_namespaces.7.html
[2]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc4/config-linux.md#user-namespace-mappings
|
|
I agree we should not have such limit which depends on kernel, I pushed a commit #693 . |
According to spec, there is a limit of 5 mappings which is the Linux kernel hard limit.
Signed-off-by: Ma Shimiao mashimiao.fnst@cn.fujitsu.com