On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks <tyhi...@canonical.com> wrote: > On 08/16/2017 09:57 AM, Paul Moore wrote: >> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks <tyhi...@canonical.com> wrote: >>> On 08/15/2017 04:14 PM, Paul Moore wrote: >>>> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks <tyhi...@canonical.com> wrote: >>>>> On 08/14/2017 04:04 PM, Paul Moore wrote: >>>>>> First, I think some clarification is in order. The only tests that >>>>>> are executed using the host kernel's seccomp mechanism are the >>>>>> tests/*-live-* tests. All of the tests/*-sim-* tests, which are the >>>>>> majority, are run through a seccomp/BPF simulator to avoid >>>>>> compatibility problems with the host kernel. >>>>> >>>>> Thanks for the clarification. It matches my understanding. Now I suppose >>>>> I should try to clarify the problem that I'm trying to solve. If this >>>>> doesn't help, I think it is best to just talk through code via the PR >>>>> that I'm preparing. :) >>>> >>>> Yes, that always helps remove any confusion and ambiguity :) >>>> >>>>> As it stands today, libseccomp validates seccomp actions specified in >>>>> seccomp_init(3) and seccomp_rule_add(3) by checking the action against a >>>>> known-good list of actions. This is possible because these were the >>>>> actions that were available when libseccomp was initially written. Those >>>>> libseccomp calls don't have to query the kernel today and that makes it >>>>> possible for the simulated tests, which call those two libseccomp >>>>> functions, to not rely on the features available in the host kernel. >>>>> >>>>> Two new actions (SECCOMP_RET_LOG, SECCOMP_RET_KILL_PROCESS) are on their >>>>> way to being merged into the kernel along with a new seccomp(2) >>>>> operation (SECCOMP_GET_ACTION_AVAIL) to validate that the kernel >>>>> supports a given action. The action validation code behind >>>>> seccomp_init(3) and seccomp_rule_add(3) will need to be changed to use >>>>> the new seccomp(2) operation to validate that the kernel supports the >>>>> action specified in the action-related parameters of those two >>>>> libseccomp functions. The logic will need to look something like this >>>>> pseudo code: >>>>> >>>>> func bool db_action_valid(action): >>>>> # Is this one of the actions that were implemented from the start? >>>>> if action in [RET_KILL, RET_TRAP, RET_ERRNO, RET_TRACE, RET_ALLOW]: >>>>> return True >>>>> >>>>> # This is potentially a new action, implemented after libseccomp's >>>>> # first release. Ask the kernel if the action is supported. Assume it >>>>> # is not supported if the seccomp(2) syscall isn't available. >>>>> if seccomp-syscall-is-not-available: >>>>> return False >>>>> else if seccomp(SECCOMP_GET_ACTION_AVAIL, 0, action) != 0: >>>>> return False >>>>> >>>>> return True >>>>> >>>>> With seccomp_init(3) and seccomp_rule_add(3) having to run actions >>>>> through this code, you can imagine how the simulated tests are no longer >>>>> decoupled from the host kernel's feature set. Their output will vary >>>>> depending on the actions that the host kernel supports. >>>> >>>> Okay, I see your concern. I'm open to suggestions, but I think we >>>> could resolve this by the creation of a new filter attribute that >>>> would force the compat/support to a specific level regardless, or >>>> independent, of the runtime detection. >>> >>> Interesting idea! It doesn't work if the default action, passed to >>> seccomp_init(3), is a new action but I'll think about some tricks that I >>> can do in that area. Thanks! >> >> One possible solution would be to modify seccomp_attr_set() such that >> a NULL ctx pointer would be allowed for specific attributes (e.g. the >> one we are discussing). While we've never needed to set system-wide >> attributes in the past, they have all been filter specific, I see no >> reason why we couldn't do this. > > Thanks! That's a good idea. > > Another idea that I came up with last night is to keep db_action_valid() > the same in that it only checks the action against a list of known-good > actions. However, it would also set a bit in a new bitmask added to > struct db_filter_col when a "new" action was used in a constructing a > filter. The code behind seccomp_load() would go through the bitmask and > verify that the kernel supports each "new" action used, using the > SECCOMP_GET_ACTION_AVAIL operation, before loading the filter.
My only concern with this approach is that the error is delayed until filter load time, making it hard for developers to trace it back to the particular line of code which is causing the problem. > I'll go with one of these two approaches and get a PR ready. Thanks for > your help in talking through this. No problem, thanks in advance for the patches :) -- paul moore www.paul-moore.com -- You received this message because you are subscribed to the Google Groups "libseccomp" group. To unsubscribe from this group and stop receiving emails from it, send an email to libseccomp+unsubscr...@googlegroups.com. To post to this group, send email to libseccomp@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
