Component Hash policy condition only supports positive matches

[francislance]

Current Behavior

When you create a policy for component hash (I tested Sha1 and 256) the conditions does not behave as what it implies to be.

Steps to Reproduce

1. Create a policy: (example) { "operator": "IS_NOT", "subject": "COMPONENT_HASH", "value": "{\"algorithm\":\"SHA-1\",\"value\":\"e1166b586cf9ca990ede7f3329853c0fe3547aa9\"}", "uuid": "5ad3139e-6f19-492a-b4ea-403d10a95a14" }

Observation:

• The above operator "IS_NOT" actually appears does not matter. Even if you change it to IS, MATCHES, or NOT_MATCH, the policy is only triggered if it is equals.
• So if your component has that same SHA1 value, it will always trigger a policy violation. Even though, the use case is to actually it must show violation if it is not match.

Expected Behavior

• if IS_NOT or NOT_MATCH operator i used, and the values does not matched then it is a violation. If it matches, then no violation.
• if IS or MATCHES operator is used, and the values matches, then no violation. If does not match, then it is a violation.

Dependency-Track Version

4.12.0

Dependency-Track Distribution

Container Image

Database Server

PostgreSQL

Database Server Version

No response

Browser

Google Chrome

Checklist

• [x] I have read and understand the contributing guidelines
• [x] I have checked the existing issues for whether this defect was already reported

[francislance]

I think, while reading this file contents: ComponentHashPolicyEvaluator.java

public List<PolicyConditionViolation> evaluate(final Policy policy, final Component component) {
        final List<PolicyConditionViolation> violations = new ArrayList<>();
        for (final PolicyCondition condition : super.extractSupportedConditions(policy)) {
            LOGGER.debug("Evaluating component (" + component.getUuid() + ") against policy condition (" + condition.getUuid() + ")");
            final Hash hash = extractHashValues(condition);
            if (matches(hash, component)) {
                violations.add(new PolicyConditionViolation(condition, component));
            }
        }
        return violations;
    }

for these lines:

 if (matches(hash, component)) {
                violations.add(new PolicyConditionViolation(condition, component));
            }

Would it mean that it only adds a violation if the hash matches and not really handling any other comparison? If so, perhaps we can improve this.

Hope can get some help on this case. Thank you

[francislance]

I had made small changes to the code to add a logic to check operator of IS and IS_NOT as follows:

    private boolean conditionApplies(Hash hash, Component component, PolicyCondition.Operator operator) {
        boolean matchFound = matches(hash, component);

        switch (operator) {
            case IS:
                return matchFound;  // Violation if the hash match

            case IS_NOT:
                return !matchFound;  // Violation if the hash does not match

            default:
                LOGGER.warn("Unsupported operator: " + operator);
                return false;
        }
    }

I have tested the above in my local and appears to be working. Here is the link to the whole code: ComponentHashPolicyEvaluator.java

Hope to hear from Developers about this soon.

Thank you

[nscuro]

@francislance Thanks for looking into this. Your proposed change makes sense.

Separately, I think we really need to add some validation to PolicyConditionResource such that clients are informed immediately when the condition they created/updated is not supported.

[francislance]

Thanks @nscuro

Shall I do a pull request for this?

[nscuro]

That would be fantastic!

[francislance]

@nscuro just wanted to know to which branch should i be requesting to merge it? Thank you

Edit: I just created the pull request here: https://github.com/DependencyTrack/dependency-track/pull/4306

Thank you

[francislance]

Hi @nscuro, I am re-evaluating this and would like to get your opinion if it makes sense for your to do the logic this way:

Operator	Policy Hash	Component Hash	Outcome	Explanation
IS	1ef6...7c9	1ef6...7c9	No Violation	Hashes match, so no violation.
IS	1ef6...7c9	705e...af8	Violation	Hashes differ, so violation occurs.
IS_NOT	1ef6...7c9	e116...7aa9	No Violation	Hashes differ, so no violation.
IS_NOT	1ef6...7c9	1ef6...7c9	Violation	Hashes match, so violation occurs.

I made a mistake on how I evaluated the logic earlier so I am changing it as below. Let me know how you think about it. Then I will be updating the code and adding the automated test later on. Thank you. :pray:

:x: earlier code:

    private boolean conditionApplies(Hash hash, Component component, PolicyCondition.Operator operator) {
        boolean matchFound = matches(hash, component);

        switch (operator) {
            case IS:
                return matchFound;  // Violation if the hash match

            case IS_NOT:
                return !matchFound;  // Violation if the hash does not match

            default:
                LOGGER.warn("Unsupported operator: " + operator);
                return false;
        }
    }

:white_check_mark: new code:

    private boolean conditionApplies(Hash hash, Component component, PolicyCondition.Operator operator) {
        boolean matchFound = matches(hash, component);

        switch (operator) {
            case IS:
                return !matchFound;  // Violation if the hash does not match

            case IS_NOT:
                return matchFound;  // Violation if the hash matches

            default:
                LOGGER.warn("Unsupported operator: " + operator);
                return false;
        }
    }

[nscuro]

@francislance Actually the previous logic was correct. Violations are supposed to occur when their respective conditions match.

[francislance]

I see.

Edit: I have reverted to earlier described logic @nscuro Thank you 🙏