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

Make internal rate limiting configurable #143

Closed
francislavoie opened this issue Sep 9, 2021 · 2 comments
Closed

Make internal rate limiting configurable #143

francislavoie opened this issue Sep 9, 2021 · 2 comments
Labels
feature request Request for new feature or functionality

Comments

@francislavoie
Copy link
Member

francislavoie commented Sep 9, 2021

What would you like to have changed?

Certmagic should make available some options to configure the rate limiting used to throttle ACME operations. The defaults are currently set to a 20 per minute sliding window.

Also, consider raising the default rate limit.

Why is this feature a useful, necessary, and/or important addition to this project?

I noticed that UptimeRobot is maintaining a fork of Certmagic and Caddy to allow them to configure a less aggressive rate limit. I assume they ran into problems with managing too many domains with their custom domains feature, using On-Demand TLS, where this limit was restricting them too much.

The limit of 10/min was originally set as a "safe" limit, then later raised to 20/min in 14b314f, but at this point since multi-issuer support exists, and ZeroSSL actually has no rate limits currently, this is probably too aggressive limit for some big companies.

What alternatives are there, or what are you doing in the meantime to work around the lack of this feature?

Forking (see UptimeRobot's fork uptimerobot@5a0ba0f)

Please link to any relevant issues, pull requests, or other discussions.

See uptimerobot@5a0ba0f where they hacked in an environment variable to override the rate limiting.

/cc @nordstern if you'd like to confirm this is why you guys made a fork. I'd be curious to know what values you found were appropriate for your usecase. Since you've forked, Certmagic has been updated to use 20/min as the limit, it was 10/min previously.

@francislavoie francislavoie added the feature request Request for new feature or functionality label Sep 9, 2021
@mholt
Copy link
Member

mholt commented Sep 10, 2021

I think this rate limit for on-demand TLS was implemented on day 1 as a safeguard, but since then CertMagic has a more proper, more intelligent throttling implementation for all transactions. We can probably remove the hard-coded on-demand throttle. I'll double-check soon.

@mholt
Copy link
Member

mholt commented Sep 13, 2021

Hm, so this is different from what I thought. I think the other rate limit I was thinking of was removed a while ago. This rate limit is important so we don't hammer CAs with requests -- even though they can implement their own rate limiting, we don't want to blast them with 10,000 requests in less than a minute, for example.

I might raise the limit to 10 every 10 seconds though. That should allow for much more throughput and still be reasonable for a CA to handle.

Keep in mind that Let's Encrypt still doesn't allow more than 2 per minute per ACME account, so raising or configuring the internal limit doesn't do much in most cases (as it's still the most popular CA). Making it configurable is technically tricky because the throttler is global, and scoping that per-config is either tricky or incorrect (not sure which yet, haven't given it much thought). So I will just raise the limit for now.

@mholt mholt closed this as completed in 047b545 Sep 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for new feature or functionality
Projects
None yet
Development

No branches or pull requests

2 participants