oasis-open / openc2-lycan-python

OASIS TC Open Repository: A GitHub public repository for development of a python library to transform between data-interchange formats (such as JSON) and python language objects
https://github.com/oasis-open/openc2-lycan-python
Other
11 stars 12 forks source link

Unable to create empty Features #27

Closed twodayslate closed 4 years ago

twodayslate commented 4 years ago
import openc2

cmd = openc2.Command(action="query", target=openc2.Features())
% python feature_test.py         
Traceback (most recent call last):
  File "feature_test.py", line 3, in <module>
    cmd = openc2.Command(action="query", target=openc2.Features(), args=args)
  File "/Users/user/.pyenv/versions/kube/lib/python3.7/site-packages/openc2/base.py", line 57, in __init__
    super(_OpenC2Base, self).__init__(allow_custom, **kwargs)
  File "/Users/user/.pyenv/versions/kube/lib/python3.7/site-packages/stix2/base.py", line 197, in __init__
    self._check_object_constraints()
  File "/Users/user/.pyenv/versions/kube/lib/python3.7/site-packages/openc2/v10/targets.py", line 81, in _check_object_constraints
    self._check_at_least_one_property()
  File "/Users/user/.pyenv/versions/kube/lib/python3.7/site-packages/stix2/base.py", line 134, in _check_at_least_one_property
    raise AtLeastOnePropertyError(self.__class__, list_of_properties)
stix2.exceptions.AtLeastOnePropertyError: At least one of the (features) properties for Features must be populated.

According to the specification:

A Producer MAY send a 'query' Command containing an empty list of features.

The following also fails

import openc2

cmd = openc2.Command(action="query", target=openc2.Features(features=[]))

It also appears that you can only send the 4 default features and no other ones.

I was at the latest Plug Fest but just getting around to using this library now.

mstair commented 4 years ago

Thanks, this was identified at plugfest, I'll get addressed.

mstair commented 4 years ago

Handled in #29, both ways work:

>>> import openc2
>>> cmd = openc2.Command(action="query", target=openc2.Features())
>>> cmd.serialize()
'{"action": "query", "target": {"features": []}}'
>>> cmd = openc2.Command(action="query", target=openc2.Features(features=[]))
>>> cmd.serialize()
'{"action": "query", "target": {"features": []}}'

wrt only send the 4 defaults and no others, my understand is that is correct behavior. Anything beyond would be defined as an extension and a custom target, https://docs.oasis-open.org/openc2/oc2ls/v1.0/cs02/oc2ls-v1.0-cs02.html#3424-feature. I have a request to the TC to verify that assumption, if I am interpreting incorrectly, I'll submit another PR.

mstair commented 4 years ago

1.0.3 pushed to pip

twodayslate commented 4 years ago

Have they responded about the custom feature yet?

The specification says there can be 10 unique items, the current implementation basically says it is 4. #31 reverts it but can be changed. If it is to be kept, a test case with a custom feature target should be added.

mstair commented 4 years ago

I think its allowed, I just asked for clarification in Slack on how it gets serialized

davaya commented 4 years ago

The philosophy of the tables is to define upper bounds wherever possible in order to detect and reject attacks with, e.g., a million random feature keywords. There's nothing special about an upper bound of 10; it just seemed like a reasonable number that allowed some room for growth (from 4) before needing modification.

Actuator profiles cannot define custom feature keywords - "Features" is the set of keywords defined by the language spec, fixed until the LS is updated. Actuator profiles CAN define custom targets, and could define a Features target containing custom keywords. It might be less confusing to pick a different name, but "query features" is a different command than "query ap-av/features", so it is technically possible to use the same name for the LS target and an AP target. They are two distinct targets, and two distinct results however, so the same command couldn't ask for both standard and custom features mixed together.

https://github.com/oasis-tcs/openc2-usecases/blob/master/Cybercom-Plugfest/TestData/language/commands/good/query_features_all.json is

{
  "action": "query",
  "target": {
    "features": [ "profiles", "versions", "pairs", "rate_limit" ]
  }
}

A custom

twodayslate commented 4 years ago

@davaya

To clarify...

So it sounds like the check to make sure only versions, profiles, pairs, and rate_limit are used should be put back in.

It might be less confusing to pick a different name, but "query features" is a different command than "query ap-av/features", so it is technically possible to use the same name for the LS target and an AP target. They are two distinct targets, and two distinct results however, so the same command couldn't ask for both standard and custom features mixed together.

Custom targets must be prefixed with x- correct? So the same name of features cannot be used. To duplicate the functionality a non-standard actuator could create a custom target x-custom:features and define it's own Feature-like enum.

davaya commented 4 years ago

Only those 4 values are legal in the LS Features. If an implementation checks that commands are valid, those 4 values would be included in the check.

Version 1.0 has inconsistent treatment between targets and the other extensible lists (args, specifiers, and results). The next version will treat targets consistently with args, specifiers and results, so "x-custom:features": ["pairs"] would become "x_custom": {"features": ["pairs"]}. For v1.0, you are correct.

V1.0 says custom targets should be prefixed with x-, but the naming conventions say property name use underscores, not dashes. That inconsistency will need to be fixed in the next version too.

V1.0 has the bizarre use of a colon between property names. The next version will use standard RFC 6901 syntax "/" in strings that refer to profile-defined targets: the response to query features pairs command for the "slpf": {"rule_id": 32} target would be the RFC 6901-compliant string ["slpf/rule_id"].

twodayslate commented 4 years ago

@davaya Those all sound like great improvements. Looking forward to it. For now I'll try to separate the test cases to specify they are v1.0. ETA on the next version?