syslog facility config should be set in config file

[zux]

the syslog facility config is set at compile time, it would be much easier if i could change it in config file.

thanks.

[Samuli Seppänen]

This sounds reasonable, if it would be easy to implement. Developers: any opinions on whether this could go to 2.4?

[Gert Döring]

If someone does a patch, I think 2.4 is perfectly fine.

For 2.3, I'm more sceptical - translating syslog strings to the syslog(3) facility values needs a translation table, lookup, some systems might not have all the facilities the rest has, so it's not a one-line change and needs cross-platform testing.

[tlhackque]

At least for POSIX systems, this is well-defined. But yes, it is not a 1-line change!

IEEE Std 1003.1-2001 (and goes back to Issue 4, version 2)

syslog.h must define:

LOG_KERN
    Reserved for message generated by the system.
LOG_USER
    Message generated by a process.
LOG_MAIL
    Reserved for message generated by mail system.
LOG_NEWS
    Reserved for message generated by news system.
LOG_UUCP
    Reserved for message generated by UUCP system.
LOG_DAEMON
    Reserved for message generated by system daemon.
LOG_AUTH
    Reserved for message generated by authorization daemon.
LOG_CRON
    Reserved for message generated by clock daemon.
LOG_LPR
    Reserved for message generated by printer system.
LOG_LOCAL0
    Reserved for local use.
LOG_LOCAL1
    Reserved for local use.
LOG_LOCAL2
    Reserved for local use.
LOG_LOCAL3
    Reserved for local use.
LOG_LOCAL4
    Reserved for local use.
LOG_LOCAL5
    Reserved for local use.
LOG_LOCAL6
    Reserved for local use.
LOG_LOCAL7
    Reserved for local use.

So, the following patch should be quite portable. However, it's only been tested on Debian Linux so far. Perhaps others can test on other platforms....

The patch allows a couple of other common facility codes if they are defined on the platform.

To specify a facility, simply add the desired facility name in [] anywhere in the name string of the --syslog or --daemon directive. This was chosen to avoid adding another option; it's backward compatible.

Also, it works with init scripts that use a config file name for the daemon name.

The default is whatever facility was used prior to the patch - e.g. 'daemon', but as before obeys LOG_OPENVPN if #defined.

An invalid facility name (e.g. []) will log all the facility names supported by the platform.

Examples:

--syslog []

List supported facility names (and will start)

--daemon staff

Log as program 'staff', facility (probably 'daemon')

--daemon [local1]staff

Log as program 'staff', facility 'local1'

--syslog [local1]

Log as PACKAGE_NAME (usually openvpn), facility 'local1'

Given /etc/openvpn/[authpriv]staff-vpn, Debian Linux will start OpenVPN with:
--daemon ovpn-[authpriv]staff-vpn

Log as ovpn-staff-vpn, facility 'authpriv'

Before someone asks, the SYSLOG_NAMES conditional in <sys/syslog.h> on many systems is non-standard and includes some facility codes that are not valid for OpenVPN.

Patch:

diff --git a/src/openvpn/error.c b/src/openvpn/error.c
index fd9f19d..284ee41 100644
--- a/src/openvpn/error.c
+++ b/src/openvpn/error.c
@@ -89,9 +89,10 @@ static bool machine_readable_output;   /* GLOBAL */
 /* Should timestamps be included on messages to stdout/stderr? */
 static bool suppress_timestamps; /* GLOBAL */

-/* The program name passed to syslog */
+/* The program name and facility passed to syslog */
 #if SYSLOG_CAPABILITY
 static char *pgmname_syslog;  /* GLOBAL */
+static int facility_syslog = LOG_OPENVPN;
 #endif

 /* If non-null, messages should be written here (used for debugging only) */
