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

acme module: support client.answer_challenge for TLS-ALPN-01 #6676

Closed
cpu opened this issue Jan 18, 2019 · 12 comments
Closed

acme module: support client.answer_challenge for TLS-ALPN-01 #6676

cpu opened this issue Jan 18, 2019 · 12 comments

Comments

@cpu
Copy link
Contributor

cpu commented Jan 18, 2019

After #6144 landed Boulder had to switch away from using an up to date Certbot clone for our integration tests.

We don't rely on Certbot's acme module to create or serve TLS-ALPN-01 challenge response certificates (we use pebble-challtestsrv for this), but we do rely on the acme module for initiating challenges. E.g. in do_tls_alpn_challenges in our chisel.py test client we call:

 client.answer_challenge(c, c.response(client.key))

where c is a acme.ChallengeBody with type TLSALPN01.

If we remove this commit pin then answer_challenge begins to fail:

Traceback (most recent call last):
  File "test/integration-test.py", line 888, in <module>
    main()
  File "test/integration-test.py", line 821, in main
    run_chisel(args.test_case_filter)
  File "test/integration-test.py", line 845, in run_chisel
    value()
  File "test/integration-test.py", line 328, in test_tls_alpn_challenge
    auth_and_issue(domains, chall_type="tls-alpn-01")
  File "/go/src/github.com/letsencrypt/boulder/test/chisel.py", line 224, in auth_and_issue
    cleanup = do_tlsalpn_challenges(client, authzs)
  File "/go/src/github.com/letsencrypt/boulder/test/chisel.py", line 198, in do_tlsalpn_challenges
    client.answer_challenge(c, c.response(client.key))
  File "/certbot/acme/acme/challenges.py", line 179, in response
    key_authorization=self.key_authorization(account_key))
TypeError: 'NotImplementedType' object is not callable

We could potentially work around this by figuring out the right way to use the acme module to prepare a signed JWS body using the account key that we can POST to the challenge URI ourselves but I think that's a little yucky: we'd be duplicating a lot of the acme module's existing code. It also means other projects that use the acme module may have to do the same in the future.

With all of the specified ACME challenge types that exist today the client can POST a JWS with the trivial {} payload to initiate the challenge without needing to know anything about the actual process of solving the challenge. E.g. it can be totally challenge type agnostic.

Would the Certbot team be willing to add support for initiating a TLS-ALPN-01 challenge?

Thanks!

@ohemorange
Copy link
Contributor

@bmw why did we revert this again? Something about OpenSSL versions?

@bmw
Copy link
Member

bmw commented Jan 23, 2019

Yeah. We reverted our TLS-ALPN support for the reasons described in #6100 (comment).

I personally think we should revisit that decision, but regardless of that, I think we can add support the minor additional support the boulder team wants here.

Keeping with our current patterns to make the code work it should be as simple as:

  1. Adding a TLSALPN01Response class that is properly registered with the right parent class (see the other *Response classes in the file).
  2. Add a response_cls = TLSALPN01Response class variable to the class definition of TLSALPN01.

@cpu
Copy link
Contributor Author

cpu commented Jan 23, 2019

@bmw I can probably write a PR based on your description if it would be helpful and you folks can help knock the dust off of my Python knowledge.

@bmw
Copy link
Member

bmw commented Jan 23, 2019

@cpu, that'd be great!

@cpu
Copy link
Contributor Author

cpu commented Jan 23, 2019

I put a PR up with a potential fix based on @bmw's suggestions: #6689 I primarily modelled it off of the DNS01Response implementation.

@schoen schoen added the has pr label Jan 28, 2019
@jurodr
Copy link

jurodr commented Jan 29, 2019

Hi all!

I believe I faced a situation that relates to this issue in some level.

