Skip to content
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.

Sign up for GitHub

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

RFE: add support for comparisons against 32-bit arguments #384

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

RFE: add support for comparisons against 32-bit arguments #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ad65b76
api: Add a SCMP_CMP_32BIT flag bit that can be combined with existing…
jhenstridge May 9, 2022
b6b2e21
db: extract per db_api_arg portion of _db_rule_gen_32 into its own fu…
jhenstridge May 10, 2022
9090c42
db: extract 64-bit comparison generation into _db_rule_gen_arg_64
jhenstridge May 11, 2022
0936a72
db: merge _db_rule_gen_64 and _db_rule_gen_32
jhenstridge May 11, 2022
b86348b
db: generate 32-bit comparisons on 64-bit architectures if SCMP_CMP_3…
jhenstridge May 11, 2022
3a13808
tests: add a test covering forced 32-bit comparisons
jhenstridge May 12, 2022
d472727
db: return EINVAL if there are any unknown flags set in the compariso…
jhenstridge May 12, 2022
518493f
tests: add Python version of 32-bit comparison test program
jhenstridge May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions include/seccomp.h.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ enum scmp_compare {
SCMP_CMP_GT = 6, /**< greater than */
SCMP_CMP_MASKED_EQ = 7, /**< masked equality */
_SCMP_CMP_MAX,

SCMP_CMP_OPMASK = 0xFFFF,
SCMP_CMP_32BIT = 1 << 16, /**< operation is 32-bit */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still working my way through this PR, but I think we want to move SCMP_CMP_OPMASK and SCMP_CMP_32BIT out of the enum and have them be standalone preprocessor macros/constants.

Copy link
Member

@pcmoore pcmoore May 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess if we do that we need to add a "filler" value to cause the enum to be a certain size, which feels a little icky but I think it's okay if we make sure to represent all of the flag space, e.g.

enum scmp_compare {
	_SCMP_CMP_MIN = 0,
	SCMP_CMP_NE = 1,
	SCMP_CMP_LT = 2,
	SCMP_CMP_LE = 3,
	SCMP_CMP_EQ = 4,
	SCMP_CMP_GE = 5,
	SCMP_CMP_GT = 6,
	SCMP_CMP_MASKED_EQ = 7,
	_SCMP_CMP_MAX,
	_SCMP_CMP_OPMASK = 0x0000FFFF,
	_SCMP_CMP_FLAGMASK = 0xFFFF0000,
};

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... which somewhat questions the value of moving the flag definitions out of the enum and into their own macros/constants, but that still seems like a goodish idea to me at this point.

};

/**
Expand Down
791 changes: 395 additions & 396 deletions src/db.c
View file
Open in desktop

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ struct db_api_arg {
scmp_datum_t datum;

bool valid;
bool is_32bit;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to drop the is_32bit flag and just preserve the SCMP_CMP_XXX flags in the db_api_arg::op field; there aren't that many places that check the comparison op so masking them shouldn't be too terrible. If necessary we can always wrap it in a preprocessor macro, e.g.

#define CMP_OP(x) (x & __SCMP_CMP_OPMASK)
#define CMP_FLAGS(x) (x & __SCMP_CMP_FLAGMASK)

};

struct db_api_rule_list {
Expand Down
1 change: 1 addition & 0 deletions src/python/libseccomp.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ cdef extern from "seccomp.h":
SCMP_CMP_GE
SCMP_CMP_GT
SCMP_CMP_MASKED_EQ
SCMP_CMP_32BIT

cdef enum:
SCMP_ACT_KILL_PROCESS
Expand Down
1 change: 1 addition & 0 deletions src/python/seccomp.pyx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ EQ = libseccomp.SCMP_CMP_EQ
GE = libseccomp.SCMP_CMP_GE
GT = libseccomp.SCMP_CMP_GT
MASKED_EQ = libseccomp.SCMP_CMP_MASKED_EQ
CMP_32BIT = libseccomp.SCMP_CMP_32BIT

def system_arch():
""" Return the system architecture value.
Expand Down
1 change: 1 addition & 0 deletions tests/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ util.pyc
57-basic-rawsysrc
58-live-tsync_notify
59-basic-empty_binary_tree
60-sim-32b_args_on_64b
86 changes: 86 additions & 0 deletions tests/60-sim-32b_args_on_64b.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* Seccomp Library test program
*
* Copyright (c) 2022 Canonical Ltd.
* Author: James Henstridge <[email protected]>
*/

