Skip to content

Commit 40f1468

Browse files
committed
keyring: handle ENOSYS with keyctl(KEYCTL_JOIN_SESSION_KEYRING)
While all modern kernels (and I do mean _all_ of them -- this syscall was added in 2.6.10 before git had begun development!) have support for this syscall, LXC has a default seccomp profile that returns ENOSYS for this syscall. For most syscalls this would be a deal-breaker, and our use of session keyrings is security-based there are a few mitigating factors that make this change not-completely-insane: * We already have a flag that disables the use of session keyrings (for older kernels that had system-wide keyring limits and so on). So disabling it is not a new idea. * While the primary justification of using session keys *is* security-based, it's more of a security-by-obscurity protection. The main defense keyrings have is VFS credentials -- which is something that users already have better security tools for (setuid(2) and user namespaces). * Given the security justification you might argue that we shouldn't silently ignore this. However, the only way for the kernel to return -ENOSYS is either being ridiculously old (at which point we wouldn't work anyway) or that there is a seccomp profile in place blocking it. Given that the seccomp profile (if malicious) could very easily just return 0 or a silly return code (or something even more clever with seccomp-bpf) and trick us without this patch, there isn't much of a significant change in how much seccomp can trick us with or without this patch. Given all of that over-analysis, I'm pretty convinced there isn't a security problem in this very specific case and it will help out the ChromeOS folks by allowing Docker to run inside their LXC container setup. I'd be happy to be proven wrong. Ref: https://bugs.chromium.org/p/chromium/issues/detail?id=860565 Signed-off-by: Aleksa Sarai <[email protected]>
1 parent 70ca035 commit 40f1468

File tree

3 files changed

+31
-10
lines changed

3 files changed

+31
-10
lines changed

libcontainer/keys/keyctl.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import (
77
"strconv"
88
"strings"
99

10+
"github.com/pkg/errors"
11+
1012
"golang.org/x/sys/unix"
1113
)
1214

@@ -15,7 +17,7 @@ type KeySerial uint32
1517
func JoinSessionKeyring(name string) (KeySerial, error) {
1618
sessKeyId, err := unix.KeyctlJoinSessionKeyring(name)
1719
if err != nil {
18-
return 0, fmt.Errorf("could not create session key: %v", err)
20+
return 0, errors.Wrap(err, "create session key")
1921
}
2022
return KeySerial(sessKeyId), nil
2123
}

libcontainer/setns_init_linux.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/opencontainers/runc/libcontainer/seccomp"
1212
"github.com/opencontainers/runc/libcontainer/system"
1313
"github.com/opencontainers/selinux/go-selinux/label"
14+
"github.com/pkg/errors"
1415

1516
"golang.org/x/sys/unix"
1617
)
@@ -29,9 +30,15 @@ func (l *linuxSetnsInit) getSessionRingName() string {
2930

3031
func (l *linuxSetnsInit) Init() error {
3132
if !l.config.Config.NoNewKeyring {
32-
// do not inherit the parent's session keyring
33+
// Do not inherit the parent's session keyring.
3334
if _, err := keys.JoinSessionKeyring(l.getSessionRingName()); err != nil {
34-
return err
35+
// Same justification as in standart_init_linux.go as to why we
36+
// don't bail on ENOSYS.
37+
//
38+
// TODO(cyphar): And we should have logging here too.
39+
if errors.Cause(err) != unix.ENOSYS {
40+
return errors.Wrap(err, "join session keyring")
41+
}
3542
}
3643
}
3744
if l.config.CreateConsole {

libcontainer/standard_init_linux.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,25 @@ func (l *linuxStandardInit) Init() error {
4848
ringname, keepperms, newperms := l.getSessionRingParams()
4949

5050
// Do not inherit the parent's session keyring.
51-
sessKeyId, err := keys.JoinSessionKeyring(ringname)
52-
if err != nil {
53-
return errors.Wrap(err, "join session keyring")
54-
}
55-
// Make session keyring searcheable.
56-
if err := keys.ModKeyringPerm(sessKeyId, keepperms, newperms); err != nil {
57-
return errors.Wrap(err, "mod keyring permissions")
51+
if sessKeyId, err := keys.JoinSessionKeyring(ringname); err != nil {
52+
// If keyrings aren't supported then it is likely we are on an
53+
// older kernel (or inside an LXC container). While we could bail,
54+
// the security feature we are using here is best-effort (it only
55+
// really provides marignal protection since VFS credentials are
56+
// the only significant protection of keyrings).
57+
//
58+
// TODO(cyphar): Log this so people know what's going on, once we
59+
// have proper logging in 'runc init'.
60+
if errors.Cause(err) != unix.ENOSYS {
61+
return errors.Wrap(err, "join session keyring")
62+
}
63+
} else {
64+
// Make session keyring searcheable. If we've gotten this far we
65+
// bail on any error -- we don't want to have a keyring with bad
66+
// permissions.
67+
if err := keys.ModKeyringPerm(sessKeyId, keepperms, newperms); err != nil {
68+
return errors.Wrap(err, "mod keyring permissions")
69+
}
5870
}
5971
}
6072

0 commit comments

Comments
 (0)