No gRPC response when no separate server is used & quarkus-keycloak-authorization is present

[SivaM07]

Describe the bug

Combination of trying to use the same http vertx server for grpc and having keycloak-authorization, seems to break the flow and stopping actual grpc service implementation being invoked.

On debugging the only difference between with & without keycloack is that, when keycloak is present the HttpAuthorizer gets executed in a blocking fashion as a runnable and the subsequent grpc handler too gets executed on this executor thread instead of the eventloop thread.

Please suggest whether this a bug.
Happy to help with additional information if needed

Expected behavior

On setting quarkus.grpc.server.use-separate-server=false when keycloak-authorization is present, the security Identity to be validated via the HttpAuthorizer and respective grpc service implementation triggered and response returned.

Actual behavior

On setting quarkus.grpc.server.use-separate-server=false when keycloak-authorization is present, the security identity is validated via the HttpAuthorizer and gets validated.
Post which the grpc server handler and interceptors are invoked, but the grpc service implementation is not invoked.
Eventually no grpc response leaving the grpc client hanging.

How to Reproduce?

Reproducer:

1. Use grpc-plain-text-quickstart
2. Add property quarkus.grpc.server.use-separate-server=false
3. Add quarkus-oidc & quarkus-keycloak-authorization dependency
4. Copy the security configurations from security-keycloak-authorization-quickstart
5. Generate a bearer token and add it as part of the grpc request ( I have used grpcul for this ./grpcurl -plaintext -H 'authorization: Bearer ${TOKEN}' -expand-headers -d '{"name":"test"}' -vv localhost:8080 helloworld.Greeter/sayHello

Output of uname -a or ver

Microsoft Windows [Version 10.0.22621.1778]

Output of java -version

openjdk version "11.0.11" 2021-04-20 OpenJDK Runtime Environment AdoptOpenJDK-11.0.11+9 (build 11.0.11+9) OpenJDK 64-Bit Server VM AdoptOpenJDK-11.0.11+9 (build 11.0.11+9, mixed mode)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.16.7.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.6 (84538c9988a25aec085021c365c560670ad80f63)

Additional information

No response

[quarkus-bot]

/cc @alesj (grpc), @cescoffier (grpc), @pedroigor (keycloak), @sberyozkin (keycloak)

[sberyozkin]

@SivaM07 Hi, I think this issue may be invalid because:

• OIDC depends on the security architecture which is currently HTTP-centric
• AFAIK, with GRPC and HTTP2, one can use only Basic and MTLS

But in any case, @alesj, please check

[cescoffier]

I agree with @sberyozkin analysis. The current OIDC only works for HTTP 1.1 and not with gRPC (It may work with HTTP/2 but not when using gRPC).

[SivaM07]

@cescoffier @sberyozkin , Hi, issue doesn't seem to be with the OIDC module. On sending Bearer token as part of the grpc metadata, the existing HttpAuthenticator did pick up the right BearerAuthMechanism and extracted the token from the header and triggered the oidc flows to create the secuityIdentity.

Issue seems to be when used in combination with the keycloack extension, where the keycloack policy authorizer tries to execute things as a runnable in separate executor thread and leading to the grpc server handler too being ran on the executor thread instead of eventloop.

So basically OIDC + GRPC does work.
It kind of breaks when OIDC + Keycloak + GRPC is used.

As per logs, even the keycloak part went through successfully where it used the servicename/method name as URI and tried to retrieve the respective response. And passed over the flow to the grpc server handler.

As for as I have debug at this point, the difference I could find is the grpc handler did not get executed on the eventloop and even though the inbound buffer and the respective handler was triggered, the actual grpc service implementation was not triggered.

So my understanding is that with the new grpc on vertx http server, the oidc & keycloak does handle the HttpServerRequest and process them successfully and only the async part of HttpAuthorizer seems to impact the subsequent handlers like grpc server handler.

[sberyozkin]

Hey @SivaM07 @cescoffier

On sending Bearer token as part of the grpc metadata, the existing HttpAuthenticator did pick up the right BearerAuthMechanism

Interesting, not sure why it works, my only theory is that HTTP/2 is auto-negotiated somehow, @stuartwdouglas suggested GRPC authentication should work over HTTP/2.

So my understanding is that with the new grpc on vertx http server, the oidc & keycloak does handle the HttpServerRequest and process them successfully and only the async part of HttpAuthorizer seems to impact the subsequent handlers like grpc server handler.

I suppose GRPC runtime needs to run after HTTP security check is complete, unfortunately I don't know much about GRPC or how GRPC and Vert.x HTTP can sync with each other. Vert.x HTTP recorder sets up the authentication filter before the service code is run, perhaps it should be tuned further it is run before GRPC service code is run

[cescoffier]

With the new mechanism, gRPC is 'just' a route on the router. I can't see why it would happen before the authentication filter.

I would need to see a reproducer to debug.

[SivaM07]

@cescoffier @sberyozkin , grpc route did get invoked after the httpauthenticator and httpauthorizer handlers as expected.

The httpauthorizer executes in a blocking executor thread, post which the grpc route is invoked. At this point it seems to mess with the way the grpc request is played and eventually leading to the request getting lost and unhandled by the actual service implementation.

I am continuing to debug the difference, will share the details shortly.

[cescoffier]

Ah ah, yes, good point. gRPC expects to be called on the I/O thread, and even need a duplicated context. With the previous server we were doing that work ourself, but with the new one, we considered that granted... but, nope, it's not always the case.

So, either we re-add the interceptor, or we make the authorizer send it's value on the captured event loop (and using a duplicated context). I would say that the later is more correct, and would avoid loosing tracing and other metadata stored in the duplicated context.

[SivaM07]

@cescoffier , yes given that you have got the essence of the problem here is there a workaround, may be to get httpauthorizer execute in the eventloop instead of on a blocking executor thread ? May be that might help the subsequent grpc flow too to execute on the eventloop.

Here is the reproducer project : https://github.com/SivaM07/quarkus-grpc-keycloak-reproducer-34085

This reproducer is composed of

• grpc plaintext quickstart project
• Added the property quarkus.grpc.server.use-separate-server=false
• Added the oidc and keycloak dependencies
• Copied over the application properties and config folder from keycloak quickstart project

Steps to reproduce the issue:

• Post startup on the dev mode, open the keycloak (test container) admin page and navigate to the admin resources to add "/helloworld.Greeter/SayHello"
  
• Get the access token for admin user as described on the https://quarkus.io/guides/security-oidc-bearer-token-authentication-tutorial#test-the-application
• Set the access token to TOKEN variable and invoke the grpc service using grpcurl tool
  ./grpcurl -plaintext -H 'authorization: Bearer ${TOKEN}' -expand-headers -d '{"name":"John Wick"}' -import-path '/quarkus/quarkus-grpc-keycloak-reproducer-34085/grpc-plain-text-quickstart/src/main/proto' -proto helloworld.proto -vv localhost:8080 helloworld.Greeter/SayHello

Observation:
Even though the debug logs of httpauthorizer mentions that the path access is granted, there wont be any response.

Note:
Post which if the same is retried with removing the keycloak dependency (which disables the httpauthorizer), we can observe successful response after authentication. On sending a invalid token we can also see the expected unauthenticated status response(This makes the authentication flow work as expected)

There is even a 403 response when tried after removing the grpc service name from the keycloak admin resources (which we have added).

Only in the flow where successful response is expected after a successful authorization, we don't receive a response at all.

[sberyozkin]

@SivaM07 I'm not sure setting a Keycloak is needed if we want to prove Quarkus security and its HttpAuthorizer part in particular does not play well with GRPC. Quarkus Keycloak authorization is an instance of HttpSecurityPolicy, so having some test application scoped HttpSecurityPolicy bean should do.

In fact, Basic authentication and a custom HttpSecurityPolicy bean which would just return Allow should do it. No need to involve OIDC, keycloak-authorization, hopefully...

I guess we should be careful with changing the way HttpAuthorizer makes its decision available, to avoid affecting the performance for HTTP endpoints, hopefully won't be a problem. Also CC @michalvavrik @stuartwdouglas

[SivaM07]

@sberyozkin , this could be a blocking issue in scenarios where grpc (on same vertx http server) is added to an existing rest service using oidc and keycloak to secure the rest endpoint.

This is a valid usecase where we are trying to configure a grpc endpoint next to its equivalent rest endpoint in keycloak.

[sberyozkin]

@SivaM07 I'm not saying the use case is invalid, I'm just trying to suggest a simpler reproducer. It does not matter for Quarkus Security how a given HttpSecurityPolicy is implemented, with quarkus-keycloak-authorization being just one such HttpSecurityPolicy instance, likewise, it does not have to be a token that has to be verified but a simpler setup with Basic authentication should do. It would be easier to test Basic Auth + a test HttpSecurityPolicy set up

[SivaM07]

@sberyozkin thanks for the clarification 🙂 I was trying to put up the scenario at which we are blocked currently.

[SivaM07]

@cescoffier @sberyozkin do you think this can be fixed ? I can contribute to this fix if there is any suggestion/direction on this.

[SivaM07]

@alesj , if you can take a look at this one, if you have not done it already, that would be really great.

@cescoffier as a workaround, I tried to inject an handler to be executed right after the httpauthorizer. Which basically tried to execute the routingContext.next() on runOnContext if the context is not onEventLoopThread. And this seem to take the flow back to eventloopThread from the executor thread and execute the grpc service implementation with right input as expected.
Please let me know if you see any issues with this approach.

[michalvavrik]

do you think this can be fixed ?

@SivaM07 this can be fixed and it is clear where the problem is as you yourself shown :-)

I can contribute to this fix if there is any suggestion/direction on this.

@cescoffier provided suggestion, but personally I am not able to assess impact of using duplicated context before I spend time on it.

@SivaM07 I'm not saying the use case is invalid, I'm just trying to suggest a simpler reproducer. It does not matter for Quarkus Security how a given HttpSecurityPolicy is implemented, with quarkus-keycloak-authorization being just one such HttpSecurityPolicy instance, likewise, it does not have to be a token that has to be verified but a simpler setup with Basic authentication should do. It would be easier to test Basic Auth + a test HttpSecurityPolicy set up

I agree it would be nice to have simpler reproducer, but KeycloakPolicyEnforcerAuthorizer is only policy running blocking I know about. Basic auth + custom policy should do though. @SivaM07 could you simplify reproducer, please? If not, no worry.

[michalvavrik]

So, either we re-add the interceptor, or we make the authorizer send it's value on the captured event loop (and using a duplicated context). I would say that the later is more correct, and would avoid loosing tracing and other metadata stored in the duplicated context.

I gave a try to duplicated context @cescoffier and if I understand you suggestion right, it won't work. Here, blocking execution is run before Grpc route is invoked and so, we can't make authorized send a value on the captured event loop because it happens before Grpc route is invoked (proactive auth). I can see all interceptors running and then, on ready is invoked of SimpleForwardingServerCallListener from GrpcRequestContextGrpcInterceptor and that's the end. What fixed it was to invoke

quarkus/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/GrpcServerRecorder.java

Line 186 in 66e1053

server.handle(ctx.request());

on captured Vert.X context.

if (!Context.isOnEventLoopThread()) {
    Context capturedVertxContext = Vertx.currentContext();
    capturedVertxContext.runOnContext(new Handler<Void>() {
        @Override
        public void handle(Void unused) {
            server.handle(ctx.request());
        }
    });
} else {
    server.handle(ctx.request());
}

Unless I hear objections, I'll try to create simplified test and PR in the evening. Please let me know if you have better idea. Global interceptor should probably do as well.