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

Filter out mis-configured domains before sending the certificate request to LetsEncrypt #36

Open
marckohlbrugge opened this issue Feb 20, 2017 · 5 comments

Comments

@marckohlbrugge
Copy link

Right now when a verification fails in rake letsencrypt:renew it raises Letsencrypt::Error::VerificationError and aborts verifying any further domains.

Would it be worth making this behaviour configurable, so rather than completely aborting the Rake task it would continue verifying other domains?

Users of my app can add a custom domain which we then secure through Let's Encrypt. With the current functionality we're unable to renew ANY certificates of any customer the moment one of those domains stops resolving (e.g. when a customer removes their CNAME record and we're now longer able to verify the domain for them.)

@jalada
Copy link
Collaborator

jalada commented Feb 20, 2017

@marckohlbrugge Thanks for your feedback!

Can you explain your set up a little bit more? If one of n domains fails to verify, surely LetsEncrypt wouldn't issue you your certificate? You use certificate in the plural in your message, so I'm just wondering what exactly your set up is? Are you generating multiple certificates, one per domain?

@marckohlbrugge
Copy link
Author

Ah sorry, there's indeed only one certificate with multiple domains included.

Imagine I'm running a service like squarespace.com where anyone can host a site either on a subdomain (amazing-bananas-inc.squarespace.com) or their own domain (amazing-bananas.com)

The problem I'm running into is that when a customer's domain ( e.g. amazing-bananas.com ) doesn't resolve to my Rails app anymore I can't verify it for Let's Encrypt and thus rake letsencrypt:renew fails.

What I'd like to do in this case is exclude that domain from being included in the certificate.

Long term a better approach would be for me to use logic separate from this gem (e.g. do a daily check whether each customer's domain name still resolves and if not send them an email, etc), but for now it would be really handy if rake letsencrypt:renew had an option to just ignore domains that fail verification.

@jalada
Copy link
Collaborator

jalada commented Feb 20, 2017

If a requested domain fails verification, LetsEncrypt won't release a certificate. LetsEncrypt does the verification, not this gem, so it's not something that can be 'ignored'. It's also outside the scope of this gem right now to pre-check the domains before sending them as part of the request to LetsEncrypt.

I do appreciate the problem you have though. We are considering adding additional DNS checks to the rake task to help users understand when they have configured their domains incorrectly, and maybe from there we could add an additional feature that filters some mis-configured domains.

@jalada jalada changed the title Rescue from Letsencrypt::Error::VerificationError and continue verifying domains Filter out mis-configured domains before sending the certificate request to LetsEncrypt Feb 20, 2017
@marckohlbrugge
Copy link
Author

Makes sense. Thanks!

I thought I had found a quick workaround for this part of the code:

https://github.com/pixielabs/letsencrypt-rails-heroku/blob/master/lib/tasks/letsencrypt.rake#L65

By assuming that an OpenURI::HTTPError means the domain isn't properly configured anymore, and no error means it still is. However, if the domain simply shows a 404 page instead it wouldn't result in a OpenURI::HTTPError so it would incorrectly assume the domain is still properly configured.

I might add some of those DNS checks you mentioned myself. Will send a pull request if I do.

@jalada
Copy link
Collaborator

jalada commented Feb 20, 2017

That's definitely the right spot! We'd kinda like to move away from OpenURI anyway as it's not the neatest of libraries (because it patches open()). A pull request would be greatly appreciated 🌟

For now, I'll leave this issue open to track the request, thanks again @marckohlbrugge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants