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

Implement ACMEv2, since ACMEv1 is now EOL 😱 #151

Closed
glyph opened this issue Nov 11, 2019 · 27 comments · Fixed by #161
Closed

Implement ACMEv2, since ACMEv1 is now EOL 😱 #151

glyph opened this issue Nov 11, 2019 · 27 comments · Fixed by #161

Comments

@glyph
Copy link
Member

glyph commented Nov 11, 2019

txacme.client.ServerError: (Error(typ=u'urn:acme:error:unauthorized', detail=u'Account creation on ACMEv1 is disabled. Please upgrade your ACME client to a version that supports ACMEv2 / RFC 8555. See https://community.letsencrypt.org/t/end-of-life-plan-for-acmev1/88430 for details.', title=None), <treq.response._Response 403 'application/problem+json' 280 bytes>)

@adiroiban
Copy link
Member

Is txacme still active?

I have PR #140 open for some time without much feedback.

I am using txacme and will work at implementing v2 support
.
Let me know if someone can help with a review.

@adiroiban
Copy link
Member

But before going to ACMEv2 I would like to have the API with support for SAN

Right now, I am using a forked txame version and I need SAN

#37

@glyph
Copy link
Member Author

glyph commented Nov 19, 2019

@adiroiban SAN support is important, but ACME v2 seems more pressing due to the fact that txacme will stop working entirely if we don’t get to it in time :).

@adiroiban
Copy link
Member

I understand. But before being able to work on ACME v2 and write tests for txacme I need to get familiar with the txacme testing tools.

This is why I start working on #140 and was planning to do #37 . After that I can look into ACME v2 support.

SAN support should be a quick fix...and I am already using it in production for my project.

@glyph
Copy link
Member Author

glyph commented Dec 22, 2019

@adiroiban I appreciate any work you can do on anything related to txacme, of course. I just hope somebody picks up ACMEv2 soon.

@adiroiban
Copy link
Member

I understand that ACME v2 is critical. But before being able to work on ACME v2 and write tests for txacme v2 I need to get familiar with the txacme testing tools... testing-cabal/testtools needs extra effort to learn

This is why I start working on #140 and was planning to do #37 . After that I can look into ACME v2 support.

SAN support should be a quick fix...and I am already using it in production for my project.

@adiroiban
Copy link
Member

I have start working on the ACME 2.

So far is a proof of concept ... as I still don't understand the eltot loging and all the callback chains.

Once I get a basic example up and running I will also update the tests

https://github.com/twisted/txacme/tree/151-acme-v2

There will be an API break for the low level ACME client... but I think that ACME service can continue to work as it is.

@glyph glyph pinned this issue Jan 28, 2020
@adiroiban
Copy link
Member

I have implemented the v2 protocol based on a local pebble server.

I have not yet updated the AcmeIssuingService to use the new implementation

In the 151-acme-v2 there is an example script that I used for manual testing

https://github.com/twisted/txacme/compare/151-acme-v2

The API is changed as v2 has a different process and objects.

Will continue to work on the ACME service part with support for SAN

@babolivier
Copy link

Amazing! 🙂

Any idea of when this implementation will be usable in the mainline txacme?

@adiroiban
Copy link
Member

I don't know... using eliot and testtools.matchers and not using defer.inlineCallbacks is still a pain for me ... so I don't know when the PR can be merged.

In order to get things done, at least from now, I have removed some eliot logging and no automated tests were written... only the example script which can server as a manual happy path test.

I hope to get AcmeIssuingService in the next week

@adiroiban
Copy link
Member

I have implemented the Service part of txacme .

as far as I can tell, there was no API change for the txacme.service.AcmeIssuingService

Please give it a try from the in work branch and let me know if anything is broke.

The low level txacme.client.Client API was changed

The current implementation also support SAN via CSV names.

@adiroiban
Copy link
Member

I think that for the initial release of ACME v2 we should remove the endpoint support.

The endpoint support for ACME v2 can be added later.

The current ACME v2 branch is already huge.

@perkinslr
Copy link

What is still required to get this to a usable state?
The src/txacme/client.py file from https://github.com/chevah/txacme/tree/chevah-acme-v2 appears to make this work, if it's cherry-picked into master. (I don't know if the service stuff works, as I don't have need of that).

I'm going to see if I can put together a minimal example using it and a private_key in from certbot. I can also give the patched client.py a once-over and open a PR with just it, as that would restore at least basic functionality here.

@adiroiban
Copy link
Member

@perkinslr I think "it takes 2 to tango"

Can you try the version from here?
Are you happy with the API?

pip install --index=https://bin.chevah.com:20443/pypi/simple/ txacme==1.0.0.chevah4

The thing that since there wasn't much public interest into txacme, I kept using my fork and private test - a dump is here https://gist.github.com/adiroiban/b895a62b4d28197dfdc8b803aa23ab0c

it needs some feedback in terms of API, to make sure the API can be use in multiple scenarios.

but if you are happy to try a pre-release I am happy to continue working on this.

@perkinslr
Copy link

Doesn't look like you've made any further changes to src/txacme/client.py in that version, so yes, it works too.
So far, I've only used the example_client.py from your github fork, but I'll be implementing a real version presently. It looks like the API is roughly like I'd have done it, so no objection there, but I'll let you know once I've actually used it.

As for the lack of public interest, I think that's a matter of messaging. I actually got about 1/4 of the way through RFC8555 to start working on it because I failed to find this before stumbling upon it. Probably because I was looking for twisted-certbot rather than twisted-acme integration. Not sure how to get this to show in searches for "twisted certbot", probably mention it's "an alternative to certbot" somewhere in the readme, but that can be addressed once it's functional.

@perkinslr
Copy link

Hm, the imports in client.py are wrong. It should be from . import __version__ (and the like), rather than from txacme import __version__.

@adiroiban
Copy link
Member

ok. let me put all the code and tests that I have in the current PR.

We can discuss based on the exact code...

the PR will be a monster, but as long as the tests are green and have some sort of coverage, I think that we should merge the PR.

Then we can create separate PR to do various other changes or bugfixes.

I have also canibalized the existing project as I have removed versioneeer as I remember that it was not playing nice with private pypi mirrors...and I had to build from source for AIX and Alpine Linux... but maybe versioneer works with those cases and is just that I don't know how to use it.

And eliot usage and hyphotesis usage ...and I didn't know how to use them TDD and they were just stading in my way.

@perkinslr
Copy link

You're still using eliot inside client.py. Since it isn't available in my distro's repos, I'd kinda prefer not to use it, but am not terribly picky.

@perkinslr
Copy link

The dns01 challenge appears to possibly be broken, as I can't figure out from the provided parameters what value should be set in the txt record.

@perkinslr
Copy link

Yup. The call to start_responding includes a pregenerated response value, which is used for the http challenge, but does not include a pregenerated validation. You can get it from challenge.validation(client_key), but it is not perfectly obvious.

@adiroiban
Copy link
Member

I have only implemented HTTP-01 for v2.
The libcloud will be removed as a start, and we can get DNS-01 in a separate PR.

@perkinslr
Copy link

Really? Because I have DNS01 working. The challenge type is different, and the way to get the verification string is slightly different, but otherwise it's quite simple. (The current API just requires the dev to keep a handle to the client secret key in order to generate the string inside the start_responding method). Returning a Deferred from start_responding already works, so the significant delay in deploying DNS01 responses is not a major problem.

@adiroiban
Copy link
Member

I have never tried DNS-01 with my v2 branch.

From you comment, I am not sure if the existing LibcloudDNSResponder is working, or you were able to use the library to create your own DNS responser.

@perkinslr
Copy link

I created my own DNS responder, since I need to reach out to my DNS provider to change the TXT record via their API. Don't see any reference to LibcloundDNSResponder, since I'm not using anything from your branch except the client.py.

@adiroiban
Copy link
Member

@perkinslr ok. The AcmeIssuingService is nice. You just set it up and it will take care of long term renewal.

If you have time, it would be nice to take a look at the AcmeIssuingService API and see what it's missing from making it usable for you test case.

see the code on the new #161 ... that is the code that I am trying to merge

@perkinslr
Copy link

Fundamentally, the problem with AcmeIssuingService is it's designed to run as a twisted service (which are usually standalone, I know you can run multiple services within a process, I generally don't like to). Also, it cannot interoperate cleanly with certbot, at least not without implementing an ICertificateStore.

So far, I have implemented a simple periodic certificate check that goes through the list of certbot certs and renews them when needed. It does this with a dead-simple reactor.callLater loop at the top level of my webserver. When this signals a cert must be reloaded, it updates the cert used by the webserver. Slightly longer term, my mail server (and other similar servers) will request their certificates via dbus, and be notified via dbus message of certificate renewals.

The whole implementation, including challenge responders, weighs in at 200 loc. Were I to use AcmeIssuingService to replace what it can, it would save me at most 25 lines.

From looking at AcmeIssuingService, it's fine for what it is, but it's far more than I need.

@adiroiban adiroiban unpinned this issue Apr 12, 2022
@adiroiban
Copy link
Member

I have merged the code.

I will have to look into updating the tests for AcmeIssuingService and do a release.

And also check the docs. The endpoint is no longer supported at SNI-01 is no longer supported

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

Successfully merging a pull request may close this issue.

5 participants