Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-17 Thread Paul Moore
On Thu, Aug 17, 2017 at 1:12 AM, Tyler Hicks  wrote:
> On 08/16/2017 04:12 PM, Paul Moore wrote:
>> On Wed, Aug 16, 2017 at 4:38 PM, Tyler Hicks  wrote:
>>> On 08/16/2017 03:23 PM, Paul Moore wrote:
 On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks  
 wrote:
> On 08/16/2017 09:57 AM, Paul Moore wrote:
>> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  
>> wrote:
>>> On 08/15/2017 04:14 PM, Paul Moore wrote:
 On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  
 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.

 

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Tyler Hicks
On 08/16/2017 04:12 PM, Paul Moore wrote:
> On Wed, Aug 16, 2017 at 4:38 PM, Tyler Hicks  wrote:
>> On 08/16/2017 03:23 PM, Paul Moore wrote:
>>> On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks  wrote:
 On 08/16/2017 09:57 AM, Paul Moore wrote:
> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  
> wrote:
>> On 08/15/2017 04:14 PM, Paul Moore wrote:
>>> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  
>>> 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 w

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Paul Moore
On Wed, Aug 16, 2017 at 4:38 PM, Tyler Hicks  wrote:
> On 08/16/2017 03:23 PM, Paul Moore wrote:
>> On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks  wrote:
>>> On 08/16/2017 09:57 AM, Paul Moore wrote:
 On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  wrote:
> On 08/15/2017 04:14 PM, Paul Moore wrote:
>> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  
>> 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 was thinking that I would add a new API call to external projects to
> see if an action is supported under the current kernel. It wouldn't need
> a scmp_

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Tyler Hicks
On 08/16/2017 03:23 PM, Paul Moore wrote:
> On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks  wrote:
>> On 08/16/2017 09:57 AM, Paul Moore wrote:
>>> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  wrote:
 On 08/15/2017 04:14 PM, Paul Moore wrote:
> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  
> 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 was thinking that I would add a new API call to external projects to
see if an action is supported under the current kernel. It wouldn't need
a scmp_filter_ctx and could even be called before seccomp_init(). They
could construct their filter based on whether the new action is
supported or not (s

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Paul Moore
On Wed, Aug 16, 2017 at 12:09 PM, Tyler Hicks  wrote:
> On 08/16/2017 09:57 AM, Paul Moore wrote:
>> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  wrote:
>>> On 08/15/2017 04:14 PM, Paul Moore wrote:
 On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  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 

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Tyler Hicks
On 08/16/2017 09:57 AM, Paul Moore wrote:
> On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  wrote:
>> On 08/15/2017 04:14 PM, Paul Moore wrote:
>>> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  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.

I'll go with one of these two approaches and get a PR ready. Thanks for
your help in talking through this.

Tyler

-- 
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.


signature.asc
Description: OpenPGP digital signature


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-16 Thread Paul Moore
On Tue, Aug 15, 2017 at 5:24 PM, Tyler Hicks  wrote:
> On 08/15/2017 04:14 PM, Paul Moore wrote:
>> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  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.

-- 
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.


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-15 Thread Tyler Hicks
On 08/15/2017 04:14 PM, Paul Moore wrote:
> On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  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!

Tyler

> 
>> You made the decision that the new/modified tests should pass on newer
>> kernels that support the new actions and that, while it isn't preferred,
>> they can be considered expected failures when they fail on old kernels
>> that don't support the new actions.
> 
> For the *live* tests, not the simulated tests.  The "make check"
> command *should* successfully complete everywhere, although I don't
> regularly test on older kernels; as you mentioned previously there may
> be some known failures on older kernels.
> 
>> Unfortunately, that policy doesn't help with test failures induced by
>> valgrind. The seccomp-syscall-is-not-available conditional will always
>> evaluate to true in the bpf-valgrind tests because valgrind doesn't
>> implement wrapping of seccomp(2) and simply returns ENOSYS in all
>> situations (even if the host kernel implements the syscall). This means
>> that it is not possible to run valgrind on the simulated tests that use
>> new actions because seccomp_rule_add(3) will always fail when a new
>> action is passed to it.
> 
> See the attribute idea above.
> 

-- 
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.


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-15 Thread Paul Moore
On Mon, Aug 14, 2017 at 5:49 PM, Tyler Hicks  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.

> You made the decision that the new/modified tests should pass on newer
> kernels that support the new actions and that, while it isn't preferred,
> they can be considered expected failures when they fail on old kernels
> that don't support the new actions.

For the *live* tests, not the simulated tests.  The "make check"
command *should* successfully complete everywhere, although I don't
regularly test on older kernels; as you mentioned previously there may
be some known failures on older kernels.

> Unfortunately, that policy doesn't help with test failures induced by
> valgrind. The seccomp-syscall-is-not-available conditional will always
> evaluate to true in the bpf-valgrind tests because valgrind doesn't
> implement wrapping of seccomp(2) and simply returns ENOSYS in all
> situations (even if the host kernel implements the syscall). This means
> that it is not possible to run valgrind on the simulated tests that use
> new actions because seccomp_rule_add(3) will always fail when a new
> action is passed to it.

See the attribute idea above.

-- 
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.


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-14 Thread Tyler Hicks
On 08/14/2017 04:04 PM, Paul Moore wrote:
> On Mon, Aug 14, 2017 at 4:57 PM, Tyler Hicks  wrote:
>> On 08/14/2017 03:53 PM, Paul Moore wrote:
>>> On Fri, Aug 11, 2017 at 4:31 PM, Tyler Hicks  wrote:
 I had forgotten about an aspect of this problem. We'll want to test new
 return actions in the simulated BPF tests but it won't be possible
 because the seccomp(2) syscall will return ENOSYS under valgrind (under
 both old and new kernels). For example, the following change breaks the
 bpf-valgrind test of 06-sim-actions even when running under a kernel
 that supports the new return action:

 https://github.com/tyhicks/libseccomp/commit/2d0b7648a86a33b983d76b91055b22195887080c
>>>
>>> That's a simulated test, not a live test, why is the seccomp() syscall
>>> being called at all?  I think we have a disconnect somewhere ...
>>
>> When would you expect the seccomp() syscall to be used to verify that
>> the action specified in calls to seccomp_init() or seccomp_rule_add()
>> are valid? I have it coded up to verify the action inside of those
>> libseccomp calls. Are you saying that it should be done at some later
>> point such as seccomp_load()?
> 
> 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. :)


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.

You made the decision that the new/modified tests should pass on newer
kernels that support the new actions and that, while it isn't preferred,
they can be considered expected failures when they fail on old kernels
that don't support the new actions.

Unfortunately, that policy doesn't help with test failures induced by
valgrind. The seccomp-syscall-is-not-available conditional will always
evaluate to true in the bpf-valgrind tests because valgrind doesn't
implement wrapping of seccomp(2) and simply returns ENOSYS in all
situations (even if the host kernel implements the syscall). This means
that it is not possible to run valgrind on the simulated tests that use
new actions because seccomp_rule_add(3) will always fail when a new
action is passed to it.

> I would expect that we would have simulated tests which verify the new
> actions, as well as a live test, if the live test can be done in such
> a way that it is backwards compatible (e.g. doesn't fail/error on
> older kernels, skipping is okay).

Sounds reasonable.

Tyler


-- 
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.


signature

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-14 Thread Paul Moore
On Mon, Aug 14, 2017 at 4:57 PM, Tyler Hicks  wrote:
> On 08/14/2017 03:53 PM, Paul Moore wrote:
>> On Fri, Aug 11, 2017 at 4:31 PM, Tyler Hicks  wrote:
>>> I had forgotten about an aspect of this problem. We'll want to test new
>>> return actions in the simulated BPF tests but it won't be possible
>>> because the seccomp(2) syscall will return ENOSYS under valgrind (under
>>> both old and new kernels). For example, the following change breaks the
>>> bpf-valgrind test of 06-sim-actions even when running under a kernel
>>> that supports the new return action:
>>>
>>> https://github.com/tyhicks/libseccomp/commit/2d0b7648a86a33b983d76b91055b22195887080c
>>
>> That's a simulated test, not a live test, why is the seccomp() syscall
>> being called at all?  I think we have a disconnect somewhere ...
>
> When would you expect the seccomp() syscall to be used to verify that
> the action specified in calls to seccomp_init() or seccomp_rule_add()
> are valid? I have it coded up to verify the action inside of those
> libseccomp calls. Are you saying that it should be done at some later
> point such as seccomp_load()?

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.

