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

Improve performance for ACL #110

Closed
wants to merge 1 commit into from
Closed

Conversation

Mygod
Copy link
Contributor

@Mygod Mygod commented Jan 17, 2024

1. What does this change do, exactly?

Skip DNS lookup if there are no IP ACL rules. This also in some sense mitigates DoS attacks by flooding servers with a lot of denied hostnames, which could lead to overloading the DNS services.

Requesting an invalid disallowed hostname will also now return 403 correctly instead of 502.

2. Please link to the relevant issues.

N/A

3. Which documentation changes (if any) need to be made because of this PR?

None.

4. Checklist

  • I have written tests and verified that they fail without my change
  • I made pull request as minimal and simple as possible. If change is not small or additional dependencies are required, I opened an issue to propose and discuss the design first
  • I have squashed any insignificant commits
  • This change has comments for package types, values, functions, and non-obvious lines of code

This also in some sense mitigates DoS attacks by flooding servers with a lot of denied hostnames, which could lead to overloading the DNS services.
@Mygod Mygod changed the title Improve performance for host-only ACL Improve performance for ACL Jan 17, 2024
@gaby
Copy link
Collaborator

gaby commented Jan 17, 2024

@Mygod Thanks for the PR! Can you add a unit-test for this?

@Mygod
Copy link
Contributor Author

Mygod commented Jan 17, 2024

I'm running it in production and it works fine. :)

@gaby
Copy link
Collaborator

gaby commented Jan 18, 2024

I'm running it in production and it works fine. :)

We still need a unit-test for it :-)

@mholt mholt deleted the branch caddyserver:caddy2 February 12, 2024 14:23
@mholt mholt closed this Feb 12, 2024
@Mygod
Copy link
Contributor Author

Mygod commented Feb 12, 2024

I wonder how to rebase on master...

@gaby
Copy link
Collaborator

gaby commented Feb 12, 2024

@Mygod Create a new PR with the new branch

@Mygod Mygod mentioned this pull request Feb 12, 2024
4 tasks
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.

3 participants