[Bug 238] New: syslog-protocol: allow NILVALUE for TIMESTAMP as specified in rfc5424
https://bugzilla.balabit.com/show_bug.cgi?id=238 Summary: syslog-protocol: allow NILVALUE for TIMESTAMP as specified in rfc5424 Product: syslog-ng Version: 3.4.x Platform: PC OS/Version: Linux Status: NEW Severity: normal Priority: unspecified Component: syslog-ng AssignedTo: bazsi@balabit.hu ReportedBy: spam@lerya.net Type of the Report: --- Estimated Hours: 0.0 RFC5424 states: - section 6: """ TIMESTAMP = NILVALUE / FULL-DATE "T" FULL-TIME """ - section 6.2.3 "" A syslog application MUST use the NILVALUE as TIMESTAMP if the syslog application is incapable of obtaining system time. "" But the 3.4 version of syslog-ng does not allow such a NILVALUE: https://github.com/balabit/syslog-ng-3.4/blob/master/modules/syslogformat/sy... For example, the following message does not work: <6>1 - - netlog - - - [ 775.895618] /bin/ping[2936] UDP 10.0.2.15:33285 -> 192.168.0.1:53 (uid=0) Whereas this one works: <6>1 2003-10-11T22:14:15.003Z - netlog - - - [ 775.895618] /bin/ping[2936] UDP 10.0.2.15:33285 -> 192.168.0.1:53 (uid=0) -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |algernon@balabit.hu --- Comment #1 from Balazs Scheidler <bazsi@balabit.hu> 2013-06-29 12:26:04 --- I have fixed this with this patch: commit fd6e77f64cc510b6b45497fca6b53eb8070f90f6 Author: Balazs Scheidler <bazsi@balabit.hu> Date: Sat Jun 29 12:24:36 2013 +0200 syslog-format: accept RFC5424 NILVALUE in timestamps RFC5424 states: In section 6: TIMESTAMP = NILVALUE / FULL-DATE "T" FULL-TIME In section 6.2.3 A syslog application MUST use the NILVALUE as TIMESTAMP if the syslog application is incapable of obtaining system time. This patch changes syslog-ng to conform to this requirement as it reported a parse error instead. Reported-By: Vincent Brillault <spam@lerya.net> Signed-off-by: Balazs Scheidler <bazsi@balabit.hu> It is available on the hotfix/3.4/syslogparse-nilvalue-in-dates branch in the syslog-ng 3.4 repository on github. @Algernon: can you please merge it to master? Thanks. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 --- Comment #2 from Vincent Brillault <spam@lerya.net> 2013-06-29 12:31:29 --- Created an attachment (id=79) --> (https://bugzilla.balabit.com/attachment.cgi?id=79) Use current timestamp if unspecified Here is a small patch for 3.2 (on gentoo, the one I'm currently using). From what I've seen in the code, porting it into other versions should be simple. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Vincent Brillault <spam@lerya.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79 is|0 |1 obsolete| | --- Comment #3 from Vincent Brillault <spam@lerya.net> 2013-06-29 12:35:04 --- (From update of attachment 79) Oh, sorry, I did not see you patch. It's perfect, thanks :) -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 --- Comment #4 from Balazs Scheidler <bazsi@balabit.hu> 2013-06-30 08:42:39 --- this is called a race condition :-) I didn't know you were submitting a patch and I just started hacking on syslog-ng when your email arrived. what I didn't do but would be great if you could work on is a unit teat for that enormous parse_date function. perhaps even split it to smaller chunks. it is indirectly tested in test_msgparse but that test program is again far from ideal. I'm trying to move toward more unit testing coverage to make it easier to develop stuff faster, you could give me a hand in that, if you feel like it. thanks in advance. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 --- Comment #5 from Vincent Brillault <spam@lerya.net> 2013-07-01 22:12:44 --- I currently do not have the time to write such unit tests, sorry (And I lack knowledge on correct time formatting). After re-reading the code, one thing is different between your patch and mine: you do not return immediately. As a result, the following block is called ( https://github.com/balabit/syslog-ng-3.4/blob/master/modules/syslogformat/sy... ): ''' if (self->timestamps[LM_TS_STAMP].zone_offset == -1) { self->timestamps[LM_TS_STAMP].zone_offset = assume_timezone; } if (self->timestamps[LM_TS_STAMP].zone_offset == -1) { self->timestamps[LM_TS_STAMP].zone_offset = get_local_timezone_ofs(self->timestamps[LM_TS_STAMP].tv_sec); } self->timestamps[LM_TS_STAMP].tv_sec = self->timestamps[LM_TS_STAMP].tv_sec + get_local_timezone_ofs(self->timestamps[LM_TS_STAMP].tv_sec) - (tm.tm_hour - unnormalized_hour) * 3600 - self->timestamps[LM_TS_STAMP].zone_offset; ''' I do not see a similar pattern on the "error" path ( https://github.com/balabit/syslog-ng-3.4/blob/master/modules/syslogformat/sy... ): ''' if (G_UNLIKELY(!success)) { gchar buf[2048]; self->timestamps[LM_TS_STAMP] = self->timestamps[LM_TS_RECVD]; [... Nothing on timestamps afterwards ...] ''' Am I missing something/Is it intended ? -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|bazsi@balabit.hu |algernon@balabit.hu -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |3.4.3 Status|NEW |ASSIGNED -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution| |FIXED Status|ASSIGNED |RESOLVED --- Comment #6 from Gergely Nagy <algernon@balabit.hu> 2013-07-04 13:47:55 --- (In reply to comment #5)
I do not see a similar pattern on the "error" path ( https://github.com/balabit/syslog-ng-3.4/blob/master/modules/syslogformat/sy... ): ''' if (G_UNLIKELY(!success)) { gchar buf[2048];
self->timestamps[LM_TS_STAMP] = self->timestamps[LM_TS_RECVD]; [... Nothing on timestamps afterwards ...] '''
Am I missing something/Is it intended ?
This copies LM_TS_RECVD to LM_TS_STAMP, with everything included, zone offsets and all, so all the magic of figuring things out is not needed there, as LM_TS_RECVD should have it all figured out at this point. Also, since I merged the patch to master, and the error path looks correct too, I'm closing this issue. The fix will be part of syslog-ng 3.4.3, but it is also available on the master branch in git. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|FIXED | Status|RESOLVED |REOPENED --- Comment #7 from Gergely Nagy <algernon@balabit.hu> 2013-07-04 16:21:40 --- Reopening, because while the zone_offset mungling does not affect the NILVALUE case, the magic with tv_sec likely does. Is it ok to enter that codepath, or should it be skipped instead? I'd say skip it, but I'm not entirely familiar with the time code. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Balazs Scheidler <bazsi@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bazsi@balabit.hu --- Comment #8 from Balazs Scheidler <bazsi@balabit.hu> 2013-07-05 19:29:26 --- it should not be skipped. that code will not do anything in the current case as the two timezones are the same. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 --- Comment #9 from Vincent Brillault <spam@lerya.net> 2013-07-05 21:15:59 --- Are you sure that it will do nothing ? I have some problems with the last line (L438): ''' self->timestamps[LM_TS_STAMP].tv_sec = self->timestamps[LM_TS_STAMP].tv_sec + get_local_timezone_ofs(self->timestamps[LM_TS_STAMP].tv_sec) - (tm.tm_hour - unnormalized_hour) * 3600 - self->timestamps[LM_TS_STAMP].zone_offset; ''' In the '-' path, aren't tm and unnormalized_hour uninitialised ? -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 --- Comment #10 from Balazs Scheidler <bazsi@balabit.hu> 2013-07-05 21:31:58 --- you are right, and I'm embarrassed. that code really needs to be split up. I simply assummed and didn't actually check -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
https://bugzilla.balabit.com/show_bug.cgi?id=238 Gergely Nagy <algernon@balabit.hu> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution| |FIXED Status|REOPENED |RESOLVED --- Comment #11 from Gergely Nagy <algernon@balabit.hu> 2013-08-13 10:38:37 --- I just fixed the last bits on master, and will release 3.4.3 in a bit. -- Configure bugmail: https://bugzilla.balabit.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
bugzilla＠bugzilla.balabit.com