Remove peer credential check from control server.

The peer credential check worked by querying the peer's UID using SO_PEERCRED.
However, when sandbox is configured with custom UID/GID mappings, which it is
in the following situations:
1. When not using directfs, sandbox has a mapping of only `65534:65534:1`. So
   all other non-65534 UIDs are unmmaped.
2. When using directfs with userns-remap. The sandbox has the same UID/GID
   mapping as the container.

In these cases, when the root user tries to connect, the host root user is not
mapped. Unmapped IDs are translated to overflowuid/gid, which happens to
default to 65534. This leads to unexpected results:
- Without directfs, this check allows any connection, because sandbox is also
  running as nobody/65534.
- In the directfs+userns-remap case, all connections are denied.

This check doesn't really enforce anything. Hence remove it.

The real enforcement mechanism is the file mode of the unix domain socket over
which the connection happens. The socket file's mode depends on umask, which
defaults to 022. Hence only the user which creates the sandbox via
`runsc create` has write permission on this socket. So only the sandbox creator
or root can connect to the sandbox, as intended.

Fixes #9356

PiperOrigin-RevId: 565836818
This commit is contained in:
Ayush Ranjan
2023-09-15 19:44:22 -07:00
committed by gVisor bot
parent 5ff1e90815
commit d81768d4c8
4 changed files with 0 additions and 56 deletions
-1
View File
@@ -11,7 +11,6 @@ go_library(
visibility = ["//:sandbox"],
deps = [
"//pkg/abi/linux",
"//pkg/log",
"//pkg/sync",
"//pkg/unet",
"//pkg/urpc",
-16
View File
@@ -28,7 +28,6 @@ import (
"golang.org/x/sys/unix"
"gvisor.dev/gvisor/pkg/abi/linux"
"gvisor.dev/gvisor/pkg/log"
"gvisor.dev/gvisor/pkg/sync"
"gvisor.dev/gvisor/pkg/unet"
"gvisor.dev/gvisor/pkg/urpc"
@@ -107,21 +106,6 @@ func (s *Server) serve() {
return
}
ucred, err := conn.GetPeerCred()
if err != nil {
log.Warningf("Control couldn't get credentials: %s", err.Error())
conn.Close()
continue
}
// Only allow this user and root.
if int(ucred.Uid) != curUID && ucred.Uid != 0 {
// Authentication failed.
log.Warningf("Control auth failure: other UID = %d, current UID = %d", ucred.Uid, curUID)
conn.Close()
continue
}
// Handle the connection non-blockingly.
s.server.StartHandling(conn)
}
-11
View File
@@ -395,17 +395,6 @@ func (s *Socket) GetPeerName() ([]byte, error) {
}
}
// GetPeerCred returns the peer's unix credentials.
func (s *Socket) GetPeerCred() (*unix.Ucred, error) {
fd, ok := s.enterFD()
if !ok {
return nil, unix.EBADF
}
defer s.gate.Leave()
return unix.GetsockoptUcred(fd, unix.SOL_SOCKET, unix.SO_PEERCRED)
}
// SocketReader wraps an individual receive operation.
//
// This may be used for doing vectorized reads and/or sending additional
-28
View File
@@ -556,22 +556,6 @@ func TestFDsReceiveSizeZero(t *testing.T) {
recvFDs(t, client, 0, []int{})
}
func TestGetPeerCred(t *testing.T) {
server, client := socketPair(t, false)
defer server.Close()
defer client.Close()
want := &unix.Ucred{
Pid: int32(os.Getpid()),
Uid: uint32(os.Getuid()),
Gid: uint32(os.Getgid()),
}
if got, err := client.GetPeerCred(); err != nil || !reflect.DeepEqual(got, want) {
t.Errorf("GetPeerCred() = %v, %v, want = %+v, %+v", got, err, want, nil)
}
}
func newClosedSocket() (*Socket, error) {
fd, err := unix.Socket(unix.AF_UNIX, unix.SOCK_STREAM, 0)
if err != nil {
@@ -587,18 +571,6 @@ func newClosedSocket() (*Socket, error) {
return s, s.Close()
}
func TestGetPeerCredFailure(t *testing.T) {
s, err := newClosedSocket()
if err != nil {
t.Fatalf("newClosedSocket got error %v want nil", err)
}
want := "bad file descriptor"
if _, err := s.GetPeerCred(); err == nil || err.Error() != want {
t.Errorf("s.GetPeerCred() = %v, want = %s", err, want)
}
}
func TestAcceptClosed(t *testing.T) {
name, err := randomFilename()
if err != nil {