[ZBX-8914] Server incorrectly parses header, if TCP packet is fragmented within first 5 bytes Created: 2014 Oct 16  Updated: 2017 Dec 23  Resolved: 2016 Jan 13

Status: Closed
Project: ZABBIX BUGS AND ISSUES
Component/s: Proxy (P), Server (S)
Affects Version/s: 2.5.0
Fix Version/s: 3.0.0alpha6

Type: Incident report Priority: Minor
Reporter: Filipp Sudanov (Inactive) Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: protocols, proxy, server, tcp
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by ZBX-10154 sender.pl fragments header, so it's n... Closed

 Description   

When server / proxy receives data, it analyses first 5 bytes to see, if they are the protocol header.
But if data is received in two TCP fragments and the first one 4 bytes long, server does not wait to receive full header, but decides that it received data without header and stores "ZBXD" in the database.



 Comments   
Comment by Filipp Sudanov (Inactive) [ 2014 Oct 16 ]

How to replicate - put ZBXD to your clipboard, run

stty raw && nc 127.0.0.1 10051

and paste into the terminal. Server does not wait for subsequent bytes, but instantly closes the connection and writes " trapper got 'ZBXD' " to it's log.

Comment by Glebs Ivanovskis (Inactive) [ 2015 Dec 22 ]

(1) Old protocols are still supported but changes should be documented in Upgrade notes anyway.

I would like to document the differences between old and new zbx_tcp_recv_ext() in details at least here. With a ',' I will denote interruptions in data flow, with a '.' I will denote socket closing moment.

# Situation Input example Old zbx_tcp_recv_ext() New zbx_tcp_recv_ext()
1. Data with header, no interruptions in <HEADER> and <DATALEN> <HEADER>,<DATALEN>,<D,A,T,A> Reads the <DATA> if <DATALEN> does not exceed 128 MB, <DATA> can easily be shorter than <DATALEN> and sometimes can be slightly longer than <DATALEN> Same, but <DATA> length must correspond to <DATALEN>, fails otherwise
2. Data with header, interruption in <HEADER> <HEA,DER><DATALEN><DATA> <HEA is assumed to be plain text Reads <HEA, waits for DER> and proceeds with <DATALEN> and <DATA>
3. Data with header, interruption in <DATALEN> <HEADER><DATA,LEN><DATA> Fails Reads <HEADER>, reads <DATA, waits for LEN> and proceeds with <DATA>
4. Plain text, read until socket closed PLAIN,T,E,X,T. Reads PLAINTEXT until '.', size must not exceed 16 MB Same
5. Plain text, don't read until socket closed PLAIN,T,E,X,T. Reads PLAIN until first ',' Same
6. XML, no interruptions in opening tag <req>X,M,L,<,/,r,e,q,> Reads until finds </req> within last 10 characters Same
7. XML, with interruptions in opening tag <re,q>XML</req> <re is assumed to be plain text Reads <re, waits for q> and reads until finds </req> within last 10 characters
8. Plain text resembling <HEADER><DATALEN> or <req> beginning <HEA or <HEADER><DAT or <re Reads <HEA as plain text and returns immediately; fails on incomplete <DATALEN>, Reads <re as plain text and returns immediately Tries to wait for the missing <HEADER>, <DATALEN> or <req> part, potentially hangs until timeout if socket is not closed earlier
9. Data with header, then less than 8 bytes and socket closed <HEADER><DAT. Fail Assume this is plain text
10. Success   Returns the number of bytes read including <HEADER> but excluding <DATALEN> Returns the number of bytes read excluding <HEADER> and <DATALEN>
11. Fail   Returns FAIL Same
12. All data is available at once and fits into 2 KB buffer   3 calls to zbx_tls_read() 1 call to zbx_tls_read()

8. is a payback for 2. and 3. Any ideas on how to improve the situation are appreciated.

glebs.ivanovskis Subtle change of return value in 10. caused a regression ZBX-10333 because such return value was used to distinguish between valid empty responses from agent and agent not willing to communicate in case of passive checks and zabbix_get requests.

Revision 58162 fixes return value in the following fashion:

# Situation Input example Old zbx_tcp_recv_ext() New zbx_tcp_recv_ext()
10. Success   Returns the number of bytes read including <HEADER> but excluding <DATALEN> Returns the number of bytes read including both <HEADER> and <DATALEN>
Comment by Glebs Ivanovskis (Inactive) [ 2015 Dec 22 ]

Fix for trunk is available in development branch svn://svn.zabbix.com/branches/dev/ZBX-8914 revision 57293.

Comment by Andris Zeila [ 2015 Dec 22 ]

(2) The following code can cause alignment error on non intel CPUs:

expected_len = zbx_letoh_uint64(*(zbx_uint64_t *)(s->buf_stat + ZBX_TCP_HEADER_LEN));

glebs.ivanovskis Can this issue be solved in this way?

memcpy(&expected_len, s->buf_stat + ZBX_TCP_HEADER_LEN, sizeof(zbx_uint64_t));
expected_len = zbx_letoh_uint64(expected_len);

wiper yes

glebs.ivanovskis RESOLVED in r57339.

wiper CLOSED

Comment by Andris Zeila [ 2015 Dec 23 ]

(3) If the initial packet contains header + length, but is larger than expected length, then 'message is shorter than expected' warning is logged.

The simple fix would be just to change the message to something like 'received data length differs from expected data length'.

glebs.ivanovskis Improved error messages as we discussed with zalex_ua to provide more information.

I've spotted a bug when we receive a slow-coming plain text message and ZBX_TCP_READ_UNTIL_CLOSE flag is set we can stick with static buffer and wont be able to read the full message if it's longer. Also fixed.

RESOLVED

wiper CLOSED

Comment by Andris Zeila [ 2016 Jan 12 ]

Successfully tested, please check my changes regarding to unrelated compilation warnings in r57558

glebs.ivanovskis Thanks, looks good!

Comment by Glebs Ivanovskis (Inactive) [ 2016 Jan 13 ]

Fixed in pre-3.0.0alpha6 (trunk) r57583.

Generated at Tue Apr 16 08:05:36 EEST 2024 using Jira 9.12.4#9120004-sha1:625303b708afdb767e17cb2838290c41888e9ff0.