Add support for NoNewPrivileges

[mrunalp]

Support NoNewPrivileges for additional security in containers.

[mrunalp]

@LK4D4 @crosbymichael ping
cc: @rhatdan

[cyphar]

Is there a spec update PR for this?

[mrunalp]

This was pulled in a previous spec update.

[rhatdan]

This looks got to me. Need to get this into Docker ASAP, since it really helps people running non privileged work loads. Eliminates a lot of worry about contained non priv processes getting access to setuid or setfcap files.

[hqhq]

Not a problem in this PR.
For json property names, we have nonewprivileges and noNewPrivileges and no_new_privileges in code, looks like we don't have code style for that yet, should we change them for consistency.

[mrunalp]

Yeah, sticking to whatever style we have in each repo for now :/

[mrunalp]

Updated.

[rhatdan]

Are we sure this work? Can you still apply seccomp rules after you have set NO_NEW_PRIVS? I thought I heard someone say that you could not.

[cyphar]

If there's no reason to place it after syncParentReady (like syncParentReady might break it), I'd prefer to place it before. Go likes to create new processes each time you do a syscall, which is the whole point of late cgroup apply (applying cgroups as late as possible means we're less likely to hit our resource limits or add any overhead during setup).

[mrunalp]

Moved it above the sync.

[cyphar]

@rhatdan I don't think that's right, if I'm reading this LWN article correctly:

Lutomirski also notes that a sandbox will be much less useful if execve() has to fail when there is any kind of security transition, as Cox suggested. The presence of a policy on a particular binary would make that binary unusable from within a sandbox, no matter what the policy is. A better solution, Lutomirski said, is to set the no_new_privs bit, then set up a sandbox (using Drewry's seccomp system call filtering for example), then execute the binary:

If you want a sandbox, call PR_SET_NO_NEW_PRIVS, then enable seccomp (or whatever) to disable ptrace, evil file access, connections on unix sockets that authenticate via uid, etc.

If you read the actual code (from kernel/seccomp.c), we have to have CAP_SYS_ADMIN while setting this up. This is fine because we drop capabilities in finalizeNamespace, which is after we enable seccomp:

    /*
     * Installing a seccomp filter requires that the task has
     * CAP_SYS_ADMIN in its namespace or be running with no_new_privs.
     * This avoids scenarios where unprivileged tasks can affect the
     * behavior of privileged children.
     */
    if (!task_no_new_privs(current) &&
        security_capable_noaudit(current_cred(), current_user_ns(),
                     CAP_SYS_ADMIN) != 0)
        return ERR_PTR(-EACCES);

So I'm fairly sure this should work perfectly fine.

[rhatdan]

Great. I am probably misremembering.

[mrunalp]

Yes, I had looked at libseccomp earlier and it makes the call as well
https://github.com/seccomp/libseccomp/blob/7f3ae6e6a12390bd38f0787b242f60c47ad076c3/src/system.c#L75

[mrunalp]

I checked that there is no harm in this getting called twice but prctl itself could get blocked by seccomp so makes sense to move this before seccomp.

[crosbymichael]

LGTM

[LK4D4]

LGTM