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

Domain set objects over 1024 bytes crash Dnsmasq #877

Closed
filippocarletti opened this issue Oct 30, 2024 · 8 comments
Closed

Domain set objects over 1024 bytes crash Dnsmasq #877

filippocarletti opened this issue Oct 30, 2024 · 8 comments
Labels
verified All test cases were verified successfully

Comments

@filippocarletti
Copy link
Member

filippocarletti commented Oct 30, 2024

If the total size of domain names exceeds 1024 bytes Dnsmasq refuses to start (syntax error).

Steps to reproduce

  • Create a Domain set object with many long names
  • Apply config

Expected behavior

Dnsmasq running

Actual behavior

Dnsmasq stopped, name resolution broken

See also

We found and fixed the problem in NethServer 7:
NethServer/nethserver-squid@931f391

@gsanchietti
Copy link
Member

Fix applied for NS7 is not applicable in this scenario, because rules can handle only one set.

I suggest to just add a validator inside the UI.

@filippocarletti
Copy link
Member Author

We need to split the single domain set into multiple sets and create a rule for every set.

@gsanchietti
Copy link
Member

We need to split the single domain set into multiple sets and create a rule for every set.

Splitting the rules means managing one rule from the UI that translates to multiple rules inside UCI: this is not possible by design.
Nor the object library nor the UI support managing multiple rules from a single entry.

@andre8244
Copy link
Collaborator

andre8244 commented Oct 31, 2024

Testing version

23.05.5-ns.1.3.0-31-g6a6ff7e

Test case

Verify the bug is no longer reproducible

@andre8244
Copy link
Collaborator

andre8244 commented Oct 31, 2024

After more thorough testing, it appears 1024 is too high as a maximum value, since additional characters will be included in /var/etc/dnsmasq.conf. Gonna submit a PR setting the maximum to 960 for safety.

@andre8244

This comment has been minimized.

@andre8244 andre8244 added the testing Packages are available from testing repositories label Oct 31, 2024
@andre8244 andre8244 removed their assignment Oct 31, 2024
@Tbaile Tbaile removed the bug label Nov 6, 2024
@francio87 francio87 self-assigned this Nov 7, 2024
andre8244 added a commit to NethServer/python3-nethsec that referenced this issue Nov 8, 2024
Ensure the total number of characters a domain set uses in the
dnsmasq configuration file does not exceed 1024 characters.

NethServer/nethsecurity#877
andre8244 added a commit to NethServer/nethsecurity-ui that referenced this issue Nov 8, 2024
The validation of the maximum characters used by a domain set in
the dnsmasq configuration file is now handled on the backend.

NethServer/nethsecurity#877
andre8244 added a commit to NethServer/python3-nethsec that referenced this issue Nov 8, 2024
Ensure the total number of characters a domain set uses in the
dnsmasq configuration file does not exceed 1024 characters.

NethServer/nethsecurity#877
andre8244 added a commit to NethServer/nethsecurity-ui that referenced this issue Nov 8, 2024
The validation of the maximum characters used by a domain set in
the dnsmasq configuration file is now handled on the backend.

NethServer/nethsecurity#877
@andre8244
Copy link
Collaborator

Updated testing version: 23.05.5-ns.1.3.0-31-g6a6ff7e

@gsanchietti gsanchietti added this to the NethSecurity 8.4 milestone Nov 12, 2024
@francio87
Copy link
Member

Confirm fixed ✅

Image

@francio87 francio87 removed their assignment Nov 13, 2024
@francio87 francio87 added verified All test cases were verified successfully and removed testing Packages are available from testing repositories labels Nov 13, 2024
@github-project-automation github-project-automation bot moved this from Verified to Done ✅ in NethSecurity Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verified All test cases were verified successfully
Projects
Archived in project
Development

No branches or pull requests

5 participants