Skip to content

examples: use test certs for running example-tls#5763

Merged
voidzcy merged 6 commits intogrpc:masterfrom
voidzcy:bugfix/use_testing_certs_for_tls_example
May 22, 2019
Merged

examples: use test certs for running example-tls#5763
voidzcy merged 6 commits intogrpc:masterfrom
voidzcy:bugfix/use_testing_certs_for_tls_example

Conversation

@voidzcy
Copy link
Copy Markdown
Contributor

@voidzcy voidzcy commented May 20, 2019

The way of generating self-signed keys and certificates in TLS example does not work, since the generated certificates does not contain Subject Alternative Names.

Generating self-sign certificates (which requires special config for subject alternative names) is quite involved and users may encounter problems doing that (#5737). For maintainability, we do not provide a separate set of certs here. We recommend users to use our test certs for running the TLS example.

switch (args.length) {
case 2:
client = new HelloWorldClientTls(args[0], Integer.parseInt(args[1]),
buildSslContext(null, null, null));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we still want this case. Using the default ca is good for tls.

Copy link
Copy Markdown
Contributor Author

@voidzcy voidzcy May 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case will not work, with PKIX path building failed: sun.security.provider.certpath.SunCertPathBuilderException: unable to find valid certification path to requested target.

Btw, I updated the usage printout for client and trustCertCollectionFilePath is not optional.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ejona86 I think that usecase is for advanced users who are experienced with TLS. For the first time learners, they still couldn't run that usecase successfully without extra guidance. For advanced users, they mostly like would know how to use default ca for their own project even this example doesn't show that option. So adding this option only adds complexity and distract first time learners.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the test server certificate it is a failure. But with a real server certificate, it would work. We want to encourage people to use the default ca when possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I put back the use case of using default CA and put a note on it.

@voidzcy voidzcy force-pushed the bugfix/use_testing_certs_for_tls_example branch from e8388d4 to e64aa13 Compare May 22, 2019 00:00
Copy link
Copy Markdown
Contributor

@dapengzhang0 dapengzhang0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with minor comments.

@voidzcy voidzcy merged commit e237ee1 into grpc:master May 22, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants