seccomp: block IPT_SO_SET_REPLACE for setsockopt by jessfraz · Pull Request #23893 · moby/moby

jessfraz · 2016-06-23T07:19:23Z

- What I did
Blocked IPT_SO_SET_REPLACE for setsockopt

- How I did it

Added it to seccomp profile.

- How to verify it

Added a test.

- Description for the changelog

Made world better place

Prevents https://bugs.chromium.org/p/project-zero/issues/detail?id=758&redir=1

Signed-off-by: Jess Frazelle jessfraz@google.com

tonistiigi · 2016-06-24T00:44:20Z

LGTM

cc @justincormack @mrjana

mrjana · 2016-06-24T18:39:52Z

LGTM

justincormack · 2016-06-24T19:21:54Z

What about the other values that are affected eg at least ARPT_SO_SET_REPLACE?

jessfraz · 2016-06-24T19:25:05Z

wdym, the vuln is via IPT_SO_SET_REPLACE and I did not equal to?

jessfraz · 2016-06-24T19:29:54Z

ah I just noticed the part about ARPT_SO_SET_REPLACE

jessfraz · 2016-06-24T19:36:42Z

maybe can we start with this and then I can work on a different PR and test for blocking ARPT_SO_SET_REPLACE as well

justincormack · 2016-06-24T21:05:21Z

No, the vuln is also in ARPT_SO_SET_REPLACE and some other calls.

Also, 64 may be a valid socket option for different address families, this is only for ipv4 sockets, which is somewhat problematic.

jessfraz · 2016-06-24T21:13:20Z

Ya working on some edits

On Friday, June 24, 2016, Justin Cormack notifications@github.com wrote:

No, the vuln is also in ARPT_SO_SET_REPLACE and some other calls.

Also, 64 may be a valid socket option for different address families, this
is only for ipv4 sockets, which is somewhat problematic.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#23893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABYNbAjxF-z2E6UEilXYd7Zt8k1WbnZIks5qPEbqgaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

jessfraz · 2016-06-24T23:47:47Z

Updated the patch so it blocks IPT_SO_SET_REPLACE, IP6T_SO_SET_REPLACE, ARPT_SO_SET_REPLACE and added the correct CVE numbers to non-events, it also effects ipv6 codepath so we dont need to worry about iv4 v 6.

jessfraz · 2016-06-27T21:18:27Z

this should be good now 😇

thaJeztah · 2016-06-28T17:20:31Z

Don't merge yet, @justincormack just told me he's still looking into this PR, and wants to check some things

jessfraz · 2016-06-28T17:36:37Z

cool cool :)

On Tue, Jun 28, 2016 at 10:21 AM, Sebastiaan van Stijn <
notifications@github.com> wrote:

Don't merge yet, @justincormack https://github.com/justincormack just
told me he's still looking into this PR, and wants to check some things

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#23893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABYNbBcCsOoDb13tGIZ8beMqIj8TuKK3ks5qQVg3gaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

justincormack · 2016-07-05T11:23:34Z

profiles/seccomp/seccomp_default.go

We should only disallow IPT_SO_SET_REPLACE and ARPT_SO_SET_REPLACE when arg 1 (level) is SOL_IP (0) and IP6T_SO_SET_REPLACE when arg 1 is SOL_IPV6 (41), otherwise we might be blocking valid socket options on different address families.

kk will update

On Tue, Jul 5, 2016 at 4:25 AM, Justin Cormack notifications@github.com
wrote:

In profiles/seccomp/seccomp_default.go
#23893 (comment):

@@ -1297,7 +1297,15 @@ func DefaultProfile(rs *specs.Spec) *types.Seccomp {
{
Name: "setsockopt",
Action: types.ActAllow,

Args: []*types.Arg{},

Args: []*types.Arg{

{

// disallow IPT_SO_SET_REPLACE, IP6T_SO_SET_REPLACE, ARPT_SO_SET_REPLACE

Index: 2,

We should only disallow IPT_SO_SET_REPLACE and ARPT_SO_SET_REPLACE when
arg 1 (level) is SOL_IP (0) and IP6T_SO_SET_REPLACE when arg 1 is SOL_IPV6
(41), otherwise we might be blocking valid socket options on different
address families.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/docker/docker/pull/23893/files/32c532c6c1d98735e604fa3aee36a6c67c29b608#r69546695,
or mute the thread
https://github.com/notifications/unsubscribe/ABYNbNUluby0bnhXCYtjMzpI40vR9hVjks5qSj8RgaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

…T_REPLACE for setsockopt Prevents https://bugs.chromium.org/p/project-zero/issues/detail?id=758&redir=1 and http://www.openwall.com/lists/oss-security/2016/06/24/5 Signed-off-by: Jess Frazelle <jessfraz@google.com>

jessfraz · 2016-07-06T17:01:03Z

so I updated but this is still blocking a bit too much, open to your opinions on what the logic should be @justincormack, should we explicitally allow each level here: https://github.com/torvalds/linux/blob/a7fd20d1c476af4563e66865213474a2f9f473a4/include/linux/socket.h#L297 (also might have missed something obvious since I'm back from long weekend haha)

justincormack · 2016-07-07T15:29:11Z

@jfrazelle thats definitely not right (setsockopt options are numbers not bits, so you cant test by masking, among other issues). I am jetlagged so it might take me a while to work out the right form. Also I wonder if the JSON format can even express it sanely. Will get back to you...

jessfraz · 2016-07-07T15:33:30Z

Ya I was thinking about this more yesterday and still haven't thought of a
good way to do it sanely if at all but I feel like I'm missing something,
so thanks appreciate your thoughts and time :)

On Thursday, July 7, 2016, Justin Cormack notifications@github.com wrote:

@jfrazelle https://github.com/jfrazelle thats definitely not right. I
am jetlagged so it might take me a while to work out the right form. Also I
wonder if the JSON format can even express it sanely. Will get back to
you...

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#23893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABYNbDi_e0wp41aeu55zgO_3eTbHeqgsks5qTRurgaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

justincormack · 2016-07-11T15:17:11Z

I don't think it is possible to express the right condition, have raised an issue on libseccomp seccomp/libseccomp#44

jessfraz · 2016-07-13T15:52:50Z

Nice so I'm so glad I'm not insane

jessfraz · 2016-07-13T15:54:11Z

what do you think about going back to the original implementation I had, that had the tradeoff of blocking too much?

justincormack · 2016-07-13T16:18:55Z

I am reluctant to block potentially valid calls, especially as this CVE is not exploitable without cap_sys_admin anyway...

jessfraz · 2016-07-13T16:24:29Z

makes sense, I'm happy to close til we figure out a better route

On Wed, Jul 13, 2016 at 10:20 AM, Justin Cormack notifications@github.com
wrote:

I am reluctant to block potentially valid calls, especially as this CVE is
not exploitable without cap_sys_admin anyway...

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#23893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ABYNbFEd9xKVKqm20w-sKVRrRDj8N3m8ks5qVRBLgaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

justincormack · 2016-07-13T16:30:44Z

Yes, am going to look at how to fix upstream at some point, as there may well be this requirement again...

justincormack · 2016-07-21T13:07:38Z

@jfrazelle I just remembered, this PR did have the addition for the non events file, we still want that!

jessfraz · 2016-07-21T13:15:05Z

Ah I can reopen just that part

On Thursday, July 21, 2016, Justin Cormack notifications@github.com wrote:

@jfrazelle https://github.com/jfrazelle I just remembered, this PR did
have the addition for the non events file, we still want that!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#23893 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABYNbL4FO4i86wFXq21rNGrv3T_3mMWFks5qX291gaJpZM4I8hV5
.

Jessie Frazelle
4096R / D4C4 DD60 0D66 F65A 8EFC 511E 18F3 685C 0022 BFF3
pgp.mit.edu http://pgp.mit.edu/pks/lookup?op=get&search=0x18F3685C0022BFF3

justincormack · 2016-07-21T13:58:24Z

Or do a different PR just for that, it is kind of useful to have this one here as a reminder...

GordonTheTurtle added the status/0-triage label Jun 23, 2016

jessfraz force-pushed the seccomp-block-IPT_SO_SET_REPLACE-setsockopt branch from 2773a78 to 4b07da4 Compare June 23, 2016 14:30

mlaventure added area/security area/security/seccomp labels Jun 23, 2016

tonistiigi added status/2-code-review and removed status/0-triage labels Jun 24, 2016

jessfraz force-pushed the seccomp-block-IPT_SO_SET_REPLACE-setsockopt branch 2 times, most recently from f6f6852 to 823c696 Compare June 24, 2016 23:45

jessfraz force-pushed the seccomp-block-IPT_SO_SET_REPLACE-setsockopt branch from 823c696 to 32c532c Compare June 25, 2016 00:04

justincormack reviewed Jul 5, 2016
View reviewed changes

jessfraz force-pushed the seccomp-block-IPT_SO_SET_REPLACE-setsockopt branch from 32c532c to 4eaeca3 Compare July 6, 2016 16:59

justincormack mentioned this pull request Jul 11, 2016

BUG: cannot add blacklist items to a whitelist seccomp/libseccomp#44

Closed

jessfraz closed this Jul 13, 2016

jessfraz mentioned this pull request Jul 25, 2016

update security non-events #25020

Merged

Conversation

jessfraz commented Jun 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tonistiigi commented Jun 24, 2016

Uh oh!

mrjana commented Jun 24, 2016

Uh oh!

justincormack commented Jun 24, 2016

Uh oh!

jessfraz commented Jun 24, 2016

Uh oh!

jessfraz commented Jun 24, 2016

Uh oh!

jessfraz commented Jun 24, 2016

Uh oh!

justincormack commented Jun 24, 2016

Uh oh!

jessfraz commented Jun 24, 2016

Uh oh!

jessfraz commented Jun 24, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jessfraz commented Jun 27, 2016

Uh oh!

thaJeztah commented Jun 28, 2016

Uh oh!

jessfraz commented Jun 28, 2016

Uh oh!

justincormack Jul 5, 2016

Choose a reason for hiding this comment

Uh oh!

jessfraz Jul 5, 2016

Choose a reason for hiding this comment

Uh oh!

jessfraz commented Jul 6, 2016

Uh oh!

justincormack commented Jul 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jessfraz commented Jul 7, 2016

Uh oh!

justincormack commented Jul 11, 2016

Uh oh!

jessfraz commented Jul 13, 2016

Uh oh!

jessfraz commented Jul 13, 2016

Uh oh!

justincormack commented Jul 13, 2016

Uh oh!

jessfraz commented Jul 13, 2016

Uh oh!

justincormack commented Jul 13, 2016

Uh oh!

justincormack commented Jul 21, 2016

Uh oh!

jessfraz commented Jul 21, 2016

Uh oh!

justincormack commented Jul 21, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

jessfraz commented Jun 23, 2016 •

edited

Loading

jessfraz commented Jun 24, 2016 •

edited

Loading

justincormack commented Jul 7, 2016 •

edited

Loading