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

Force SSL Options Prevents Let's Encrypt Renewals #4275

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

MeCJay12
Copy link

@MeCJay12 MeCJay12 commented Jan 5, 2025

The force-ssl.conf file does the following:

  1. Checks if the request is HTTP and sets $test to H if so
  2. Checks if the request is to "test-challenge" in the normal Let's Encrypt challenge directory and appends T to $test if so
  3. If $test = H (and only H) upgrades the request to HTTPS

This works but only for "test-challenge". All other Let's Encrypt renewals/challenges will fall through and be upgraded to HTTPS which causes them to fail (not sure why, see thoughts below). By modifying step two to catch all Let's Encrypt challenges and not upgrade those connections, renewals succeed.

It looks like Let's Encrypt should work without the modification since letsencrypt-acme-challenge.conf is included in the server with both the listen 80 and listen 443 directives. Either way, this has never worked for me. I've already implemented this change in my environment by mounting the modified force-ssl.conf file directly and it fixed the issue immedeatly.

Modify Force SSL to bypass all Let's Encrypt requests
@jc21
Copy link
Member

jc21 commented Jan 6, 2025

This acme challenge thing has always been a thorn in my side.

  1. When a new host/certificate is being created, this should go to NPMs file structure
  2. The community wants to pass through the letsencrypt challenge to their upstream services that may also use it
  3. When the NPM renewal process is happening, the codebase doesn't know which host might get renewed so it doesn't attempt to hijack that path

Point 1 is fine and works as expected, point 2 also AFAIK (I don't do this myself). But your issue is with point 3. It's hard to respect point 2 at the same time.

The backend code fires off the certbot renewal process, so it could put a temporary hijack in place during that execution.

@MeCJay12
Copy link
Author

MeCJay12 commented Jan 7, 2025

This change should only affect Let's Encrypt flows by preventing them from being redirected to HTTPS. I don't think that this will impact anyone given that LE challenges on HTTP port 80 is standard. This should not impact the host routing of the requests; only the port & protocol.

That said, I do also have another fix for host routing of LE challenges to upstream hosts. Let me know if you are interested and I can open another pull request to discuss that in.

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.

2 participants