Trying to run my client (that uses the acme module) against Pebble I got an error for sending the keyAuthorization. (Relates to #5686)

I understand that makes sense since the RFC section 7.5.1 states the following:

"The client indicates to the server it is ready for the challenge
validation by sending an empty JSON body ("{}"), carried in a POST
request to the challenge URL (not authorization URL)."

Please correct me if I'm wrong, but I believe the acme module does not support this part of the spec.

Trying to work around it I tried using an empty JSONObjectWithFields, but the signature method _wrap_in_jws evaluates this to false and generates an empty payload (not what should be, an empty JSON body as payload), which make the request looking like a post-as-get and doesn't trigger the CA to validate the challenge (at least with pebble).

Another option is to use a fresh HTTP01Response, then keyAuthorization will be null, but the payload will look like:

{
"resource": "challenge",
"keyAuthorization": null,
"type": "http-01"
}

Which makes pebble happy but it is still not conforming with spec.

Shouldn't the answer_challenge NOT expect a ChallengeResponse for ACMEv2? Which means that no knowledge of what type of challenge is necessary.

Please let me know if I misunderstood something.

@adferrand
Copy link
Collaborator

adferrand commented Jan 29, 2019

You are right, I encountered the same problem with my work on integration tests. acme module uses an approach that is functional, but deprecated: passes keyAuthorization to initiate the challenge is not necessary, and thus has been deprecated server-side.

By default from the docker-compose.yml that is in the Pebble git repository, Pebble will start in strict mode, because the flag -strict is set in the start command. In this case Pebble follows strictly the current ACME spec and so fails if the client is not completely compliant.

Remove the -strict flag to make Pebble happy with current implementation of ACME protocol in certbot acme module.

We should certainly do something about this, first by adding the capability to send an empty Json as a payload, that is different from an empty payload as you said, and implement that in the challenge negotiation process.

@cpu
Copy link
Contributor Author

cpu commented Jan 29, 2019

I think the concern about sending the deprecated key authorization when POSTing a challenge is valid but separate from this issue (specifically about implementing challenge POSTs for TLS-ALPN-01)

@bmw
Copy link
Member

bmw commented Jan 29, 2019

I agree that this is a separate issue.

Thanks for reporting it though @jrodcla! If you're interested, I would love to see an issue/PR created for the problem you describe.

@nngo
Copy link

nngo commented Jan 30, 2019

Is there a timeline to add support to Certbot for certificate renews using TLS-ALPN-01? I am using the nginx plugin.

I am getting the following trying to upgrade to the more secure TLS-ALPN-01 challenge (vs HTTP-01)

certbot --preferred-challenges tls-alpn-01
Saving debug log to /var/log/letsencrypt/letsencrypt.log
Plugins selected: Authenticator nginx, Installer nginx

Which names would you like to activate HTTPS for?
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
1: xxx.yyy.com
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Select the appropriate numbers separated by commas and/or spaces, or leave input
blank to select all options shown (Enter 'c' to cancel): 1
Cert not yet due for renewal

You have an existing certificate that has exactly the same domains or certificate name you requested and isn't close to expiry.
(ref: /usr/local/etc/letsencrypt/renewal/xxx.yyy.com.conf)

What would you like to do?
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
1: Attempt to reinstall this existing certificate
2: Renew & replace the cert (limit ~5 per 7 days)
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Select the appropriate number [1-2] then [enter] (press 'c' to cancel): 2
Renewing an existing certificate
Performing the following challenges:
None of the preferred challenges are supported by the selected plugin

@bmw
Copy link
Member

bmw commented Jan 30, 2019

@nngo, the issue discussed here is separate from the one you're describing. If you'd like to discuss the possibility of getting TLS-ALPN-01 support getting added to Certbot, please open a new issue.

@enoch85
Copy link

enoch85 commented Jan 30, 2019

please open a new issue.

#6724

@bmw bmw closed this as completed in #6689 Feb 4, 2019
@bmw bmw added this to the 0.31.0 milestone Feb 4, 2019
bmw pushed a commit that referenced this issue Feb 4, 2019
The existing `acme.TLSALPN01` challenge class did not have
a `response_cls`, meaning it was not possible to use a tls-alpn-01
challenge with `client.answer_challenge`.

To support the above a simple `TLSALPN01Response` class is added that
doesn't provide the ability to solve a tls-alpn-01 challenge end to end
(e.g. generating and serving the correct response certificate) but that
does allow the challenge to be initiated. This is sufficient for users
that have set up the challenge response independent of the `acme`
module code.

Resolves #6676
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

8 participants