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

caddytls: Require email for ZeroSSL issuer #6101

Closed
wants to merge 1 commit into from
Closed

Conversation

mholt
Copy link
Member

@mholt mholt commented Feb 12, 2024

This patch is in preparation for upcoming changes that will help prevent abuse without imposing strict rate limits.

My current approach is simply to return an error if an email isn't specified.

Right now I am pretty aggressive about this: I return an error at provision-time if an email isn't found, and this will actually break a lot of deployments (not strictly speaking: as long as they're using caddy reload or the API their sites will stay up, but their new configs will need to add an email in order to run successfully). This is actually preferable if they are relying on ZeroSSL for certs. (Requiring an email address is a low price to effectively remove rate limits.)

I also return an error at account-create time, which is probably more effective, as this can allow deployments to continue without having to add their email address, they just lose the redundancy of having ZeroSSL. If they are also using Let's Encrypt, then this will not be noticeable since they will keep using Let's Encrypt.

(Frankly, using another CA just to circumvent LE rate limits is a brittle plan anyway, as it typically represents a shortcut taken in the design of one's infrastructure.)

I'm thinking of maybe removing the provision-time error and turning it into a warning to soften the blow, at least at first.

Another approach we could take is to not even wire up the ZeroSSL issuer (implicitly) if there's no email address provided. Looking at the code, this might be more complicated (or it's not, I didn't spend much time on this approach yet).

Open question remains: Are existing ACME accounts going to be blocked if they don't have an email? I'll try to find out.

This patch is in preparation for upcoming changes that will help prevent abuse without
imposing strict rate limits.
@mholt mholt added do not merge ⛔ Not ready yet! needs info 📭 Requires more information labels Feb 12, 2024
@francislavoie francislavoie added this to the v2.8.0 milestone Feb 12, 2024
@mholt
Copy link
Member Author

mholt commented Apr 11, 2024

Superseded by #6229

@mholt mholt closed this Apr 11, 2024
@mholt mholt removed this from the v2.8.0 milestone Apr 11, 2024
@mholt mholt deleted the zerossl-email branch May 20, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge ⛔ Not ready yet! needs info 📭 Requires more information
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants