Navigation Menu

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

proposal: x/crypto/acme/autocert: offer safe defaults for Manager #46439

Open
jsha opened this issue May 29, 2021 · 1 comment
Open

proposal: x/crypto/acme/autocert: offer safe defaults for Manager #46439

jsha opened this issue May 29, 2021 · 1 comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Milestone

Comments

@jsha
Copy link

jsha commented May 29, 2021

In autocert's Manager, there are three fields for which the zero value is unsafe:

Email: The zero value means "provide no email." This is unsafe because the CA can't reach the user if their client is being blocked, or if APIs are changing such that their renewals will fail soon. It also means the user can't receive renewal emails. The ACME spec and Let's Encrypt allow for registering with no email, which should be a supported use case but not the default.

The Certbot project has had good success by requiring one of --email or --register-unsafely-without-email. In autocert, this could take the form of a sentinel value autocert.UnsafeRegisterWithoutEmail that indicates to Manager that the user really does want to register without email.

HostPolicy: The zero value means "TLS handshakes can trigger a certificate request for any hostname." This is unsafe because anyone on the internet can cause a server running autocert to send many requests to a CA, possibly exceeding its rate limits and/or getting blocked. The documentation says this but I suspect many people still run with this policy.

A safe alternative would be to consider nil equivalent to "reject all hosts," and offer an explicit UnsafeIssueForAnyHost HostPolicy. A couple of other HostPolicies that might be popular and provide lower risk: IssueForSubdomains, which would contain a list of domains and accept issuance requests for any of those domains' subdomains; and IssueForResolvedDomains, which would resolve a domain and attempt to issue only on success (or only if it resolves to a specific set of IP addresses).

Cache: The zero value means "no cache." This is unsafe because it's common for servers to wind up in a crash loop. If there is no cache, crash-looping servers will not have their certificates ready on startup, and will send excessive traffic to their CA. This is behavior we've seen from other clients in the past, like cert-manager.

@gopherbot gopherbot added this to the Unreleased milestone May 29, 2021
@seankhliao seankhliao changed the title x/crypto/acme/autocert: offer safe defaults for Manager proposal: x/crypto/acme/autocert: offer safe defaults for Manager May 30, 2021
@seankhliao seankhliao added the Proposal-Crypto Proposal related to crypto packages or other security issues label May 30, 2021
@seankhliao
Copy link
Member

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Jun 8, 2021
@ianlancetaylor ianlancetaylor modified the milestones: Unreleased, Proposal Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal Proposal-Crypto Proposal related to crypto packages or other security issues
Projects
Status: Incoming
Development

No branches or pull requests

4 participants