feat(k8sdisallowanonymous): allow disallowing system:authenticated #579
Conversation
There was a problem hiding this comment.
Hey @julianKatz - Don't forget to also bump this to 1.1.0: https://github.com/julianKatz/gatekeeper-library/blob/disallow-authenticated/src/general/disallowanonymous/constraint.tmpl#L7
julianKatz
changed the title
julianKatz
changed the title
artifacthub/library/general/disallowanonymous/1.0.1/artifacthub-pkg.yml
Outdated
Show resolved
Hide resolved
julianKatz
force-pushed
the
disallow-authenticated
branch
4 times, most recently
from
7809cea to
be4b35d
Compare
There was a problem hiding this comment.
Thanks for the contribution @julianKatz!
Made a few suggestions, and this will still need to be reviewed by another maintainer.
| review(subject) = true { | ||
| subject.name == "system:anonymous" | ||
| message(group) := val { | ||
| val := sprintf("%v is not allowed as a subject name in %v %v", [group, input.review.object.kind, input.review.object.metadata.name]) |
There was a problem hiding this comment.
Just calling out that by including group we are slightly changing the original behavior. As it's just the message I don't think it a breaking change, but wanted to note that is some cases it will increases the number of violations as they are now group specific.
There was a problem hiding this comment.
Good eye!
I confirmed with Max that going from 1 to multiple violations is not a breaking change.
My thinking was that before:
-
Someone with
system:anonymousas the subject would get a message saying thatUnuthenticatedwas the problem. -
Someone with a binding to both
system:anonymousANDsystem:unauthenticatedwould get only a single violation. Once they fix that violation, they'd then get another violation for the other binding. This UX is confusing.
There was a problem hiding this comment.
Yup, agreed, and makes sense to me!
|
FYI: Probably a fresh |
julianKatz
force-pushed
the
disallow-authenticated
branch
3 times, most recently
from
c5c3188 to
fc229d5
Compare
julianKatz
force-pushed
the
disallow-authenticated
branch
from
fc229d5 to
c662242
Compare
Previously, the k8sdisallowanonymous would prevent bindings to the system:anonymous and system:unauthenticated groups. For Google Kubernetes Engine, the system:authenticated group is a potential security threat. This was described by Orca Security in this blog post: https://orca.security/resources/blog/sys-all-google-kubernetes-engine-risk/ This PR updates the k8sdisallowanonymous to prevent bindings to `system:authenticated` if the parameters.disallowAuthenticated toggle is set to true. This will not break existing customers as the default boolean value is false, leaving this functionality disabled. Signed-off-by: juliankatz <[email protected]>
julianKatz
force-pushed
the
disallow-authenticated
branch
from
c662242 to
0025c06
Compare
Previously, the k8sdisallowanonymous would prevent bindings to the
system:anonymous and system:unauthenticated groups.
For Google Kubernetes Engine, the system:authenticated group is a
potential security threat. This was described by Orca Security in this
blog post:
https://orca.security/resources/blog/sys-all-google-kubernetes-engine-risk/
This PR updates the k8sdisallowanonymous to prevent bindings to
system:authenticatedif the parameters.disallowAuthenticated toggle isset to true. This will not break existing customers as the default
boolean value is false, leaving this functionality disabled.
Fixes #561