OPA security assessment #275
Conversation
There was a problem hiding this comment.
quick nit: could you please wrap 80 columns, since it's easier to see diffs / comments if there are future changes, see writing style in CONTRIBUTING.md
ashutosh-narkar
force-pushed
the
opa-summary
branch
from
d6cab85 to
a1d4099
Compare
|
Done.
|
ultrasaurus
requested review from
lumjjb,
pragashj,
JustinCappos and
justincormack
There was a problem hiding this comment.
LGTM!
before merging requires approval from...
- SIG chair: @pragashj
- Lead reviewer: @JustinCappos
- the review team: @lumjjb @justincormack @rficcaglia (ideally everyone would LGTM this PR, but everyone already reviewed the Google doc, and the process requires only 2 reviewers, so @JustinCappos can merge after reasonable time for people to take a look)
|
|
||
| * Pushes to master are forbidden by convention | ||
|
|
||
| * OPA is at the passing criteria in the [Core Infrastructure Initiative (CII) badging process](https://bestpractices.coreinfrastructure.org/en/projects/1768) |
There was a problem hiding this comment.
While I concur that OPA's entry in the CII shows passing, I did a review of the detailed requirements per the worksheet here and there were some items I felt might need clarification or supporting details to accurately assert that they are met. That said, I don't think any of them would block this assessment as currently defined.
I would suggest changing the wording to "OPA asserts that it meets the criteria..."
There was a problem hiding this comment.
Updated the wording.
ashutosh-narkar
force-pushed
the
opa-summary
branch
from
a1d4099 to
59af8d5
Compare
assessments/projects/opa/README.md
Outdated
| Related Issues: [OPA-1781](https://github.com/open-policy-agent/opa/issues/1781) | ||
|
|
||
|
|
||
| ### CNCF recommendations |
assessments/projects/opa/README.md
Outdated
| for confusion. Examining the language’s potential for confusion and mitigating | ||
| situations where these problems arise would help to improve user security. | ||
|
|
||
| ### Project recommendations |
ashutosh-narkar
force-pushed
the
opa-summary
branch
from
59af8d5 to
4d59583
Compare
| #### The *input* Document | ||
|
|
||
| In some cases, policies require input values. In addition to the built-in data | ||
| document,OPA also has a built-in input document. When you query OPA, you can set |
There was a problem hiding this comment.
minor nit, space after ","
|
|
||
| * The OPA daemon can be configured to authenticate and authorize requests. This | ||
| could prove beneficial especially in scenarios where the deployment environment | ||
| is untrusted. More details on deploying OPA securely can be found [here](https://www.openpolicyagent.org/docs/latest/security/). Additionally OAuth2.0 client credentials can be used to authenticate |
There was a problem hiding this comment.
this didn't get 80-col wrapped.
| issue. | ||
|
|
||
| * Since OPA does not contain any policies by default when started, it’s | ||
| recommended that the application fail-open in case of any non-200 response |
There was a problem hiding this comment.
Fail open? Surely it should fail closed?
|
|
||
| ### Related Projects / Vendors | ||
|
|
||
| * OPA’s policy language REGO, allows policy decisions that are more than |
There was a problem hiding this comment.
for consistent capitalisation should be "Rego".
ashutosh-narkar
force-pushed
the
opa-summary
branch
from
4d59583 to
a0400f7
Compare
This change includes the documents related to the OPA self-assessment and OPA recommendations. Signed-off-by: Ashutosh Narkar <[email protected]>
ashutosh-narkar
force-pushed
the
opa-summary
branch
from
a0400f7 to
cf7e272
Compare
|
Merging as JJ recommended |
assessments/projects/opa/README.md
Outdated
| for confusion. Examining the language’s potential for confusion and mitigating | ||
| situations where these problems arise would help to improve user security. | ||
|
|
||
| ### Project recommendations |
There was a problem hiding this comment.
Could we get consequence section for each of the issues? The objective here is to have a follow-through on those consequences - follow through could take one of the forms of workflow to be specified/process to be documented/guidelines to be provided and agreed by community/tools to be built.
There was a problem hiding this comment.
Just to clarify - do you mean examples of known consequences, or just an enumeration of hypothetical consequences? I myself would like to see issues relate to concrete actual known problems that can be drawn from existing forensic analysis of other projects or CVEs, etc. That said, we can't necessarily rely on 20/20 hindsight to anticipate all future problems so I acknowledge the need to allow for unknown unknowns.
| from a wide range of adopters ([77 contributors](https://github.com/open-policy-agent/opa/graphs/contributors), | ||
| with Chef engineer in top 4 and Google engineer co-authoring RegoV2 specification). | ||
|
|
||
| ## Summary |
There was a problem hiding this comment.
Add in Risks as a section to highlight areas of concerns, like for example trade-offs for solving for heterogeneity vs increased surface area of attack (as specified in the SIG Security call)
There was a problem hiding this comment.
probably an aside to this PR but it would be interesting to plumb deeper into the notion of solving for X vs increased surface area. Is there a theoretical underpinning that requires that solving X always requires an increase in complexity or surface area? If complexity or surface area does go up (whether or not it MUST), is that necessarily a bad thing if compensating controls are in place? ie how to quantify the "breakeven" point on increased risk vs solved risk. anyway, something for another thread to be sure...
| from a wide range of adopters ([77 contributors](https://github.com/open-policy-agent/opa/graphs/contributors), | ||
| with Chef engineer in top 4 and Google engineer co-authoring RegoV2 specification). | ||
|
|
||
| ## Summary |
There was a problem hiding this comment.
Can you please add in the personas, primary and secondary, as expected to be used. This was brought up by @ckemper67 in the SIG call
OPA security assessment
This change includes the documents related to the OPA self-assessment and OPA recommendations.
Signed-off-by: Ashutosh Narkar [email protected]