Make WARNING the default verbosity level #630

mpreisler · 2017-01-04T21:44:00Z

~~DO NOT MERGE, up for discussion.~~

@jan-cerny I have noticed that your oscap_set_verbose code defaults to unknown verbosity and no output. Do you recall what was the reasoning behind that? I expected WARNING level by default and stderr output, which is what this PR does.

Does the UNKNOWN verbose level have any meaning? It seems it's just used to flag that oscap_set_verbose was never run with non-NULL verbosity level.

Perhaps I am missing something.

mpreisler · 2017-01-04T23:15:43Z

(The Jenkins failure is expected)

jan-cerny · 2017-01-17T11:17:36Z

@mpreisler

I think that UNKNOWN level doesn't have any meaning, probably it is just a fallback value for oscap_string_to_enum.

However, now, in OpenSCAP you must directly specify verbosity level. The '--verbose' has a required argument.

If you omit the level, you will get an error:

oscap oval eval --verbose  /usr/share/xml/scap/ssg/content/ssg-fedora-oval.xml 
ERROR:    Invalid verbosity level! Verbosity level must be one of: DEVEL, INFO, WARNING, ERROR.

What's the reason behind this pull request? Do you want to reuse the function somewhere else? Or did you want to make the code more readable?

mpreisler · 2017-01-17T15:31:48Z

I want to make "warnings and up" visible by default, even without --verbose

jan-cerny · 2017-01-17T15:45:57Z

@mpreisler OK, That makes sense. Great idea! I think we can ignore the UNKNOWN value and default to WARNING.

mpreisler · 2017-01-19T17:32:43Z

@dahaic How do you feel about changing this in a maintenance branch? maint-1.2. If everything goes well there will be no change in output, if there are any warnings they will be shown in stderr.

dahaic · 2017-01-27T15:52:54Z

Personally, I see it as a bugfix, because proposed behaviour is something I would expect from any utility. So I have no issues with inclusion into the maintenance branch.

mpreisler · 2017-01-29T21:27:06Z

Ok, so we have a consensus here. I will change the tests accordingly and finish this up.

jan-cerny · 2017-02-01T11:16:49Z

@mpreisler The Jenkins fail here looks legit, there are multiple failing test cases in XCCDF directory.

mpreisler · 2017-02-01T16:38:31Z

@mpreisler The Jenkins fail here looks legit, there are multiple failing test cases in XCCDF directory.

Yeah, it's expected. Check out comment #630 (comment)

I need to change the tests one by one.

mpreisler · 2017-02-01T17:27:42Z

TODO:

~~tests/API/XCCDF/applicability~~ FIXED, warning filtered out
~~tests/API/XCCDF/unittests~~ FIXED, error loading SCE
~~tests/API/XCCDF/variable_instance~~ FIXED, error loading SCE
~~tests/bz2~~ FIXED, error loading SCE
~~tests/DS~~ FIXED, error loading SCE
~~tests/probes/sysctl~~ FIXED
~~tests/probes/sql57~~ FIXED

…ype.sh

It's not a serious warning and in many use-cases is expected.

…l.sh

If SCE is not installed it will fail in an expected way and it makes no sense to clutter the error log with it. Explicit plugin loading via the API will still fail with errors.

mpreisler · 2017-02-08T20:47:15Z

$ cat /tmp/test_remediation_subs_unresolved.out.RClcIQ
W: oscap: The xccdf:rule-result/xccdf:instance element was not found.

This looks like a serious bug found by this change, the instance is there, it's just that the code fails to find it. I will investigate further after I fix up the rest.

rule results XCCDF:

<?xml version="1.0" encoding="UTF-8"?>
<Benchmark xmlns="http://checklists.nist.gov/xccdf/1.2" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" id="xccdf_moc.elpmaxe.www_benchmark_test" resolved="1">
  <status>accepted</status>
  <version>1.0</version>
  <model system="urn:xccdf:scoring:default"/>
  <Rule id="xccdf_moc.elpmaxe.www_rule_1" selected="true">
    <title xmlns:xhtml="http://www.w3.org/1999/xhtml">Ensure that file exists and it is not executable</title>
    <description xmlns:xhtml="http://www.w3.org/1999/xhtml">
      <xhtml:object id="#xccdf:value:asdfg"/>
    </description>
    <fix xmlns:xhtml="http://www.w3.org/1999/xhtml" system="urn:xccdf:fix:script:sh">
        <instance xmlns="http://checklists.nist.gov/xccdf/1.2"/>
        touch test_file
        chmod a-x test_file
    </fix>
    <check system="http://oval.mitre.org/XMLSchema/oval-definitions-5">
      <check-content-ref name="oval:moc.elpmaxe.www:def:1" href="test_remediation_simple.oval.xml"/>
    </check>
  </Rule>
  <TestResult id="xccdf_org.open-scap_testresult_default-profile" start-time="2017-02-08T15:53:13" end-time="2017-02-08T15:53:13" version="1.0" test-system="cpe:/a:redhat:openscap:1.2.14">
    <benchmark href="./test_remediation_subs_unresolved.xccdf.xml" id="xccdf_moc.elpmaxe.www_benchmark_test"/>
    <title>OSCAP Scan Result</title>
    <identity authenticated="false" privileged="false">mpreisle</identity>
    <target>t450s</target>
    <target-address>127.0.0.1</target-address>
    <target-address>0:0:0:0:0:0:0:1</target-address>
    <target-facts>
      <fact name="urn:xccdf:fact:scanner:name" type="string">OpenSCAP</fact>
      <fact name="urn:xccdf:fact:scanner:version" type="string">1.2.14</fact>
      <fact name="urn:xccdf:fact:ethernet:MAC" type="string">00:00:00:00:00:00</fact>
    </target-facts>
    <rule-result idref="xccdf_moc.elpmaxe.www_rule_1" time="2017-02-08T15:53:13" weight="1.000000">
      <result>fail</result>
      <message severity="info">Fix execution was aborted: Text substitution failed.</message>
      <fix xmlns:xhtml="http://www.w3.org/1999/xhtml" system="urn:xccdf:fix:script:sh">
        <instance xmlns="http://checklists.nist.gov/xccdf/1.2"/>
        touch test_file
        chmod a-x test_file
    </fix>
      <check system="http://oval.mitre.org/XMLSchema/oval-definitions-5">
        <check-content-ref name="oval:moc.elpmaxe.www:def:1" href="test_remediation_simple.oval.xml"/>
      </check>
    </rule-result>
    <score system="urn:xccdf:scoring:default" maximum="100.000000">0.000000</score>
  </TestResult>
</Benchmark>

mpreisler · 2017-02-08T22:33:11Z

Does anybody understand the code at https://github.com/OpenSCAP/openscap/blob/maint-1.2/src/XCCDF_POLICY/xccdf_policy_substitute.c#L168 ? I think it might be mistaking fix/instance with rule-result/instance but I don't know enough about this part of OpenSCAP to know for sure.

@jan-cerny @ybznek

rsprudencio · 2017-02-09T16:28:49Z

The only instance I can recall in the specs is the one related to textfilecontent result and according to the code, that's the correct relation: xccdf_rule_result_get_instances(data->rule_result);

I don't know the relation between those instances and this text substitution though, sorry 😞

mpreisler · 2017-02-09T16:43:35Z

I took another look and the instance it's looking for is really not in the rule-results. So it could be that the code is fine. I am leaning towards backlisting this warning in the test. So we will keep warning about this situation but the tests will pass when this warning is in that particular test. I will make the commit shortly.

mpreisler · 2017-02-10T18:08:11Z

@openscap-jenkins test this please

rsprudencio · 2017-02-13T16:29:12Z

@openscap-jenkins test this please

mpreisler · 2017-02-13T16:35:56Z

@openscap-jenkins test this please

This has passed before AFAIK.

rsprudencio · 2017-02-14T11:52:47Z

This was passed before AFAIK.

Yes I know, I was testing Jenkins, it's only failing on SSG PR's 😞

jan-cerny · 2017-02-15T13:14:19Z

tests/API/OVAL/unittests/test_external_variable.sh

 $OSCAP oval eval --results $result --variables $srcdir/external_variables.xml $srcdir/$name.oval.xml 2> $stderr
-[ ! -s $stderr ] && rm $stderr
+# filter out the expected warnings in stderr
+sed -i -E "/^W: oscap:     Referenced variable has no values \(oval:x:var:[13689]\)/d" "$stderr"


What if the indentation level will change in future? I would put here instead of 6 spaces a regular expression matching at least one space.

…paces

jan-cerny · 2017-02-20T08:43:34Z

@mpreisler Great job! Thank you.

mpreisler added this to the 1.2.14 milestone Jan 4, 2017

mpreisler added the enhancement label Jan 29, 2017

mpreisler mentioned this pull request Jan 30, 2017

The <benchmark> ref can also appear in XCCDF 1.1 tailoring extension #669

Merged

mpreisler force-pushed the warning_default_verbose_level branch from d2743b6 to 810b52a Compare February 1, 2017 18:51

rsprudencio changed the title ~~Make WARNING the default verbosity level~~ [DO NOT MERGE] Make WARNING the default verbosity level Feb 2, 2017

Martin Preisler added 5 commits February 8, 2017 14:51

Make WARNING the default verbosity level

bb66711

Changed expected number of lines in stderr in test_object_component_t…

1ba592e

…ype.sh

Changed a warning about parsing without benchmark reference to info

54d66d5

It's not a serious warning and in many use-cases is expected.

Filter expected permission errors from stderr in test_sysctl_probe_al…

3ef6685

…l.sh

Filter out the expected error in sql57 probe test

789486e

mpreisler force-pushed the warning_default_verbose_level branch from 8365ac2 to 789486e Compare February 8, 2017 19:51

Do not output errors if check engine plugin auto-loading fails

cdff2e8

If SCE is not installed it will fail in an expected way and it makes no sense to clutter the error log with it. Explicit plugin loading via the API will still fail with errors.

Filter out the expected warning in test_remediation_subs_unresolved

703c104

mpreisler changed the title ~~[DO NOT MERGE] Make WARNING the default verbosity level~~ Make WARNING the default verbosity level Feb 10, 2017

Filter out expected warnings in test_external_variable.sh

ca3ea12

jan-cerny self-assigned this Feb 15, 2017

jan-cerny reviewed Feb 15, 2017

View reviewed changes

Adapted the regex to detect at least one space instead of exactly 6 s…

c4583ff

…paces

jan-cerny merged commit 91feb5c into OpenSCAP:maint-1.2 Feb 20, 2017

jan-cerny mentioned this pull request Feb 20, 2017

Recategorize some warning messages #684

Merged

jan-cerny mentioned this pull request Mar 14, 2017

Error messages are duplicated #707

Closed

mpreisler deleted the warning_default_verbose_level branch May 19, 2017 09:44

Make WARNING the default verbosity level #630

Make WARNING the default verbosity level #630

Uh oh!

Conversation

mpreisler commented Jan 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpreisler commented Jan 4, 2017

Uh oh!

jan-cerny commented Jan 17, 2017

Uh oh!

mpreisler commented Jan 17, 2017

Uh oh!

jan-cerny commented Jan 17, 2017

Uh oh!

mpreisler commented Jan 19, 2017

Uh oh!

dahaic commented Jan 27, 2017

Uh oh!

mpreisler commented Jan 29, 2017

Uh oh!

jan-cerny commented Feb 1, 2017

Uh oh!

mpreisler commented Feb 1, 2017

Uh oh!

mpreisler commented Feb 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpreisler commented Feb 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mpreisler commented Feb 8, 2017

Uh oh!

rsprudencio commented Feb 9, 2017

Uh oh!

mpreisler commented Feb 9, 2017

Uh oh!

mpreisler commented Feb 10, 2017

Uh oh!

rsprudencio commented Feb 13, 2017

Uh oh!

mpreisler commented Feb 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsprudencio commented Feb 14, 2017

Uh oh!

jan-cerny Feb 15, 2017

Choose a reason for hiding this comment

Uh oh!

mpreisler Feb 16, 2017

Choose a reason for hiding this comment

Uh oh!

jan-cerny commented Feb 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mpreisler commented Jan 4, 2017 •

edited

Loading

mpreisler commented Feb 1, 2017 •

edited

Loading

mpreisler commented Feb 8, 2017 •

edited

Loading

mpreisler commented Feb 13, 2017 •

edited

Loading