acme-tiny icon indicating copy to clipboard operation
acme-tiny copied to clipboard

Implement externalAccountBinding

Open systemcrash opened this issue 2 years ago • 19 comments

https://datatracker.ietf.org/doc/html/rfc8555#section-7.3.4

Also send contact at register time if available (some CAs mandate this)

This increases line-count, while maintaining readability of the bare minimum code necessary to achieve eAB. Tested and verified with some CAs that mandate eAB. ( Zerossl, Buypass )

systemcrash avatar Sep 06 '21 21:09 systemcrash

@systemcrash can you please resolve the conflicts so tests can run? thanks!

diafygi avatar Sep 06 '21 21:09 diafygi

Sorry - pushed from an older branch copy 👍

systemcrash avatar Sep 06 '21 21:09 systemcrash

OK - rebased and updated.

systemcrash avatar Sep 06 '21 21:09 systemcrash

@systemcrash looks like some tests weren't successful

diafygi avatar Sep 06 '21 22:09 diafygi

AssertionError: False is not true for your

        self.assertTrue((  # can be in either order
            "Updated contact details:\nmailto:[email protected]\nmailto:[email protected]" in log_string.decode("utf8")
            or "Updated contact details:\nmailto:[email protected]\nmailto:[email protected]" in log_string.decode("utf8")
        ))

My code amends one thing there: if contact and code == 200: # 200 == already reg --> update because of your earlier test/assignment: reg_payload = {"termsOfServiceAgreed": True} if contact is None else {"termsOfServiceAgreed": True, "contact": contact}

Edit: there is not much point to update contact details if this is already (implicitly) done because contact was already provided (at reg). So the only interesting thing to do is to update the contact at this point, if it actually changed after initial registration (as I understand the process).

Edit 2: Summary: you can always provide contact, but the only time it is actually 'updated' (using this code) is when it has changed from the previous registration. So to test for this, something like this must happen: first, register, second, run again with different contact.

Edit 3: I did it this way so we save a round-trip, and only 'update' if something has changed: https://datatracker.ietf.org/doc/html/rfc8555#section-7.3.2

systemcrash avatar Sep 06 '21 22:09 systemcrash

A plausible fix for your test case is to add around L228 in test_module.py:

        result = acme_tiny.main([
            "--account-key", self.KEYS['account_key'].name,
            "--csr", self.KEYS['domain_csr'].name,
            "--acme-dir", self.tempdir,
            "--directory-url", self.DIR_URL,
            "--check-port", self.check_port,
            "--contact", "mailto:[email protected]",
        ])

systemcrash avatar Sep 06 '21 22:09 systemcrash

e.g. https://github.com/systemcrash/acme-tiny/commit/d05a2757523c27fe857c1e8acb98af6dc405a398

systemcrash avatar Sep 07 '21 00:09 systemcrash

@systemcrash It looks like this pull request puts acme-tiny over the 200 line limit, so I'm not going to merge it for now. However, I'll keep this pull request open if I decide to increase the line limit.

diafygi avatar Oct 26 '21 11:10 diafygi

I just wanted to express support for this pull request.

While I can respect the desire to keep acme-tiny short and auditable. I feel like there is merit to supporting other SSL providers that are not LetsEncrypt.

In the (extremely unlikely) event of LetsEncrypt ever suffers from a prolonged outage or became no longer trustworthy. The ability to immediately switch to a different issuer would be valuable to many sites worldwide.

Still, I expect if such an outage did come to pass this could be merged in a hurry.

anthonyryan1 avatar Jan 19 '22 16:01 anthonyryan1

Well, I use other ACME providers, so this PR is actual. But if we want to exclude the other 90% of providers out there for the sake of 10-15 lines of code, then sure.

systemcrash avatar Jan 20 '22 00:01 systemcrash

@diafygi I do appreciate the efforts to keep this library simple.

Althrought letsencrypt is a widely used provider and doesn't require EAB, but please consider that EAB is part of the ACME protocol, not a CA specific requirement.

Now, this is the only obstacle on migrating from certbot cli automation.

sanan-go avatar Mar 07 '22 17:03 sanan-go

I'd like to be another voice in support of merging this PR! :) We're using the DFN PKI (sorry, German only) who use EAB for 'sub-accounts', and acme-tiny would be a great fit here since it is easily auditable in comparison to the likes of Certbot and acme.sh.

runiq avatar May 09 '22 08:05 runiq

Also +1 for EAB from here... I also appreciate desire to keep thing small, or like with this tool keeping it specific to what its function is, but I also don't quite understand the desire for arbitrarily decided line count as alone it doesn't measure much anything ...

olmari avatar Nov 17 '22 18:11 olmari

210 or 220 doesn't sound as straight forward as 200 I guess. But the functionality this opens is pretty good. :)

systemcrash avatar Nov 17 '22 18:11 systemcrash

200 may look/sound more relatable due to common sense, but this is base10ish concept. let's go with 256, it is less arbitrary (for the audience) and still nice.

sanan-go avatar Nov 28 '22 16:11 sanan-go

Squeaked in eAB in under 200 lines. Skipped the _build_eab function and run everything on one line. Skip error checks for valid base64 eabHMAC. If you copy/pasted alright, there shouldn't be any problem.

systemcrash avatar Nov 29 '22 19:11 systemcrash

Why no merge? Is project dead?

Coachonko avatar Apr 07 '23 15:04 Coachonko

You're welcome to try the copy in my repo:eAB :)

systemcrash avatar Apr 14 '23 21:04 systemcrash

Please... merge this, so no need for manual tracking of many sources... :)

olmari avatar Dec 02 '23 01:12 olmari