Enhancing Quarkus Security for ROOT Group Access

[gbourant]

Description

Assume you are using JWT for security which has a group called ROOT. The user with the group ROOT should be able to access all JAX-RS (all beans in general) despite what is defined in @RolesAllowed, @PermissionsAllowed or security config properties.

For example, the following configuration behavior does not allow that. The root-policy would work for all paths except the ones defined in quarkus.http.auth.permission.*.paths properties. In the following case the ROOT won't be able to access /api/hello due to the longest path wins principle (/api/hello > /*).

quarkus.http.auth.policy.root-policy.roles-allowed=ROOT
quarkus.http.auth.permission.root.paths=/*
quarkus.http.auth.permission.root.policy=root-policy
quarkus.http.auth.policy.customer-policy.roles-allowed=CUSTOMER
quarkus.http.auth.permission.customer.paths=/api/hello
quarkus.http.auth.permission.customer.policy=customer-policy

The current behaviour does offer a solution which would mean that i will have to add the ROOT role to all quarkus.http.auth.policy.*.roles-allowed properties.

Implementation ideas

A potential solution could involve incorporating a mechanism where, if a SecurityInterceptor is defined, it operates subsequent to the Quarkus Engine's determination of whether a call should be permitted or denied. In this proposed solution, the SecurityInterceptor would yield the conclusive result for the call after the Quarkus Engine has made its decision.

interface SecurityInterceptor {
       Uni<CheckResult> isPermitted(CheckResult quarkusCheckResult);
}

@RequestScoped
class CustomSecurityInterceptor implements SecurityInterceptor {

       @Inject
       JsonWebToken jwt;

       Uni<CheckResult> isPermitted(Uni<CheckResult> quarkusCheckResult) {

           if(jwt.getGroups().contains("ROOT")){
               return Uni.createFrom().item(CheckResult.PERMIT);
           }

           return quarkusCheckResult;
       }
}

[quarkus-bot]

/cc @sberyozkin (security)

[sberyozkin]

Thanks @gbourant, I guess we need to make sure that when quarkus.security.jaxrs.default-roles-allowed=ROOT is set then it is also checked even when path specific role policies exist. CC @michalvavrik

[michalvavrik]

Thanks @gbourant, I guess we need to make sure that when quarkus.security.jaxrs.default-roles-allowed=ROOT is set then it is also checked even when path specific role policies exist. CC @michalvavrik

My problem with this suggestion is that:

• it is magic. Documentation related to web endpoint authorization is already big enough, if users don't read every single line, they won't mention / expect this behavior
• that is breaking change as it will expose policies that were secured against ROOT (or as I like to call this role: minimal-default-role); now, minimal-default-role will have access everywhere!

I think @gbourant have a point that there should be a way to do that, but I'm worried to do that based on quarkus.security.jaxrs.default-roles-allowed value because:

• they are 2 different concepts:
  • right at the top of https://quarkus.io/guides/security-authorize-web-endpoints-reference we suggest to use quarkus.security.jaxrs.default-roles-allowed
  • they work differently
  • that property is specific for standard security annotations (and JAX-RS endpoints), it has no relation to HTTP paths

Again, I suggest that @gbourant have a good idea, but IMHO the easiest and clearest way to do that is

quarkus.http.auth.shared-policy-roles-allowed=ROOT

Please replace property name with the one you like.

Hey @gbourant, would you like to implement this? I can do that, but I can see you are interested and contributions are welcome! This should be simple enough to implement. Let's wait for @sberyozkin on concrete implementation ideas and let me know if you plan to do this.

[michalvavrik]

btw. you implementation idea won't work, I won't go into details unless you require to as my suggest is not related to that.

[michalvavrik]

However policy share role won't have effect on other annotation. To create some root level role allowed that will have access everywhere will be way more complex. But it sounds like a good idea.

I agree there should be some property that will give access to that role everywhere, WDYT @sberyozkin @gbourant ?

[michalvavrik]

If there is an agreement on such root role, I'll put this issue on my list.

[sberyozkin]

@michalvavrik

quarkus.http.auth.shared-policy-roles-allowed=ROOT is interesting, but we also have role mappings, permissions, etc.
Can we define a policy shared attribute and if yes then it is applied even if a more specific path in some other policy wins ? So whatever a shared policy defines it will be applied to all matching paths

[michalvavrik]

@michalvavrik

quarkus.http.auth.shared-policy-roles-allowed=ROOT is interesting, but we also have role mappings, permissions, etc. Can we define a policy shared attribute and if yes then it is applied even if a more specific path in some other policy wins ? So whatever a shared policy defines it will be applied to all matching paths

no, it can't be done on policy level. such a "passthrough for selected role" over all security checks needs to be done differently, but can be (also) configured with config properties. I like this idea, I'll have a look.

[sberyozkin]

@michalvavrik Thanks, I guess we just should not focus on the role only, because then when someone asks for a default permission or default role mappings, we'll have a problem. which is I why I thought, given an HTTP Policy instance already has all those properties, it might make sense to just apply it to everywhere where it matches. It is not really a passthrouh, but more like augmenting the existing policy with one or more matching shared policy data.
Why do you think it won't work ?

[michalvavrik]

@michalvavrik Thanks, I guess we just should not focus on the role only, because then when someone asks for a default permission or default role mappings, we'll have a problem.

I thought to make this "passthrough" CDI bean that is more like predicate applied on SecurityIdentity. default impl. if build time properties were defined would allow roles as root, but you can do same for anything else

which is I why I thought, given an HTTP Policy instance already has all those properties, it might make sense to just apply it to everywhere where it matches.

problem is that it has no relation to RBAC security checks and we do not have a control ever any of:

• user/extension defined security checks for methods (I don't expect to be major case, but it is possible)
• user/extension defined HTTP Security policies

we just control default ones and applying HTTP Policy won't make RBAC security checks passing. Extensions / users may define their own security checks and policies for methods, we don't have a way to augment them

It is not really a passthrouh,

scary word, I don't want to relax security, augmenting sounds better, there is just one catch - augmenting can work only for builtin policies / checks

but more like augmenting the existing policy with one or more matching shared policy data. Why do you think it won't work ?

It will work for builtin policies, and I think we should do that if we don't allow what I suggest.
It will not work for RBAC annotations as they have no relation to builtin HTTP policies.

So I can see 2 ways:

1. augmentation - allow something like roles-allowed in configuration properties that will be added to every security check (AKA: annotation RolesAllowed) and role-based HTTP Security policy
2. passthrough - give users a way to define bean that for SecurityIdentity can say has all access and no other security check/policy will be applied.

I'm perfectly fine with 1., but please bear in mind it won't work on anything not provided by Quarkus @sberyozkin

[sberyozkin]

Michal, sorry, I find it difficult to follow the above, can you please answer the question why simply designating a given policy as shared can not work, for example:

quarkus.http.auth.policy.role-policy1.roles-allowed=root
quarkus.http.auth.permission.roles1.paths=/*      
quarkus.http.auth.permission.roles1.policy=role-policy1
# add permissions and role mapping as well
quarkus.http.auth.permission.roles1.shared=true


quarkus.http.auth.policy.role-policy2.roles-allowed=user
quarkus.http.auth.permission.roles2.paths=/a/*,/b/*,/c/*      
quarkus.http.auth.permission.roles2.policy=role-policy2

Why we can't have the above implemented such that /a/*, /b/*, /c/* can be accessed by root users ?

[michalvavrik]

Michal, sorry, I find it difficult to follow the above, can you please answer the question why simply designating a given policy as shared can not work, for example:

quarkus.http.auth.policy.role-policy1.roles-allowed=root
quarkus.http.auth.permission.roles1.paths=/*      
quarkus.http.auth.permission.roles1.policy=role-policy1
# add permissions and role mapping as well
quarkus.http.auth.permission.roles1.shared=true


quarkus.http.auth.policy.role-policy2.roles-allowed=user
quarkus.http.auth.permission.roles2.paths=/a/*,/b/*,/c/*      
quarkus.http.auth.permission.roles2.policy=role-policy2

Why we can't have the above implemented such that /a/*, /b/*, /c/* can be accessed by root users ?

Your example can work and:

• There is always one path matched (winning one), therefore having quarkus.http.auth.permission.roles1.shared=true is not ideal because it would require matching more then one path
• You example can be nicely implemented if *.shared=true is only allowed for .paths=/*
• It would be much easier with something like quarkus.http.auth.shared-policy=policy-name, because your suggestion would add real complexity to already complex path matching algorithm

You example does not mention it would not work for:

@Path("/a/b")
public class HelloResource {
    @RolesAllowed("user")
    @GET
    public String hello() {
       return "hello";
    }
}

because HTTP policy role-policy1 or role-policy2 has no effect on standard security annotations. I thought we are discussion mechanism that will work for both policy and annotations. Implementing your example is fine as well, I think it makes sense.

[sberyozkin]

@michalvavrik

It would be much easier with something like quarkus.http.auth.shared-policy=policy-name

Sure it is easier but this is a variation of what this issue aims to avoid ( where they can already do it right now anyway by repeating ROOT in any other security policy to which ROOT has to apply), a user wants to say only once that a given role applies to /* etc and this is it

You example can be nicely implemented if .shared=true is only allowed for .paths=/

My thinking has been not to try to merge this shared policy with all other polices whose paths can be matched, though I initially thought this is how it would work. My proposal would be to make it simple:

• first, the usual longest path wins is applied to the non-shared policies - this is what we do now
• second, repeat the first step, but select one of the shared-only policies which has the longest winning path

That should do I guess.

You example does not mention it would not work for...because HTTP policy role-policy1 or role-policy2 has no effect on standard security annotations. I thought we are discussion mechanism that will work for both policy and annotations.

IMHO we should not consider this case in isolation: if HTTP policy has no effect on standard security annotations in Quarkus Security right now, so be it, then we will have one solution like a shared HTTP policy when users prefer working with HTTP policies only and another one for the case where users prefer to have annotations only.

I can imagine we can have some new property which will work for roles, but as I said HTTP policy that can also have permissions, and role mappings and will probably have more things added, which is why I'm quite keen to have an HTTP policy configuration reusable/shareable

Can you give an example of how your solution would look like ?

Thanks

[michalvavrik]

It would be much easier with something like quarkus.http.auth.shared-policy=policy-name
Sure it is easier but this is a variation of what this issue aims to avoid ( where they can already do it right now anyway by repeating ROOT in any other security policy to which ROOT has to apply), a user wants to say only once that a given role applies to /* etc and this is it

You example can be nicely implemented if .shared=true is only allowed for .paths=/

My thinking has been not to try to merge this shared policy with all other polices whose paths can be matched, though I initially thought this is how it would work. My proposal would be to make it simple:

first, the usual longest path wins is applied to the non-shared policies - this is what we do now
second, repeat the first step, but select one of the shared-only policies which has the longest winning path
That should do I guess.

I understand you now. Yes, what you propose would work if you applied shared policy first and if it succeeded, you would skip not shared one. Or if you augmented identity in shared policy in a way it would make pass not-shared policy.

I will make your example work by merging policies based on path pattern during creation of path HTTP Security policy bean. Thanks for suggestion.

I can imagine we can have some new property which will work for roles, but as I said HTTP policy that can also have permissions, and role mappings and will probably have more things added, which is why I'm quite keen to have an HTTP policy configuration reusable/shareable
Can you give an example of how your solution would look like ?

Based on when you are heading with HTTP Security policies, it needs to be independent solution. My suggestion would be to just augment identity in HTTP Security policy to have required roles. We are talking about few words. Let's not add complexity to Quarkus. If user augment identity in HTTP Security policy, let say for root to have roles user, there is extra value for not doing some magic hidden on line XYZ in documentation. Please let's discuss shared HTTP Security policies now.

[michalvavrik]

I forgot to mention is that what we should want (and do) is to merge shared policy with others once, during bean creation. So that 2 path matching do not need to be applied for every single HTTP request.

[sberyozkin]

@michalvavrik That sounds good, but it can be tricky trying to figure out how to merge, like you said, one non-shared policy can have /a/*/b/*, a shared one /a/c... You are right, we can just restrict shared to /* only but it will also be limiting. Performance wise, doing 2 matches for such cases may not be a big problem...
In any case, please take your time, I agree @gbourant's proposal will make it possible to avoid a lot of duplication across multiple policies, but it is an optimization, so not a critical one.

[michalvavrik]

I've played with this little bit and merging permissions is very difficult, because you can also have less specific permission that is enhanced by more specific permission (root one), which means that even when less specific permission wins, it only sometimes will match root permission; conclusion: I agree with @sberyozkin suggestion to not merge them.

[michalvavrik]

There is one else thing though - this #37414 (comment) proposal by @sberyozkin doesn't take into consideration that many permission checks can be applied on one path (not just roles-allowed that can be matched) and also sometimes shared policy will be custom named policy that can't be merged somehow.

I belief we need to follow what already exists in path-matching HTTP security policies - merging. You can merge many permission checks for same path and all are applied => the relationship between permission checks is logical AND, not OR as suggested in the example. It can do same job though as there is role mapping where you can give root all roles on all authenticated paths.

[michalvavrik]

On the other hand, the fact that we would stick to logical AND between permissions allows you to require authenticated via shared permission for same shared path like /secured/* and only define additional requirements (like roles) for subpaths like /secured/admin for admin etc. which wasn't possible till now (or with suggestions above).