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

Certbot library usage #4065

Open
jaddison opened this Issue Jan 16, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@jaddison

jaddison commented Jan 16, 2017

Would it be possible to refactor certbot to be more of a library+tool than just a tool? Or two packages python-certbot and certbot, with the latter having the current functionality but using the former at its core?

Referencing #1793, #358, #1299 as semi-related items, although they are more focused on refactoring the certbot the tool. I'm interested in a refactor into a libary+tool in one package, or separate packages for each.

I think this would lead to greater uptake from hosting providers (easier implementation), as it becomes much simpler to integrate into a fully automated certificate generation process.

I created a wrapper around certbot[1], but I'd rather see a native Python implementation than running certbot in a separate process within Python as I've done. A short conversation on the topic here [2].

[1] https://github.com/jaddison/certbot_py
[2] https://community.letsencrypt.org/t/python-wrapper-around-certbot-auto-0-10-0/25582/2

@bmw

This comment has been minimized.

Contributor

bmw commented Jan 17, 2017

@jaddison

This comment has been minimized.

jaddison commented Jan 17, 2017

Thanks - I have seen acme. As I mention on those referenced pages, it doesn't appear to be as user-friendly as something like a generate_certificates(domains=domain_list, auth_callback=auth_callback_func, **options) function, however. (obviously just an example)

As I also mention, it's nice to maintain compatibility with certbot renew.

A fair bit of boilerplate is needed and as things change moving forward, an implementation using acme could break. If there was a stable interface in certbot, the likelihood is lower (but not removed, I will admit). Let's Encrypt engineers can change the code behind the interface without affecting users.

Just putting the thought out there - if it isn't desirable, then by all means, close this issue.

@bmw

This comment has been minimized.

Contributor

bmw commented Jan 17, 2017

No this is definitely desirable and actually something we've talked about internally. Certbot/acme is rarely used by hosting providers who support Let's Encrypt and we believe the problems you describe in this issue are largely the reason. These companies need a library and acme is too low level.

To be honest, I doubt we'll see this change for a while, but we'll certainly keep it in mind. We're currently working on getting things polished up and increasing stability so we can get Certbot 1.0 out the door. We do, however, have big plans for Certbot 2.0 as we've learned a lot while writing and deploying Certbot to a large user base with a changing protocol. I think this is something important for us to keep in mind then.

cc @ohemorange

@jaddison

This comment has been minimized.

jaddison commented Jan 17, 2017

Great, thanks for the clarification and confirmation!

@jsha

This comment has been minimized.

Contributor

jsha commented Jan 23, 2017

Adding my +1 here. Over the weekend I refactored the Boulder integration test to use the acme module internally: https://github.com/letsencrypt/boulder/pull/2515/files#diff-ba551430a8d669438a949f2de20ecea8.

It was harder than I expected. Main pain points:

  • acme is under-documented. It would benefit from a top-level introduction with examples that explains how the various pieces fit together.
  • The distinction between FooResource and FooBody is hard to grasp.
  • It requires directly using the cryptography.hazmat packages in order to generate keys. The library should provide functions to generate account keys and CSRs without touching cryptography directly.
  • Some functionality is left to Certbot when it should be in acme. For instance, downcasing of DNS names. If you try to issue a cert with capitalized letters in its domain name, acme will throw the confusing UnexpectedUpdate error (in general, I'm not sure UnexpectedUpdate is a good error to throw at all).
  • If validation fails, poll_and_request_issuance returns a PollError. If you want to get the validation errors, you have to iterate through the elements of the PollError, fetch and parse the authorizations, and extract the error field. It would be natural for poll_and_request_issuance to do this itself.
  • Constructing HTTP01Resources to pass to HTTP01Server is surprisingly complex. It would make more sense to directly pass a list of challenges to HTTP01Server.
  • To use HTTP01Server, you need to starts a thread, call poll_and_request_issuance, then call server.shutdown() and server.server_close() and join the thread. You also have to make sure to do this if there is an exception, which will be non-intuitive to people not familiar with threaded programming. There are also some tricky race conditions around when you start shut down the server (for instance, looping until the server is actually listening; shutting down only once Boulder has attempted issuance). There should be a convenience wrapper for poll_and_request_issuance that both starts and joins the HTTP01Server thread (roughly, some of the functionality from Certbot's standalone plugin should be moved into acme).
@bmw

This comment has been minimized.

Contributor

bmw commented Jan 23, 2017

Thanks for all the specific points. This will be really valuable when we try and resolve the issue.

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