Libguestfs - Oct 2020 - Re: [PATCH nbdkit v2 3/3] ocaml: Add bindings for nbdkit_peer_{pid, uid, gid}.

This is just a simple update to:

https://www.redhat.com/archives/libguestfs/2020-October/msg00015.html

rebased on top of current nbdkit master because I pushed a few simple
refactorings.

Rich.

New nbdkit_peer_pid, nbdkit_peer_uid and nbdkit_peer_gid calls can be
used on Linux (only) to read the peer PID, UID and GID from clients
connected over a Unix domain socket.  This can be used in the
preconnect phase to add additional filtering.

One use for this is to add an extra layer of authentication for local
connections.  A subsequent commit will enhance the now misnamed
nbdkit-ip-filter to allow filtering on these extra fields.

It appears as if it would be possible to implement this for FreeBSD
too (see comment in code).
---
 docs/nbdkit-plugin.pod  |  47 +++++++++++++++--
 include/nbdkit-common.h |   3 ++
 server/nbdkit.syms      |   3 ++
 server/public.c         | 108 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 5 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 62feae47..c2b9fb12 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -656,9 +656,10 @@ B<not> been negotiated at this point.
 For security reasons (to avoid denial of service attacks) this
 callback should be written to be as fast and take as few resources as
 possible.  If you use this callback, only use it to do basic access
-control, such as checking C<nbdkit_peer_name> against a list of
-permitted IP addresses (see L</PEER NAME> and
L<nbdkit-ip-filter(1)>).
-It may be better to do access control outside the server, for example
+control, such as checking C<nbdkit_peer_name>, C<nbdkit_peer_pid>,
+C<nbdkit_peer_uid>, C<nbdkit_peer_gid> against a list of permitted
+source addresses (see L</PEER NAME> and L<nbdkit-ip-filter(1)>). 
It
+may be better to do access control outside the server, for example
 using TCP wrappers or a firewall.
 
 The C<readonly> flag informs the plugin that the server was started
@@ -1614,8 +1615,8 @@ C<.open>), C<nbdkit_error> is called and the
call returns C<-1>.
 
 =head1 PEER NAME
 
-It is possible to get the address of the client when you are running
-in any connected callback.
+It is possible to get the source address of the client when you are
+running in any connected callback.
 
 =head2 C<nbdkit_peer_name>
 
@@ -1635,6 +1636,42 @@ specify the current connection.
 On success this returns C<0>.  On error, C<nbdkit_error> is called
and
 this call returns C<-1>.
 
+=head2 C<nbdkit_peer_pid>
+
+(nbdkit E<ge> 1.24)
+
+ int nbdkit_peer_pid (void);
+
+Return the peer process ID.  This is only available when the client
+connected over a Unix domain socket, and only works for Linux.
+
+On success this returns the peer process ID.  On error,
+C<nbdkit_error> is called and this call returns C<-1>.
+
+=head2 C<nbdkit_peer_uid>
+
+(nbdkit E<ge> 1.24)
+
+ int nbdkit_peer_uid (void);
+
+Return the peer user ID.  This is only available when the client
+connected over a Unix domain socket, and only works for Linux.
+
+On success this returns the user ID.  On error, C<nbdkit_error> is
+called and this call returns C<-1>.
+
+=head2 C<nbdkit_peer_gid>
+
+(nbdkit E<ge> 1.24)
+
+ int nbdkit_peer_gid (void);
+
+Return the peer group ID.  This is only available when the client
+connected over a Unix domain socket, and only works for Linux.
+
+On success this returns the user ID.  On error, C<nbdkit_error> is
+called and this call returns C<-1>.
+
 =head1 DEBUGGING
 
 Run the server with I<-f> and I<-v> options so it doesn't fork
and you
diff --git a/include/nbdkit-common.h b/include/nbdkit-common.h
index eb4416df..7690b57e 100644
--- a/include/nbdkit-common.h
+++ b/include/nbdkit-common.h
@@ -130,6 +130,9 @@ NBDKIT_EXTERN_DECL (char *, nbdkit_realpath, (const char
*path));
 NBDKIT_EXTERN_DECL (int, nbdkit_nanosleep, (unsigned sec, unsigned nsec));
 NBDKIT_EXTERN_DECL (int, nbdkit_peer_name,
                     (struct sockaddr *addr, socklen_t *addrlen));
