[patch] Stop to_syslog_ietf() from incorrectly escaping carriage return and newline characters

#1 ron-macneil-ice 7 years ago

Hi,

RFC5424 and all transports (except obsolete non-octet-counted TCP) can handle MSG containing ANY character including newlines and carriage returns.

In violation of the above, NxLog's to_syslog_ietf() function backslash-escapes these two characters. Furthermore, the escaping scheme is broken because it doesn't also escape the escape character itself (the backslash) so there's no way to reliably un-escape the MSG on the receiving end.

The correct behaviour is to stop escaping these characters altogether. In the rare case that someone needs to send multiline messages over non-octet-counted TCP, they can escape/unescape the $Message themselves using NxLog's replace() function.

Patch below.

RFC References: https://tools.ietf.org/html/rfc5424#section-6.4 https://tools.ietf.org/html/rfc6587#section-3.4

Regards, Ron MacNeil

--- src/modules/extension/syslog/syslog.c.orig 2014-07-19 23:52:06.000000000 +1000 +++ src/modules/extension/syslog/syslog.c 2016-07-26 14:01:57.296175500 +1000 @@ -1321,16 +1321,8 @@ nx_syslog_add_structured_data(logdata);

 // Append message

i = (int) logdata->raw_event->len; nx_string_append(logdata->raw_event, " ", 1); nx_string_append(logdata->raw_event, msg.string->buf, (int) msg.string->len);
for ( ; i < (int) logdata->raw_event->len; i++ )
{ // replace linebreaks with space

  if ( (logdata-&gt;raw_event-&gt;buf[i] == '\n') || (logdata-&gt;raw_event-&gt;buf[i] == '\r') )

```
  {
```

      logdata-&gt;raw_event-&gt;buf[i] = ' ';

```
  }
```
}

if (tmpmsg != NULL) { // clean up temp copy

Permalink

#2 adm Nxlog ✓ 7 years ago

#1 ron-macneil-ice

Hi, RFC5424 and all transports (except obsolete non-octet-counted TCP) can handle MSG containing ANY character including newlines and carriage returns. In violation of the above, NxLog's to_syslog_ietf() function backslash-escapes these two characters. Furthermore, the escaping scheme is broken because it doesn't also escape the escape character itself (the backslash) so there's no way to reliably un-escape the MSG on the receiving end. The correct behaviour is to stop escaping these characters altogether. In the rare case that someone needs to send multiline messages over non-octet-counted TCP, they can escape/unescape the $Message themselves using NxLog's replace() function. Patch below. RFC References: https://tools.ietf.org/html/rfc5424#section-6.4 https://tools.ietf.org/html/rfc6587#section-3.4 Regards, Ron MacNeil --- src/modules/extension/syslog/syslog.c.orig 2014-07-19 23:52:06.000000000 +1000 +++ src/modules/extension/syslog/syslog.c 2016-07-26 14:01:57.296175500 +1000 @@ -1321,16 +1321,8 @@ nx_syslog_add_structured_data(logdata); // Append message i = (int) logdata->raw_event->len; nx_string_append(logdata->raw_event, " ", 1); nx_string_append(logdata->raw_event, msg.string->buf, (int) msg.string->len); for ( ; i < (int) logdata->raw_event->len; i++ ) { // replace linebreaks with space if ( (logdata->raw_event->buf[i] == '\n') || (logdata->raw_event->buf[i] == '\r') ) { logdata->raw_event->buf[i] = ' '; } } if (tmpmsg != NULL) { // clean up temp copy

The code you removed with the patch does what it says in the comment (// replace linebreaks with space) and there is no escaping done there.

Note that this has been added because linebreaks cause issues with TCP/TLS transport when octet framing is not used. I know this too can be done with the replace() function but it seemed better to do this explicitly. Perhaps a RemoveLinebreaks configuration option could be added to xm_syslog to satisfy everyone.