Cannot Compose Insecure Credentials
jonahbron opened this issue · 20 comments
Is your feature request related to a problem? Please describe.
When composing an InsecureChannelCredentials instance with CallCredentials, it throws the following error:
Cannot compose insecure credentials
I'm guessing this is to discourage users from communicating credentials over a unencrypted connection. The good intent here is obvious, but mis-placed in my opinion. Not only does it make it more difficult to implement a perfectly legitimate use case (communication within a VPC where encryption is redundant), but it still doesn't prevent unencrypted credential passing, since the user can simply add tokens to the metadata directly at call time.
I still would like to use the CallCredentials feature so I can attach an auth token to a Client instance upon instantiation. As of right now, I have to provide a metadata object in my code anywhere I'm making a call. This makes SoC and testing more difficult.
Describe the solution you'd like
I'd simply like to be able to compose insecure credentials with call credentials.
Describe alternatives you've considered
In the interim, I'm probably going to make my own InsecureChannelCredentialsImpl class that permits what I'm trying to do.
I'm sorry, but we have to decline this request. This restriction is part of the API now, so even if we did want to make this change, we're not willing to break the API to do so.
Also, would you mind mentioning which gRPC implementation you are using?
You've described it perfectly: this is indeed to prevent passing credentials over a plaintext connection. That's a hard stance not just from the node grpc implementation, but from the grpc project as a whole. In fact, while this error comes from the node wrapping code, the common core code is designed in such a way that we wouldn't be able to compose them anyway in the first place.
I don't think there's any intention on softening this stance, so I'm going to close this one preemptively as a "won't fix". But I am still going to ping @ejona86, @jboeuf, @a11r, and @jiangtaoli2016 about design decisions such as this one, hence leaving the discussion open.
In Java and Go, you could attach call credentials over insecure channel (depend on configuration of call credentials).
In c core, it is not allowed to send call credential over an insecure channel. We do have the concept of local channel credentials (https://github.com/grpc/grpc/blob/master/src/core/lib/security/credentials/local/local_credentials.h, where you could send call credentials over local channel. So far local channel types only support UDS. In the future, we may add TCP localhost connection as local channel.
communication within a VPC where encryption is redundant
There's encryption and there's authentication. A lack of authentication means you may try to connect outside of the VPC. Yes, you could have firewalls/airgaps in place to prevent that, but it should make a security person worried.
The stance right now is "grpc's pre-built credentials won't support insecure connections." There was going to be support for creating your own credentials that could choose what security level they needed. I don't know the state of that cross-language, but that's what Java's been doing for a while (although the decision is like 1-2 months old).
@murgatroid99 I'm using the native version, on NPM as grpc.
You've described it perfectly: this is indeed to prevent passing credentials over a plaintext connection.
@nicolasnoble I'd argue that it doesn't prevent it, it just makes it harder, and makes my code less idiomatic.
We do have the concept of local channel credentials
@jiangtaoli2016 That would be great, I'd love to have a third ChannelCredentials that has a concept of being "safely unencrypted".
There's encryption and there's authentication. A lack of authentication means you may try to connect outside of the VPC.
@ejona86 It might help if I clarify my situation better. Any given GRPC microservice I'm running in my VPC can only be hit from another microservice within that VPC. Each microservice also has a GRPC-Web proxy that is publicly facing. That proxy does require HTTPS, since it's exposed to the Internet.
Even though encryption seems unnecessary (change_my_mind.jpg) when talking to GRPC in this case, I still need to be able to pass around credentials so I know what user is performing any given action. I can do it now by setting the metadata at call time, it just sucks.
There was going to be support for creating your own credentials that could choose what security level they needed.
I think ChannelCredentials is exported from the library, so I'm pretty sure I can already do this. Haven't tried though. More explicitly supporting it would be nice though.
You will not be able to do this yourself. It's true that the ChannelCredentials class is exported from the library, but all of the actual logic is in the C and C++ layers.
@murgatroid99 Hm, I see. Does the pure JS client work well enough for me to switch over to it?
In general, I would say that it does, but it really depends on what features you need, exactly. And if you do try to do this in the pure JS client, keep in mind that it will almost definitely require poking at some private APIs, so don't be surprised if it gets broken in future versions.
To be more specific, this document compares the available features in the two libraries.
@murgatroid99 Okay. Besides "you have to use HTTPS if you want good SoC in your app", is there any plan to implement something like what @jiangtaoli2016 mentioned? I feel I'm not the only one going to be at odds with this policy, and a better solution seems warranted.
We don't currently have any specific plans to do that, but if and when that feature is added to the core library, it may eventually make its way into the Node libraries.
Okay, thank you @murgatroid99. Sounds like I just need to use HTTPS across the board (which is gonna be a pain but 🤷♂️ )
Is this the reason why we can't do the following?
const metadataUpdater = (_, cb) => {
const metadata = new grpc.Metadata()
metadata.set('key', 'test11')
cb(null, metadata)
}
const metadataCreds = grpc.credentials.createFromMetadataGenerator(
metadataUpdater
)
const insecureSSL = grpc.credentials.createInsecure()
obClient = new serviceProto(
`${host:port}`,
grpc.credentials.combineChannelCredentials(insecureSSL, metadataCreds)
)
Also seeing Cannot compose insecure credential
@willyyang yes.
Ok gotcha.
Is there any way to pass metadata through an insecure channel?
You can pass a metadata argument when calling a method on a Client. It's an optional argument that comes right after the message, or first for methods that don't take a message.
@murgatroid99 Doesn't the method only take 2 arguments . the request (proto request message) and the callback?
const insecureSSL = grpc.credentials.createInsecure()
obClient = new serviceProto(
`${host:port}`,
grpc.credentials.createInsecure()
)
obClient.myMethod(request, callback) // this should only take 2 arguments?
It takes two required arguments and another two optional arguments in the middle: metadata and options. The Client API documentation doesn't show exactly that method signature, but if you ignore the first three arguments of the methods in the docs it's the same.
Thanks @murgatroid99 .