Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for gRPC bearer token authentication #1019

Open
nathanperkins opened this issue May 10, 2023 · 6 comments
Open

Add support for gRPC bearer token authentication #1019

nathanperkins opened this issue May 10, 2023 · 6 comments
Assignees
Labels
kind/enhancement This would improve or streamline existing functionality.

Comments

@nathanperkins
Copy link

We would like to properly authenticate and authorize access to a hubble-relay service, which uses Istio for IAM.

In order to authenticate, we need to add a bearer token to gRPC requests. My understanding is that it can be added to the request context like this:

reference

ctx := context.Background()
md := metadata.Pairs("Authorization", fmt.Sprintf("Bearer %s", token))
ctx = metadata.NewOutgoingContext(ctx, md)

Adding it to the clients based on an env var or flag might look something like this:

reference

// hubble/cmd/common/conn/bearer.go
package conn

func init() {
  token := vp.GetString(config.KeyBearerToken)
  if token != "" {
    opt := WithUnaryInterceptor(bearerTokenInterceptor(token))
    GRPCOptionFuncs = append(GRPCOptionFuncs, opt)
  }
}

func bearerTokenInterceptor(token string) grpc.UnaryClientInterceptor {
    return func(ctx context.Context, method string, req, reply interface{}, cc *grpc.ClientConn, invoker grpc.UnaryInvoker, opts ...grpc.CallOption) error {
        md := metadata.Pairs("authorization", "Bearer "+token)
        ctx = metadata.NewOutgoingContext(ctx, md)
        return invoker(ctx, method, req, reply, cc, opts...)
    }
}

If this looks good, I may be able to work on contributing it.

@gandro
Copy link
Member

gandro commented May 10, 2023

cc @chancez

@gandro gandro added the kind/enhancement This would improve or streamline existing functionality. label May 10, 2023
@chancez
Copy link
Contributor

chancez commented May 10, 2023

@nathanperkins this seems like a reasonable feature request, if your interested, feel free to open a PR.

The idea of using a Unary interceptor is interesting. I would generally use https://pkg.go.dev/google.golang.org/grpc/credentials#PerRPCCredentials for authentication like this, as that's typically the interface used for gRPC authentication. You can create an implementation of it and use grpc.WithPerRPCCredentials as part of the GRPCOptionFunc.

Another thing you'll find is you can't do the check for token in init, because viper is initialized as part of the command being executed, so you'll need to handle a lot of the logic to check if the token is provided inside the optionFunc/PerRPCCredentials implementation. See the existing grpcOptionTLS implementation for an example of how you will have to get values from viper.

Here's a few things you'll want to make sure your PR includes:

  • Add validation that TLS is configured when token-file is set
  • We should probably have a --token-type flag with a default value of Bearer, just in case some users require a different type?

@nathanperkins
Copy link
Author

nathanperkins commented May 11, 2023

@chancez, those are great suggestions thank you.

One thing that has come up in our discussions is how we could refresh tokens. JWT token expiration can be very short, like 5-30 minutes. Ideally, somebody would be able to run a packet capture which can exceed the expiration of a single token, probably by catching the expiration failure, refreshing the token, and restarting the capture.

I see somebody has implemented a token refresh, although it calls out to a refresh function that is imported alongside their client code. That seems difficult in our case since hubble CLI is distributed as a compiled binary.

In our case, we can get tokens by executing a command. Maybe we could provide a flag like --auth-token-command="authtool token --refresh --uri=<url>" which would be called in PerRPCCredentials to get the token for each request?

@chancez
Copy link
Contributor

chancez commented May 11, 2023

@nathanperkins I think as a starting point we should focus on adding support to use an existing token in a file. Afterwards, we can revisit if, and how we would support tokens that need to be renewed. This shouldn't prevent us from supporting more advanced versions of authentication that require refreshing/etc in the future, which will require a more thorough design/RFC.

Additionally, with just --token-file, refreshing could in theory be done entirely out-of-band of Hubble CLI, as long as the implementation of --token-file supports re-reading the token from the file on each RPC.

@nathanperkins
Copy link
Author

nathanperkins commented May 11, 2023

For prior art, it seems like docker's credential helpers are implemented as separate binaries which can be called with commands like osxkeychain get with the request details fed via stdin and the credential exposed via stdout.

edit: we replied at the same time :) --token-file sounds good, let me check on it and get back to you.

@shivanshuraj1333
Copy link

Hi, I'm interested in opening a PR for the initial version i.e. auth based on token file, can I get it assigned to me?
Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement This would improve or streamline existing functionality.
Projects
None yet
Development

No branches or pull requests

4 participants