/*
* This library is free software; you can redistribute it and/or modify it
* under the terms of version 2.1 of the GNU Lesser General Public License as
* published by the Free Software Foundation.
*
* This library is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
* for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this library; if not, see <http://www.gnu.org/licenses>.
*/

#include <errno.h>
#include <unistd.h>
#include <inttypes.h>

#include <seccomp.h>

#include "util.h"

int main(int argc, char *argv[])
{
int rc;
struct util_options opts;
scmp_filter_ctx ctx = NULL;

rc = util_getopt(argc, argv, &opts);
if (rc < 0)
goto out;

ctx = seccomp_init(SCMP_ACT_KILL);
if (ctx == NULL)
return ENOMEM;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1001, 1,
SCMP_A0(SCMP_CMP_NE | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1002, 1,
SCMP_A0(SCMP_CMP_LT | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1003, 1,
SCMP_A0(SCMP_CMP_LE | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1004, 1,
SCMP_A0(SCMP_CMP_EQ | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1005, 1,
SCMP_A0(SCMP_CMP_GE | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1006, 1,
SCMP_A0(SCMP_CMP_GT | SCMP_CMP_32BIT, 0x10));
if (rc != 0)
goto out;

rc = seccomp_rule_add_exact(ctx, SCMP_ACT_ALLOW, 1007, 1,
SCMP_A0(SCMP_CMP_MASKED_EQ | SCMP_CMP_32BIT, 0xff, 0x10));
if (rc != 0)
goto out;

rc = util_filter_output(&opts, ctx);
if (rc)
goto out;

out:
seccomp_release(ctx);
return (rc < 0 ? -rc : rc);
}
47 changes: 47 additions & 0 deletions tests/60-sim-32b_args_on_64b.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#!/usr/bin/env python

#
# Seccomp Library test program
#
# Copyright (c) 2022 Canonical Ltd.
# Author: James Henstridge <[email protected]>
#

#
# This library is free software; you can redistribute it and/or modify it
# under the terms of version 2.1 of the GNU Lesser General Public License as
# published by the Free Software Foundation.
#
# This library is distributed in the hope that it will be useful, but WITHOUT
# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
# FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
# for more details.
#
# You should have received a copy of the GNU Lesser General Public License
# along with this library; if not, see <http://www.gnu.org/licenses>.
#

import argparse
import sys

import util

from seccomp import *

def test(args):
f = SyscallFilter(KILL)
f.add_rule_exactly(ALLOW, 1001, Arg(0, NE | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1002, Arg(0, LT | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1003, Arg(0, LE | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1004, Arg(0, EQ | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1005, Arg(0, GE | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1006, Arg(0, GT | CMP_32BIT, 0x10))
f.add_rule_exactly(ALLOW, 1007, Arg(0, MASKED_EQ | CMP_32BIT, 0xff, 0x10))
return f

args = util.get_opt()
ctx = test(args)
util.filter_output(args, ctx)

# kate: syntax python;
# kate: indent-mode python; space-indent on; indent-width 4; mixedindent off;
54 changes: 54 additions & 0 deletions tests/60-sim-32b_args_on_64b.tests
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
#
# libseccomp regression test automation data
#
# Copyright (c) 2022 Canonical Ltd.
# Author: James Henstridge <[email protected]>
#

test type: bpf-sim

# Testname Arch Syscall Arg0 Arg1 Arg2 Arg3 Arg4 Arg5 Result
60-sim-32b_args_on_64b all 1001 0x1 N N N N N ALLOW
60-sim-32b_args_on_64b all 1001 0x10 N N N N N KILL
60-sim-32b_args_on_64b all 1001 0xffffffff00000001 N N N N N ALLOW
60-sim-32b_args_on_64b all 1001 0xffffffff00000010 N N N N N KILL

60-sim-32b_args_on_64b all 1002 0x1 N N N N N ALLOW
60-sim-32b_args_on_64b all 1002 0x20 N N N N N KILL
60-sim-32b_args_on_64b all 1002 0xffffffff00000001 N N N N N ALLOW
60-sim-32b_args_on_64b all 1002 0xffffffff00000020 N N N N N KILL

60-sim-32b_args_on_64b all 1003 0x10 N N N N N ALLOW
60-sim-32b_args_on_64b all 1003 0x20 N N N N N KILL
60-sim-32b_args_on_64b all 1003 0xffffffff00000010 N N N N N ALLOW
60-sim-32b_args_on_64b all 1003 0xffffffff00000020 N N N N N KILL

60-sim-32b_args_on_64b all 1004 0x10 N N N N N ALLOW
60-sim-32b_args_on_64b all 1004 0x20 N N N N N KILL
60-sim-32b_args_on_64b all 1004 0xffffffff00000010 N N N N N ALLOW
60-sim-32b_args_on_64b all 1004 0xffffffff00000020 N N N N N KILL

60-sim-32b_args_on_64b all 1005 0x10 N N N N N ALLOW
60-sim-32b_args_on_64b all 1005 0x01 N N N N N KILL
60-sim-32b_args_on_64b all 1005 0xffffffff00000010 N N N N N ALLOW
60-sim-32b_args_on_64b all 1005 0xffffffff00000001 N N N N N KILL

60-sim-32b_args_on_64b all 1006 0x20 N N N N N ALLOW
60-sim-32b_args_on_64b all 1006 0x01 N N N N N KILL
60-sim-32b_args_on_64b all 1006 0xffffffff00000020 N N N N N ALLOW
60-sim-32b_args_on_64b all 1006 0xffffffff00000001 N N N N N KILL

60-sim-32b_args_on_64b all 1007 0xff10 N N N N N ALLOW
60-sim-32b_args_on_64b all 1007 0x01 N N N N N KILL
60-sim-32b_args_on_64b all 1007 0xffffffff0000ff10 N N N N N ALLOW
60-sim-32b_args_on_64b all 1007 0xffffffff00000001 N N N N N KILL

test type: bpf-sim-fuzz

# Testname StressCount
60-sim-32b_args_on_64b 5

test type: bpf-valgrind

# Testname
60-sim-32b_args_on_64b
9 changes: 6 additions & 3 deletions tests/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ check_PROGRAMS = \
56-basic-iterate_syscalls \
57-basic-rawsysrc \
58-live-tsync_notify \
59-basic-empty_binary_tree
59-basic-empty_binary_tree \
60-sim-32b_args_on_64b
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the new tests :)


EXTRA_DIST_TESTPYTHON = \
util.py \
Expand Down Expand Up @@ -154,7 +155,8 @@ EXTRA_DIST_TESTPYTHON = \
56-basic-iterate_syscalls.py \
57-basic-rawsysrc.py \
58-live-tsync_notify.py \
59-basic-empty_binary_tree.py
59-basic-empty_binary_tree.py \
60-sim-32b_args_on_64b.tests.py
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double bonus points for the Python tests :)


EXTRA_DIST_TESTCFGS = \
01-sim-allow.tests \
Expand Down Expand Up @@ -215,7 +217,8 @@ EXTRA_DIST_TESTCFGS = \
56-basic-iterate_syscalls.tests \
57-basic-rawsysrc.tests \
58-live-tsync_notify.tests \
59-basic-empty_binary_tree.tests
59-basic-empty_binary_tree.tests \
60-sim-32b_args_on_64b.tests

EXTRA_DIST_TESTSCRIPTS = \
38-basic-pfc_coverage.sh 38-basic-pfc_coverage.pfc \
Expand Down