[lxc-devel] [lxd/master] lxc/ucred: Simplify logic

stgraber on Github lxc-bot at linuxcontainers.org
Fri May 22 15:19:50 UTC 2020


A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200522/976bb98d/attachment.bin>
-------------- next part --------------
From 352cbc8c3d35159bb3e94ec91ec655568044760a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Fri, 22 May 2020 11:19:21 -0400
Subject: [PATCH] lxc/ucred: Simplify logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/devlxd.go          | 13 +++++----
 lxd/devlxd_test.go     |  4 ++-
 lxd/seccomp/seccomp.go | 24 ++++++++--------
 lxd/ucred/ucred.go     | 65 +++++++-----------------------------------
 lxd/ucred/ucred_gc.go  |  3 --
 5 files changed, 32 insertions(+), 77 deletions(-)
 delete mode 100644 lxd/ucred/ucred_gc.go

diff --git a/lxd/devlxd.go b/lxd/devlxd.go
index b7871737ee..7256ab6525 100644
--- a/lxd/devlxd.go
+++ b/lxd/devlxd.go
@@ -15,6 +15,7 @@ import (
 	"unsafe"
 
 	"github.com/gorilla/mux"
+	"golang.org/x/sys/unix"
 
 	"github.com/lxc/lxd/lxd/daemon"
 	"github.com/lxc/lxd/lxd/instance"
@@ -173,22 +174,22 @@ func hoistReq(f func(*Daemon, instance.Instance, http.ResponseWriter, *http.Requ
 			return
 		}
 
-		c, err := findContainerForPid(cred.PID, d.State())
+		c, err := findContainerForPid(cred.Pid, d.State())
 		if err != nil {
 			http.Error(w, err.Error(), 500)
 			return
 		}
 
 		// Access control
-		rootUID := int64(0)
+		rootUID := uint32(0)
 
 		idmapset, err := c.CurrentIdmap()
 		if err == nil && idmapset != nil {
 			uid, _ := idmapset.ShiftIntoNs(0, 0)
-			rootUID = int64(uid)
+			rootUID = uint32(uid)
 		}
 
-		if rootUID != cred.UID {
+		if rootUID != cred.Uid {
 			http.Error(w, "Access denied for non-root user", 401)
 			return
 		}
@@ -241,10 +242,10 @@ func devLxdAPI(d *Daemon) http.Handler {
  * from our http handlers, since there appears to be no way to pass information
  * around here.
  */
-var pidMapper = ConnPidMapper{m: map[*net.UnixConn]*ucred.UCred{}}
+var pidMapper = ConnPidMapper{m: map[*net.UnixConn]*unix.Ucred{}}
 
 type ConnPidMapper struct {
-	m     map[*net.UnixConn]*ucred.UCred
+	m     map[*net.UnixConn]*unix.Ucred
 	mLock sync.Mutex
 }
 
diff --git a/lxd/devlxd_test.go b/lxd/devlxd_test.go
index c851481a77..ddc8098731 100644
--- a/lxd/devlxd_test.go
+++ b/lxd/devlxd_test.go
@@ -10,6 +10,8 @@ import (
 	"strings"
 	"testing"
 
+	"golang.org/x/sys/unix"
+
 	"github.com/lxc/lxd/lxd/sys"
 	"github.com/lxc/lxd/lxd/ucred"
 )
@@ -110,7 +112,7 @@ func TestCredsSendRecv(t *testing.T) {
 			result <- -1
 			return
 		}
-		result <- cred.PID
+		result <- cred.Pid
 	}()
 
 	conn, err := connect(fmt.Sprintf("%s/test-devlxd-sock", testDir))
diff --git a/lxd/seccomp/seccomp.go b/lxd/seccomp/seccomp.go
index 0c78860b1a..9db1dcbb67 100644
--- a/lxd/seccomp/seccomp.go
+++ b/lxd/seccomp/seccomp.go
@@ -580,7 +580,7 @@ type Server struct {
 
 // Iovec defines an iovec to move data between kernel and userspace.
 type Iovec struct {
-	ucred  *ucred.UCred
+	ucred  *unix.Ucred
 	memFd  int
 	procFd int
 	msg    *C.struct_seccomp_notify_proxy_msg
@@ -591,7 +591,7 @@ type Iovec struct {
 }
 
 // NewSeccompIovec creates a new seccomp iovec.
-func NewSeccompIovec(ucred *ucred.UCred) *Iovec {
+func NewSeccompIovec(ucred *unix.Ucred) *Iovec {
 	msgPtr := C.malloc(C.sizeof_struct_seccomp_notify_proxy_msg)
 	msg := (*C.struct_seccomp_notify_proxy_msg)(msgPtr)
 	C.memset(msgPtr, 0, C.sizeof_struct_seccomp_notify_proxy_msg)
@@ -666,25 +666,25 @@ func (siov *Iovec) IsValidSeccompIovec(size uint64) bool {
 	}
 	if siov.msg.__reserved != 0 {
 		logger.Warnf("Disconnected from seccomp socket after client sent non-zero reserved field: pid=%v",
-			siov.ucred.PID)
+			siov.ucred.Pid)
 		return false
 	}
 
 	if siov.msg.sizes.seccomp_notif != C.expected_sizes.seccomp_notif {
 		logger.Warnf("Disconnected from seccomp socket since client uses different seccomp_notif sizes: %d != %d, pid=%v",
-			siov.msg.sizes.seccomp_notif, C.expected_sizes.seccomp_notif, siov.ucred.PID)
+			siov.msg.sizes.seccomp_notif, C.expected_sizes.seccomp_notif, siov.ucred.Pid)
 		return false
 	}
 
 	if siov.msg.sizes.seccomp_notif_resp != C.expected_sizes.seccomp_notif_resp {
 		logger.Warnf("Disconnected from seccomp socket since client uses different seccomp_notif_resp sizes: %d != %d, pid=%v",
-			siov.msg.sizes.seccomp_notif_resp, C.expected_sizes.seccomp_notif_resp, siov.ucred.PID)
+			siov.msg.sizes.seccomp_notif_resp, C.expected_sizes.seccomp_notif_resp, siov.ucred.Pid)
 		return false
 	}
 
 	if siov.msg.sizes.seccomp_data != C.expected_sizes.seccomp_data {
 		logger.Warnf("Disconnected from seccomp socket since client uses different seccomp_data sizes: %d != %d, pid=%v",
-			siov.msg.sizes.seccomp_data, C.expected_sizes.seccomp_data, siov.ucred.PID)
+			siov.msg.sizes.seccomp_data, C.expected_sizes.seccomp_data, siov.ucred.Pid)
 		return false
 	}
 
@@ -705,13 +705,13 @@ retry:
 			logger.Debugf("Caught EINTR, retrying...")
 			goto retry
 		}
-		logger.Debugf("Disconnected from seccomp socket after failed write for process %v: %s", siov.ucred.PID, err)
-		return fmt.Errorf("Failed to send response to seccomp client %v", siov.ucred.PID)
+		logger.Debugf("Disconnected from seccomp socket after failed write for process %v: %s", siov.ucred.Pid, err)
+		return fmt.Errorf("Failed to send response to seccomp client %v", siov.ucred.Pid)
 	}
 
 	if uint64(bytes) != uint64(C.SECCOMP_MSG_SIZE_MIN) {
-		logger.Debugf("Disconnected from seccomp socket after short write: pid=%v", siov.ucred.PID)
-		return fmt.Errorf("Failed to send full response to seccomp client %v", siov.ucred.PID)
+		logger.Debugf("Disconnected from seccomp socket after short write: pid=%v", siov.ucred.Pid)
+		return fmt.Errorf("Failed to send full response to seccomp client %v", siov.ucred.Pid)
 	}
 
 	return nil
@@ -765,7 +765,7 @@ func NewSeccompServer(s *state.State, path string, findPID func(pid int32, state
 					return
 				}
 
-				logger.Debugf("Connected to seccomp socket: pid=%v", ucred.PID)
+				logger.Debugf("Connected to seccomp socket: pid=%v", ucred.Pid)
 
 				unixFile, err := c.(*net.UnixConn).File()
 				if err != nil {
@@ -776,7 +776,7 @@ func NewSeccompServer(s *state.State, path string, findPID func(pid int32, state
 					siov := NewSeccompIovec(ucred)
 					bytes, err := siov.ReceiveSeccompIovec(int(unixFile.Fd()))
 					if err != nil {
-						logger.Debugf("Disconnected from seccomp socket after failed receive: pid=%v, err=%s", ucred.PID, err)
+						logger.Debugf("Disconnected from seccomp socket after failed receive: pid=%v, err=%s", ucred.Pid, err)
 						c.Close()
 						return
 					}
diff --git a/lxd/ucred/ucred.go b/lxd/ucred/ucred.go
index 86557fec1b..028bf92017 100644
--- a/lxd/ucred/ucred.go
+++ b/lxd/ucred/ucred.go
@@ -1,72 +1,27 @@
 package ucred
 
 import (
-	"fmt"
 	"net"
-	"reflect"
 
 	"golang.org/x/sys/unix"
 )
 
-// GetUCred returns the file descriptor's ucreds.
-func GetUCred(fd int) (uint32, uint32, int32, error) {
-	cred, err := unix.GetsockoptUcred(fd, unix.SOL_SOCKET, unix.SO_PEERCRED)
-	if err != nil {
-		return 0, 0, -1, err
-	}
-
-	return cred.Uid, cred.Gid, cred.Pid, nil
-}
-
-// UCred represents the credentials being used to access a unix socket.
-type UCred struct {
-	PID int32
-	UID int64
-	GID int64
-}
-
 // GetCred returns the credentials being used to access a unix socket.
-func GetCred(conn *net.UnixConn) (*UCred, error) {
-	fd, err := extractUnderlyingFd(conn)
-	if err != nil {
-		return nil, err
-	}
-
-	uid, gid, pid, err := GetUCred(fd)
+func GetCred(conn *net.UnixConn) (*unix.Ucred, error) {
+	rawConn, err := conn.SyscallConn()
 	if err != nil {
 		return nil, err
 	}
 
-	return &UCred{PID: pid, UID: int64(uid), GID: int64(gid)}, nil
-}
-
-/*
- * I also don't see that golang exports an API to get at the underlying FD, but
- * we need it to get at SO_PEERCRED, so let's grab it.
- */
-func extractUnderlyingFd(unixConnPtr *net.UnixConn) (int, error) {
-	conn := reflect.Indirect(reflect.ValueOf(unixConnPtr))
-
-	netFdPtr := conn.FieldByName("fd")
-	if !netFdPtr.IsValid() {
-		return -1, fmt.Errorf("Unable to extract fd from net.UnixConn")
-	}
-	netFd := reflect.Indirect(netFdPtr)
-
-	fd := netFd.FieldByName("sysfd")
-	if !fd.IsValid() {
-		// Try under the new name
-		pfdPtr := netFd.FieldByName("pfd")
-		if !pfdPtr.IsValid() {
-			return -1, fmt.Errorf("Unable to extract pfd from netFD")
+	var ucred *unix.Ucred
+	err = rawConn.Control(func(fd uintptr) {
+		cred, err := unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED)
+		if err != nil {
+			return
 		}
-		pfd := reflect.Indirect(pfdPtr)
 
-		fd = pfd.FieldByName("Sysfd")
-		if !fd.IsValid() {
-			return -1, fmt.Errorf("Unable to extract Sysfd from poll.FD")
-		}
-	}
+		ucred = cred
+	})
 
-	return int(fd.Int()), nil
+	return ucred, nil
 }
diff --git a/lxd/ucred/ucred_gc.go b/lxd/ucred/ucred_gc.go
deleted file mode 100644
index ec3492fd84..0000000000
--- a/lxd/ucred/ucred_gc.go
+++ /dev/null
@@ -1,3 +0,0 @@
-// +build gc
-
-package ucred


More information about the lxc-devel mailing list