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

x/net/http2/h2demo: contains expired certificates #34729

Closed
johha opened this issue Oct 7, 2019 · 6 comments

Comments

@johha
Copy link

commented Oct 7, 2019

Hi,

the rootCA.pem and server.crt in net/http2/h2demo/ are expired a long time ago.
Shouldn't they be removed and instead be created by user as instructed in the readme?

@agnivade

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

@dmitshur dmitshur changed the title net/http2/h2demo/ contains expired certificates x/net/http2/h2demo: contains expired certificates Oct 7, 2019
@gopherbot gopherbot added this to the Unreleased milestone Oct 7, 2019
@dmitshur

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

Seems reasonable to me to delete them. They're not used in prod, only for people wanting to try h2demo locally.

As part of removing them, we can improve error handling in log.Fatal(srv.ListenAndServeTLS("server.crt", "server.key")) to print a more helpful error message like "A certificate and matching private key not found; see README for instructions to create them." when xerrors.Is(err, os.ErrNotExist).

Brad has more context here so he might have a better suggestion.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

That they're expired doesn't matter much: the developer will have to click through cert warnings regardless. That's just one more reason the browser won't like the cert. That's okay.

I'd rather not delete them until there's an easy way to make the server start up in localhost TLS otherwise.

@dmitshur

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

I thought there'd be no warnings in browser because the README suggested to "install [root CA] to Firefox". If there are warnings, then the above rationale makes sense.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Oct 7, 2019

I don't remember what that Firefox bit was about. Maybe it only let you do opt-in HTTP/2 back in the day for trusted sites?

@gopherbot

This comment has been minimized.

Copy link

commented Oct 7, 2019

Change https://golang.org/cl/199579 mentions this issue: http2/h2demo: update README

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.