Skip to content

IPFilterRules: add anylocal and multicast keywords, fail on unparseable rules - #2119

Open
abhinav-phi wants to merge 1 commit into
apache:mainfrom
abhinav-phi:issue-2086-ipfilter-linklocal
Open

IPFilterRules: add anylocal and multicast keywords, fail on unparseable rules#2119
abhinav-phi wants to merge 1 commit into
apache:mainfrom
abhinav-phi:issue-2086-ipfilter-linklocal

Conversation

@abhinav-phi

Copy link
Copy Markdown
Contributor

Fixes #2086 (the part not already covered by #2077).

A rule which is neither a keyword, a CIDR block nor a single IP address was logged and dropped, silently widening what the crawler will connect to. Configuration now throws an IllegalArgumentException instead, so a typo in an exclude list stops the topology rather than removing a rule the operator believed was in force.

  • adds the anylocal and multicast keywords
  • documents linklocal in the class javadoc (it was added by Only follow robots.txt redirects on the same host #2077 but missing from the docs)
  • updates the commented example in crawler-default.yaml to also cover CGNAT and IPv6 unique-local space
  • invalidRuleIsIgnored is replaced by invalidRuleFailsConfiguration, which asserts the throw

…le rules (apache#2086)

A rule that is neither a keyword, a CIDR block nor a single address was
logged and dropped, silently widening what the crawler will connect to.
Configuration now fails with an IllegalArgumentException instead, so a
typo in an exclude list stops the topology rather than removing a rule
the operator believed was in force.

Adds the anylocal and multicast keywords, documents linklocal (already
present but missing from the javadoc), and updates the example in
crawler-default.yaml to also cover CGNAT and IPv6 unique-local space.

@rzo1 rzo1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Currently I am short on time, so first round AI-based review)

Approve, with two comments.

parseIPRules is reached only from HttpProtocol.configure (line 239), so throwing aborts topology start rather than a fetch. That is the behaviour the new javadoc claims.

Please state in the PR description that an existing deployment with a typo'd rule will now fail to start. That is the intent, but operators need to read it before upgrading.

Note this overlaps with #2127, which sets http.filter.ipaddress.exclude for real. Worth agreeing which PR owns the shipped value so the two do not conflict on merge.

# allowed), addresses matching an exclude rule are always blocked.
# http.filter.ipaddress.include:
# http.filter.ipaddress.exclude: "localhost,sitelocal,linklocal"
# http.filter.ipaddress.exclude: "localhost,sitelocal,linklocal,100.64.0.0/10,fd00::/8"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The doc block above now advertises anylocal and multicast, but the example does not use them. fd00::/8 also only covers the locally-assigned half of ULA, while the comment above says "IPv6 unique-local", which is fc00::/7.

fc00::/8 is unassigned in practice, so this is completeness rather than an open hole.

Suggested change
# http.filter.ipaddress.exclude: "localhost,sitelocal,linklocal,100.64.0.0/10,fd00::/8"
# http.filter.ipaddress.exclude: "localhost,sitelocal,linklocal,anylocal,multicast,100.64.0.0/10,fc00::/7"

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.

IPFilterRules has no keyword for link-local addresses and the documented example leaves them allowed

2 participants