Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

Add support for negative values of the "instance" element of the textfilecontent54_object and textfilecontent54_state elements #146

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iankko
Copy link

@iankko iankko commented Aug 4, 2016

In certain scenarios it is necessary to obtain not the first, but the last match of a specific match in the <textfilecontent54_object> and <textfilecontent54_state> elements.

This situation happens when the textfilecontent file allows multiple occurrences of the same directive to be present, and have different values. And at the end, value of the directive specified as the last one is truly used for the configuration of the system. Examples of such configuration files:

  • /etc/login.defs,
  • /etc/profile,
  • /etc/bashrc,
  • /etc/csh.cshrc

There are more configuration files / services following this behavioral pattern. More representatives of them can be provided upon request if necessary.

The instance element of <textfilecontent54_object> and <textfilecontent54_state> allows to specify particular match, that should be retrieved (first match will have value of 1, second value of 2, and so on.) But since when retrieving textfilecontent54 file content it is not know, how many instances of specific directive will be present, it's currently not possible to obtain the last one (that will be taken in effect).

Therefore we propose enhancement of the instance element of the <textfilecontent54_object> and <textfilecontent54_state>complex elements to support also negative values with the meaning as follows:

  • The last match is given an instance of -1,
  • The penultimate match is given an instance value of -2,
  • and so on.

Additional Information: It was not necessary to change the current OVAL datatype for the <instance> element, since the current type (oval-def:EntityObjectIntType) is able to handle also negative numbers. Therefore we enhanced just the documentation for the <instance> element of these two complex element to documented the desired / modified behavior.

Regards, Jan

the textfilecontent54_object and textfilecontent54_state elements
@mpreisler
Copy link
Contributor

+1 to this. A very useful change that doesn't break compatibility because it defines undefined behavior.

@mpreisler
Copy link
Contributor

Shouldn't this be changing the independent schema instead of introducing a new file? It's very difficult to see what the actual change is.

@solind
Copy link
Contributor

solind commented Sep 8, 2016

See also, #18

@solind
Copy link
Contributor

solind commented Sep 9, 2016

Question for you Martin,

Let's say that there are 5 instances of a match in a file. If the object is constructed with -1, then will the matching item have (a) 5, or (b) -1?

I am thinking (a) -- the item's instance value should be the same as it was originally.

However, the way you've documented the instance entity of the textfilecontent54_state actually implies that scanners should produce TWO items for every instance found: one with the original value, and another with the negative value. That is the only way that permitting a negative number in the state will work reliably, as I think we should not permit the collection methodology to dictate the usability of a state. And if we were to do that, of course, we'd break any content that's actually counting the number of items, and potentially also any content that's using variable values based on textfilecontent54_item components.

LMKWYT!

@solind
Copy link
Contributor

solind commented Sep 9, 2016

For that matter... what to do, with a negative object/instance value and an operation like "greater than" or "less than"? I would say, perhaps we should require use of the "equals" operator in conjunction with a negative number (and enforce with a schematron rule), or assume in the case of negative numbers that the values end at 0?

Say you wanted all but the last value. You could use "less than" -1.

Say that you want only the last three values. You might think to use "greater than or equals" -3, but you certainly wouldn't want to start getting item matches with positive instance values!

Did you have any thoughts about how to deal with these cases?

@mpreisler
Copy link
Contributor

Hmm, now I understand why the previous similar proposal had a new direction attribute instead of the negative instance. Maybe we will have to do that instead of negative instance unless we find an elegant solution to this.

@iankko I had the impression that negative instance will only be allowed in state we are comparing to and not the collected items. Can you confirm that that's the case? How do the operators work with your proposal?

@solind
Copy link
Contributor

solind commented Sep 15, 2016

If this proposal somehow only applied to the state entity, it would be the only state entity in the entire OVAL language that would require special item-set-aware processing, and I'd object to it on those grounds.

My idea to make this proposal workable is to restrict negative values to the textfilecontent54_object/instance ONLY, with the following documentation for that object entity:

The instance entity calls out a specific match of the pattern. It can have both positive and negative values. If the value is positive, the index of the specific match of the pattern is counted from the beginning of the set of matches of that pattern. The first match is given an instance value of 1, the second match is given an instance value of 2, and so on. For positive values, the 'less than' and 'less than or equals' operations imply the the object is operating only on positive values. Frequently, this entity will be defined as 'greater than or equals' 1, which results in the object representing the set of all matches of the pattern.

