search

sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.38k stars 533 forks source link

Support errors from Rego validation consistent with policy-controller's convention #2871

Open SaiJyothiGudibandi opened 1 year ago

SaiJyothiGudibandi commented 1 year ago

Description We are trying to print error messages when cosign verify-attestation failed as per the rego policy rules. I have a policy to satisfy my requirement, it’s working from the rego playground as you see below.

SUCCESS CASE: INPUT: { "environment": "snapshot", "type": "twistlockquality", "stage_properties": { "enabled": true, "scan_status": "PASSED", "type": "TWISTLOCK_SCAN", "running_on": "02", "scan_results": { "ImageName": "docker_build1", "vulnerabilitiesCount": "94", "complianceIssuesCount": "1", "vulnerabilityDistribution": { "critical": "4", "high": "6", "medium": "2", "low": "82", "total": "94" } } } }

Policy: `package signature

allow[msg]{ input.type == "twistlockquality" result := less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical) msg := sprintf("FAILED - twistlockquality, REASON: Found %v criticals", [input.stage_properties.scan_results.vulnerabilityDistribution.critical]) result != true }

less_than(x) := to_number(x) < 5

larger_than(x) := to_number(x) > 1`

OUTPUT: { "allow": [ ] }

FAIL CASE: INPUT: { "environment": "snapshot", "type": "twistlockquality", "stage_properties": { "enabled": true, "scan_status": "PASSED", "type": "TWISTLOCK_SCAN", "running_on": "02", "scan_results": { "ImageName": "docker_build1", "vulnerabilitiesCount": "94", "complianceIssuesCount": "1", "vulnerabilityDistribution": { "critical": "4", "high": "6", "medium": "2", "low": "82", "total": "94" } } } }

Policy: `package signature

allow[msg]{ input.type == "twistlockquality" result := less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical) msg := sprintf("FAILED - twistlockquality, REASON: Found %v criticals", [input.stage_properties.scan_results.vulnerabilityDistribution.critical]) result != true }

less_than(x) := to_number(x) < 2

larger_than(x) := to_number(x) > 1`

OUTPUT: { "allow": [ "FAILED - twistlockquality, REASON: Found 4 criticals" ] }

But when I integrate and use the same policy at cosign verify-attestation, even in the policy success case, cosign verification is failing with expression value, [], is not true and main.go:74: error during command execution: 7 validation errors occurred.

Policy success case:

Policy fail case:

SaiJyothiGudibandi commented 1 year ago

@hectorj2f @developer-guy @wlynch

hectorj2f commented 1 year ago

@SaiJyothiGudibandi Please, try following this format for your policy:

package sigstore
default isCompliant = false
isCompliant {
...
}

or a format like:

package sigstore
isCompliant[response] {
   ...

  response :={
   "result" : result,
    "error" : errorMsg,
  }
}
SaiJyothiGudibandi commented 1 year ago

@hectorj2f

I have tried the below, but no luck.

Rego Playground: Input:

{
   "environment": "snapshot",
   "type": "twistlockquality",
   "stage_properties":    {
     "enabled": true,
     "scan_status": "PASSED",
     "type": "TWISTLOCK_SCAN",
     "running_on": "02",
     "scan_results":      {
       "ImageName": "docker_build1",
       "vulnerabilitiesCount": "94",
       "complianceIssuesCount": "1",
       "vulnerabilityDistribution":        {
         "critical": "4",
         "high": "6",
         "medium": "2",
         "low": "82",
         "total": "94"
       }
     }
   }
}

Policy:

package sigstore
isCompliant[response] {
  input.type == "twistlockquality"
  result := less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical)
#   msg := sprintf("FAILED - twistlockquality, REASON: Found %v criticals", [input.predicate.stage_properties.scan_results.vulnerabilityDistribution.critical])
  result != true
  response :={
   "criticals" : result,
   "error" : "FAILED - twistlockquality, REASON: Found more criticals than expected",
  }
}
less_than(x) := to_number(x) < 5
larger_than(x) := to_number(x) > 1

Output:

[
   {
     "isCompliant": []
   }
]

——————— POLICY:

package sigstore
isCompliant[response] {
  input.type == "twistlockquality"
  result := less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical)
#   msg := sprintf("FAILED - twistlockquality, REASON: Found %v criticals", [input.predicate.stage_properties.scan_results.vulnerabilityDistribution.critical])
  result != true
  response :={
   "criticals" : result,
   "error" : "FAILED - twistlockquality, REASON: Found more criticals than expected",
  }
}
less_than(x) := to_number(x) < 2
larger_than(x) := to_number(x) > 1

OUTPUT:

{
   "isCompliant": [
     {
       "criticals": false,
       "error": "FAILED - twistlockquality, REASON: Found more criticals than expected"
     }
   ]
}

———————

+ COSIGN_EXPERIMENTAL=1
+ cosign verify-attestation --key cosign.pub --type spdxjson [snapshot/docker_build1:1.1.11](http://snapshot/docker_build1:1.1.11) --policy policy.rego
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
will be validating against Rego policies: [rekor-policy/gto-policy.rego]
There are 8 number of errors occurred during the validation:
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
- result is undefined for query 'data.signature.allow'
Error: 8 validation errors occurred
main.go:74: error during command execution: 8 validation errors occurred
hectorj2f commented 1 year ago

Your response format doesn't follow the same format I shared above. That is why you are getting a result is undefined for query ... Please, wrap the result of your policy as part of the result field in the response object.

SaiJyothiGudibandi commented 1 year ago

@hectorj2f,

Can you please point whats wrong in the below policy. I wasnt able to figure out whats going wrong.

package signature

isCompliant[response] { input.predicate.type == "twistlockquality" result := less_than(input.predicate.stage_properties.scan_results.vulnerabilityDistribution.critical) result != true

response :={ "result" : result, "error" : "FAILED - twistlockquality, REASON: Found more criticals than expected", } }

less_than(x) := to_number(x) < 1 larger_than(x) := to_number(x) > 1

lcarva commented 1 year ago

cosign will use rego to make the following query: data.signature.allow

It looks like your policy does have the right package name, signature, and produces the allow value. Good.

The issue is that cosign expects the value of that query to be the boolean true.

In your case, data.signature.allow is a string with an error message.

package signature

allow[msg]{
  input.type == "twistlockquality"
  result := less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical)
  msg := sprintf(
    "FAILED - twistlockquality, REASON: Found %v criticals", 
    [input.stage_properties.scan_results.vulnerabilityDistribution.critical],
  )
  result != true
}

...

I suggest changing it to something like this:

package signature

allow {
  input.type == "twistlockquality"
  less_than(input.stage_properties.scan_results.vulnerabilityDistribution.critical)
}

...

It looks like you're trying to pass an error message if the rule does fail, is that correct? If so, I don't think that's possible with allow rules. In rego, if a rule fails, it means the value is never produced, thus no option to provide a meaningful error message.

Other projects that integrate with rego query for deny rules instead. When the rule fails, the deny rule emits a value which contains relevant information about the violation. The downside is that you have to be a little more careful when writing the policy to ensure processing the deny rule doesn't fail for other reasons (e.g. missing value in attestation). That would cause an incorrect evaluation success (no deny values were found). :grimacing:

For cosign's current use case, looking for the allow rule probably makes the most sense. The policy evaluation support in cosign is somewhat primitive and feels very experimental. It would be great to improve that. conftest might be something we can leverage to make using rego a little bit more flexible. It allows using deny, warn, and conditional skips.

NOTE: cosign used to look for a deny rule. https://github.com/sigstore/cosign/commit/3b597b999ad083afd2ce0dba68fa2715ddec0f06 (from yours truly :bow: ) changed it to allow and fixed a bunch of other bugs in the rego policy evaluation code. That may have additional context.

SaiJyothiGudibandi commented 1 year ago

@lcarva Thanks for the details.

I am trying to pint an error message, if my repo policy rule doesnt satisfy. Any suggestions here?

lcarva commented 1 year ago

@SaiJyothiGudibandi, how are you trying to do that?

hectorj2f commented 1 year ago

@SaiJyothiGudibandi Why don't you try to follow the suggested format ? 

package sigstore
isCompliant[response] {
   ...

  response :={
   "result" : result,
    "error" : errorMsg,
  }
}

I can see how the package name isn't correct signature. We expect a package name sigstore.

SaiJyothiGudibandi commented 1 year ago

lcarva / hectorj2f Some one suggested to do something like below.

package signature

default allow = true

isCompliant[response] { input.predicate.type == "sonarquality" result := input.predicate.stage_properties.enabled result != true

response :={ "result" : result, "error" : "FAILED - sonarquality", } }

It is working with OPA, but when I integrate with cosign, not working as required.

Do you have any smaple example to achieve this?

lcarva commented 1 year ago

When using OPA's rego with cosign, allow has to be a boolean. That's the the only value that will be queried. You cannot return a response error message because of this.

Setting a default value for allow is a good idea. You may want something like this:

package signature

# Not allowed unless proven otherwise
default allow = false

allow {
  input.predicate.type == "sonarquality"
  input.predicate.stage_properties.enabled
  # If it gets here, allow will be set to true
}
SaiJyothiGudibandi commented 1 year ago

Thanks for the quick response.

Understand the above code, using that how can I print error message if allow set to false? I am looking to print the error message if allow set to false (as per your above code) Please suggest.

lcarva commented 1 year ago

Thanks for the quick response.

Understand the above code, using that how can I print error message if allow set to false? I am looking to print the error message if allow set to false (as per your above code) Please suggest.

That is not possible with cosign today.

SaiJyothiGudibandi commented 1 year ago

Can you guys support printing messages functionality with cosign integration similar to OPA?

It makesense to print the message if some rule doesnt satisfy. Users will get to know more details on the failure.

hectorj2f commented 1 year ago

This error message is only supported in the policy-controller. The code calls EvaluatePolicyAgainstJSON function in cosign. However the cli code doesn't use it, that is why this format won't work for you at the moment.

Can you guys support printing messages functionality with cosign integration similar to OPA?

In relation to your question, I'd let @znewman01 @priyawadhwa to chime in and decide on this.

SaiJyothiGudibandi commented 1 year ago

@znewman01 and @priyawadhwa, I would love to hear your thoughts on this feature request. What are your insights or opinions on this?

znewman01 commented 1 year ago

Offhand, seems reasonable. And leads to better consistency between cosign/policy-controller. I'll rename the issue.

SaiJyothiGudibandi commented 1 year ago

Great. Could you please provide an ETA for the support of this feature?

znewman01 commented 1 year ago

No ETA as of yet. But we would take a PR to thread these errors through and print them, provided it was done in a backwards compatible way.

warroyo commented 9 months ago

I came across this will getting errors using the example here so based on these comments , it seems like this documentation is wrong?