[Proftpd-devel] [PATCH 1/1] mod_wrap: fix logging so that TCPAccessSyslogLevels works

[Philip Prindeville]

I'm seeing logging output on "allows" even though I've got:

<IfModule mod_wrap.c>
TCPAccessFiles /etc/ftpd.allow /etc/ftpd.deny
TCPAccessSyslogLevels debug warn
TCPServiceName ftpd
</IfModule mod_wrap.c>

configured. Logs look like:

May 5 22:26:47 mail proftpd[31230]: 192.168.1.3 (192.168.1.17[192.168.1.17]) - mod_wrap/1.2.3: allowed connection from macbook.redfish-solutions.com

this message shouldn't be present.

[TJ Saunders]

Post by Philip Prindeville
<IfModule mod_wrap.c>
TCPAccessFiles /etc/ftpd.allow /etc/ftpd.deny
TCPAccessSyslogLevels debug warn
TCPServiceName ftpd
</IfModule mod_wrap.c>
May 5 22:26:47 mail proftpd[31230]: 192.168.1.3 (192.168.1.17[192.168.1.17]) - mod_wrap/1.2.3: allowed connection from macbook.redfish-solutions.com
this message shouldn't be present.

I've opened:

http://bugs.proftpd.org/show_bug.cgi?id=3562

to track this.

I'm wondering about this part of your patch:

- allow_severity = log_getfacility() | allow_severity;
- deny_severity = log_getfacility() | deny_severity ;

Why is removing these lines necessary? And with your patch, is a
SyslogFacility directive also honored properly (a la Bug#3317)?

Thanks,
TJ

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

He that complies against his will
Is of his own opinion still.

-Samuel Butler

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[Philip Prindeville]

Post by TJ Saunders

Post by Philip Prindeville
<IfModule mod_wrap.c>
TCPAccessFiles /etc/ftpd.allow /etc/ftpd.deny
TCPAccessSyslogLevels debug warn
TCPServiceName ftpd
</IfModule mod_wrap.c>
May 5 22:26:47 mail proftpd[31230]: 192.168.1.3 (192.168.1.17[192.168.1.17]) - mod_wrap/1.2.3: allowed connection from macbook.redfish-solutions.com
this message shouldn't be present.

http://bugs.proftpd.org/show_bug.cgi?id=3562
to track this.
- allow_severity = log_getfacility() | allow_severity;
- deny_severity = log_getfacility() | deny_severity ;
Why is removing these lines necessary? And with your patch, is a
SyslogFacility directive also honored properly (a la Bug#3317)?
Thanks,
TJ

Let's walk through the calling sequence.

static void wrap_log_request_allowed(int priority,
struct request_info *request) {
int facility;

/* Mask off the facility bit. */
facility = log_getfacility();
priority &= ~facility;

pr_log_pri(priority, MOD_WRAP_VERSION ": allowed connection from %s",
eval_client(request));

/* done */
return;
}


Then we hit:

static int facility = LOG_DAEMON;
...

void pr_log_pri(int priority, const char *fmt, ...) {
...
log_write(priority, facility, buf);
}



So we've got two things going on here:

(1) We OR the facility into the priority argument, only to remove it later (which is redundant);
(2) When we do mask it out, we do it incorrectly... it should be "priority &= 0x07" instead.

essentially my patch skips these two self-canceling steps.

The bottom line is that all of the other modules call pr_log_pri() with just a priority (no facility) and they work just fine.

-Philip

[Philip Prindeville]

[snip]
(1) We OR the facility into the priority argument, only to remove it later (which is redundant);
(2) When we do mask it out, we do it incorrectly... it should be "priority &= 0x07" instead.
essentially my patch skips these two self-canceling steps.
The bottom line is that all of the other modules call pr_log_pri() with just a priority (no facility) and they work just fine.
-Philip

Ok, spoke too soon.

The steps aren't exactly "redundant" because allow_severity and deny_severity are also imported into libwrap.

So I've revised the patch:

We pass in either variable into the function as appropriate, in the variable "severity".

We compute the new variable "priority" as "severity & PR_LOG_PRIMASK". We then pass "priority" back to pr_log_pri() as it expects only the priority to be present.

I retested, and it works.

I attempted to verify that I had undone the regression of 3317, but couldn't get SyslogFacility to send to a different file with the following directive in /etc/rsyslog.conf:

local5.* /var/log/five

even after doing a "touch /var/log/five ; chmod 600 /var/log/five" and "service rsyslog restart"...

Not sure if that belies a compatibility issue with rsyslogd or not.

Hopefully someone using the syslogd "classic" can reproduce this test.

-Philip