I would expect that we would have simulated tests which verify the new
actions, as well as a live test, if the live test can be done in such
a way that it is backwards compatible (e.g. doesn't fail/error on
older kernels, skipping is okay).

-- 
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.


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-14 Thread Tyler Hicks
On 08/14/2017 03:53 PM, Paul Moore wrote:
> On Fri, Aug 11, 2017 at 4:31 PM, Tyler Hicks  wrote:
>> I had forgotten about an aspect of this problem. We'll want to test new
>> return actions in the simulated BPF tests but it won't be possible
>> because the seccomp(2) syscall will return ENOSYS under valgrind (under
>> both old and new kernels). For example, the following change breaks the
>> bpf-valgrind test of 06-sim-actions even when running under a kernel
>> that supports the new return action:
>>
>> https://github.com/tyhicks/libseccomp/commit/2d0b7648a86a33b983d76b91055b22195887080c
> 
> That's a simulated test, not a live test, why is the seccomp() syscall
> being called at all?  I think we have a disconnect somewhere ...

When would you expect the seccomp() syscall to be used to verify that
the action specified in calls to seccomp_init() or seccomp_rule_add()
are valid? I have it coded up to verify the action inside of those
libseccomp calls. Are you saying that it should be done at some later
point such as seccomp_load()?

Tyler

> 
>> I'm convinced that we're going to need a way for the tests to tell
>> libseccomp to skip the kernel compatibility checks but don't know how to
>> do it cleanly.
> 
> I think we just need to make sure that the live tests don't do
> anything that isn't backwards compatible.  The simulated tests
> obviously don't need that same restriction since they aren't ever
> loaded into the kernel.
> 


-- 
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.


signature.asc
Description: OpenPGP digital signature


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-14 Thread Paul Moore
On Fri, Aug 11, 2017 at 4:31 PM, Tyler Hicks  wrote:
> I had forgotten about an aspect of this problem. We'll want to test new
> return actions in the simulated BPF tests but it won't be possible
> because the seccomp(2) syscall will return ENOSYS under valgrind (under
> both old and new kernels). For example, the following change breaks the
> bpf-valgrind test of 06-sim-actions even when running under a kernel
> that supports the new return action:
>
> https://github.com/tyhicks/libseccomp/commit/2d0b7648a86a33b983d76b91055b22195887080c

That's a simulated test, not a live test, why is the seccomp() syscall
being called at all?  I think we have a disconnect somewhere ...

> I'm convinced that we're going to need a way for the tests to tell
> libseccomp to skip the kernel compatibility checks but don't know how to
> do it cleanly.

I think we just need to make sure that the live tests don't do
anything that isn't backwards compatible.  The simulated tests
obviously don't need that same restriction since they aren't ever
loaded into the kernel.

-- 
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.


Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-11 Thread Tyler Hicks
On 08/10/2017 03:02 PM, Paul Moore wrote:
> On Thu, Aug 10, 2017 at 3:24 PM, Tyler Hicks  wrote:
>> On 08/10/2017 01:56 PM, Paul Moore wrote:
>>> On Thu, Aug 10, 2017 at 2:24 PM, Tyler Hicks  wrote:
 Hello - I'm working on some libseccomp patches to support new kernel
 filter flags (SECCOMP_FILTER_FLAG_LOG and maybe
 SECCOMP_FILTER_FLAG_KILL_PROCESS) and return actions (SECCOMP_RET_LOG)
 being discussed upstream. I've bumped into an issue with the libseccomp
 test suite and would like to get some direction on how to proceed.
>>>
>>> Hi Tyler,
>>
>> Hey Paul!
>>
>>>
>>> I must apologize, I've seen your patches upstream but I haven't had a
>>> chance to give them any sort of review.
>>
>> No worries. I wasn't trying to drum up a review from you. :)
> 
> Okay, I feel less bad now :)
> 
 The problems stem from the (new) need for libseccomp to call the
 seccomp() syscall in order to verify that the kernel supports the new
 filter flags and new return action. The seccomp() syscall can already be
 used to verify that specific filter flags are supported and will likely
 soon get a new operation that allows the caller to check if a specific
 action is supported.

 The first problem is that the build tests may be running under an older
 kernel that doesn't support the new features. If specifying the new
 SECCOMP_RET_LOG as an action, seccomp_rule_add() could fail due to the
 kernel not supporting the action and there's no way in the current test
 infrastructure to handle that. Additionally, seccomp_attr_set() may fail
 when trying to set one of the new filter flags.
>>>
>>> The good news is that we already have some provisions for runtime
>>> checking of kernel capabilities, specifically the seccomp() syscall;
>>> see src/system.c, sys_chk_seccomp_syscall() and
>>> sys_chk_seccomp_flag().  I would need to better understand exactly how
>>> the detection would work with your new patches, but I expect you could
>>> do something similar.
>>
>> I'm aware of sys_chk_seccomp_syscall() and I'm building on top of it.
>> Unfortunately, it doesn't address the test issues that I'm describing.
>>
>> I suspect that tests/13-basic-attrs.c would fail today if ran on a
>> kernel that doesn't have the seccomp() syscall:
>>
>> rc = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1);
>> if (rc != 0 && rc != -EOPNOTSUPP)
>> goto out;
>> rc = seccomp_attr_get(ctx, SCMP_FLTATR_CTL_TSYNC, &val);
>> if (rc != 0)
>> goto out;
>> if (val != 1) {
>> rc = -1;
>> goto out;
>> }
>>
>> The seccomp_attr_set() would fail with EOPNOTSUPP so the test would
>> continue but then seccomp_attr_get() will return with val set to 0.
> 
> True, the 13-basic-attrs.c test would likely fail on a non-seccomp()
> system as val would always be zero, but that really isn't too much of
> a problem as we can treat that as a "known failure" when running the
> tests on older systems.  The important part is that seccomp_attr_set()
> and seccomp_attr_get() work correctly on both new and old systems, and
> they do.  The problem isn't with the libseccomp library code, but
> rather the test.
> 
>> This isn't a problem in practice because everyone is building
>> libseccomp() on a kernel with seccomp(2) because that syscall has been
>> around for a while.
> 
> ... and for those few who are using an older system, the "known
> failure" route is a reasonable one.  They may even patch around the
> test.
> 
>> It will be a problem in practice once new filter flags land (in the
>> kernel and libseccomp) and someone tries building libseccomp on a kernel
>> that doesn't yet support them.
> 
> The #1 priority is to make sure that the library API works correctly
> (see my comments above about seccomp_attr_{get,set}(...)), the #2
> priority is to make the test work on both old and new systems.  If we
> can't do #1, then we will not ever be able to use the new
> functionality in libseccomp.

Agreed. I've already laid the foundation, at the kernel level, for the
#1 priority:

* For actions:

https://lkml.kernel.org/r/1502426037-3777-3-git-send-email-tyhi...@canonical.com
* For flags:

https://lkml.kernel.org/r/1502426037-3777-5-git-send-email-tyhi...@canonical.com

I've got libseccomp patches that take advantage of these concepts to
validate actions and flags:

* For actions:

https://github.com/tyhicks/libseccomp/commit/1f7daa016433fa8f9fd9b5eaadd2ad87816223f7
* For flags:

https://github.com/tyhicks/libseccomp/commit/d47e68c83fbb18d5f0036de19d7e7c9a3a50264b

