Skip to contentSkip to conversation

Add support for specifying the syslog facility, requested in, trac #188. (Rev 2)

PATCH New Timothe Litt 1 participant 1 message May 2, 2014

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Add support for specifying the syslog facility, requested in, trac #188. (Rev 2)

Reflects review comments from David; doc changes, single patch, one name change, some clarification in the commit message. No functional changes.

Detailed response/rationale in previous email.

From 4af33b94ad4e2509fb9bc195eb50404c0d2b7581 Mon Sep 17 00:00:00 2001
From: Timothe Litt <[email protected]...>
Date: Thu, 1 May 2014 20:57:45 -0400
Subject: [PATCH] Add support for specifying the syslog facility, requested in trac #188.

Revision 2, reflects review comments from David.

Users want this because it allows them to have syslog put all OpenVPN messages in specific file(s) by using syslog.conf.

Adds --syslog-facility name option, which must precede --daemon and --syslog.

Adds ability to specify facility as [name] in --daemon and --syslog's progname argument, which makes it possible to specify the syslog facility name without modifying certain system init scripts. --syslog-facility takes precedence.

For example, Debian automatically generates a --daemon vpn-{configname} directive. If you name the config foo[local1].conf, it's as though you were able to specify --syslog-facility local1 --daemon vpn-foo --config foo[local1].conf. Absent this feature, this isn't possible without modifying the distribution-provided init script.

--syslog-facility list (or any other undefined name) will list the facilities that the platform supports.

The old method of -DLOG_OPENVPN still provides the facility used by default.

Thus, syslog facility can be specified in one of three ways:
--syslog-facility
--daemon or --syslog's progname argument with embedded [facility] -DLOG_OPENVPN

For a given configuration:
if --syslog-facility is present, its argument determines the facility. Otherwise:
if --daemon or --syslog has a progname argument that contains a
[facility] string,
the [facility] will be used.
Otherwise:
The compile-time default LOG_OPENVPN, usually 'daemon', is used.

This is forward and backward compatible with existing scripts & initfiles.

---
 doc/openvpn.8         |   17 +++++++
 src/openvpn/error.c   |  129 
+++++++++++++++++++++++++++++++++++++++++++++++--
 src/openvpn/error.h   |    1 +
 src/openvpn/options.c |   19 ++++++++
 4 files changed, 163 insertions(+), 3 deletions(-)
doc/openvpn.8 +6 -0
index 34894e5..e25a39c 100644
@@ -2169,6 +2169,12 @@ When unspecified,
 .B progname
 defaults to "openvpn".
 
+If --syslog-facility has not been specified and the name
+includes [facility], the specified syslog facility name will be used with
+the system logger.  This method of specifying the facility name may be
+useful when initialization scripts generate --daemon progname from
+the config file name.  This avoids the need to edit the script.
+
 When OpenVPN is run with the
 .B \-\-daemon
 option, it will try to delay daemonization until the majority of 

initialization
@@ -2188,6 +2194,17 @@ directive above for description of
.B progname
parameter.
.TP
+.B \-\-syslog-faciity name
+When logging to syslog, use name (e.g. local1 or daemon) for the facility. +If name is absent or not supported, a platform-dependent list of valid +facility names is provided.
+
+Must be specified before --daemon and --syslog.
+If this is not convenient, the facility name may be specified as [name] in +the name parameter of either directive. See --daemon.
+
+If this option is not specified, the default facility is usually 'daemon'. +.TP
.B \-\-errors-to-stderr
Output errors to stderr instead of stdout unless log output is
redirected by one of the
.B \-\-log

src/openvpn/error.c +2 -1
index fd9f19d..53048bf 100644
@@ -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 = -1; /* Unspec (RFC5424 - can not be negative) */
 #endif
 
 /* If non-null, messages should be written here (used for debugging 

only) */
@@ -439,6 +440,93 @@ out_of_memory (void)
exit (1);
}

+#if SYSLOG_CAPABILITY
+static int lookup_syslog_facility_code (const char *name, int default_code) +{
+ 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;
+
+ if (use_syslog)
+ {
+ msg (M_ERR, "syslog facility can not be changed after logging has started");
+ return;
+ }
+
+ /* Lookup facility code by name */
+
+ for (fac = facnames; fac->name; fac++)
+ {
+ if (!strcmp (fac->name, name))
+ {
+ return fac->code;
+ }
+ }
+
+ /* Not found, look for name of facility that will be used */ +
+ if (default_code == -1)
+ default_code = LOG_OPENVPN;
+
+ for (fac = facnames; fac->name; fac++)
+ {
+ if (fac->code == default_code)
+ {
+ break;
+ }
+ }
+
+ /* Warn, and list valid names (they are platform-dependent) */ +
+ msg (M_WARN, "syslog: %s is not a valid facility name, using %s", name,
+ (fac->name? fac->name : "default"));
+ for (fac = facnames; fac->name; fac++)
+ {
+ msg (M_INFO, "syslog: %s facility is valid on this platform", fac->name);
+ }
+ return default_code;
+}
+
+void set_syslog_facility (const char *name)
+{
+ facility_syslog = lookup_syslog_facility_code (name, -1); +}
+
+#endif
+
void
open_syslog (const char *pgmname, bool stdio_to_null)
{
@@ -447,8 +535,43 @@ 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);
+         char *pname = pgmname_syslog = string_alloc (pgmname ?

+ pgmname : PACKAGE, NULL);
+ int facility = facility_syslog;
+ if (facility == -1)
+ {
+ /* Unspecified, default */
+
+ facility = LOG_OPENVPN;
+
+ /* Attempt to extract from program name (simplifies init files) */
+ char *facb, *face;
+
+ /* Decode optional [facilityname] prefix */
+
+ if ((facb = strchr (pgmname_syslog, '[')) != NULL
+ && (face = strchr (facb+1, ']')) != NULL)
+ {
+ 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;
+
+ /* Convert extracted facility name to code */
+
+ facility = lookup_syslog_facility_code (facname, facility);
+ free (facname);
+ }
+ }

+         openlog (pname, LOG_PID, facility);
          use_syslog = true;

          /* Better idea: somehow pipe stdout/stderr output to msg() */
src/openvpn/error.h +1 -0
index 1e1f2ac..cbd1980 100644
@@ -234,6 +234,7 @@ void msg_forked (void);
 
 /* syslog output */
 
+void set_syslog_facility (const char *name);
 void open_syslog (const char *pgmname, bool stdio_to_null);
 void close_syslog ();
 
src/openvpn/options.c +12 -0
index 40210e6..e6cc1cc 100644
@@ -333,9 +333,21 @@ static const char usage_message[] =
   "--setcon context: Apply this SELinux context after initialization.\n"
 #endif
   "--cd dir        : Change to this directory before initialization.\n"
+#if SYSLOG_CAPABILITY
+  "--syslog-facility name : When logging to syslog, use name (e.g. \n"
+  "                  local1) for the facility.  If name is absent or not\n"
+  "                  supported, a platform-dependent list of valid\n"
+  "                  facility names is provided.  Must be specified\n"
+  "                  before --daemon and --syslog.\n"
+#endif
   "--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 SYSLOG_CAPABILITY
+  "                  If --syslog-facility has not been specified and \n"
+  "                  the name includes [facility], the specified\n"
+  "                  syslog facility name will be used.\n"
+#endif
   "--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"
@@ -4661,6 +4673,13 @@ add_option (struct options *options,

       VERIFY_PERMISSION (OPT_P_GENERAL);
       options->up_restart = true;
     }

+#if SYSLOG_CAPABILITY
+ else if (streq (p[0], "syslog-facility"))
+ {
+ VERIFY_PERMISSION (OPT_P_GENERAL);
+ set_syslog_facility (p[1]);
+ }
+#endif
else if (streq (p[0], "syslog"))

     {
       VERIFY_PERMISSION (OPT_P_GENERAL);

--
1.7.10.4

Signature · 5 lines
Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed.