Skip to content

Commit da4610e

Browse files
authored
Merge pull request #1410 from DominiqueDevinci/contrib1403
Remediation verbosity : don't apply fix for multicheck ? issue #1403
2 parents 490983b + 729bcf5 commit da4610e

File tree

6 files changed

+68
-41
lines changed

6 files changed

+68
-41
lines changed

src/XCCDF/result.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -532,8 +532,16 @@ void xccdf_message_free(struct xccdf_message *msg)
532532
}
533533
}
534534

535+
bool xccdf_message_set_content(struct xccdf_message *obj, const char *newval){
536+
// in debug mode, we handle all new messages in case where the xccdf_message is never displayed
537+
dD("XCCDF_SET_MESSAGE: %s", newval);
538+
free(obj->content);
539+
obj->content=oscap_strdup(newval);
540+
return true;
541+
}
542+
535543
OSCAP_ACCESSOR_SIMPLE(xccdf_message_severity_t, xccdf_message, severity)
536-
OSCAP_ACCESSOR_STRING(xccdf_message, content)
544+
OSCAP_GETTER(const char*, xccdf_message, content)
537545

538546
struct xccdf_target_fact* xccdf_target_fact_new(void)
539547
{

src/XCCDF_POLICY/xccdf_policy_remediate.c

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ static int _rule_add_info_message(struct xccdf_rule_result *rr, ...)
6363

6464
msg = xccdf_message_new();
6565
xccdf_message_set_content(msg, text);
66+
dI("[%s]->msg: %s", xccdf_rule_result_get_idref(rr), text);
6667
free(text);
6768
xccdf_message_set_severity(msg, XCCDF_MSG_INFO);
6869
xccdf_rule_result_add_message(rr, msg);
@@ -379,9 +380,11 @@ static inline int _xccdf_fix_decode_xml(struct xccdf_fix *fix, char **result)
379380
#if defined(unix) || defined(__unix__) || defined(__unix)
380381
static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_fix *fix)
381382
{
382-
if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL))
383+
if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
384+
_rule_add_info_message(rr, "No fix available.");
383385
return 1;
384-
386+
}
387+
385388
const char *interpret = NULL;
386389
if ((interpret = _get_supported_interpret(xccdf_fix_get_system(fix), NULL)) == NULL) {
387390
_rule_add_info_message(rr, "Not supported xccdf:fix/@system='%s' or missing interpreter.",
@@ -478,10 +481,13 @@ static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_
478481
#else
479482
static inline int _xccdf_fix_execute(struct xccdf_rule_result *rr, struct xccdf_fix *fix)
480483
{
481-
if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL))
484+
if (fix == NULL || rr == NULL || oscap_streq(xccdf_fix_get_content(fix), NULL)) {
485+
_rule_add_info_message(rr, "No fix available.");
482486
return 1;
483-
else
487+
} else {
484488
_rule_add_info_message(rr, "Cannot execute the fix script: not implemented");
489+
}
490+
485491
return 1;
486492
}
487493
#endif
@@ -493,39 +499,48 @@ int xccdf_policy_rule_result_remediate(struct xccdf_policy *policy, struct xccdf
493499
if (xccdf_rule_result_get_result(rr) != XCCDF_RESULT_FAIL)
494500
return 0;
495501

502+
// if a miscellaneous error happens (fix unsuitable or if we want to skip it for any reason
503+
// we set misc_error to one, and the fix will be reported as error (and not skipped without log like before)
504+
int misc_error=0;
505+
496506
if (fix == NULL) {
497507
fix = _find_suitable_fix(policy, rr);
498-
if (fix == NULL)
499-
// We may want to append xccdf:message about missing fix.
500-
return 0;
508+
if (fix == NULL) {
509+
// We want to append xccdf:message about missing fix.
510+
_rule_add_info_message(rr, "No suitable fix found.");
511+
xccdf_rule_result_set_result(rr, XCCDF_RESULT_FAIL);
512+
misc_error=1;
513+
}
501514
}
502515

503516
struct xccdf_check *check = NULL;
504517
struct xccdf_check_iterator *check_it = xccdf_rule_result_get_checks(rr);
505518
while (xccdf_check_iterator_has_more(check_it))
506519
check = xccdf_check_iterator_next(check_it);
507520
xccdf_check_iterator_free(check_it);
508-
if (check != NULL && xccdf_check_get_multicheck(check))
509-
// Do not try to apply fix for multi-check.
510-
return 0;
511-
512-
/* Initialize the fix. */
513-
struct xccdf_fix *cfix = xccdf_fix_clone(fix);
514-
int res = xccdf_policy_resolve_fix_substitution(policy, cfix, rr, test_result);
515-
xccdf_rule_result_add_fix(rr, cfix);
516-
if (res != 0) {
517-
_rule_add_info_message(rr, "Fix execution was aborted: Text substitution failed.");
518-
return res;
519-
}
520521

521-
/* Execute the fix. */
522-
res = _xccdf_fix_execute(rr, cfix);
523-
if (res != 0) {
524-
_rule_add_info_message(rr, "Fix was not executed. Execution was aborted.");
525-
return res;
522+
if(misc_error == 0){
523+
/* Initialize the fix. */
524+
struct xccdf_fix *cfix = xccdf_fix_clone(fix);
525+
int res = xccdf_policy_resolve_fix_substitution(policy, cfix, rr, test_result);
526+
xccdf_rule_result_add_fix(rr, cfix);
527+
if (res != 0) {
528+
_rule_add_info_message(rr, "Fix execution was aborted: Text substitution failed.");
529+
xccdf_rule_result_set_result(rr, XCCDF_RESULT_ERROR);
530+
misc_error=1;
531+
}else{
532+
533+
/* Execute the fix. */
534+
res = _xccdf_fix_execute(rr, cfix);
535+
if (res != 0) {
536+
_rule_add_info_message(rr, "Fix was not executed. Execution was aborted.");
537+
xccdf_rule_result_set_result(rr, XCCDF_RESULT_ERROR);
538+
misc_error=1;
539+
}
540+
}
526541
}
527542

528-
/* We report rule during remediation only when the fix was actually executed */
543+
/* We report rule during remediation even if fix isn't executed due to a miscellaneous error */
529544
int report = 0;
530545
struct xccdf_rule *rule = _lookup_rule_by_rule_result(policy, rr);
531546
if (rule == NULL) {
@@ -538,18 +553,20 @@ int xccdf_policy_rule_result_remediate(struct xccdf_policy *policy, struct xccdf
538553
return report;
539554
}
540555

541-
/* Verify applied fix by calling OVAL again */
542-
if (check == NULL) {
543-
xccdf_rule_result_set_result(rr, XCCDF_RESULT_ERROR);
544-
_rule_add_info_message(rr, "Failed to verify applied fix: Missing xccdf:check.");
545-
} else {
546-
int new_result = xccdf_policy_check_evaluate(policy, check);
547-
if (new_result == XCCDF_RESULT_PASS)
548-
xccdf_rule_result_set_result(rr, XCCDF_RESULT_FIXED);
549-
else {
556+
if(misc_error == 0){
557+
/* Verify fix if applied by calling OVAL again */
558+
if (check == NULL) {
550559
xccdf_rule_result_set_result(rr, XCCDF_RESULT_ERROR);
551-
_rule_add_info_message(rr, "Failed to verify applied fix: Checking engine returns: %s",
552-
new_result <= 0 ? "internal error" : xccdf_test_result_type_get_text(new_result));
560+
_rule_add_info_message(rr, "Failed to verify applied fix: Missing xccdf:check.");
561+
} else {
562+
int new_result = xccdf_policy_check_evaluate(policy, check);
563+
if (new_result == XCCDF_RESULT_PASS)
564+
xccdf_rule_result_set_result(rr, XCCDF_RESULT_FIXED);
565+
else {
566+
xccdf_rule_result_set_result(rr, XCCDF_RESULT_ERROR);
567+
_rule_add_info_message(rr, "Failed to verify applied fix: Checking engine returns: %s",
568+
new_result <= 0 ? "internal error" : xccdf_test_result_type_get_text(new_result));
569+
}
553570
}
554571
}
555572

tests/API/XCCDF/applicability/test_remediate_fix_notapplicable.sh

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@ assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profil
2727
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/result'
2828
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/result[text()="fail"]'
2929
assert_exists 0 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/fix'
30-
assert_exists 0 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/message'
30+
31+
# one message expected signalling no suitable fix found.
32+
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/message'
3133

3234
#
3335
# Second, make sure that the fix is applied, when CPE is recognized as appplicable

tests/API/XCCDF/unittests/test_remediate_unresolved.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ assert_exists 2 '//TestResult'
2424
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]'
2525
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result'
2626
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/result'
27-
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/result[text()="fail"]'
27+
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/result[text()="error"]'
2828
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/fix'
2929
assert_exists 1 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/fix/something'
3030
assert_exists 0 '//TestResult[@id="xccdf_org.open-scap_testresult_default-profile001"]/rule-result/fix/object'

tests/API/XCCDF/unittests/test_remediation_fix_without_system.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ assert_exists 1 '//rule-result'
2323
assert_exists 1 '//rule-result/result'
2424
assert_exists 1 '//rule-result/result[text()="fail"]'
2525
assert_exists 0 '//rule-result/fix'
26-
assert_exists 0 '//rule-result/message'
26+
assert_exists 1 '//rule-result/message[text()="No suitable fix found."]'
2727
assert_exists 1 '//score'
2828
assert_exists 1 '//score[text()="0.000000"]'
2929

tests/API/XCCDF/unittests/test_remediation_subs_unresolved.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ assert_exists 0 '/Benchmark/Rule/fix/sub'
4848
assert_exists 1 '/Benchmark/Rule/fix/instance'
4949
assert_exists 1 '//rule-result'
5050
assert_exists 1 '//rule-result/result'
51-
assert_exists 1 '//rule-result/result[text()="fail"]'
51+
assert_exists 1 '//rule-result/result[text()="error"]'
5252
assert_exists 1 '//rule-result/fix'
5353
assert_exists 1 '//rule-result/fix[@system="urn:xccdf:fix:script:sh"]'
5454
assert_exists 1 '//rule-result/fix/instance'

0 commit comments

Comments
 (0)