+NBDKIT_EXTERN_DECL (int, nbdkit_peer_pid, (void));
+NBDKIT_EXTERN_DECL (int, nbdkit_peer_uid, (void));
+NBDKIT_EXTERN_DECL (int, nbdkit_peer_gid, (void));
 NBDKIT_EXTERN_DECL (void, nbdkit_shutdown, (void));
 
 NBDKIT_EXTERN_DECL (const char *, nbdkit_strdup_intern,
diff --git a/server/nbdkit.syms b/server/nbdkit.syms
index 1eb18bb0..3d6b2235 100644
--- a/server/nbdkit.syms
+++ b/server/nbdkit.syms
@@ -67,7 +67,10 @@
     nbdkit_parse_uint32_t;
     nbdkit_parse_uint64_t;
     nbdkit_parse_unsigned;
+    nbdkit_peer_gid;
     nbdkit_peer_name;
+    nbdkit_peer_pid;
+    nbdkit_peer_uid;
     nbdkit_printf_intern;
     nbdkit_read_password;
     nbdkit_realpath;
diff --git a/server/public.c b/server/public.c
index 7636a16b..8b7ca437 100644
--- a/server/public.c
+++ b/server/public.c
@@ -47,6 +47,7 @@
 #include <limits.h>
 #include <errno.h>
 #include <signal.h>
+#include <sys/types.h>
 
 #ifdef HAVE_TERMIOS_H
 #include <termios.h>
@@ -801,6 +802,113 @@ nbdkit_peer_name (struct sockaddr *addr, socklen_t
*addrlen)
   return 0;
 }
 
+#ifdef SO_PEERCRED
+
+static int
+get_peercred (int s, int *pid, int *uid, int *gid)
+{
+  struct ucred ucred;
+  socklen_t n = sizeof ucred;
+
+  if (getsockopt (s, SOL_SOCKET, SO_PEERCRED, &ucred, &n) == -1) {
+    nbdkit_error ("getsockopt: SO_PEERCRED: %m");
+    return -1;
+  }
+
+  if (pid && ucred.pid >= 1) {
+    if (ucred.pid <= INT_MAX)
+      *pid = ucred.pid;
+    else
+      nbdkit_error ("pid out of range: cannot be mapped to int");
+  }
+  if (uid && ucred.uid >= 0) {
+    if (ucred.uid <= INT_MAX)
+      *uid = ucred.uid;
+    else
+      nbdkit_error ("uid out of range: cannot be mapped to int");
+  }
+  if (gid && ucred.gid >= 0) {
+    if (ucred.gid <= INT_MAX)
+      *gid = ucred.gid;
+    else
+      nbdkit_error ("gid out of range: cannot be mapped to int");
+  }
+
+  return 0;
+}
+
+#else /* !SO_PEERCRED */
+
+/* Note this could be ported to FreeBSD, see LOCAL_PEERCRED in:
+ * https://www.freebsd.org/cgi/man.cgi?query=unix&sektion=4
+ */
+static int
+get_peercred (int s, int *pid, int *uid, int *gid)
+{
+  nbdkit_error ("nbdkit_peer_pid, nbdkit_peer_uid and nbdkit_peer_gid
"
+                "are not supported on this platform");
+  return -1;
+}
+
+#endif /* !SO_PEERCRED */
+
+static int
+get_peercred_common (int *pid, int *uid, int *gid)
+{
+  struct connection *conn = threadlocal_get_conn ();
+  int s;
+
+  if (pid) *pid = -1;
+  if (uid) *uid = -1;
+  if (gid) *gid = -1;
+
+  if (!conn) {
+    nbdkit_error ("no connection in this thread");
+    return -1;
+  }
+
+  s = conn->sockin;
+  if (s == -1) {
+    nbdkit_error ("socket not open");
+    return -1;
+  }
+
+  return get_peercred (s, pid, uid, gid);
+}
+
+int
+nbdkit_peer_pid ()
+{
+  int pid;
+
+  if (get_peercred_common (&pid, NULL, NULL) == -1)
+    return -1;
+
+  return pid;
+}
+
+int
+nbdkit_peer_uid ()
+{
+  int uid;
+
+  if (get_peercred_common (NULL, &uid, NULL) == -1)
+    return -1;
+
+  return uid;
+}
+
+int
+nbdkit_peer_gid ()
+{
+  int gid;
+
+  if (get_peercred_common (NULL, NULL, &gid) == -1)
+    return -1;
+
+  return gid;
+}
+
 /* Functions for manipulating intern'd strings. */
 
 static string_vector global_interns;
-- 
2.27.0

---
 filters/ip/nbdkit-ip-filter.pod | 64 +++++++++++++++++++++++++-----
 tests/Makefile.am               | 14 ++++++-
 filters/ip/ip.c                 | 69 +++++++++++++++++++++++++++++---
 tests/test-ip-filter-gid.sh     | 51 ++++++++++++++++++++++++
 tests/test-ip-filter-pid.sh     | 70 +++++++++++++++++++++++++++++++++
 tests/test-ip-filter-uid.sh     | 51 ++++++++++++++++++++++++
 6 files changed, 301 insertions(+), 18 deletions(-)

diff --git a/filters/ip/nbdkit-ip-filter.pod b/filters/ip/nbdkit-ip-filter.pod
index 17108617..aa91cff2 100644
--- a/filters/ip/nbdkit-ip-filter.pod
+++ b/filters/ip/nbdkit-ip-filter.pod
@@ -1,6 +1,7 @@
 =head1 NAME
 
-nbdkit-ip-filter - filter clients by IP address
+nbdkit-ip-filter - filter clients by IP address, process ID, user ID
+or group ID
 
 =head1 SYNOPSIS
 
@@ -14,6 +15,10 @@ address.  Usually it is better to control this outside
nbdkit, for
 example using TCP wrappers or a firewall, but this filter can be used
 if these are not available.
 
+nbdkit E<ge> 1.24 added the ability to filter clients connecting over
+local Unix domain sockets by client process ID, user ID and group ID.
+This currently only works on Linux.
+
 =head1 EXAMPLES
 
  nbdkit --filter=ip [...] allow=127.0.0.1,::1 deny=all
@@ -28,13 +33,29 @@ network.
 
  nbdkit --filter=ip [...] allow=anyipv6 deny=all
 
-Allow IPv6 clients to connect from anywhere, deny all IPv4
-connections.
+Allow IPv6 clients to connect from anywhere, deny all other sources.
+
+ nbdkit -U sock --filter=ip [...] allow=pid:1234 deny=all
+
+Only process ID 1234 can connect to the server over the local Unix
+domain socket.
+
+ nbdkit -U $tmpdir/sock --filter=ip [...] allow=uid:`id -u` deny=all
+
+Only allow the current user (S<C<id -u>>) to connect over the
socket.
+It is better to use this as an additional line of defence — also
+create a temporary directory, make sure it is only accessible by the
+user, and place the socket there.
+
+ nbdkit -U sock --filter=ip [...] allow=gid:`id -g` deny=all
+
+Allow anyone in the same group as the current user to connect to the
+Unix domain socket.
 
 =head1 RULES
 
-When a client connects, this filter checks its IP address against the
-allow and deny lists as follows:
+When a client connects, this filter checks its source address against
+the allow and deny lists as follows:
 
 =over 4
 
@@ -66,8 +87,7 @@ list of any of the following:
 
 =item B<any>
 
-These keywords (which both have the same meaning) match any IP
-address.
+These keywords (which both have the same meaning) match any source.
 
 =item B<allipv4>
 
@@ -100,6 +120,27 @@ address representations can be used (see S<RFC
5952>).
 
 This matches a range of IPv6 addresses C<A:B:.../NN>.
 
+=item B<pid:>PID
+
+(nbdkit E<ge> 1.24, Linux only)
+
+This matches the process ID C<PID>, if the client connects
+over a Unix domain socket.
+
+=item B<uid:>UID
+
+(nbdkit E<ge> 1.24, Linux only)
+
+This matches the numeric user ID C<UID>, if the client connects over a
+Unix domain socket.
+
+=item B<gid:>GID
+
+(nbdkit E<ge> 1.24, Linux only)
+
+This matches the numeric group ID C<GID>, if the client connects over
+a Unix domain socket.
+
 =back
 
 =head2 Not filtered
@@ -107,8 +148,11 @@ This matches a range of IPv6 addresses C<A:B:.../NN>.
 If neither the C<allow> nor the C<deny> parameter is given the
filter
 does nothing.
 
-The filter permits non-IP connections, such as Unix domain sockets or
-AF_VSOCK.
+C<AF_VSOCK> connections are always unfiltered.
+
+Unix domain sockets were always unfiltered in S<nbdkit E<le> 1.22>.
+In S<nbdkit E<ge> 1.24> it is possible to apply filtering to them
on
+Linux.
 
 =head1 PARAMETERS
 
@@ -155,4 +199,4 @@ Richard W.M. Jones
 
 =head1 COPYRIGHT
 
-Copyright (C) 2019 Red Hat Inc.
+Copyright (C) 2019-2020 Red Hat Inc.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b5b06810..cacbbce9 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -1437,8 +1437,18 @@ test_gzip_CFLAGS = $(WARNINGS_CFLAGS)
$(LIBGUESTFS_CFLAGS)
 test_gzip_LDADD = libtest.la $(LIBGUESTFS_LIBS)
 
 # ip filter test.
-TESTS += test-ip-filter.sh
-EXTRA_DIST += test-ip-filter.sh
+TESTS += \
+	test-ip-filter.sh \
+	test-ip-filter-pid.sh \
+	test-ip-filter-uid.sh \
+	test-ip-filter-gid.sh \
+	$(NULL)
+EXTRA_DIST += \
+	test-ip-filter.sh \
+	test-ip-filter-pid.sh \
+	test-ip-filter-uid.sh \
+	test-ip-filter-gid.sh \
+	$(NULL)
 
 # limit filter test.
 TESTS += test-limit.sh
diff --git a/filters/ip/ip.c b/filters/ip/ip.c
index b16ec55c..24c0147c 100644
--- a/filters/ip/ip.c
+++ b/filters/ip/ip.c
@@ -62,12 +62,13 @@ int ip_debug_rules;
 
 struct rule {
   struct rule *next;
-  enum { BAD = 0, ANY, ANYV4, ANYV6, IPV4, IPV6 } type;
+  enum { BAD = 0, ANY, ANYV4, ANYV6, IPV4, IPV6, PID, UID, GID } type;
   union {
-    struct in_addr ipv4;
+    struct in_addr ipv4;        /* for IPV4, IPV6 */
     struct in6_addr ipv6;
+    int id;                     /* for PID, UID and GID */
   } u;
-  unsigned prefixlen;
+  unsigned prefixlen;           /* for IPV4, IPV6 */
 };
 
 static struct rule *allow_rules, *allow_rules_last;
@@ -100,6 +101,16 @@ print_rule (const char *name, const struct rule *rule,
const char *suffix)
     nbdkit_debug ("%s=ipv6:[%s]/%u%s", name, u.addr6,
rule->prefixlen, suffix);
     break;
 
+  case PID:
+    nbdkit_debug ("%s=pid:%d%s", name, rule->u.id, suffix);
+    break;
+  case UID:
+    nbdkit_debug ("%s=uid:%d%s", name, rule->u.id, suffix);
+    break;
+  case GID:
+    nbdkit_debug ("%s=gid:%d%s", name, rule->u.id, suffix);
+    break;
+
   case BAD:
     nbdkit_debug ("%s=BAD(!)%s", name, suffix);
     break;
@@ -227,6 +238,37 @@ parse_rule (const char *paramname,
     return 0;
   }
 
+  if (n >= 4 && ascii_strncasecmp (value, "pid:", 4) == 0)
{
+    new_rule->type = PID;
+    if (nbdkit_parse_int ("pid:", &value[4],
&new_rule->u.id) == -1)
+      return -1;
+    if (new_rule->u.id <= 0) {
+      nbdkit_error ("pid: parameter out of range");
+      return -1;
+    }
+    return 0;
+  }
+  if (n >= 4 && ascii_strncasecmp (value, "uid:", 4) == 0)
{
+    new_rule->type = UID;
+    if (nbdkit_parse_int ("uid:", &value[4],
&new_rule->u.id) == -1)
+      return -1;
+    if (new_rule->u.id < 0) {
+      nbdkit_error ("uid: parameter out of range");
+      return -1;
+    }
+    return 0;
+  }
+  if (n >= 4 && ascii_strncasecmp (value, "gid:", 4) == 0)
{
+    new_rule->type = GID;
+    if (nbdkit_parse_int ("gid:", &value[4],
&new_rule->u.id) == -1)
+      return -1;
+    if (new_rule->u.id < 0) {
+      nbdkit_error ("gid: parameter out of range");
+      return -1;
+    }
+    return 0;
+  }
+
   /* Address with prefixlen. */
   if ((p = strchr (value, '/')) != NULL) {
     size_t pllen = &value[n] - &p[1];
@@ -401,6 +443,19 @@ matches_rule (const struct rule *rule,
     sin6 = (struct sockaddr_in6 *) addr;
     return ipv6_equal (sin6->sin6_addr, rule->u.ipv6,
rule->prefixlen);
 
+    /* Note these work even if the underlying nbdkit_peer_* call fails. */
+  case PID:
+    if (family != AF_UNIX) return false;
+    return nbdkit_peer_pid () == rule->u.id;
+
+  case UID:
+    if (family != AF_UNIX) return false;
+    return nbdkit_peer_uid () == rule->u.id;
+
+  case GID:
+    if (family != AF_UNIX) return false;
+    return nbdkit_peer_gid () == rule->u.id;
+
   case BAD:
   default:
     abort ();
@@ -430,8 +485,10 @@ check_if_allowed (const struct sockaddr *addr)
 {
   int family = ((struct sockaddr_in *)addr)->sin_family;
 
-  /* There's an implicit allow all for non-IP sockets, see the manual. */
-  if (family != AF_INET && family != AF_INET6)
+  /* There's an implicit allow all for non-IP, non-Unix sockets,
+   * see the manual.
+   */
+  if (family != AF_INET && family != AF_INET6 && family !=
AF_UNIX)
     return true;
 
   if (matches_rules_list ("ip: match source with allow",
@@ -457,7 +514,7 @@ ip_preconnect (nbdkit_next_preconnect *next, void *nxdata,
int readonly)
   /* Follow the rules. */
   if (check_if_allowed ((struct sockaddr *) &addr) == false) {
     nbdkit_error ("client not permitted to connect "
-                  "because of IP address restriction");
+                  "because of source address restriction");
     return -1;
   }
 
diff --git a/tests/test-ip-filter-gid.sh b/tests/test-ip-filter-gid.sh
new file mode 100755
index 00000000..d02407f3
--- /dev/null
+++ b/tests/test-ip-filter-gid.sh
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Test the ip filter with gid: parameter.
+
+source ./functions.sh
+set -e
+set -x
+
+requires qemu-img --version
+# This requires Linux.
+requires_linux_kernel_version 2.6
+
+nbdkit -U - -v -D ip.rules=1 --filter=ip null allow=gid:`id -g` deny=all \
+       --run 'qemu-img info $nbd'
+
+# This is expected to fail.
+if nbdkit -U - -v -D ip.rules=1 --filter=ip null deny=gid:`id -g` \
+          --run 'qemu-img info $nbd'; then
+    echo "$0: expected test to fail"
+    exit 1
+fi
diff --git a/tests/test-ip-filter-pid.sh b/tests/test-ip-filter-pid.sh
new file mode 100755
index 00000000..4a7f55a3
--- /dev/null
+++ b/tests/test-ip-filter-pid.sh
@@ -0,0 +1,70 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Test the ip filter with pid: parameter.
+
+source ./functions.sh
+set -e
+set -x
+
+requires nbdinfo --version
+requires nbdsh --version
+requires_nbdsh_uri
+# This requires Linux.
+requires_linux_kernel_version 2.6
+
+# This is expected to fail because the shell ($$) is not connecting to
+# the server.
+if nbdkit -U - -v -D ip.rules=1 --filter=ip null allow=pid:$$ deny=all \
+          --run 'nbdinfo --size "$uri"'; then
+    echo "$0: expected test to fail"
+    exit 1
+fi
+
+# This is expected to work because we can deny the shell.
+nbdkit -U - -v -D ip.rules=1 --filter=ip null deny=pid:$$ \
+       --run 'nbdinfo --size "$uri"'
+
+# This is a better test using nbdsh and passing the PID of nbdsh
+# itself to nbdkit.  Note this only works because nbd_connect_command
+# uses a socketpair which is a kind of nameless Unix domain socket.
+nbdsh -c - <<'EOF'
+import os
+
+h.connect_command(["nbdkit", "-s",
+                   "-v", "-D", "ip.rules=1",
+                   "null", "size=512",
+                   "--filter=ip",
+                   "allow=pid:" + str(os.getpid()),
+                   "deny=all"])
+assert h.get_size() == 512
+EOF
diff --git a/tests/test-ip-filter-uid.sh b/tests/test-ip-filter-uid.sh
new file mode 100755
index 00000000..dc6ab679
--- /dev/null
+++ b/tests/test-ip-filter-uid.sh
@@ -0,0 +1,51 @@
+#!/usr/bin/env bash
+# nbdkit
+# Copyright (C) 2020 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+# Test the ip filter with uid: parameter.
+
+source ./functions.sh
+set -e
+set -x
+
+requires qemu-img --version
+# This requires Linux.
+requires_linux_kernel_version 2.6
+
+nbdkit -U - -v -D ip.rules=1 --filter=ip null allow=uid:`id -u` deny=all \
+       --run 'qemu-img info $nbd'
+
+# This is expected to fail.
+if nbdkit -U - -v -D ip.rules=1 --filter=ip null deny=uid:`id -u` \
+          --run 'qemu-img info $nbd'; then
+    echo "$0: expected test to fail"
+    exit 1
+fi
-- 
2.27.0

---
 plugins/ocaml/NBDKit.mli |  7 +++++++
 plugins/ocaml/NBDKit.ml  |  4 ++++
 plugins/ocaml/bindings.c | 24 ++++++++++++++++++++++++
 3 files changed, 35 insertions(+)

diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli
index ececd5fd..8abfeb49 100644
--- a/plugins/ocaml/NBDKit.mli
+++ b/plugins/ocaml/NBDKit.mli
@@ -162,3 +162,10 @@ val shutdown : unit -> unit
 
 (** Print a debug message when nbdkit is in verbose mode. *)
 val debug : ('a, unit, string, unit) format4 -> 'a
+
+(** Binding for [nbdkit_peer_pid]. *)
+val peer_pid : unit -> int
+(** Binding for [nbdkit_peer_uid]. *)
+val peer_uid : unit -> int
+(** Binding for [nbdkit_peer_gid]. *)
+val peer_gid : unit -> int
diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml
index 739210fa..76fa3a77 100644
--- a/plugins/ocaml/NBDKit.ml
+++ b/plugins/ocaml/NBDKit.ml
@@ -257,3 +257,7 @@ external _debug : string -> unit =
"ocaml_nbdkit_debug" "noalloc"
 
 let debug fs    ksprintf _debug fs
+
+external peer_pid : unit -> int = "ocaml_nbdkit_peer_pid"
+external peer_uid : unit -> int = "ocaml_nbdkit_peer_uid"
+external peer_gid : unit -> int = "ocaml_nbdkit_peer_gid"
diff --git a/plugins/ocaml/bindings.c b/plugins/ocaml/bindings.c
index 6c92cd3e..d231ec89 100644
--- a/plugins/ocaml/bindings.c
+++ b/plugins/ocaml/bindings.c
@@ -190,3 +190,27 @@ ocaml_nbdkit_debug (value strv)
 
   return Val_unit;
 }
+
+value
+ocaml_nbdkit_peer_pid (value unitv)
+{
+  int id = nbdkit_peer_pid ();
+  if (id == -1) caml_failwith ("nbdkit_peer_pid");
+  return Val_int (id);
+}
+
+value
+ocaml_nbdkit_peer_uid (value unitv)
+{
+  int id = nbdkit_peer_uid ();
+  if (id == -1) caml_failwith ("nbdkit_peer_uid");
+  return Val_int (id);
+}
+
+value
+ocaml_nbdkit_peer_gid (value unitv)
+{
+  int id = nbdkit_peer_gid ();
+  if (id == -1) caml_failwith ("nbdkit_peer_gid");
+  return Val_int (id);
+}
-- 
2.27.0

On 10/3/20 1:50 PM, Richard W.M. Jones wrote:
> New nbdkit_peer_pid, nbdkit_peer_uid and nbdkit_peer_gid calls can be
> used on Linux (only) to read the peer PID, UID and GID from clients
> connected over a Unix domain socket.  This can be used in the
> preconnect phase to add additional filtering.
> 
> One use for this is to add an extra layer of authentication for local
> connections.  A subsequent commit will enhance the now misnamed
> nbdkit-ip-filter to allow filtering on these extra fields.
> 
> It appears as if it would be possible to implement this for FreeBSD
> too (see comment in code).
> ---
>  docs/nbdkit-plugin.pod  |  47 +++++++++++++++--
>  include/nbdkit-common.h |   3 ++
>  server/nbdkit.syms      |   3 ++
>  server/public.c         | 108 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 156 insertions(+), 5 deletions(-)
> 
> +=head2 C<nbdkit_peer_pid>
> +
> +(nbdkit E<ge> 1.24)
> +
> + int nbdkit_peer_pid (void);
> +
> +Return the peer process ID.  This is only available when the client
> +connected over a Unix domain socket, and only works for Linux.
> +
> +On success this returns the peer process ID.  On error,
> +C<nbdkit_error> is called and this call returns C<-1>.
Is int always going to be sufficient?  Or are there platforms with
64-bit pid_t?  Mingw is an interesting beast; I've seen conflicting
stories on whether 64-bit windows has 32- or 64-bit pids (the spawn APIs
manage 64-bit handles, but other windows APIs return 32-bit int), so
64-bit pid_t on mingw does seem to be a real concern.
> +
> +=head2 C<nbdkit_peer_uid>
> +
> +(nbdkit E<ge> 1.24)
> +
> + int nbdkit_peer_uid (void);
> +
> +Return the peer user ID.  This is only available when the client
> +connected over a Unix domain socket, and only works for Linux.
> +
> +On success this returns the user ID.  On error, C<nbdkit_error> is
> +called and this call returns C<-1>.
> +
> +=head2 C<nbdkit_peer_gid>
> +
> +(nbdkit E<ge> 1.24)
> +
> + int nbdkit_peer_gid (void);
int for these two is probably fine.
> +
> +Return the peer group ID.  This is only available when the client
> +connected over a Unix domain socket, and only works for Linux.
> +
> +On success this returns the user ID.  On error, C<nbdkit_error> is
> +called and this call returns C<-1>.
> +
>  =head1 DEBUGGING
>  
> +static int
> +get_peercred (int s, int *pid, int *uid, int *gid)
> +{
> +  struct ucred ucred;
> +  socklen_t n = sizeof ucred;
> +
> +  if (getsockopt (s, SOL_SOCKET, SO_PEERCRED, &ucred, &n) == -1) {
> +    nbdkit_error ("getsockopt: SO_PEERCRED: %m");
> +    return -1;
> +  }
> +
> +  if (pid && ucred.pid >= 1) {
> +    if (ucred.pid <= INT_MAX)
> +      *pid = ucred.pid;
> +    else
> +      nbdkit_error ("pid out of range: cannot be mapped to
int");
> +  }
well, at least you are acknowledging that int might not always map to pid_t.

Otherwise, looks fine to me.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 10/3/20 1:50 PM, Richard W.M. Jones wrote:
> ---
>  filters/ip/nbdkit-ip-filter.pod | 64 +++++++++++++++++++++++++-----
>  tests/Makefile.am               | 14 ++++++-
>  filters/ip/ip.c                 | 69 +++++++++++++++++++++++++++++---
>  tests/test-ip-filter-gid.sh     | 51 ++++++++++++++++++++++++
>  tests/test-ip-filter-pid.sh     | 70 +++++++++++++++++++++++++++++++++
>  tests/test-ip-filter-uid.sh     | 51 ++++++++++++++++++++++++
>  6 files changed, 301 insertions(+), 18 deletions(-)
> 
> +
> + nbdkit -U $tmpdir/sock --filter=ip [...] allow=uid:`id -u` deny=all
> +
> +Only allow the current user (S<C<id -u>>) to connect over the
socket.
> +It is better to use this as an additional line of defence — also
defense
> +create a temporary directory, make sure it is only accessible by the
> +user, and place the socket there.
> +
> + nbdkit -U sock --filter=ip [...] allow=gid:`id -g` deny=all
> +
> +Allow anyone in the same group as the current user to connect to the
> +Unix domain socket.
>  
> +++ b/filters/ip/ip.c
> @@ -62,12 +62,13 @@ int ip_debug_rules;
>  
>  struct rule {
>    struct rule *next;
> -  enum { BAD = 0, ANY, ANYV4, ANYV6, IPV4, IPV6 } type;
> +  enum { BAD = 0, ANY, ANYV4, ANYV6, IPV4, IPV6, PID, UID, GID } type;
>    union {
> -    struct in_addr ipv4;
> +    struct in_addr ipv4;        /* for IPV4, IPV6 */
>      struct in6_addr ipv6;
> +    int id;                     /* for PID, UID and GID */
Do you want to use id_t here? POSIX requires that type to be a superset
of pid_t, uid_t, and gid_t - on Linux, it is still 32-bit, but it might
be more robust if we have to compile on systems with 64-bit pid_t.
Sadly, there is no handy printf specifier for id_t, so using int does
make life easier elsewhere.
>    } u;
> -  unsigned prefixlen;
> +  unsigned prefixlen;           /* for IPV4, IPV6 */
>  };
>  
>  static struct rule *allow_rules, *allow_rules_last;
> @@ -100,6 +101,16 @@ print_rule (const char *name, const struct rule *rule,
const char *suffix)
>      nbdkit_debug ("%s=ipv6:[%s]/%u%s", name, u.addr6,
rule->prefixlen, suffix);
>      break;
>  
> +  case PID:
> +    nbdkit_debug ("%s=pid:%d%s", name, rule->u.id, suffix);
For example, this becomes more complicated if we use id_t instead of int.

ACK

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On 10/3/20 1:50 PM, Richard W.M. Jones wrote:
> ---
>  plugins/ocaml/NBDKit.mli |  7 +++++++
>  plugins/ocaml/NBDKit.ml  |  4 ++++
>  plugins/ocaml/bindings.c | 24 ++++++++++++++++++++++++
>  3 files changed, 35 insertions(+)
> 
> diff --git a/plugins/ocaml/NBDKit.mli b/plugins/ocaml/NBDKit.mli
> index ececd5fd..8abfeb49 100644
> --- a/plugins/ocaml/NBDKit.mli
> +++ b/plugins/ocaml/NBDKit.mli
> @@ -162,3 +162,10 @@ val shutdown : unit -> unit
>  
>  (** Print a debug message when nbdkit is in verbose mode. *)
>  val debug : ('a, unit, string, unit) format4 -> 'a
> +
> +(** Binding for [nbdkit_peer_pid]. *)
> +val peer_pid : unit -> int
> +(** Binding for [nbdkit_peer_uid]. *)
> +val peer_uid : unit -> int
> +(** Binding for [nbdkit_peer_gid]. *)
> +val peer_gid : unit -> int
Is int sufficient on 32-bit platforms, or do you need int32?  But on
64-bit platforms, I don't see a system ever having enough valid
uid_t/gid_t/pid_t to overflow int to the point that int64 would have
been better.

Otherwise looks fine.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

On Sat, Oct 03, 2020 at 07:50:01PM +0100, Richard W.M. Jones
wrote:
> ---
>  filters/ip/nbdkit-ip-filter.pod | 64 +++++++++++++++++++++++++-----
>  tests/Makefile.am               | 14 ++++++-
>  filters/ip/ip.c                 | 69 +++++++++++++++++++++++++++++---
>  tests/test-ip-filter-gid.sh     | 51 ++++++++++++++++++++++++
>  tests/test-ip-filter-pid.sh     | 70 +++++++++++++++++++++++++++++++++
>  tests/test-ip-filter-uid.sh     | 51 ++++++++++++++++++++++++
>  6 files changed, 301 insertions(+), 18 deletions(-)
> 
> diff --git a/filters/ip/nbdkit-ip-filter.pod
b/filters/ip/nbdkit-ip-filter.pod
> index 17108617..aa91cff2 100644
> --- a/filters/ip/nbdkit-ip-filter.pod
> +++ b/filters/ip/nbdkit-ip-filter.pod
> @@ -1,6 +1,7 @@
>  =head1 NAME
>  
> -nbdkit-ip-filter - filter clients by IP address
> +nbdkit-ip-filter - filter clients by IP address, process ID, user ID
> +or group ID
>  
>  =head1 SYNOPSIS
>  
> @@ -14,6 +15,10 @@ address.  Usually it is better to control this outside
nbdkit, for
>  example using TCP wrappers or a firewall, but this filter can be used
>  if these are not available.
>  
> +nbdkit E<ge> 1.24 added the ability to filter clients connecting
over
> +local Unix domain sockets by client process ID, user ID and group ID.
> +This currently only works on Linux.
> +
>  =head1 EXAMPLES
>  
>   nbdkit --filter=ip [...] allow=127.0.0.1,::1 deny=all
> @@ -28,13 +33,29 @@ network.
>  
>   nbdkit --filter=ip [...] allow=anyipv6 deny=all
>  
> -Allow IPv6 clients to connect from anywhere, deny all IPv4
> -connections.
> +Allow IPv6 clients to connect from anywhere, deny all other sources.
> +
> + nbdkit -U sock --filter=ip [...] allow=pid:1234 deny=all
> +
> +Only process ID 1234 can connect to the server over the local Unix
> +domain socket.
NB using  PID as an access control token on its own is racy due to the
possibility of PID reuse.  There was a major CVE against polkit many
years back due to use of PID alone:

   https://access.redhat.com/security/cve/CVE-2013-4288

The safe way to check PIDs is to use the (PID, start time, uid) triple.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|