[ZBXNEXT-2160] Wildcard mask for network discovery Created: 2014 Feb 13 Updated: 2019 Mar 29 Resolved: 2015 Jan 20 |
|
Status: | Closed |
Project: | ZABBIX FEATURE REQUESTS |
Component/s: | API (A), Frontend (F), Proxy (P), Server (S) |
Affects Version/s: | None |
Fix Version/s: | 2.4.4, 2.5.0 |
Type: | New Feature Request | Priority: | Blocker |
Reporter: | Andrey Protsenko | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 3 |
Labels: | networkdiscovery | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified |
Issue Links: |
|
Description |
Issue: Autodiscovery is not have possibility to add all 10.100.NN.1 addresses to discovery rule at that moment. Another example: https://www.zabbix.com/forum/showthread.php?t=6791 I belive, cisco wildcard mask will be good solution for that. |
Comments |
Comment by Andris Zeila [ 2014 Oct 27 ] |
Specifications http://www.zabbix.org/wiki/Docs/specs/ZBXNEXT-2160 |
Comment by Andris Zeila [ 2014 Nov 12 ] |
Server part done in development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-2160 . |
Comment by Aleksandrs Saveljevs [ 2014 Nov 13 ] |
(1) [S] Functions iprangev4_parse() and iprangev6_parse() do not seem to validate the mask integer, so it is possible to write masks like "192.168.0.0/asdf". wiper while it does not have explicit check if the mask is given correctly, it would still fail 192.168.0.0/asdf as the mask would be 0. Though it would accept 192.168.0.0/16asdf range. wiper RESOLVED in r50633 asaveljevs I wonder whether is_uint31() function should be used instead of is_uint32(), considering that bits variable is of type "int", rather than "unsigned int". Also, while according to (5) we do not validate that the size of the range is 64K, we should probably validate that mask is within reasonable bounds (1 to 32 for IPv4, 1 to 128 for IPv6). If we validate that, then the added "0 <= i" check in iprange_apply_mask() does not seem to be necessary. REOPENED. wiper mask range is now limited to 0-32 (IPv4) and 0-128 (IPv6). asaveljevs There was a bug in a call to is_uint_n_range(), where a mask like "192.168.0.0/16999999" would be accepted. This is fixed in r50672. RESOLVED. wiper CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 14 ] |
(2) [S] Function iprangev6_parse() accesses a byte beyond the terminating NUL in the following conditional: check_fill: /* check if the next group is empty */ if (':' == ptr[1]) ... wiper RESOLVED in r50632 asaveljevs CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 14 ] |
(3) [S] Please review r50624. It mostly suggests stylistic fixes and minor improvements. wiper Thanks, CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 14 ] |
(4) [S] How about we remove ip4_str2dig(), ip6_str2dig(), and ip6_dig2str() functions? Those are no longer used. wiper RESOLVED in r50636 asaveljevs CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 14 ] |
(5) [S] Server does not seem to impose the 64K limit on the number of addresses specified using ranges. wiper yes, I understand that it should be done by frontend. But server does limit the bitmask to 16+ (112+) and I'm now wondering if it's right. wiper Iprange parsing functions should not check the range for any limits. However a new function to check the range size should be added and used in discoverer to validate the range before trying to iterate through it. wiper RESOLVED in r50653 asaveljevs A new constant was introduced: #define ZBX_DISCOVERER_IPRANGE_LIMIT (64 * ZBX_KIBIBYTE) It is then used like so: zabbix_log(LOG_LEVEL_WARNING, "discovery rule \"%s\": IP range exceeds %d KB address limit", drule->name, ZBX_DISCOVERER_IPRANGE_LIMIT / ZBX_KIBIBYTE); However, IP ranges are not exactly measured in kibibytes. There is also a suggestion to rename iprange_get_address_count() to iprange_volume(). Finally, iprange_volume() can overflow a 64-bit integer. Its return value should probably be limited by ZBX_MAX_UINT64 (a new constant). Whie at it, please review r50664. REOPENED. wiper RESOLVED in r50668 asaveljevs r50672 removes bytes completely from ZBX_DISCOVERER_IPRANGE_LIMIT. RESOLVED. wiper CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 19 ] |
(6) [S] Please review r50681 - it simplifies calls to is_uint_n_range() in IP range parse functions. If this looks good, we might also wish to simplify calls to is_uint_n_range() in include/common.h. wiper reviewed, simpliefied is_uint_* defines in common.h asaveljevs Removing "LL" from long constants gave warnings on 32-bit FreeBSD: integer constant is too large for 'long' type This is fixed in r50694. Please review. RESOLVED. asaveljevs Replaced "LL" suffix with __UINT64_C() macro in r50695, as per your suggestion. RESOLVED. wiper CLOSED |
Comment by Aleksandrs Saveljevs [ 2014 Nov 20 ] |
(7) [S] There was an infinite loop in iprange_apply_mask() due to "bits" variable being unsigned. This is fixed in r50707 wiper and r50710 (to get rid of compilation warning) CLOSED |
Comment by Andris Zeila [ 2014 Nov 20 ] |
(8) [F] Frontend also must support the new IP range specifications arvids.godjuks RESOLVED in r50776 iivs Provided better solution. RESOLVED in r51015, r51088, r51094 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Nov 21 ] |
(9) [PS] broadcast and network addresses should not be included in a range when we using IPv4 masks wiper RESOLVED in r50727 asaveljevs Looks good, but please see r50744. RESOLVED. wiper thanks! CLOSED |
Comment by Andris Zeila [ 2014 Nov 21 ] |
Server side ready for retesting |
Comment by Arvids Godjuks (Inactive) [ 2014 Nov 24 ] |
(10) Changes in translation sasha Translation string changes: Removed:
Added:
RESOLVED. iivs CLOSED. |
Comment by Alexei Vladishev [ 2014 Nov 25 ] |
(11) IPs like '/192.168.2.33', '192.168.2/10000.33', '192.168.2/.33' must not be valid arvids.godjuks RESOLVED in r50830 iivs CLOSED. |
Comment by Alexei Vladishev [ 2014 Nov 25 ] |
(12) Function isValidMask() always returns true, therefore it does not validate anything arvids.godjuks RESOLVED in r50830 iivs CLOSED. |
Comment by Alexei Vladishev [ 2014 Nov 25 ] |
(13) Function hasError() is (wrongly) used in isValidMask() but not used at all in isValidRange(). The function must be removed. arvids.godjuks RESOLVED in r50830 iivs CLOSED. |
Comment by Ivo Kurzemnieks [ 2014 Dec 04 ] |
(14) [F] Part of IP validation was in validate.inc.php, so I moved that functionality to CIPValidator class. (Notice IP is in upper case, since it is an abbrevation). Then I extended CIPRangeValidator class from CIPValidator to additionally handle IP ranges. I also fixed several problems, like in case when IP comma-separated list skipped validating IP addresses after fist comma, and possibility to pass other type of variables in CIPValidator and CIPRangeValidator validate methods. Before that even array could be passed causing SQL errors. And I also fixed several other coding style issues. RESOLVED in r51015, r51088 sasha CLOSED |
Comment by richlv [ 2014 Dec 08 ] |
subissues still open : |
Comment by Ivo Kurzemnieks [ 2014 Dec 09 ] |
(15) [F] Added frontend unit tests for IP and IP range validators. RESOLVED in r51088, r51094 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(16) CIPValidator class shouldn't support macros. It is not its task! iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(17) frontends/php/include/classes/validators/CIPValidator.php:101 - here is unnecessary with is_numeric() function call
if (!is_numeric($matches[$i]) || $matches[$i] > 255 || $matches[$i] < 0 ) {
iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(18) CIPValidator.php, CIPRangeValidator.php - $empty variable should be renamed. May be $allowEmpty? iivs Will be removed in (21). |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(19) isIPv4() and isIPv6() functions should be removed. If we will use these functions any string with dot or colon will be a valid IP address. iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(20) Please review my changes in r51128. sasha This changes will be removed under (21). CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(21) Unused property CIPRangeValidator.empty. CIPValidator.empty should be removed too. iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(22) All regular expressions should be unified. For example: '/^\d{2}$/' => '/^[0-9]{2}$/' iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 12 ] |
(23) isValidRange() should be divided into three functions: isValidRange(), isValidRangeIPv4() and isValidRangeIPv6() iivs RESOLVED in r51199 sasha CLOSED |
Comment by Alexander Vladishev [ 2014 Dec 18 ] |
(24) Please review my changes in r51241, r51249. iivs Looks good. Thanks for improving the code! sasha Thanks a lot! CLOSED |
Comment by Ivo Kurzemnieks [ 2014 Dec 19 ] |
TESTED, but don't forget to close |
Comment by Alexander Vladishev [ 2014 Dec 20 ] |
Available in pre-2.4.4 r51286 and pre-2.5.0 (trunk) r51287. |
Comment by Alexander Vladishev [ 2014 Dec 20 ] |
(25) Documentation changes:
martins-v Updated:
sasha Great! Also updated: Removed outdated note from:
martins-v Looking good. I'll set this as CLOSED. |
Comment by Aleksandrs Saveljevs [ 2015 Jan 19 ] |
(26) Windows agent does not compile: misc.o : error LNK2019: unresolved external symbol _iprange_validate referenced in function _ip_in_list misc.o : error LNK2019: unresolved external symbol _iprange_first referenced in function _ip_in_list misc.o : error LNK2019: unresolved external symbol _iprange_parse referenced in function _ip_in_list asaveljevs Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-2160 in r51693. RESOLVED. wiper CLOSED |
Comment by Aleksandrs Saveljevs [ 2015 Jan 19 ] |
Fix of (26) available in pre-2.4.4 r51695 and pre-2.5.0 (trunk) r51696. |