(I'm waiting for the dust to settle in the kernel before creating a PR.)

This thread is only about the #2 priority.

 The second problem is with the valgrind tests. Valgrind doesn't wrap
 seccomp(2):

   https://bugs.kde.org/show_bug.cgi?id=345414
   https://bugs.kde.org/show_bug.cgi?id=380183

 This means that the valgrind test

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-10 Thread Paul Moore
On Thu, Aug 10, 2017 at 3:24 PM, Tyler Hicks  wrote:
> On 08/10/2017 01:56 PM, Paul Moore wrote:
>> On Thu, Aug 10, 2017 at 2:24 PM, Tyler Hicks  wrote:
>>> Hello - I'm working on some libseccomp patches to support new kernel
>>> filter flags (SECCOMP_FILTER_FLAG_LOG and maybe
>>> SECCOMP_FILTER_FLAG_KILL_PROCESS) and return actions (SECCOMP_RET_LOG)
>>> being discussed upstream. I've bumped into an issue with the libseccomp
>>> test suite and would like to get some direction on how to proceed.
>>
>> Hi Tyler,
>
> Hey Paul!
>
>>
>> I must apologize, I've seen your patches upstream but I haven't had a
>> chance to give them any sort of review.
>
> No worries. I wasn't trying to drum up a review from you. :)

Okay, I feel less bad now :)

>>> The problems stem from the (new) need for libseccomp to call the
>>> seccomp() syscall in order to verify that the kernel supports the new
>>> filter flags and new return action. The seccomp() syscall can already be
>>> used to verify that specific filter flags are supported and will likely
>>> soon get a new operation that allows the caller to check if a specific
>>> action is supported.
>>>
>>> The first problem is that the build tests may be running under an older
>>> kernel that doesn't support the new features. If specifying the new
>>> SECCOMP_RET_LOG as an action, seccomp_rule_add() could fail due to the
>>> kernel not supporting the action and there's no way in the current test
>>> infrastructure to handle that. Additionally, seccomp_attr_set() may fail
>>> when trying to set one of the new filter flags.
>>
>> The good news is that we already have some provisions for runtime
>> checking of kernel capabilities, specifically the seccomp() syscall;
>> see src/system.c, sys_chk_seccomp_syscall() and
>> sys_chk_seccomp_flag().  I would need to better understand exactly how
>> the detection would work with your new patches, but I expect you could
>> do something similar.
>
> I'm aware of sys_chk_seccomp_syscall() and I'm building on top of it.
> Unfortunately, it doesn't address the test issues that I'm describing.
>
> I suspect that tests/13-basic-attrs.c would fail today if ran on a
> kernel that doesn't have the seccomp() syscall:
>
> rc = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1);
> if (rc != 0 && rc != -EOPNOTSUPP)
> goto out;
> rc = seccomp_attr_get(ctx, SCMP_FLTATR_CTL_TSYNC, &val);
> if (rc != 0)
> goto out;
> if (val != 1) {
> rc = -1;
> goto out;
> }
>
> The seccomp_attr_set() would fail with EOPNOTSUPP so the test would
> continue but then seccomp_attr_get() will return with val set to 0.

True, the 13-basic-attrs.c test would likely fail on a non-seccomp()
system as val would always be zero, but that really isn't too much of
a problem as we can treat that as a "known failure" when running the
tests on older systems.  The important part is that seccomp_attr_set()
and seccomp_attr_get() work correctly on both new and old systems, and
they do.  The problem isn't with the libseccomp library code, but
rather the test.

> This isn't a problem in practice because everyone is building
> libseccomp() on a kernel with seccomp(2) because that syscall has been
> around for a while.

... and for those few who are using an older system, the "known
failure" route is a reasonable one.  They may even patch around the
test.

> It will be a problem in practice once new filter flags land (in the
> kernel and libseccomp) and someone tries building libseccomp on a kernel
> that doesn't yet support them.

The #1 priority is to make sure that the library API works correctly
(see my comments above about seccomp_attr_{get,set}(...)), the #2
priority is to make the test work on both old and new systems.  If we
can't do #1, then we will not ever be able to use the new
functionality in libseccomp.

>>> The second problem is with the valgrind tests. Valgrind doesn't wrap
>>> seccomp(2):
>>>
>>>   https://bugs.kde.org/show_bug.cgi?id=345414
>>>   https://bugs.kde.org/show_bug.cgi?id=380183
>>>
>>> This means that the valgrind tests will always fail because libseccomp
>>> will see ENOSYS when attempting to verify that the kernel supports those
>>> new filter flags and the new action.
>>
>> At present libseccomp does not run valgrind on the "live" tests, only
>> the simulated BPF tests so this shouldn't be an issue.  If it was, we
>> would be seeing failures with the current (and past) releases.
>
> Currently libseccomp only changes behavior based on the presence of the
> seccomp() syscall when setting the TSYNC attribute and when loading a
> filter. As you said, valgrind doesn't run on the live tests so the
> latter isn't an issue.
>
> I think we'd see a failure in tests/13-basic-attrs.tests if valgrind was
> enabled for that test (only the "basic" test type is specified).

We don't run any valgrind tests on 13-basic-attrs, so it isn't worth
worrying about.

>>> The best 

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-10 Thread Tyler Hicks
On 08/10/2017 01:56 PM, Paul Moore wrote:
> On Thu, Aug 10, 2017 at 2:24 PM, Tyler Hicks  wrote:
>> Hello - I'm working on some libseccomp patches to support new kernel
>> filter flags (SECCOMP_FILTER_FLAG_LOG and maybe
>> SECCOMP_FILTER_FLAG_KILL_PROCESS) and return actions (SECCOMP_RET_LOG)
>> being discussed upstream. I've bumped into an issue with the libseccomp
>> test suite and would like to get some direction on how to proceed.
> 
> Hi Tyler,

Hey Paul!

> 
> I must apologize, I've seen your patches upstream but I haven't had a
> chance to give them any sort of review.

No worries. I wasn't trying to drum up a review from you. :)

> 
>> The problems stem from the (new) need for libseccomp to call the
>> seccomp() syscall in order to verify that the kernel supports the new
>> filter flags and new return action. The seccomp() syscall can already be
>> used to verify that specific filter flags are supported and will likely
>> soon get a new operation that allows the caller to check if a specific
>> action is supported.
>>
>> The first problem is that the build tests may be running under an older
>> kernel that doesn't support the new features. If specifying the new
>> SECCOMP_RET_LOG as an action, seccomp_rule_add() could fail due to the
>> kernel not supporting the action and there's no way in the current test
>> infrastructure to handle that. Additionally, seccomp_attr_set() may fail
>> when trying to set one of the new filter flags.
> 
> The good news is that we already have some provisions for runtime
> checking of kernel capabilities, specifically the seccomp() syscall;
> see src/system.c, sys_chk_seccomp_syscall() and
> sys_chk_seccomp_flag().  I would need to better understand exactly how
> the detection would work with your new patches, but I expect you could
> do something similar.

I'm aware of sys_chk_seccomp_syscall() and I'm building on top of it.
Unfortunately, it doesn't address the test issues that I'm describing.

I suspect that tests/13-basic-attrs.c would fail today if ran on a
kernel that doesn't have the seccomp() syscall:

rc = seccomp_attr_set(ctx, SCMP_FLTATR_CTL_TSYNC, 1);
if (rc != 0 && rc != -EOPNOTSUPP)
goto out;
rc = seccomp_attr_get(ctx, SCMP_FLTATR_CTL_TSYNC, &val);
if (rc != 0)
goto out;
if (val != 1) {
rc = -1;
goto out;
}

The seccomp_attr_set() would fail with EOPNOTSUPP so the test would
continue but then seccomp_attr_get() will return with val set to 0.

This isn't a problem in practice because everyone is building
libseccomp() on a kernel with seccomp(2) because that syscall has been
around for a while.

It will be a problem in practice once new filter flags land (in the
kernel and libseccomp) and someone tries building libseccomp on a kernel
that doesn't yet support them.

>> The second problem is with the valgrind tests. Valgrind doesn't wrap
>> seccomp(2):
>>
>>   https://bugs.kde.org/show_bug.cgi?id=345414
>>   https://bugs.kde.org/show_bug.cgi?id=380183
>>
>> This means that the valgrind tests will always fail because libseccomp
>> will see ENOSYS when attempting to verify that the kernel supports those
>> new filter flags and the new action.
> 
> At present libseccomp does not run valgrind on the "live" tests, only
> the simulated BPF tests so this shouldn't be an issue.  If it was, we
> would be seeing failures with the current (and past) releases.

