Repository navigation
Conversation
FuzzySetMatch() handled every pattern notation and address family in one long function. It now determines the notation (prefix, CIDR or range) and calls one helper per notation and address family. ParseHostPort() now splits host and port first and determines the address type afterwards. Also removed the declaration of IsLoopbackAddress(), which was never defined, and made the parameter names in addr_lib.h match the definitions. No change in behaviour, except that FuzzySetMatch() returns -1 instead of the strncmp() result when an address prefix does not match. All callers only compare the result to 0. Ticket: ENT-14276 Signed-off-by: Victor Moene <victor.moene@northern.tech> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- IPv6 range patterns compared groups without a range as decimal numbers, so groups such as "fe80" failed to parse and stale values from the previous group were compared instead. For example, "fe80:1-3:0:0:0:0:0:1" matched "fe81:2:0:0:0:0:0:1". This affects admit_ips/deny_ips in cf-serverd. - IPv6 range patterns excluded the lower bound of a range. - IPv6 range patterns did not support "::", neither in the pattern nor in the address, so "2001:db8:1-5::1" never matched "2001:db8:3::1". - IPv4 range patterns reused values from the previous octet when an octet failed to parse. - The address in an IPv4 CIDR pattern was never validated, so an uninitialized address was compared, and e.g. "bogus/0" matched every address. - FuzzyMatchParse() (used by iprange() and isipinsubnet()) always returned true, since its check for an address stopped at the first '.' or ':'. It now validates addresses, CIDR masks (0-32 for IPv4, 0-128 for IPv6) and ranges, using the same parser as FuzzySetMatch(). - FuzzyHostMatch() (used by hostrange()) read before the start of its buffer when the host name was empty or only digits. - ParseHostPort() classified any host name starting with a digit as an IPv4 address, e.g. "1password.com". It now uses inet_pton(). Signed-off-by: Victor Moene <victor.moene@northern.tech> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There is nothing fuzzy about them. They now share a prefix with the policy functions they implement, iprange() and hostrange(): FuzzySetMatch() -> IPRangeMatch() FuzzyMatchParse() -> IPRangeIsValid() FuzzyHostMatch() -> HostRangeMatch() FuzzyHostParse() -> HostRangeIsValid() IPRangeMatch() and HostRangeMatch() now return true on match, like StringMatch(), instead of 0. Signed-off-by: Victor Moene <victor.moene@northern.tech> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
larsewi
requested changes
Nov 11, 2026
larsewi
left a comment
larsewi
left a comment
Contributor
There was a problem hiding this comment.
Please separate each bug fix into their own commits with a changelog.
Comment on lines
+48
to
+49
| const bool is_cidr = (strchr(pattern, '/') != NULL); | ||
| const bool is_range = (strchr(pattern, '-') != NULL); |
Contributor
There was a problem hiding this comment.
Since you've already touched all of this code, you can replace this and other strchr() with StringContainsChar(). Similar with strstr() and StringEqual() in a separate commit. If you want to.
| /* TODO rename to AddrSubnetMatch() */ | ||
| int FuzzySetMatch(const char *s1, const char *s2) | ||
| /** | ||
| * Determines the notation of an address pattern. |
Contributor
There was a problem hiding this comment.
Should mention what notation is, and what it means for notation to be true / false
| { | ||
| isCIDR = true; | ||
| } | ||
| *notation = is_cidr ? PATTERN_CIDR : (is_range ? PATTERN_RANGE : PATTERN_PREFIX); |
Contributor
There was a problem hiding this comment.
Maybe it would be easier to read if this was an actual if statement, rather than nesting two ternary operators?
| #include <string_lib.h> | ||
|
|
||
| #define CF_ADDRSIZE 128 | ||
| #define IPV4_OCTETS 4 |
Contributor
There was a problem hiding this comment.
Each of these bullets (in the commit message) deserve their own commit, and their own changelog entry.
| const char *ipaddr, const char *hostname, | ||
| const char *key, const char *username) | ||
| { | ||
| assert(acl != NULL); |
Contributor
There was a problem hiding this comment.
Maybe this assert should go into a separate commit. Because the commit message only mentions a rename if a function
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.