Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Component Hash policy condition only supports positive matches #4230

Open
2 tasks done
francislance opened this issue Oct 9, 2024 · 6 comments
Open
2 tasks done

Component Hash policy condition only supports positive matches #4230

francislance opened this issue Oct 9, 2024 · 6 comments
Labels
defect Something isn't working good first issue Good for newcomers p2 Non-critical bugs, and features that help organizations to identify and reduce risk size/S Small effort

Comments

@francislance
Copy link

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

@francislance francislance added defect Something isn't working in triage labels Oct 9, 2024
@francislance
Copy link
Author

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
Copy link
Author

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
Copy link
Member

nscuro commented Oct 24, 2024

@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.

@nscuro nscuro changed the title Policy: Component Hash Component Hash policy condition only supports positive matches Oct 24, 2024
@nscuro nscuro added p2 Non-critical bugs, and features that help organizations to identify and reduce risk good first issue Good for newcomers size/S Small effort and removed in triage labels Oct 24, 2024
@francislance
Copy link
Author

Thanks @nscuro

Shall I do a pull request for this?

@nscuro
Copy link
Member

nscuro commented Oct 24, 2024

That would be fantastic!

@francislance
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect Something isn't working good first issue Good for newcomers p2 Non-critical bugs, and features that help organizations to identify and reduce risk size/S Small effort
Projects
None yet
Development

No branches or pull requests

2 participants