Currently libseccomp only changes behavior based on the presence of the
seccomp() syscall when setting the TSYNC attribute and when loading a
filter. As you said, valgrind doesn't run on the live tests so the
latter isn't an issue.

I think we'd see a failure in tests/13-basic-attrs.tests if valgrind was
enabled for that test (only the "basic" test type is specified).

>> The best solution that I can think of is for libseccomp to call
>> secure_getenv(), prior to calling seccomp() to check feature support,
>> and always blindly assume that a feature is supported if a "magic"
>> environment variable is set. The test runner would set that env variable
>> prior to running each test. Is this an acceptable solution? If not, do
>> you have any ideas that you like better?
> 
> My initial reaction to using environment variables in this manner is
> not positive.  Let's try to do runtime checking in a similar manner as
> we are doing now (see above comments) and if that doesn't work we can
> reevaluate things, sound good?

I don't like the env variable approach either, which is why I wanted to
bring it up early.

I have a slightly out of date (local) git branch that adds support for
SECCOMP_FILTER_FLAG_LOG and SECCOMP_RET_LOG, using runtime checking that
you're suggesting, that runs into the test failures that I initially
described in this email thread.

Note that I'm fine with delaying this discussion until the kernel
patches are accepted and the libseccomp PR is proposed but I wanted to
plant the seed ahead of 

Re: [libseccomp] Preferred way of telling libseccomp that it is being tested

2017-08-10 Thread Paul Moore
On Thu, Aug 10, 2017 at 2:24 PM, Tyler Hicks  wrote:
> Hello - I'm working on some libseccomp patches to support new kernel
> filter flags (SECCOMP_FILTER_FLAG_LOG and maybe
> SECCOMP_FILTER_FLAG_KILL_PROCESS) and return actions (SECCOMP_RET_LOG)
> being discussed upstream. I've bumped into an issue with the libseccomp
> test suite and would like to get some direction on how to proceed.

Hi Tyler,

I must apologize, I've seen your patches upstream but I haven't had a
chance to give them any sort of review.

> The problems stem from the (new) need for libseccomp to call the
> seccomp() syscall in order to verify that the kernel supports the new
> filter flags and new return action. The seccomp() syscall can already be
> used to verify that specific filter flags are supported and will likely
> soon get a new operation that allows the caller to check if a specific
> action is supported.
>
> The first problem is that the build tests may be running under an older
> kernel that doesn't support the new features. If specifying the new
> SECCOMP_RET_LOG as an action, seccomp_rule_add() could fail due to the
> kernel not supporting the action and there's no way in the current test
> infrastructure to handle that. Additionally, seccomp_attr_set() may fail
> when trying to set one of the new filter flags.

The good news is that we already have some provisions for runtime
checking of kernel capabilities, specifically the seccomp() syscall;
see src/system.c, sys_chk_seccomp_syscall() and
sys_chk_seccomp_flag().  I would need to better understand exactly how
the detection would work with your new patches, but I expect you could
do something similar.

> The second problem is with the valgrind tests. Valgrind doesn't wrap
> seccomp(2):
>
>   https://bugs.kde.org/show_bug.cgi?id=345414
>   https://bugs.kde.org/show_bug.cgi?id=380183
>
> This means that the valgrind tests will always fail because libseccomp
> will see ENOSYS when attempting to verify that the kernel supports those
> new filter flags and the new action.

At present libseccomp does not run valgrind on the "live" tests, only
the simulated BPF tests so this shouldn't be an issue.  If it was, we
would be seeing failures with the current (and past) releases.

> The best solution that I can think of is for libseccomp to call
> secure_getenv(), prior to calling seccomp() to check feature support,
> and always blindly assume that a feature is supported if a "magic"
> environment variable is set. The test runner would set that env variable
> prior to running each test. Is this an acceptable solution? If not, do
> you have any ideas that you like better?

My initial reaction to using environment variables in this manner is
not positive.  Let's try to do runtime checking in a similar manner as
we are doing now (see above comments) and if that doesn't work we can
reevaluate things, sound good?

-- 
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.