Negative values are used to simplify collection of pattern match occurrences counting backwards from the last match. To find the last match, use an instance of -1; the penultimate match is found using an instance value of -2, and so on. For negative values, the 'greater than' and 'greater than or equals' operations imply the object is operating only on negative values. For example, searching for instances greater than or equal to -2 would yield only the last two maches.

Note that the main purpose of the instance item entity is to provide uniqueness for different textfilecontent_items that results from multiple matches of a given pattern against the same file, and they will always have positive values.

See beginning at line 1430:
https://github.com/OVALProject/Language/blob/5.11.2/schemas/independent-definitions-schema.xsd

@radzy
Copy link

radzy commented Sep 30, 2016

I prefer the idea of a direction directive. And for convenience, it would be nice to have some immediate indicator directive to specify using the first or last, which would be identical to first: instance=1 direction=forward or last: instance=1 direction=reverse

@solind
Copy link
Contributor

solind commented Sep 30, 2016

I could add a "behavior" attribute. That would require adding an entity to the item/state types containing the direction, as adding an attribute to an item/state entity would deviate from the current operating conventions of the language, and hence wouldn't be appropriate for a minor version update.

The major difference between the approaches would be how the items would combine, originating from different directions, when they are used together in the context of sets. The approach of permitting a negative value for the object instance makes it possible for identical item instances to be identified as such in sets, whereas that would not be possible if we use a behavior.

@radzy
Copy link

radzy commented Sep 30, 2016

@solind : I'm not sure what that would look like. Would you mind giving a specific example, perhaps using object_etc_security_limitsd_conf_maxlogins_exists from https://github.com/OpenSCAP/scap-security-guide/pull/1487/files

@solind
Copy link
Contributor

solind commented Sep 30, 2016

What what would look like?

With the existing proposal, to find the last instance of a pattern, you create this object:

<textfilecontent54_object>
  <filepath>/foo/bar.txt</filepath>
  <pattern>^.*$</pattern>
  <instance datatype="int">-1</instance>
</textfilecontent54_object>

This would produce the following item:

<textfilecontent54_item>
  <filepath>/foo/bar.txt</filepath>
  <pattern>^.*$</pattern>
  <instance datatype="int">100</instance>
  <text>This is the last line of bar.txt</text>
  <subexpression status="does not exist"/>
</textfilecontent54_item>

Using a behavior, the object would look like this:

<textfilecontent54_object>
  <behaviors direction="backwards"/>
  <filepath>/foo/bar.txt</filepath>
  <pattern>^.*$</pattern>
  <instance datatype="int">1</instance>
</textfilecontent54_object>

Which would produce an item looking like this:

<textfilecontent54_item>
  <filepath>/foo/bar.txt</filepath>
  <pattern>^.*$</pattern>
  <instance datatype="int">1</instance>
  <text>This is the last line of bar.txt</text>
  <subexpression status="does not exist"/>
  <direction>backwards</direction>
</textfilecontent54_item>

Notice such an item is quite distinct from a representation of the exact same line that would be produced by a different object:

<textfilecontent54_item>
  <filepath>/foo/bar.txt</filepath>
  <pattern>^.*$</pattern>
  <instance datatype="int">100</instance>
  <text>This is the last line of bar.txt</text>
  <subexpression status="does not exist"/>
  <direction>forwards</direction>
</textfilecontent54_item>

So, it would not be possible for these items to match one-another if used in a set.

Does that make sense?

@radzy
Copy link

radzy commented Sep 30, 2016

Thanks, @solind . Yes, it makes sense.

Of the two, behaviors direction="backwards" will certainly be easier for a newbie to understand. I suspect that it would result in fewer errors in the long term, due to human factors, even among experts.

@solind
Copy link
Contributor

solind commented Sep 30, 2016

@iankko, @mpreisler, any comment?

I'd personally prefer the negative object instance entity because it remains potentially useful in sets (and because I already implemented the change in the 5.11.2 branch, so, less new work for me), but I could be talked into the change if there's a consensus.

@iankko
Copy link
Author

iankko commented Oct 3, 2016

@solind

I could add a "behavior" attribute. That would require adding an entity to the item/state types containing the direction, as adding an attribute to an item/state entity would deviate from the current operating conventions of the language, and hence wouldn't be appropriate for a minor version update.

Hi David, AFAIK <textfilecontent54_object> already has (voluntary) behaviors element:
https://oval.mitre.org/language/version5.8/ovaldefinition/documentation/independent-definitions-schema.html

Wouldn't new behavior attribute be confusing for potential users?
(will comment on other questions so far too).

@iankko
Copy link
Author

iankko commented Oct 3, 2016

@mpreisler

@iankko I had the impression that negative instance will only be allowed in state we are comparing to and not the collected items. Can you confirm that that's the case? How do the operators work with your proposal?

The issue here is sometimes it might be necessary for the content author to specify they want last item in the object, not just in the state. That's why the proposal touches / changes both.

@iankko
Copy link
Author

iankko commented Oct 3, 2016

@solind

For that matter... what to do, with a negative object/instance value and an operation like "greater than" or "less than"? I would say, perhaps we should require use of the "equals" operator in conjunction with a negative number (and enforce with a schematron rule), or assume in the case of negative numbers that the values end at 0?

In the light of the above I agree the negative indices might be confusing when used in connection with greater than or less than operations.

Say you wanted all but the last value. You could use "less than" -1.

Correct.

Say that you want only the last three values. You might think to use "greater than or equals" -3, but you certainly wouldn't want to start getting item matches with positive instance values!

Agree here too.

Did you have any thoughts about how to deal with these cases?

The original proposal was thinking about negative index like li[-n] == li[len(li) - n]. In that case greater than or equal -3, and having the matches like ['a', 'b', 'mpilgrim', 'z', 'example'], it would return all items > -3 till the end of the list of matches, thus ['mpilgrim', 'z', 'example'] (thus items at indices -3, -2, and -1) . On the other hand less than or equal -1, would return all items from the -1 index till the beginning, thus ['a', 'b', 'mpilgrim', 'z'] (items at indices -2, -3, -4, -5 == 0) for the example above.

But having said the above I realize this Python notation might be confusing to "traditional" meaning of greater / less than in the OVAL language as they are implemented / used now.

Long story short, after reviewing the proposal I agree the direction attribute makes more sense (and maybe will be also easier to implement).

While on the topic of adding attributes to the behaviors element - our plan in the future is to also add a new order attribute (having possibly two values lexicographical and natural) specifying the order also the requirement for the order how the future collected objects should be obtained from the system (see e.g. http://stackoverflow.com/questions/6810619/how-to-explain-sorting-numerical-lexicographical-and-collation-with-examples/6810681#6810681 for the difference). The motivation for introducing this attribute being that there exist certain services, that honour the system settings from multiple *.conf files either lexicographically (e.g. systemd) or in natural order (e.g. augenrules and its /etc/audit/rules.d/* files). This new order attribute would be just <textfilecontent54_object> specific. (mentioning this here just for the case, we would agree on introduction of the direction attribute, it wouldn't possibly collide with the another proposal).

@solind
Copy link
Contributor

solind commented Oct 3, 2016

@iankko

Wouldn't new behavior attribute be confusing for potential users?

I meant a new "direction" attribute in the existing behavior entity. A corresponding direction entity must then be added to the state and item types, as illustrated by the examples I provided.

@iankko
Copy link
Author

iankko commented Oct 3, 2016

@solind

@iankko

Wouldn't new behavior attribute be confusing for potential users?

I meant a new "direction" attribute in the existing behavior entity. A corresponding direction entity must then be added to the state and item types, as illustrated by the examples I provided.

Yeah, got this once have looked at all the comments. Ignore that comment. Thanks.

@solind
Copy link
Contributor

solind commented Oct 4, 2016

The issue I have with adding a direction to the behaviors is that it exposes the direction as a "first-class" concept -- something that will always show up in the item results themselves. Whereas just allowing the object to have a negative instance number to count back from the end feels like more of a convenient "trick". I'd think 99% of the time, you'll use the test as it currently exists, or use <instance>-1</instance> as a quick way to get to the last instance, without actually changing the underlying instance number in the item and adding a garbage entity.

These are all easy to implement, that's not the issue. It's just a question of how polluted and overloaded we want to make the textfilecontent54_object.

And I'm really not sure about adding a sorting behavior. Sorting seems like something that should be handled by a function, not by the textfilecontent54_object.

Maybe we should have a SelectFunctionType instead? For example, this function would let you order the items for some object by one field, selecting the (first or last) value of another field:

<local_variable id="oval:x:var:1">
  <select object_id="oval:x:obj:1" item_field="text" order_field="instance" instance="last" order="ascending"/>
</local_variable>

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants