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.

Sign up for GitHub

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

Jump to bottom

fix(dns): should reject with nx instead of 0.0.0.0 #141

Merged
merged 3 commits into from
Jul 16, 2023
Merged

fix(dns): should reject with nx instead of 0.0.0.0 #141

merged 3 commits into from
Jul 16, 2023

Conversation

mzz2017
Copy link
Contributor

@mzz2017 mzz2017 commented Jun 11, 2023 •
edited by miooochi

Background

The DNS rejection procedure now (reject with 0.0.0.0) causes some problems #136; to avoid DDoS to local services like nginx listening at port 80.

Checklist

  • The Pull Request has been fully tested

Full changelog

  • Reject DNS look-up with NX instead of 0.0.0.0.
  • Do not cache DNS records if rejecting.

Issue reference

Fix #136
Related #63

@miooochi
Copy link
Contributor

This feature is NOT quite stable - I've tested it for 2 days.

@mzz2017
Copy link
Contributor Author

mzz2017 commented Jun 15, 2023

This feature is NOT quite stable - I've tested it for 2 days.

Yes. It is known.

@mzz2017 mzz2017 marked this pull request as draft June 15, 2023 14:40
@mzz2017 mzz2017 changed the title fix(dns): should reject with nx instead of 0.0.0.0 and fix response reject fix(dns): should reject with nx instead of 0.0.0.0 Jul 14, 2023
@mzz2017 mzz2017 marked this pull request as ready for review July 14, 2023 15:48
@mzz2017 mzz2017 changed the title fix(dns): should reject with nx instead of 0.0.0.0 fix(dns): should reject with nx instead of 0.0.0.0 and fix resp reject Jul 14, 2023
@mzz2017 mzz2017 added the fix label Jul 14, 2023
@mzz2017 mzz2017 changed the title fix(dns): should reject with nx instead of 0.0.0.0 and fix resp reject fix(dns): should reject with nx instead of 0.0.0.0 Jul 14, 2023
dae-prow[bot]
dae-prow bot approved these changes Jul 15, 2023
View reviewed changes
Copy link
Contributor

@dae-prow dae-prow bot left a comment

Choose a reason for hiding this comment

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

🧪 Since the PR has been fully tested, please consider merging it.

@miooochi
Copy link
Contributor

miooochi commented Jul 15, 2023 •
edited

I've been testing it in the past 12 hours, and no obvious bugs are detected. Let's consider closing and merging it. Thanks for your efforts! 🤛🏻

@miooochi miooochi requested a review from a team July 15, 2023 14:45
@kunish
Copy link
Member

kunish commented Jul 15, 2023

@mzz2017 Good to go?

miooochi
miooochi approved these changes Jul 15, 2023
View reviewed changes
Copy link
Contributor

@miooochi miooochi left a comment

Choose a reason for hiding this comment

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

LGTM. Please merge it when you feel it is ready.

@mzz2017 mzz2017 merged commit 00b39df into main Jul 16, 2023
17 checks passed
@mzz2017 mzz2017 deleted the fix_dns_nx branch July 16, 2023 03:28
This was referenced Jul 16, 2023
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kunish kunish kunish approved these changes

@miooochi miooochi miooochi approved these changes

@dae-prow dae-prow[bot] dae-prow approved these changes

Assignees

@mzz2017 mzz2017

Labels
fix priority/low tested
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ipversion_prefer might cause failures to access website the first time
3 participants
@mzz2017 @miooochi @kunish