Permalink
Browse files

api: create an API level construct as part of the supported API

WORK IN PROGRESS, DO NOT SHIP

XXX - manpage needed
XXX - tests needed

Signed-off-by: Paul Moore <paul@paul-moore.com>
  • Loading branch information...
pcmoore committed Sep 21, 2017
1 parent 9e61fd7 commit 355953c00ae34083f8acd89eac3360707e02dfaf
Showing with 98 additions and 3 deletions.
  1. +10 −0 include/seccomp.h.in
  2. +55 −0 src/api.c
  3. +30 −3 src/system.c
  4. +3 −0 src/system.h
View
@@ -274,6 +274,16 @@ struct scmp_arg_cmp {
*/
const struct scmp_version *seccomp_version(void);
/**
* XXX
*/
const unsigned int seccomp_api_get(void);
/**
* XXX
*/
int seccomp_api_set(unsigned int level);
/**
* Initialize the filter state
* @param def_action the default filter action
View
@@ -44,6 +44,8 @@ const struct scmp_version library_version = {
.micro = SCMP_VER_MICRO,
};
unsigned int seccomp_api_level = 0;
/**
* Validate a filter context
* @param ctx the filter context
@@ -75,12 +77,65 @@ static int _syscall_valid(const struct db_filter_col *col, int syscall)
return 0;
}
/**
* Update the API level
*
* XXX
*/
static void _seccomp_api_update(void)
{
unsigned int level = 1;
/* if seccomp_api_level > 0 then it's already been set, we're done */
if (seccomp_api_level >= 1)
return;
/* NOTE: level 1 is the base level, start checking at 2 */
/* level 2 */
if (sys_chk_seccomp_syscall() &&
sys_chk_seccomp_flag(SECCOMP_FILTER_FLAG_TSYNC))
level = 2;
/* update the stored api level */
seccomp_api_level = level;
}
/* NOTE - function header comment in include/seccomp.h */
API const struct scmp_version *seccomp_version(void)
{
return &library_version;
}
/* NOTE - function header comment in include/seccomp.h */
API const unsigned int seccomp_api_get(void)
{
/* update the api level, if needed */
_seccomp_api_update();
return seccomp_api_level;
}
/* NOTE - function header comment in include/seccomp.h */
API int seccomp_api_set(unsigned int level)
{
switch (level) {
case 1:
sys_set_seccomp_syscall(false);
sys_set_seccomp_flag(SECCOMP_FILTER_FLAG_TSYNC, false);
break;
case 2:
sys_set_seccomp_syscall(true);
sys_set_seccomp_flag(SECCOMP_FILTER_FLAG_TSYNC, true);
break;
default:
return -EINVAL;
}
seccomp_api_level = level;
return 0;
}
/* NOTE - function header comment in include/seccomp.h */
API scmp_filter_ctx seccomp_init(uint32_t def_action)
{
View
@@ -40,6 +40,7 @@
static int _nr_seccomp = -1;
static int _support_seccomp_syscall = -1;
static int _support_seccomp_flag_tsync = -1;
/**
* Check to see if the seccomp() syscall is supported
@@ -97,25 +98,51 @@ int sys_chk_seccomp_syscall(void)
return 1;
}
/**
* XXX
*/
void sys_set_seccomp_syscall(bool enable)
{
_support_seccomp_syscall = (enable ? 1 : 0);
}
/**
* Check to see if a seccomp() flag is supported
* @param flag the seccomp() flag
*
* This function checks to see if a seccomp() flag is supported by the system.
* If the flag is supported one is returned, zero if unsupported, negative
* values on error.
* Return one if the syscall is supported, zero if unsupported, negative values
* on error.
*
*/
int sys_chk_seccomp_flag(int flag)
{
int rc;
switch (flag) {
case SECCOMP_FILTER_FLAG_TSYNC:
return sys_chk_seccomp_syscall();
if (_support_seccomp_flag_tsync < 0) {
rc = sys_chk_seccomp_syscall();
_support_seccomp_flag_tsync = (rc == 1 ? 1 : 0);
}
return _support_seccomp_flag_tsync;
}
return -EOPNOTSUPP;
}
/**
* XXX
*/
void sys_set_seccomp_flag(int flag, bool enable)
{
switch (flag) {
case SECCOMP_FILTER_FLAG_TSYNC:
_support_seccomp_flag_tsync = (enable ? 1 : 0);
break;
}
}
/**
* Loads the filter into the kernel
* @param col the filter collection
View
@@ -109,7 +109,10 @@ typedef struct sock_filter bpf_instr_raw;
#endif
int sys_chk_seccomp_syscall(void);
void sys_set_seccomp_syscall(bool enable);
int sys_chk_seccomp_flag(int flag);
void sys_set_seccomp_flag(int flag, bool enable);
int sys_filter_load(const struct db_filter_col *col);

5 comments on commit 355953c

@tyhicks

This comment has been minimized.

Show comment
Hide comment
@tyhicks

tyhicks Sep 21, 2017

Contributor

Ah, very nice design. I never quite got on the same page as you when we were discussing this verbally but now it is clear.

I was initially expecting seccomp_init() to call _seccomp_api_update() but, after thinking about it some more, I don't think it is needed. Applications that care about the API level can trigger the API level checks themselves before calling seccomp_init().

I like the fact that the API level is a simple integer instead of attempting to come up with a meaningful name for each level. The man page can take care of explaining what each integer represents. I'm curious if you foresee the new log filter flag, log action, and process kill action all being represented by level '3' or would, for example, the logging flag and action be level '3' and the process kill action be level '4'? My question is assuming that the new flag and actions would all be supported in the upcoming 2.4 libseccomp release. If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

Contributor

tyhicks replied Sep 21, 2017

Ah, very nice design. I never quite got on the same page as you when we were discussing this verbally but now it is clear.

I was initially expecting seccomp_init() to call _seccomp_api_update() but, after thinking about it some more, I don't think it is needed. Applications that care about the API level can trigger the API level checks themselves before calling seccomp_init().

I like the fact that the API level is a simple integer instead of attempting to come up with a meaningful name for each level. The man page can take care of explaining what each integer represents. I'm curious if you foresee the new log filter flag, log action, and process kill action all being represented by level '3' or would, for example, the logging flag and action be level '3' and the process kill action be level '4'? My question is assuming that the new flag and actions would all be supported in the upcoming 2.4 libseccomp release. If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

@tyhicks

This comment has been minimized.

Show comment
Hide comment
@tyhicks

tyhicks Sep 21, 2017

Contributor

If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

The only value in splitting them up would be at the distro level where a package maintainer wanted to backport, for example, the logging changes (level '3') but not the process kill changes (level '4'). However, that feature backport model breaks down completely if the package maintainer wanted to backport the process kill changes but not the logging changes since the process kill changes are a higher API level. Therefore, I think that the API level should only be assigned at a libseccomp release granularity and not at an individual feature granularity.

Contributor

tyhicks replied Sep 21, 2017

If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

The only value in splitting them up would be at the distro level where a package maintainer wanted to backport, for example, the logging changes (level '3') but not the process kill changes (level '4'). However, that feature backport model breaks down completely if the package maintainer wanted to backport the process kill changes but not the logging changes since the process kill changes are a higher API level. Therefore, I think that the API level should only be assigned at a libseccomp release granularity and not at an individual feature granularity.

@pcmoore

This comment has been minimized.

Show comment
Hide comment
@pcmoore

pcmoore Sep 21, 2017

Member

Ah, very nice design. I never quite got on the same page as you when we were discussing this verbally but now it is clear.

Yes, sometimes having code, even if it is incomplete, is helpful in providing clarity.

I like the fact that the API level is a simple integer instead of attempting to come up with a meaningful name for each level. The man page can take care of explaining what each integer represents. I'm curious if you foresee the new log filter flag, log action, and process kill action all being represented by level '3' or would, for example, the logging flag and action be level '3' and the process kill action be level '4'? My question is assuming that the new flag and actions would all be supported in the upcoming 2.4 libseccomp release. If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

Yes, I agree, my thought was that all of the stuff you are proposing would be combined in level 3.

Member

pcmoore replied Sep 21, 2017

Ah, very nice design. I never quite got on the same page as you when we were discussing this verbally but now it is clear.

Yes, sometimes having code, even if it is incomplete, is helpful in providing clarity.

I like the fact that the API level is a simple integer instead of attempting to come up with a meaningful name for each level. The man page can take care of explaining what each integer represents. I'm curious if you foresee the new log filter flag, log action, and process kill action all being represented by level '3' or would, for example, the logging flag and action be level '3' and the process kill action be level '4'? My question is assuming that the new flag and actions would all be supported in the upcoming 2.4 libseccomp release. If they were all initially present in the same libseccomp release, I'd think that representing them all with level '3' would be fine.

Yes, I agree, my thought was that all of the stuff you are proposing would be combined in level 3.

@tyhicks

This comment has been minimized.

Show comment
Hide comment
@tyhicks

tyhicks Oct 4, 2017

Contributor

FYI, I rebased my patch set onto this commit to get a better idea how it would all work together. The tests all pass under a v4.14-rc3-102-gd81fa66 kernel. Under an older kernel, the only tests that fail are some of the Python tests due to the Python bindings not yet having API level support.

If it helps, here's my working branch:

https://github.com/tyhicks/libseccomp/tree/working-improved_logging_api_level

Contributor

tyhicks replied Oct 4, 2017

FYI, I rebased my patch set onto this commit to get a better idea how it would all work together. The tests all pass under a v4.14-rc3-102-gd81fa66 kernel. Under an older kernel, the only tests that fail are some of the Python tests due to the Python bindings not yet having API level support.

If it helps, here's my working branch:

https://github.com/tyhicks/libseccomp/tree/working-improved_logging_api_level

@pcmoore

This comment has been minimized.

Show comment
Hide comment
@pcmoore

pcmoore Oct 4, 2017

Member

Thanks.

I'm going to give my self a deadline of Monday morning, October 9th to get this RFC polished up and merged into master. If you don't see something in the repo by then, send me some hate mail.

Member

pcmoore replied Oct 4, 2017

Thanks.

I'm going to give my self a deadline of Monday morning, October 9th to get this RFC polished up and merged into master. If you don't see something in the repo by then, send me some hate mail.

Please sign in to comment.