@@ -447,8 +448,89 @@ open_syslog (const char *pgmname, bool stdio_to_null)
     {
       if (!use_syslog)
        {
-         pgmname_syslog = string_alloc (pgmname ? pgmname : PACKAGE, NULL);
-         openlog (pgmname_syslog, LOG_PID, LOG_OPENVPN);
+         int facility = facility_syslog;
+         char *pname = pgmname_syslog = string_alloc (pgmname ? pgmname : PACKAGE, NULL);
+         char *facb, *face;
+
+         /* Decode optional [facilityname] prefix */
+
+         if ((facb = strchr (pgmname_syslog, '[')) != NULL
+             && (face = strchr (pgmname_syslog, ']')) != NULL) {
+           static struct {
+             int code;
+             const char *const name;
+           } facnames[] = {
+             { LOG_AUTH, "auth" },
+#ifdef LOG_AUTHPRIV  /* Prefered (private), but non-POSIX */
+             { LOG_AUTHPRIV, "authpriv" },
+#else
+             { LOG_AUTH, "authpriv" }, /* Map to non-secure code */
+#endif
+             { LOG_CRON, "cron" },
+             { LOG_DAEMON, "daemon" },
+#ifdef LOG_FTP
+             { LOG_FTP, "ftp" },
+#endif
+             { LOG_LPR, "lpr" },
+             { LOG_MAIL, "mail" },
+             { LOG_NEWS, "news" },
+#ifdef LOG_SYSLOG
+             { LOG_SYSLOG, "syslog" },
+#endif
+             { LOG_USER, "user" },
+#ifdef LOG_UUCP
+             { LOG_UUCP, "uucp" },
+#endif
+             { LOG_LOCAL0, "local0" },
+             { LOG_LOCAL1, "local1" },
+             { LOG_LOCAL2, "local2" },
+             { LOG_LOCAL3, "local3" },
+             { LOG_LOCAL4, "local4" },
+             { LOG_LOCAL5, "local5" },
+             { LOG_LOCAL6, "local6" },
+             { LOG_LOCAL7, "local7" },
+
+             { 0, NULL },
+           }, *fac = facnames;
+           char *facname = malloc (face - facb); /* [name] => name\0 */
+
+           /* Extract facility name and remove from option string */
+
+           *face++ = '\0';
+           strcpy (facname, facb+1);
+           memmove (facb, face, strlen(face)+1);
+
+           /* Default program name */
+
+           if (!*pname)
+             pname = PACKAGE_NAME;
+
+           /* Lookup facility code by name */
+
+           while (fac->name) {
+             if (!strcmp (fac->name, facname)) {
+               facility = fac->code;
+               break;
+             }
+             fac++;
+           }
+           if (!fac->name) {
+             /* Not found, look for name of facility that will be used */
+             for (fac = facnames; fac->name; fac++) {
+               if (fac->code == facility) {
+                 break;
+               }
+             }
+             /* Warn, and list valid names (they are platform-dependent) */
+             msg (M_WARN, "syslog: %s is not a valid facility name, using %s", facname,
+                  (fac->name? fac->name : "default"));
+             for (fac = facnames; fac->name; fac++) {
+               msg (M_INFO, "syslog: %s facility is valid on this platform", fac->name);
+             }
+           }
+           free (facname);
+         }
+         openlog (pname, LOG_PID, facility);
          use_syslog = true;

          /* Better idea: somehow pipe stdout/stderr output to msg() */
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 18cb354..4ccb138 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -336,6 +336,8 @@ static const char usage_message[] =
   "--daemon [name] : Become a daemon after initialization.\n"
   "                  The optional 'name' parameter will be passed\n"
   "                  as the program name to the system logger.\n"
+  "                  If the name includes [facility], the specified\n"
+  "                  syslog facility name will be used.\n"
   "--syslog [name] : Output to syslog, but do not become a daemon.\n"
   "                  See --daemon above for a description of the 'name' parm.\n"
   "--inetd [name] ['wait'|'nowait'] : Run as an inetd or xinetd server.\n"

[tlhackque]

Enhanced version of patch submitted to openvpn-devel.

Functionally the same, plus --syslog-facility cn be used as the more conventional way of specifying the desired facility.

Man page also updated in this rev, and some restructuring was done.

[Samuli Seppänen]

The patch on the ml seems to lack an ack or nack:

​http://thread.gmane.org/gmane.network.openvpn.devel/8690

What do we want to do with it?

[Gert Döring]

So the discussion went from "the current patch is not acceptable" to "how can it be improved" and then nothing ever happened again.

I am closing this ticket - there was not enough interest in the last 8 years to push this.