Investigate supporting defaults and overrides in AuthPolicy #91
Comments
|
@guicassolato @rahulanand16nov based on our conversation earlier, I think this is a solid use case for providing default at the gateway level. Perhaps during the next sprint we could investigate and come up with a proof of concept that would support this behaviour? I think we want the default behaviour to start as not a merge. So a simple replace. IE I have defined a default but that can be replaced entirely for a given host WDYT? |
maleck13
added
kind/investigation
|
Based on initial thoughts on how to go about solving not only defaults but also overrides, I think it makes sense for the kuadrant controller to create an internal index of the hierarchy of resources present on the cluster. This would be similar to the tree representation that authorino uses for its cache but customized for controller use cases. We need this because of the relational nature of let's go over a non-canonical example: This would look like the following in the kuadrant controller. Each KAP would add information to the index tree and after each change to the tree, reconciliation would happen to a single resource from each node based on predefined rules. Q: What happens if there are two route-level KAPs with the same host? Q: What are the rules of creation for AuthConfigs? Q: What would happen to the example shown above? Q: What is left? This is not perfect but I hope gives an idea of what I am thinking :) |
|
Just on
We could certainly add a webhook that would reject an override at the route level with a useful message such as: overrides when targeting HTTPRoutes are not allowed, please set a default It may not be required. One thought that comes to mind is if we were to allow setting a policy at the service level for example rather than just the route, but I don't see a good usecase for that right now. |
|
I think it makes sense to build up a representation of the relationships between authpolicies. This will avoid an expensive list all auth policies and end up building a temporary structure for each change. I think the actual rules need some refinment |
|
@rahulanand16nov, a couple extra observations from my side as well. In the diagram, I believe you meant for the root to be a dot ('.'), not a star ('*').
Which one prevails, the newest or the oldest resource?
Do you mean
Do you mean as 3 separate AuthConfigs? (I imagine so; just double checking.) Moreover, we're considering disabling Authorino's host name collision prevention for Kuadrant (Kuadrant/authorino#329). If that is the case, then all fine. Otherwise, in the case of 3 separate AuthConfigs for the example above, let's have in mind that It also would be interesting to see an example of the AuthConfig operations induced by each step into building the example above (IWO, what 'reconcile' means exactly in the algorithm), and them permutations of the steps (i.e. same steps in different order). |
Yep, it's supposed to be dot('.');
the oldest resource would prevail; route-level policies can have wildcards; default or override will have no implications; Note: These are the ideal expectations but as we move forward and find more problems, might need to pivot on those expectations.
Yep
Ideally, yes.
We can control the order of creation for each authconfig depending on how we traverse the tree so I hope it shouldn't be an issue.
👍 |
|
I just realized the algorithm doesn't even work for the example I wrote above 🤦 but second iteration does. Even Craig mentioned we need to isolate this logic and test the hell out of it. Edit: I noticed that we should also move Probably make sense to start talking about merge case now. |
|
@rahulanand16nov, here's my take on the algorithm to reconcile an AuthPolicy for a particular host. I took into account 8 types of AuthPolicies (i.e. 8 different cases), based on the 3 aspects (variables) of the AuthPolicy. Variables:
Cases (2 targets ⅹ 2 hosts x 2 schemes = 8):
Note: I find useful to have RLD and RLO both supported, e.g. for the use case where admins can define RLO policies, while devs are restricted to RLD. Precedence: Description of the algorithm: Pseudocode: PolicyType = enum{GWD, RWD, GLD, RLD, GWO, RWO, GLO, RLO}
func reconcile(kap AuthPolicy, host string) error
var supersededTypes []PolicyType
switch kap.PolicyType()
case GWD:
if existOthers(kap, host, []PolicyType{RWD, GWO, RWO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GWD}
case RWD:
if existOthers(kap, host, []PolicyType{GWO, RWO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GWD, RWD}
case GWO:
if existOthers(kap, host, []PolicyType{RWO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GLD, RLD, GWO}
case RWO:
supersededTypes = []PolicyType{GWD, RWD, GLD, RLD, GWO, RWO}
case GLD:
if existOthers(kap, host, []PolicyType{RLD, GWO, RWO, GLO, RLO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GLD}
case RLD:
if existOthers(kap, host, []PolicyType{GWO, RWO, GLO, RLO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GLD, RLD}
case GLO:
if existOthers(kap, host, []PolicyType{RLO})
return updateAuthPolicyStatus("%host superseded", kap)
supersededTypes = []PolicyType{GLD, RLD, GLO}
case RLO:
supersededTypes = []PolicyType{GLD, RLD, GLO, RLO}
# somehow as an atomic transaction
supersededAuthPolicies := findOthers(kap, host, supersededTypes)
updateAuthPolicyStatus("%host superseded", supersededAuthPolicies...)
deleteAuthConfigs(hosts(supersededAuthPolicies, host) - host)
createOrUpdateAuthConfig(host, kap.AuthScheme)
func findOthers(kap AuthPolicy, host string, policyTypes []PolicyType) []AuthPolicy
return []AuthPolicy{ other authpolicies targeting the same host (wildcard overlaps included) and whose type is in 'policyTypes' }
func existOthers(kap AuthPolicy, host string, policyTypes []PolicyType) bool
return len(findOthers(kap, host, policyTypes)) > 0 |
maleck13
changed the title
|
Updating my own suggestion here, so it's consistent with the Gateway API (GW-API) spec. I said before:
GW-API establishes that the hierarchy (which I called "precedence") for the different types of resources should be exactly the reverse between defaults and overrides.
Given our 4 types of resources:
The hierarchy in our case should actually be: GW[D] < RW[D] < GL[D] < RL[D] < RL[O] < GL[O] < RW[O] < GW[O] (Now noting default (D) and override (B) within square brackets.) Rewriting the above with inverted inequality symbol (to match how it's represented in the GW-API docs): GW[O] > RW[O] > GL[O] > RL[O] > RL[D] > GL[D] > RW[D] > GW[D] Finally, the updated pseudocode for the hierarchy above:
PolicyType = enum{GWO, RWO, GLO, RLO, RLD, GLD, RWD, GWD}
// reconcile(kap, host) reconciles a policy for a particular host.
// It looks for other policies targeting the same host (wildcard overlaps included), that are higher in the hierarchy and
// that according to the rules would be favoured to protect the host. If that is the case, it rejects the resource; otherwise,
// it updates the status of any existing policy that would be superseded by the resource regarding the the host, and
// deletes/creates/updates the authconfigs accordingly.
//
// Hierarchy:
// GWO > RWO > GLO > RLO > RLD > GLD > RWD > GWD
//
// General rules:
// 1. **Older policy rule:** Between policies of the same type, targeting the same host, the older policy (created first) wins, i.e. older (existing) supersedes newer (challenging).
// 2. **Strict subset rule:** Literal hosts (L) never supersede wildcards (W), because L ⊂ W; they can leave side by side (different AuthConfigs).
// 3. **Default rule:** Defaults (D) only supersede other defaults (lower in the hierarchy).
// 4. **Override rule:** Overrides (O) supersede everything that is lower in the hierarchy, as long as it doesn't violate any of the other rules.
//
// Combined rules:
// - xWO rule:
// - supersedes all types that are lower in the hierarchy (rule 4)
// - is superseded by older self and all xWO that are higher in the hierarchy (rules 1, 2, 3 and 4)
// - xLO rule:
// - supersedes all xLD and xLO that are lower in the hierarchy (rules 2 and 4)
// - is superseded by older self and all xWO and xLO that are higher in the hierarchy (rules 1, 3 and 4)
// - xLD rule:
// - supersedes all xLD that are lower in the hierarchy (rules 2 and 3)
// - is superseded by older self and all types that are higher in the hierarchy (rules 1, 3 and 4)
// - xWD rule:
// - supersedes all xWD that are lower in the hierarchy (rule 3)
// - is superseded by older self and all xWD and xWO that are higher in the hierarchy (rules 1, 2, 3 and 4)
func reconcile(kap AuthPolicy, host string) error {
var supersededTypes []PolicyType
switch kap.PolicyType() {
case GWO:
if existOthers(kap, host, []PolicyType{GWO}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{RWO, GLO, RLO, RLD, GLD, RWD, GWD}
case RWO:
if existOthers(kap, host, []PolicyType{GWO, RWO}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{GLO, RLO, RLD, GLD, RWD, GWD}
case GLO:
if existOthers(kap, host, []PolicyType{GWO, RWO, GLO}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{RLO, RLD, GLD}
case RLO:
if existOthers(kap, host, []PolicyType{GWO, RWO, GLO, RLO}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{RLD, GLD}
case RLD:
if existOthers(kap, host, []PolicyType{GWO, RWO, GLO, RLO, RLD}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{GLD}
case GLD:
if existOthers(kap, host, []PolicyType{GWO, RWO, GLO, RLO, RLD, GLD}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
case RWD:
if existOthers(kap, host, []PolicyType{GWO, RWO, RWD}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
supersededTypes = []PolicyType{GWD}
case GWD:
if existOthers(kap, host, []PolicyType{GWO, RWO, RWD, GWD}) {
return updateAuthPolicyStatus("cannot apply policy for %host", kap)
}
}
// somehow as an atomic transaction
{
supersededAuthPolicies := findOthers(kap, host, supersededTypes)
updateAuthPolicyStatus("can no longer apply policy for %host - now taken by %kap", supersededAuthPolicies...)
deleteAuthConfigs(hosts(supersededAuthPolicies, host) - host)
createOrUpdateAuthConfig(host, kap.AuthScheme)
}
}
func findOthers(kap AuthPolicy, host string, policyTypes []PolicyType) []AuthPolicy {
return []AuthPolicy{ other authpolicies targeting the same host (wildcard overlaps included) and whose type is in 'policyTypes' }
}
func existOthers(kap AuthPolicy, host string, policyTypes []PolicyType) bool {
return len(findOthers(kap, host, policyTypes)) > 0
} |
|
Ok, here goes my 2c regarding this topic… It’s an effort that aims to make the algorithm as simple as possible, redefining the precedence (hierarchy) order and the actual “compilation” of the policy rules. It aspires to also unify and be applied to the Rate Limit Policies HierarchyIt’s based on the GW API and Gui’s take, with some changes: GL[O] > GW[O] > RL[O] > RW[O] > RL[D] > GL[D] > RW[D] > GW[D] My understanding is that if you have a GW Literal Override, means that the level of specificity that the Cluster Admin had in mind needs to overtake any wildcard that most likely are general rules, therefore would always have more weight than any less specific one. Which would be the opposite when it comes to Defaults. Considerations
Reconciliation Algorithm
I know it’s a bit simplistic, and might be even naive… but hopefully this will help to craft a good algorithm that could be applied for both types of policies (RL/Auth) |
|
My contribution:
Surely I am missing tons of things, but food for thought is always welcome (I guess). |
|
@eguzki, as just discussed offline.
I believe the policy rules should play a role to the host names in the AuthConfig, otherwise we enable a situation where the AuthConfig covers more that what it suppose to, e.g.: We don't want
Maybe I can't see any use case where the user would want the ext-authz service triggered but have corresponding AuthConfig (effectively resulting in a Meaning:
That was the same reasoning that previously made me think on I will try later to rethink this so it reflects the correct hierarchy between the different types of resources as specified in GW-API and pointed out by @didierofrivia (i.e. |
|
I think I'll come somewhat out of left field on this but:
I'd like to propose a more "aggressive" approach and go for one or the other and build from there… If we really want to use the policy attachment concept, we don't really have a choice today anyways… Given the spec's:
I'd be tempted to start out with
I don't think that So, not to invalidate any of what's been said so far, nor to pun on |
* vs controller: little refactor * update go.mod
|
Superseded by #431 |
Use Case
As a gateway administrator I want to lock down access to any host attaching to this gateway. By default I want to apply a default DENY ALL policy at the gateway level.
When a policy is created targeting a HTTPRoute, I want that policy to then take precedence over my default policy.
The text was updated successfully, but these errors were encountered: