client: use a dedicated file lock to prevent concurrent changes

We were using a lock on the Unix socket. This was working on Linux but
this is not portable. Therefore, we have to use a dedicated file for
this purpose. We use /var/lock by default.

We don't do a secure creation as the lock file is only opened in
append mode, so a symlink attack could only create empty file or reset
the timestamp of a file. No content can be erased this way.

Fix #445

diff --git a/NEWS b/NEWS
index 88c67a653da47dab848a6f453af78c3737d0ae96..b9dd1c37983af914d4e3c3c2c3eceb3403b0038e 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+lldpd (1.0.12)
+  * Fix:
+    + Use a dedicated file lock to prevent concurrent changes from lldpcli.
+
 lldpd (1.0.11)
   * Changes:
     + Disable LLDP in firmware for Intel X7xx cards.

diff --git a/configure.ac b/configure.ac
index 5128569de869c1066e4bb6030867d9f2d2586012..e7e159657ad9a042aa26464d2fdd7ef581e9ba47 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -351,6 +351,7 @@ fi
 lldp_ARG_WITH([privsep-chroot], [Which directory to use to chroot lldpd], [${runstatedir}/lldpd])
 lldp_ARG_WITH([lldpd-ctl-socket], [Path to socket for communication with lldpd], [${runstatedir}/lldpd.socket])
 lldp_ARG_WITH([lldpd-pid-file], [Path to lldpd PID file], [${runstatedir}/lldpd.pid])
+lldp_ARG_WITH([lldpcli-lock-dir], [Which directory to use to put locks], [${localstatedir}/lock])
 
 # Netlink
 lldp_ARG_WITH_UNQUOTED([netlink-max-receive-bufsize], [Netlink maximum receive buffer size], [1024*1024])

diff --git a/src/client/commands.c b/src/client/commands.c
index 823176792a9015d5c09baf24f9cc9f996863b0d1..b7b77dded309f92ffdf5ed782b04044d37ca2623 100644 (file)
--- a/src/client/commands.c
+++ b/src/client/commands.c
@@ -19,8 +19,11 @@
 #include <string.h>
 #include <sys/queue.h>
 #include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include <sys/socket.h>
 #include <sys/un.h>
+#include <libgen.h>
 
 /**
  * An element of the environment (a key and a value).
@@ -372,6 +375,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
                .argp = 0
        };
        static int lockfd = -1;
+       static char *lockname = NULL;   /* Name of lockfile */
        cmdenv_push(&env, root);
        if (!completion)
                for (n = 0; n < argc; n++)
@@ -445,26 +449,30 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
                /* Push and execute */
                cmdenv_push(&env, best);
                if (best->execute) {
-                       struct sockaddr_un su;
                        if (needlock) {
                                if (lockfd == -1) {
-                                       log_debug("lldpctl", "reopen %s for locking", ctlname);
-                                       if ((lockfd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1) {
-                                               log_warn("lldpctl", "cannot open for lock %s", ctlname);
+                                       char *_ctlname = NULL;
+                                       if (lockname == NULL &&
+                                           ((_ctlname = strdup(ctlname)) == NULL ||
+                                           asprintf(&lockname, LLDPCLI_LOCK_DIR "/%s.lck",
+                                           basename(_ctlname)) == -1)) {
+                                               log_warnx("lldpctl",
+                                                   "not enough memory to build lock filename");
                                                rc = -1;
+                                               free(_ctlname);
                                                goto end;
                                        }
-                                       su.sun_family = AF_UNIX;
-                                       strlcpy(su.sun_path, ctlname, sizeof(su.sun_path));
-                                       if (connect(lockfd, (struct sockaddr *)&su, sizeof(struct sockaddr_un)) == -1) {
-                                               log_warn("lldpctl", "cannot connect to socket %s", ctlname);
+                                       free(_ctlname);
+                                       log_debug("lldpctl", "open %s for locking", lockname);
+                                       if ((lockfd = open(lockname,
+                                           O_CREAT|O_RDWR|O_NOFOLLOW, 0666)) == -1) {
+                                               log_warn("lldpctl", "cannot open lock %s", lockname);
                                                rc = -1;
-                                               close(lockfd); lockfd = -1;
                                                goto end;
                                        }
                                }
                                if (lockf(lockfd, F_LOCK, 0) == -1) {
-                                       log_warn("lldpctl", "cannot get lock on %s", ctlname);
+                                       log_warn("lldpctl", "cannot get lock on %s", lockname);
                                        rc = -1;
                                        close(lockfd); lockfd = -1;
                                        goto end;
@@ -472,7 +480,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
                        }
                        rc = best->execute(conn, w, &env, best->arg) != 1 ? -1 : rc;
                        if (needlock && lockf(lockfd, F_ULOCK, 0) == -1) {
-                               log_warn("lldpctl", "cannot unlock %s", ctlname);
+                               log_warn("lldpctl", "cannot unlock %s", lockname);
                                close(lockfd); lockfd = -1;
                        }
                        if (rc == -1) goto end;