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

Ambiguous name: credentials.NewClientTLSFromCert() #107

Closed
ghost opened this issue Mar 7, 2015 · 7 comments
Closed

Ambiguous name: credentials.NewClientTLSFromCert() #107

ghost opened this issue Mar 7, 2015 · 7 comments

Comments

@ghost
Copy link

ghost commented Mar 7, 2015

My first impression was "TLS client certificate authentication", i.e. distinguish each clients by certificate that they sent. But from cursory look, turns out it's actually certificate pinning.. making sure client talks with pinned server CA.

Am I right, or does grpc actually supports client certificate authentication ?

@iamqizhao
Copy link
Contributor

On Sat, Mar 7, 2015 at 12:02 AM, prazzt notifications@github.com wrote:

My first impression was "TLS client certificate authentication", i.e.
distinguish each clients by certificate that they sent. But from cursory
look, turns out it's actually certificate pinning.. making sure client
talks with pinned server CA.

Am I right, or does grpc actually supports client certificate
authentication ?

Currently, we do not support client certificate authentication
(i.e., NoClientCert is used). But it is not hard to add if it is needed.
The name basically means creating a TLS grpc credential for client from a
cert (ca).


Reply to this email directly or view it on GitHub
#107.

@iamqizhao
Copy link
Contributor

how about NewClientTLSFromCA?

@ghost
Copy link
Author

ghost commented Mar 9, 2015

So this is basically certificate pinning right ?

How is the expected usage here ? does clientCert == serverCert ?

@iamqizhao
Copy link
Contributor

I do not think we do extra work besides the normal TLS handshake.

This is more like a browser->web service type of usage -- clients do not have their own certs but root CA.

@ghost
Copy link
Author

ghost commented Mar 19, 2015

I see another issue got confused also by TLS client certificate ..

I propose the following signatures:

// NewClient constructs secure connection for client with optional rootCA
func NewClient(server string, rootCA *x509.CertPool) TransportAuthenticator {}

// NewClientFile constructs secure connection by loading rootCA from local file
func NewClientFile(server, rootCAFile string) TransportAuthenticator {}

// NewServer constructs a new server
func NewServer(cert *tls.Certificate) TransportAuthenticator {}

// NewServerFile constructs a new server by loading cert and key from local file
func NewServerFile(certFile, keyFile string) (TransportAuthenticator, error) {}

This way it's shorter (we know it's always TLS anyway), and people don't confuse for "TLS client authentication"

@iamqizhao
Copy link
Contributor

Nah, it is not TLS always. We will support SSH too. And we are working on some Google internal transport security protocol too. Therefore, you need to have TLS in the names. In addition, I prefer "XXXFromFile" to "XXXFile". Plus, it is not necessary a local file (e.g., it could be at NFS.).

@ghost
Copy link
Author

ghost commented Mar 20, 2015

I see. Hope it doesn't get too bloated in the future. Closing this.

@ghost ghost closed this as completed Mar 20, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Sep 27, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant