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

Properly refresh fqdn_state of DNS-based CWNPs #174

Merged
merged 1 commit into from
Jan 8, 2024

Conversation

mreiger
Copy link
Contributor

@mreiger mreiger commented Jan 3, 2024

No description provided.

@mreiger mreiger linked an issue Jan 3, 2024 that may be closed by this pull request
@mreiger mreiger marked this pull request as ready for review January 4, 2024 11:35
@mreiger mreiger requested a review from a team as a code owner January 4, 2024 11:35
Copy link
Contributor

@Gerrit91 Gerrit91 left a comment

Choose a reason for hiding this comment

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

Don't know the reasoning behind the original code but if this does everything we want, it's better than before. Do you have an idea why the status was passed originally?

@mreiger
Copy link
Contributor Author

mreiger commented Jan 5, 2024

I have no idea really why the old status was passed, since the function (re-)writes the status for all currently allowed destinations anyway. Preserving the old status gives you an insight to previously enabled destinations, but I think doing that in this way is more confusing that useful.

There is one edge case where you can still have an outdated state in fqdn_state, but I think it is probably not really relevant:

When you have a matchPattern statement without a matching entry in the DNS cache you get an empty statement like this:

    '*.f-i-ts.de': []

When you delete the statement from the rule, that empty entry remains - probably because there was never any iptables rule generated for that pattern and therefore the removal of the statement did not trigger a nftables rules change. It vanishes when there's another change to the cwnp that actually affects the nftables ruleset.

As long as there's a dns cache entry that matches the pattern removing the entry works as expected and does not leave behind a stale state.

@mreiger mreiger merged commit 9544b36 into master Jan 8, 2024
2 checks passed
@majst01 majst01 deleted the 173-stale-state-in-dns-based-cwnps branch January 8, 2024 09:42
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.

Stale state in DNS-based CWNPs
2 participants