[ZBX-10820] Potential loss of data when server/proxy processes zabbix_sender data Created: 2016 May 18  Updated: 2024 Apr 10  Resolved: 2017 Sep 07

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: Proxy (P), Server (S)
Affects Version/s: 3.0.3rc1, 3.2.0alpha1
Fix Version/s: 3.0.11rc1, 3.2.8rc1, 3.4.2rc1, 4.0.0alpha1

Type: Problem report Priority: Minor
Reporter: Glebs Ivanovskis (Inactive) Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: codequality, loss, network, sender
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Causes
causes ZBX-15955 Agent allows requests from any hosts ... Closed
causes ZBX-15399 „::/0“ in allowed hosts for Zabbix Tr... Closed
Sub-task
depends on ZBX-4252 Agent sends AAAA queries when operati... Closed
Epic Link: DEV-591
Team: Team A
Sprint: Sprint 12, Sprint 13, Sprint 14, Sprint 15, Sprint 16
Story Points: 2

 Description   

Thanks to (86) of ZBXNEXT-1263 we now save peer hostname/IP in peer field of zbx_socket_t structure and reuse when necessary.

The only exception is zbx_tcp_check_security() calling getpeername() and inet_ntoa()/zbx_getnameinfo() independently. We should reuse peer at least and possibly think about storing getpeername() result in zbx_socket_t structure too on stage of zbx_tcp_accept().

Suppose zabbix_sender disconnects (due to timeout or network failure) right after server/proxy performs zbx_tcp_accept(). Server/proxy will be able to read sender data but will have to reject it because of security restriction since second call of getpeername() will fail.

Also peer in zbx_socket_t and host in zbx_get_ip_by_socket() use different in size static buffers.

 Comments   
Comment by Glebs Ivanovskis (Inactive) [ 2017 Jul 28 ]

I want to share with you a few issues I found in the code while I was digging this task.

Comment by Glebs Ivanovskis (Inactive) [ 2017 Jul 28 ]

(1) Function which gets peer IP may return "unknown IP". Do we need to accept connection in this case? Is it good from security perspective?

andris Thanks! The incoming connection will be dropped early if peer IP address is not available.

andris RESOLVED in dev branch svn://svn.zabbix.com/branches/dev/ZBX-10820-30 r70742.

s.paskevics CLOSED

Comment by Glebs Ivanovskis (Inactive) [ 2017 Jul 28 ]

(2) Few Simple check items (net.udp.service, net.udp.service.perf for sure, have not checked TCP equivalents) silently crop DNS/IP parameter to 63 characters. It is not ideal for a number of reasons:

  • functions downstream accept const char * which implies unlimited length, but then it is dumped into 128 character long peer field of zbx_socket_t;
  • no error message for user who uses DNS names longer than 63 characters (I know this violates RFCs, but there seem to be no limitations in /etc/hosts, at least on my system).

andris ZBXNEXT-1520 is somewhat related. Thanks for noticing it! RESOLVED in r70842.

s.paskevics CLOSED

Comment by Glebs Ivanovskis (Inactive) [ 2017 Jul 28 ]

(3) If DNS is used for connection, then gethostbyname() is used. As far as I understand, this call may result in querying remote DNS server. Zabbix does this call before it starts timeout countdown, which may lead to configured timeouts not being obeyed.

andris Same problem with getaddrinfo(). We cannot set custom timeout for every gethostbyname() call. One known solution is starting gethostbyname() in a separate thread and waiting for timeout in a parent thread. Another solution - using an advanced third-party DNS lookup library. A simple modification - including a time taken by gethostbyname() into checking against Zabbix configured timeout - will not shorten waiting time on gethostbyname() but make an item NOTSUPPORTED - seems not worth to implement.
WON'T FIX (see also ZBXNEXT-1002).

s.paskevics Looks good. Please check my minor changes in r71135,

andris r71135 reverted, a new change r71439 applied.

s.paskevics Looks good. CLOSED

Comment by Andris Mednis [ 2017 Aug 02 ]

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10820-30 .

Comment by Andris Mednis [ 2017 Aug 07 ]

Why "zabbix_log(LOG_LEVEL_WARNING, "Cannot get socket IP address: %s", error_message);" was deleted from zbx_socket_peer_ip_save() ?
Because zbx_socket_peer_ip_save() sets the same error message using zbx_set_socket_strerror() and functions calling zbx_tcp_accept() do their own error logging and will log this error (except if EINTR occurred - then nothing will be logged).

Comment by Sergejs Paskevics [ 2017 Aug 21 ]

Successfully tested - branch svn://svn.zabbix.com/branches/dev/ZBX-10820-30.

Comment by Andris Mednis [ 2017 Aug 23 ]

Available in versions:

  • pre-3.0.11rc1 r71535
  • pre-3.2.8rc1 r71538

Merge into 3.4/trunk delayed because of conflicts.
Working on separate dev branch svn://svn.zabbix.com/branches/dev/ZBX-10820-34 .

Comment by Andris Mednis [ 2017 Sep 04 ]

Fixed in development branch svn://svn.zabbix.com/branches/dev/ZBX-10820-34 (for 3.4).
It also solves ZBX-4252.

Comment by Viktors Tjarve [ 2017 Sep 06 ]

Successfully tested (for 3.4).

Comment by Andris Mednis [ 2017 Sep 06 ]

Available in versions:

  • pre-3.4.2rc1 r72311
  • pre-4.0.0alpha1 r72312
Comment by Andris Mednis [ 2017 Sep 07 ]

No documentation changes required.

Generated at Wed Apr 24 14:39:34 EEST 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.