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

feat: ✨ test for ACME file over SSL, + more logging #1342

Conversation

techieshark
Copy link

@techieshark techieshark commented Dec 23, 2021

Resolves #1340

Consider this a rough initial PR, ready for review/discussion. I'm happy to make changes as needed.

Things to consider:

  • is it really necessary to have the ssl config option (maybe we just recognize a potential 301 HTTP->HTTPS redirect and follow it?
  • I haven't renamed get_status function, but now that it returns more than just the HTTP status, perhaps I should

Example output

Below is an example where I've just changed the filename to force what happens when the file is inaccessible

TASK [letsencrypt : Test Acme Challenges] **************************************
System info:
  Ansible 2.10.11; Darwin
  Trellis 1.8.0: February 12th, 2021
---------------------------------------------------
Failed to test ACME file
failed: [example.com] (item=example.com) => {"ansible_loop_var": "item", "changed": false, "failed_hosts": [{"headers": [["Date", "Thu, 23 Dec 2021 22:57:40 GMT"], ["Connection", "keep-alive"], ["Cache-Control", "max-age=3600"], ["Expires", "Thu, 23 Dec 2021 23:57:40 GMT"], ["Location", "https://example.com/.well-known/acme-challenge/missing.txt"], ["Vary", "Accept-Encoding"], ["Server", "cloudflare"], ["CF-RAY", "6c2530b19d5e62d3-SYD"]], "hostname": "example.com", "reason": "Moved Permanently", "status": 301, "uri": "example.com:80/.well-known/acme-challenge/missing.txt"}, {"headers": [["Date", "Thu, 23 Dec 2021 22:57:40 GMT"], ["Content-Type", "text/html; charset=utf-8"], ["Connection", "keep-alive"], ["X-Content-Type-Options", "nosniff"], ["X-XSS-Protection", "1; mode=block"], ["Content-Security-Policy", "frame-ancestors 'self'"], ["X-Frame-Options", "SAMEORIGIN"], ["CF-Cache-Status", "DYNAMIC"], ["Expect-CT", "max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""], ["Server", "cloudflare"], ["CF-RAY", "6c2530b1c800559f-SYD"]], "hostname": "example.com", "reason": "Not Found", "status": 404, "uri": "example.com:443/.well-known/acme-challenge/missing.txt"}], "item": {"key": "example.com", "value": {"admin_email": "user@example.org", "branch": "main", "cache": {"enabled": false}, "deploy_keep_releases": 10, "local_path": "../site", "multisite": {"enabled": false}, "repo": "git@github.com:example/wp-bedrock.git", "site_hosts": [{"canonical": "example.com", "redirects": []}], "ssl": {"enabled": true, "provider": "letsencrypt"}}}, "rc": 1}
...ignoring

TASK [letsencrypt : Notify of challenge failures] ******************************
System info:
  Ansible 2.10.11; Darwin
  Trellis 1.8.0: February 12th, 2021
---------------------------------------------------
Could not access the challenge file for the hosts/domains:
example.com, example.com

Details:
301 example.com:80/.well-known/acme-challenge/missing.txt
404 example.com:443/.well-known/acme-challenge/missing.txt

Let's Encrypt requires every domain/host be publicly accessible.
Make sure that a valid DNS record exists for each host and that
they point to this server's IP.

If you don't want these domains in your SSL certificate, then
remove them from `site_hosts`. See https://roots.io/trellis/docs/ssl for more
details.
failed: [example.com] (item=example.com) => {"ansible_loop_var": "item", "changed": false, "item": {"ansible_loop_var": "item", "changed": false, "failed": true, "failed_hosts": [{"headers": [["Date", "Thu, 23 Dec 2021 22:57:40 GMT"], ["Connection", "keep-alive"], ["Cache-Control", "max-age=3600"], ["Expires", "Thu, 23 Dec 2021 23:57:40 GMT"], ["Location", "https://example.com/.well-known/acme-challenge/missing.txt"], ["Vary", "Accept-Encoding"], ["Server", "cloudflare"], ["CF-RAY", "6c2530b19d5e6243"]], "hostname": "example.com", "reason": "Moved Permanently", "status": 301, "uri": "example.com:80/.well-known/acme-challenge/missing.txt"}, {"headers": [["Date", "Thu, 23 Dec 2021 22:57:40 GMT"], ["Content-Type", "text/html; charset=utf-8"], ["Connection", "keep-alive"], ["X-Content-Type-Options", "nosniff"], ["X-XSS-Protection", "1; mode=block"], ["Content-Security-Policy", "frame-ancestors 'self'"], ["X-Frame-Options", "SAMEORIGIN"], ["CF-Cache-Status", "DYNAMIC"], ["Expect-CT", "max-age=604800, report-uri=\"https://report-uri.cloudflare.com/cdn-cgi/beacon/expect-ct\""], ["Server", "cloudflare"], ["CF-RAY", "6c2530b1c800559e"]], "hostname": "example.com", "reason": "Not Found", "status": 404, "uri": "example.com:443/.well-known/acme-challenge/missing.txt"}], "invocation": {"module_args": {"file": "ping.txt", "hosts": ["example.com"], "path": ".well-known/acme-challenge", "ssl": true}}, "item": {"key": "example.com", "value": {"admin_email": "user@example.org", "branch": "main", "cache": {"enabled": false}, "deploy_keep_releases": 10, "local_path": "../site", "multisite": {"enabled": false}, "repo": "git@github.com:example/wp-bedrock.git", "site_hosts": [{"canonical": "example.com", "redirects": []}], "ssl": {"enabled": true, "provider": "letsencrypt"}}}, "msg": "Failed to test ACME file", "rc": 1}}

Below is an example where example.com (some good host) has a typo like example.comuu (extra 'uu') so we see the "-1" status in the details and the "Name or service not known" in the JSON holding the full details.

TASK [letsencrypt : Test Acme Challenges] **************************************
System info:
  Ansible 2.10.11; Darwin
  Trellis 1.8.0: February 12th, 2021
---------------------------------------------------
Failed to test ACME file
failed: [example.com] (item=example.com) => {"ansible_loop_var": "item", "changed": false, "failed_hosts": [{"exception": "[Errno -2] Name or service not known", "headers": null, "hostname": "example.comuu", "reason": null, "status": -1, "uri": "example.comuu:80/.well-known/acme-challenge/ping.txt"}, {"exception": "[Errno -2] Name or service not known", "headers": null, "hostname": "example.comuu", "reason": null, "status": -1, "uri": "example.comuu:443/.well-known/acme-challenge/ping.txt"}], "item": {"key": "example.com", "value": {"admin_email": "user@example.org", "branch": "main", "cache": {"enabled": false}, "deploy_keep_releases": 10, "local_path": "../site", "multisite": {"enabled": false}, "repo": "git@github.com:example/wp-bedrock.git", "site_hosts": [{"canonical": "example.com", "redirects": []}], "ssl": {"enabled": true, "provider": "letsencrypt"}}}, "rc": 1}
...ignoring

TASK [letsencrypt : Notify of challenge failures] ******************************
System info:
  Ansible 2.10.11; Darwin
  Trellis 1.8.0: February 12th, 2021
---------------------------------------------------
Could not access the challenge file for the hosts/domains:
example.comuu, example.comuu

Details:
-1 example.comuu:80/.well-known/acme-challenge/ping.txt
-1 example.comuu:443/.well-known/acme-challenge/ping.txt

Let's Encrypt requires every domain/host be publicly accessible.
Make sure that a valid DNS record exists for each host and that
they point to this server's IP.

If you don't want these domains in your SSL certificate, then
remove them from `site_hosts`. See https://roots.io/trellis/docs/ssl for more
details.
failed: [example.com] (item=example.com) => {"ansible_loop_var": "item", "changed": false, "item": {"ansible_loop_var": "item", "changed": false, "failed": true, "failed_hosts": [{"exception": "[Errno -2] Name or service not known", "headers": null, "hostname": "example.comuu", "reason": null, "status": -1, "uri": "example.comuu:80/.well-known/acme-challenge/ping.txt"}, {"exception": "[Errno -2] Name or service not known", "headers": null, "hostname": "example.comuu", "reason": null, "status": -1, "uri": "example.comuu:443/.well-known/acme-challenge/ping.txt"}], "invocation": {"module_args": {"file": "ping.txt", "hosts": ["example.com"], "path": ".well-known/acme-challenge", "ssl": true}}, "item": {"key": "example.com", "value": {"admin_email": "user@example.org", "branch": "main", "cache": {"enabled": false}, "deploy_keep_releases": 10, "local_path": "../site", "multisite": {"enabled": false}, "repo": "git@github.com:example/wp-bedrock.git", "site_hosts": [{"canonical": "example.com", "redirects": []}], "ssl": {"enabled": true, "provider": "letsencrypt"}}}, "msg": "Failed to test ACME file", "rc": 1}}

@techieshark
Copy link
Author

I think we'd also need to modify this file:

to add

  listen [::]:443;
  listen 443;

@swalkinshaw
Copy link
Member

Looks good overall!

re: adding 443 to roles/letsencrypt/templates/nginx-challenge-site.conf.j2

🤔 would we need the ssl directive too? Or is just the listen port enough?

Comment on lines +112 to +119
if int(status) != 200 and bool(ssl) == True:
failed_hosts.append(result)
# Try again, this time over SSL
result = get_status(host, 443, path, file)
status = result['status']

if int(status) != 200:
failed_hosts.append(host)
failed_hosts.append(result)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads better I think?

if int(status) != 200:
  failed_hosts.append(result)

  if bool(ssl) == True:
    # Try again, this time over SSL
    result = get_status(host, 443, path, file)
    status = result['status']

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry just realized this comment had been pending for over a month and I forgot to submit 😓

@swalkinshaw
Copy link
Member

@techieshark are you still interested in finishing this? I can take it over if not.

@swalkinshaw
Copy link
Member

Closing this since it won't be compatible with #1310

@swalkinshaw swalkinshaw closed this Aug 4, 2022
@techieshark
Copy link
Author

Just finally getting back to some Trellis work… thanks @swalkinshaw for that readability suggestion; I've updated my local code with that and will use that internally until #1310 is released.

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

Successfully merging this pull request may close these issues.

Update test_challenges.py to allow testing ping.txt over SSL
2 participants