New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block obsolete and unusual socket families in the default seccomp profile #29076

Merged
merged 1 commit into from Jan 18, 2017

Conversation

Projects
None yet
10 participants
@justincormack
Contributor

justincormack commented Dec 2, 2016

Linux supports many obsolete address families, which are usually available in
common distro kernels, but they are less likely to be properly audited and
may have security issues

This blocks all socket families in the socket (and socketcall where applicable) syscall
except

  • AF_UNIX - Unix domain sockets
  • AF_INET - IPv4
  • AF_INET6 - IPv6
  • AF_NETLINK - Netlink sockets for communicating with the ekrnel
  • AF_PACKET - raw sockets, which are only allowed with CAP_NET_RAW

All other socket families are blocked, including Appletalk (native, not
over IP), IPX (remember that!), VSOCK and HVSOCK, which should not generally
be used in containers, etc.

Note that users can of course provide a profile per container or in the daemon
config if they have unusual use cases that require these.

Signed-off-by: Justin Cormack justin.cormack@docker.com

cc @diogomonica @riyazdf @ijc25 @rneugeba

bats

@riyazdf

riyazdf approved these changes Dec 2, 2016

profile changes LGTM!

We should also update the default seccomp profile docs to capture this change

cc @mstanleyjones

{
Index: 0,
Value: 1,
Op: types.OpGreaterThan,

This comment has been minimized.

@riyazdf

riyazdf Dec 2, 2016

Contributor

to ensure I understand correctly: this rule allows for calling bind() and accept(), subsequent rules lock down socket()?

@riyazdf

riyazdf Dec 2, 2016

Contributor

to ensure I understand correctly: this rule allows for calling bind() and accept(), subsequent rules lock down socket()?

This comment has been minimized.

@justincormack

justincormack Dec 2, 2016

Contributor

socketcall is a kind of obsolete way of calling socket used on some architectures (x86 32 bit, some others), rather than there being a syscall socket and one for bind etc there is one for all socket operations, and you call socketcall(1, rest of args) to call socket, where 1 is socket, 2 is bindetc. So to block we have to block both ways of calling it. So we allowsocketcallwith the first argument greater than 1, ie all operations exceptsocket`.

@justincormack

justincormack Dec 2, 2016

Contributor

socketcall is a kind of obsolete way of calling socket used on some architectures (x86 32 bit, some others), rather than there being a syscall socket and one for bind etc there is one for all socket operations, and you call socketcall(1, rest of args) to call socket, where 1 is socket, 2 is bindetc. So to block we have to block both ways of calling it. So we allowsocketcallwith the first argument greater than 1, ie all operations exceptsocket`.

This comment has been minimized.

@justincormack

justincormack Dec 2, 2016

Contributor

So yes, as you said, just spelling it out for clarity...

@justincormack

justincormack Dec 2, 2016

Contributor

So yes, as you said, just spelling it out for clarity...

This comment has been minimized.

@riyazdf

riyazdf Dec 2, 2016

Contributor

cool, thanks 👍

@riyazdf

riyazdf Dec 2, 2016

Contributor

cool, thanks 👍

This comment has been minimized.

@diogomonica

diogomonica Dec 10, 2016

Contributor

Would be great to capture this discussion in the form of a comment on the policy itself? In 6 months we won't remember why we did this ;)

@diogomonica

diogomonica Dec 10, 2016

Contributor

Would be great to capture this discussion in the form of a comment on the policy itself? In 6 months we won't remember why we did this ;)

This comment has been minimized.

@justincormack

justincormack Jan 17, 2017

Contributor

Added a comment inline.

@justincormack

justincormack Jan 17, 2017

Contributor

Added a comment inline.

@justincormack justincormack changed the title from Block obsolete socket families in the default seccomp profile to Block obsolete and unusual socket families in the default seccomp profile Dec 2, 2016

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack
Contributor

justincormack commented Dec 9, 2016

@jessfraz

This comment has been minimized.

Show comment
Hide comment
@jessfraz

jessfraz Dec 9, 2016

Contributor

cool thanks!

Contributor

jessfraz commented Dec 9, 2016

cool thanks!

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Dec 10, 2016

Member

ping @diogomonica @ijc25 @rneugeba PTAL

(thanks for looking @jessfraz !)

Member

thaJeztah commented Dec 10, 2016

ping @diogomonica @ijc25 @rneugeba PTAL

(thanks for looking @jessfraz !)

@diogomonica

This comment has been minimized.

Show comment
Hide comment
@diogomonica

diogomonica Dec 10, 2016

Contributor

This LGTM. I agree w/ the current list of allowed socket families, and with the impression that the other families are very unlikely to be in use by any containers out there.

Do we currently have any way to test profile changes like this in a meaningful number of images? That would help us gain more confidence that this is indeed a NOP.

Good work!

Contributor

diogomonica commented Dec 10, 2016

This LGTM. I agree w/ the current list of allowed socket families, and with the impression that the other families are very unlikely to be in use by any containers out there.

Do we currently have any way to test profile changes like this in a meaningful number of images? That would help us gain more confidence that this is indeed a NOP.

Good work!

@ijc

This comment has been minimized.

Show comment
Hide comment
@ijc

ijc Dec 12, 2016

Contributor

Looking through include/linux/socket.h I think you've picked the right ones to allow by default.

It's possible some people might want AF_PACKET but I think you are right to exclude that from the default set.

Contributor

ijc commented Dec 12, 2016

Looking through include/linux/socket.h I think you've picked the right ones to allow by default.

It's possible some people might want AF_PACKET but I think you are right to exclude that from the default set.

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Dec 16, 2016

Contributor

@ijc25 AF_PACKET is specifically allowed in this patch, but it is gated by CAP_NET_RAW (which is allowed by default, which is something that needs fixing, but thats another PR).

Contributor

justincormack commented Dec 16, 2016

@ijc25 AF_PACKET is specifically allowed in this patch, but it is gated by CAP_NET_RAW (which is allowed by default, which is something that needs fixing, but thats another PR).

@ijc

This comment has been minimized.

Show comment
Hide comment
@ijc

ijc Dec 16, 2016

Contributor

I should learn to read properly!

Contributor

ijc commented Dec 16, 2016

I should learn to read properly!

@vdemeester

This comment has been minimized.

Show comment
Hide comment
@vdemeester

vdemeester Jan 12, 2017

Member

@justincormack needs a rebase 🙇

Member

vdemeester commented Jan 12, 2017

@justincormack needs a rebase 🙇

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Jan 17, 2017

Contributor

rebased and added a comment as suggested by @diogomonica

Contributor

justincormack commented Jan 17, 2017

rebased and added a comment as suggested by @diogomonica

@thaJeztah

looks like @jessfraz, @riyazdf, and @diogomonica are good, so

LGTM

docs PR is in docker/docker.github.io#776

@@ -62,7 +62,7 @@ func ensureSyscallTest(c *check.C) {
gcc, err := exec.LookPath("gcc")
c.Assert(err, checker.IsNil, check.Commentf("could not find gcc"))
tests := []string{"userns", "ns", "acct", "setuid", "setgid", "socket", "raw"}
tests := []string{"userns", "ns", "acct", "setuid", "setgid", "socket", "raw", "appletalk"}

This comment has been minimized.

@thaJeztah
@thaJeztah

thaJeztah Jan 17, 2017

Member

screen shot 2017-01-17 at 17 51 40

This comment has been minimized.

@justincormack

justincormack Jan 17, 2017

Contributor

ha!

@justincormack

justincormack Jan 17, 2017

Contributor

ha!

@vdemeester

LGTM 🐻

Block obsolete socket families in the default seccomp profile
Linux supports many obsolete address families, which are usually available in
common distro kernels, but they are less likely to be properly audited and
may have security issues

This blocks all socket families in the socket (and socketcall where applicable) syscall
except
- AF_UNIX - Unix domain sockets
- AF_INET - IPv4
- AF_INET6 - IPv6
- AF_NETLINK - Netlink sockets for communicating with the ekrnel
- AF_PACKET - raw sockets, which are only allowed with CAP_NET_RAW

All other socket families are blocked, including Appletalk (native, not
over IP), IPX (remember that!), VSOCK and HVSOCK, which should not generally
be used in containers, etc.

Note that users can of course provide a profile per container or in the daemon
config if they have unusual use cases that require these.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Jan 18, 2017

Member

all 💚 now

Member

thaJeztah commented Jan 18, 2017

all 💚 now

@thaJeztah thaJeztah merged commit 4818435 into moby:master Jan 18, 2017

4 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 29741 has succeeded
Details
janky Jenkins build Docker-PRs 38339 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 9361 has succeeded
Details

@GordonTheTurtle GordonTheTurtle added this to the 1.14.0 milestone Jan 18, 2017

thaJeztah added a commit to thaJeztah/docker that referenced this pull request Feb 22, 2017

Merge pull request moby#29076 from justincormack/seccomp-socket-to-them
Block obsolete and unusual socket families in the default seccomp profile
(cherry picked from commit 4818435)

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

fishilico added a commit to fishilico/shared that referenced this pull request Mar 19, 2017

linux/crypto_socket: support Docker containers
Since moby/moby#29076 socket(AF_ALG, ...) is
being blocked by Docker default seccomp policy. Fail nicely in this
case.
@ihac

This comment has been minimized.

Show comment
Hide comment
@ihac

ihac Apr 20, 2017

The second argument of socketcall() is a pointer(unsigned long *), so the register value what seccomp bpf filters get is an address, not the actual value it points to. it might be wrong to compare it with the socket domain(AF_LOCAL, AF_INET, etc).

ihac commented on profiles/seccomp/default.json in 7e3a596 Apr 20, 2017

The second argument of socketcall() is a pointer(unsigned long *), so the register value what seccomp bpf filters get is an address, not the actual value it points to. it might be wrong to compare it with the socket domain(AF_LOCAL, AF_INET, etc).

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Apr 26, 2017

Contributor

@ihac ah yes, I forgot about that. Hmm, this basically makes this patch useless as we can't really block socketcall at present as many programs are still compiled with it.

Contributor

justincormack commented Apr 26, 2017

@ihac ah yes, I forgot about that. Hmm, this basically makes this patch useless as we can't really block socketcall at present as many programs are still compiled with it.

@ijc

This comment has been minimized.

Show comment
Hide comment
@ijc

ijc Apr 26, 2017

Contributor

On the plus side socketcall never existed for x86-64 or ARM according to socketcall(2). Looking at the kernel source it looks like only a minority of arches had or have it:

  • arm oabi (not the modern one)
  • blackfin
  • cris
  • frv
  • m32r
  • m68k
  • microblaze
  • mips32+64
  • mn10300
  • s390
  • sh
  • sparc
  • x86-32.

Not sure what the cross of that with Moby's set of supported arches is, at least x86-32, mips and s390, I think? I don't know about the others. Even of all those I didn't check if they also have the split calls and if so what versions of glibc for those platforms used which interface.

So I don't think it renders the patch quite "useless" at least for a number of interesting platforms (x86-64, arm64). Although I suppose with CONFIG_COMPAT there are paths to socketcall even on an x86-64 host.

Contributor

ijc commented Apr 26, 2017

On the plus side socketcall never existed for x86-64 or ARM according to socketcall(2). Looking at the kernel source it looks like only a minority of arches had or have it:

  • arm oabi (not the modern one)
  • blackfin
  • cris
  • frv
  • m32r
  • m68k
  • microblaze
  • mips32+64
  • mn10300
  • s390
  • sh
  • sparc
  • x86-32.

Not sure what the cross of that with Moby's set of supported arches is, at least x86-32, mips and s390, I think? I don't know about the others. Even of all those I didn't check if they also have the split calls and if so what versions of glibc for those platforms used which interface.

So I don't think it renders the patch quite "useless" at least for a number of interesting platforms (x86-64, arm64). Although I suppose with CONFIG_COMPAT there are paths to socketcall even on an x86-64 host.

@justincormack

This comment has been minimized.

Show comment
Hide comment
@justincormack

justincormack Apr 26, 2017

Contributor

yes, as we allow 32 bit calls on 64 bit by default, and the glibc changes for supporting non socketcall on 32 bit x86 are recent (and never done for Musl) we cant disable, so the aim of removing dangerous kernel paths for exploits is impossible on amd64. Same applies to s390/s390x that changed at the same time.

Contributor

justincormack commented Apr 26, 2017

yes, as we allow 32 bit calls on 64 bit by default, and the glibc changes for supporting non socketcall on 32 bit x86 are recent (and never done for Musl) we cant disable, so the aim of removing dangerous kernel paths for exploits is impossible on amd64. Same applies to s390/s390x that changed at the same time.

justincormack added a commit to justincormack/docker that referenced this pull request May 9, 2017

Revert "Block obsolete socket families in the default seccomp profile"
This reverts commit 7e3a596.

Unfortunately, it was pointed out in moby#29076 (comment)
that the `socketcall` syscall takes a pointer to a struct so it is not possible to
use seccomp profiles to filter it. This means these cannot be blocked as you can
use `socketcall` to call them regardless, as we currently allow 32 bit syscalls.

Users who wish to block these should use a seccomp profile that blocks all
32 bit syscalls and then just block the non socketcall versions.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>

justincormack added a commit to justincormack/docker that referenced this pull request May 9, 2017

Revert "Block obsolete socket families in the default seccomp profile"
This reverts commit 7e3a596.

Unfortunately, it was pointed out in moby#29076 (comment)
that the `socketcall` syscall takes a pointer to a struct so it is not possible to
use seccomp profiles to filter it. This means these cannot be blocked as you can
use `socketcall` to call them regardless, as we currently allow 32 bit syscalls.

Users who wish to block these should use a seccomp profile that blocks all
32 bit syscalls and then just block the non socketcall versions.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>

anonymuse added a commit to anonymuse/docker that referenced this pull request May 19, 2017

Revert "Block obsolete socket families in the default seccomp profile"
This reverts commit 7e3a596.

Unfortunately, it was pointed out in moby#29076 (comment)
that the `socketcall` syscall takes a pointer to a struct so it is not possible to
use seccomp profiles to filter it. This means these cannot be blocked as you can
use `socketcall` to call them regardless, as we currently allow 32 bit syscalls.

Users who wish to block these should use a seccomp profile that blocks all
32 bit syscalls and then just block the non socketcall versions.

Signed-off-by: Justin Cormack <justin.cormack@docker.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment