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.

Sign up for GitHub

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

Jump to bottom

example: add mutual TLS example #5194

Merged

easwars merged 8 commits into grpc:master from sding3:add-mtls-example

Feb 24, 2022

Contributor

sding3 commented Feb 12, 2022 •

edited by easwars

Loading

This update adds an example for mutual TLS.

Fixes #5002.

RELEASE NOTES: none

sding3 added 3 commits

February 12, 2022 15:59


          obtain client certs necessary for example code

fb356db

this was done by copying existing certs from testdata/x509/ to
examples/data/x509 with the following shell commands:

    src=testdata/x509
    tgt=examples/data/x509

    cp ${src}/client_ca_cert.pem  ${tgt}/client_ca_cert.pem
    cp ${src}/client1_cert.pem    ${tgt}/client_cert.pem
    cp ${src}/client1_key.pem     ${tgt}/client_key.pem


          add mutual tls example

391d124


          tweak readme file

b87ad15

sding3 force-pushed the add-mtls-example branch from 3e0608d to b87ad15 Compare

February 13, 2022 00:27

easwars requested changes

View reviewed changes

examples/data/data.go Outdated

+              	for _, p := range certPaths {
+              		caBytes, err := ioutil.ReadFile(p)
+              		if err != nil {
+              			return nil, fmt.Errorf("failed to read cert %s: %v", p, err)

Contributor

easwars Feb 15, 2022

Nit: s/%s/%q/

examples/data/data.go Outdated

+              		}
+              		ok := certPool.AppendCertsFromPEM(caBytes)
+              		if !ok {
+              			return nil, fmt.Errorf("failed to parse %s", p)

Contributor

easwars Feb 15, 2022

Same here. Prefer %q.

examples/data/data.go Outdated

Comment on lines 59 to 60

		ok := certPool.AppendCertsFromPEM(caBytes)
		if !ok {

Contributor

easwars Feb 15, 2022

Could you please combine these two lines.

examples/features/mutual_tls/README.md Outdated

		@@ -0,0 +1,31 @@
		# Mutual TLS

		In mutual TLS, the client and the server authenticates each other. gRPC allows

Contributor

easwars Feb 15, 2022

Nit: s/authenticates/authenticate/

examples/features/mutual_tls/client/main.go Outdated

+               *
+               */
+              // The client demonstrates how to supply an OAuth2 token for every RPC.

Contributor

easwars Feb 15, 2022

Comment needs updating.

Something like

// Binary client is an example client which connects to the server using mTLS.

examples/features/mutual_tls/server/main.go Outdated

+              func main() {
+              	flag.Parse()
+              	fmt.Printf("server starting on port %d...\n", *port)

Contributor

easwars Feb 15, 2022

Prefer log.Infof instead of fmt.Printf.

examples/features/mutual_tls/server/main.go Outdated

+              	flag.Parse()
+              	fmt.Printf("server starting on port %d...\n", *port)
+              	serverCert, err := tls.LoadX509KeyPair(data.Path("x509/server_cert.pem"), data.Path("x509/server_key.pem"))

Contributor

easwars Feb 15, 2022

s/serverCert/cert/

examples/features/mutual_tls/server/main.go Outdated

+              		log.Fatalf("failed to load key pair: %s", err)
+              	}
+              	trustedClientCAs := x509.NewCertPool()

Contributor

easwars Feb 15, 2022

Is this line required?

Contributor Author

sding3 Feb 16, 2022

Nope - removed.

examples/features/mutual_tls/server/main.go Outdated

+              	}
+              	trustedClientCAs := x509.NewCertPool()
+              	trustedClientCAs, err = data.NewCertPool(data.Path("x509/client_ca_cert.pem"))

Contributor

easwars Feb 15, 2022

s/trustedClientCAs/{ca|roots}/

examples/features/mutual_tls/server/main.go Outdated

Comment on lines 72 to 78

+              type ecServer struct {
+              	pb.UnimplementedEchoServer
+              }
+              func (s *ecServer) UnaryEcho(ctx context.Context, req *pb.EchoRequest) (*pb.EchoResponse, error) {
+              	return &pb.EchoResponse{Message: req.Message}, nil
+              }

Contributor

easwars Feb 15, 2022

Prefer moving this to be above main.

easwars assigned sding3

easwars added the Type: Documentation label

easwars added this to the 1.45 Release milestone

Contributor

easwars commented Feb 15, 2022

@sding3 : Thank you for the PR.

Do you happen to know what signing algorithm is used in the certificates? Starting Go 1.18, support for SHA1 signatures is being dropped. We recently changed all our certificates to use SHA256 signatures.

Contributor

easwars commented Feb 15, 2022

#5002 says following regarding the location of the example:

A mutual TLS example here https://github.com/grpc/grpc-go/tree/master/examples/features/authentication

And I think that sounds better than examples/features/mutual_tls as is currently implemented. Thanks.

Contributor Author

sding3 commented Feb 15, 2022

#5002 says following regarding the location of the example:
A mutual TLS example here https://github.com/grpc/grpc-go/tree/master/examples/features/authentication
And I think that sounds better than examples/features/mutual_tls as is currently implemented. Thanks.

Yeah, I can move them to examples/features/authentication/mutual_tls, but that implies the existing content under examples/features/authentication/ would have to be moved into a new sub-directory, examples/features/authentication/oauth2, is that okay?

Contributor

easwars commented Feb 15, 2022

#5002 says following regarding the location of the example:
A mutual TLS example here https://github.com/grpc/grpc-go/tree/master/examples/features/authentication
And I think that sounds better than examples/features/mutual_tls as is currently implemented. Thanks.
Yeah, I can move them to examples/features/authentication/mutual_tls, but that implies the existing content under examples/features/authentication/ would have to be moved into a new sub-directory, examples/features/authentication/oauth2, is that okay?

Sorry, I wanted to examples/features/encryption/ where we currently have directories for TLS and ALTS. And I think we can add one for mTLS.

Contributor Author

sding3 commented Feb 16, 2022

@sding3 : Thank you for the PR.

Do you happen to know what signing algorithm is used in the certificates? Starting Go 1.18, support for SHA1 signatures is being dropped. We recently changed all our certificates to use SHA256 signatures.

Yeah, they are signed with sha256WithRSAEncryption.

sding3 added 2 commits

February 15, 2022 18:07


          address comments

9825a70


          move example to encryption/mTLS

d04bb79

Contributor Author

sding3 commented Feb 16, 2022

I've made updates and ready for you to take another look.


          fix code comment

4c136cf

easwars approved these changes

View reviewed changes

examples/features/encryption/mTLS/client/main.go Outdated

@@ @@ -0,0 +1,74 @@ @@
+              /*
+               *
+               * Copyright 2018 gRPC authors.

Contributor

easwars Feb 16, 2022

Oops ... s/2018/2022/

Contributor Author

sding3 Feb 17, 2022

fixed in cb32098


          fix copyright year to current year

cb32098

tomaswarynyca approved these changes

View reviewed changes

tomaswarynyca left a comment

Thank you for providing the example 🚀

Contributor

easwars commented Feb 22, 2022

@menghanl : Do you mind taking a look (for the second review).

easwars modified the milestones: 1.45 Release, 1.46 Release

menghanl reviewed

View reviewed changes

Contributor

menghanl left a comment

Overall LGTM. Just one comment on the helper function.

examples/data/data.go Outdated

+              // NewCertPool returns a x509.CertPool given the absolute paths of a list of
+              // PEM certificates, or an error if any failure is encountered when loading
+              // the said certificates.
+              func NewCertPool(certPaths ...string) (*x509.CertPool, error) {

Contributor

menghanl Feb 22, 2022 •

edited

Loading

I don't think this function belongs here. The data package is only to solve problems caused by relative paths.

I think it would be OK to do this inline in both client and server.
It would be easier to read, and it isn't a long function anyway.

Contributor Author

sding3 Feb 24, 2022

Ok, inlined in d66ce1d

easwars added the Status: Requires Reporter Clarification label


          inline data.NewCertPool

d66ce1d

sding3 force-pushed the add-mtls-example branch from c8a774b to d66ce1d Compare

February 24, 2022 01:29

menghanl approved these changes

View reviewed changes

Contributor

menghanl left a comment

LGTM. Thanks for the PR!

menghanl assigned easwars and unassigned sding3

easwars changed the title ~~Add mutual TLS example~~ example: add mutual TLS example

easwars merged commit 328efcc into grpc:master

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Status: Requires Reporter Clarification Type: Documentation