diff --git a/CHANGELOG.md b/CHANGELOG.md index 593e0a5..75f1bef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. ### Changed +- `rdpgw-auth` now creates its socket with mode `0660` and accepts only + connections whose peer UID is on an allow-list (default: the daemon's + own UID). Operators running rdpgw and rdpgw-auth as different users + must list the gateway's UID via `--allow-uid` or share a group via + `--allow-gid`. See [UPGRADING.md](UPGRADING.md). - `X-Forwarded-For` is now honored only when the request arrives from a `Server.TrustedProxies` CIDR. The default `Server.TrustedProxies` is empty, so by default the request's `RemoteAddr` (host portion) is @@ -25,6 +30,7 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. ### Added +- `rdpgw-auth --allow-uid` and `--allow-gid` flags (repeatable). - `Server.TrustedProxies` (`[]string`, CIDR, default empty). - `Server.AllowedDestinationPorts` (`[]int`, default `[3389]`). - `Server.AllowPrivateDestinations` (`bool`, default `false`). diff --git a/UPGRADING.md b/UPGRADING.md index 0c6c009..37569db 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -2,6 +2,27 @@ ## Unreleased +### `rdpgw-auth` only accepts connections from the daemon's own UID by default + +The auth daemon previously created its socket world-writable +(`Umask(0)`) and accepted any local UID that could `connect(2)` to it. +Two changes: + +* The socket is now created with mode `0660` (no access for `other`). +* The daemon reads `SO_PEERCRED` on every accepted connection and + rejects callers whose UID is not on the allow-list. The default + allow-list is the daemon's own UID. + +If `rdpgw` and `rdpgw-auth` run as the same user, no action is +required. Otherwise, list the gateway's UID (or a shared GID): + +``` +./rdpgw-auth -s /tmp/rdpgw-auth.sock --allow-uid 1001 +./rdpgw-auth -s /tmp/rdpgw-auth.sock --allow-gid 1100 +``` + +`--allow-uid` and `--allow-gid` are repeatable. + ### `X-Forwarded-For` is no longer trusted by default Previously rdpgw read the first `X-Forwarded-For` entry into the diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index ede837a..6f0ce26 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -24,7 +24,9 @@ const ( var opts struct { ServiceName string `short:"n" long:"name" default:"rdpgw" description:"the PAM service name to use"` SocketAddr string `short:"s" long:"socket" default:"/tmp/rdpgw-auth.sock" description:"the location of the socket"` - ConfigFile string `short:"c" long:"conf" default:"rdpgw-auth.yaml" description:"users config file for NTLM (yaml)"` + ConfigFile string `short:"c" long:"conf" default:"rdpgw-auth.yaml" description:"users config file for NTLM (yaml)"` + AllowUID []int `long:"allow-uid" description:"additional UIDs allowed to connect to the socket; the daemon's own UID is always allowed (repeatable)"` + AllowGID []int `long:"allow-gid" description:"GIDs allowed to connect to the socket (repeatable)"` } type AuthServiceImpl struct { @@ -127,12 +129,19 @@ func main() { } cleanup() - oldUmask := syscall.Umask(0) + oldUmask := syscall.Umask(0117) listener, err := net.Listen(protocol, opts.SocketAddr) syscall.Umask(oldUmask) if err != nil { log.Fatal(err) } + + // The daemon's own UID is always permitted; additional callers must + // be enumerated by the operator. This stops any local user on a + // shared host from speaking gRPC against the PAM oracle. + allowedUIDs := append([]int{os.Getuid()}, opts.AllowUID...) + listener = newGatedListener(listener, allowedUIDs, opts.AllowGID) + server := grpc.NewServer() db := database.NewConfig(conf.Users) service := NewAuthService(opts.ServiceName, db) diff --git a/cmd/auth/peercred_linux.go b/cmd/auth/peercred_linux.go new file mode 100644 index 0000000..99c0074 --- /dev/null +++ b/cmd/auth/peercred_linux.go @@ -0,0 +1,89 @@ +//go:build linux + +package main + +import ( + "fmt" + "log" + "net" + + "golang.org/x/sys/unix" +) + +// gatedListener wraps a unix-socket net.Listener and accepts only +// connections whose peer UID/GID is on the allow-list. The check is +// applied at Accept(), before any application data is read, so the +// gRPC server never sees an unauthorized caller. +type gatedListener struct { + net.Listener + allowedUIDs map[uint32]struct{} + allowedGIDs map[uint32]struct{} +} + +func newGatedListener(l net.Listener, uids []int, gids []int) net.Listener { + uidSet := make(map[uint32]struct{}, len(uids)) + for _, u := range uids { + uidSet[uint32(u)] = struct{}{} + } + gidSet := make(map[uint32]struct{}, len(gids)) + for _, g := range gids { + gidSet[uint32(g)] = struct{}{} + } + return &gatedListener{Listener: l, allowedUIDs: uidSet, allowedGIDs: gidSet} +} + +func (l *gatedListener) Accept() (net.Conn, error) { + for { + conn, err := l.Listener.Accept() + if err != nil { + return nil, err + } + ucred, err := peerCred(conn) + if err != nil { + log.Printf("rejecting connection: cannot read peer credentials: %s", err) + conn.Close() + continue + } + if !l.allowed(ucred) { + log.Printf("rejecting connection from uid=%d gid=%d pid=%d", ucred.Uid, ucred.Gid, ucred.Pid) + conn.Close() + continue + } + return conn, nil + } +} + +func (l *gatedListener) allowed(c *unix.Ucred) bool { + if _, ok := l.allowedUIDs[c.Uid]; ok { + return true + } + if _, ok := l.allowedGIDs[c.Gid]; ok { + return true + } + return false +} + +func peerCred(conn net.Conn) (*unix.Ucred, error) { + uc, ok := conn.(*net.UnixConn) + if !ok { + return nil, fmt.Errorf("connection is not a *net.UnixConn (got %T)", conn) + } + raw, err := uc.SyscallConn() + if err != nil { + return nil, err + } + var ( + ucred *unix.Ucred + credErr error + ) + ctrlErr := raw.Control(func(fd uintptr) { + ucred, credErr = unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED) + }) + if ctrlErr != nil { + return nil, ctrlErr + } + if credErr != nil { + return nil, credErr + } + return ucred, nil +} diff --git a/cmd/auth/peercred_linux_test.go b/cmd/auth/peercred_linux_test.go new file mode 100644 index 0000000..7ea14e4 --- /dev/null +++ b/cmd/auth/peercred_linux_test.go @@ -0,0 +1,103 @@ +//go:build linux + +package main + +import ( + "net" + "os" + "path/filepath" + "syscall" + "testing" + "time" +) + +func startGatedListener(t *testing.T, allowUIDs, allowGIDs []int) (string, <-chan struct{}) { + t.Helper() + dir := t.TempDir() + addr := filepath.Join(dir, "auth.sock") + + old := syscall.Umask(0117) + raw, err := net.Listen("unix", addr) + syscall.Umask(old) + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { raw.Close() }) + + accepted := make(chan struct{}, 1) + gated := newGatedListener(raw, allowUIDs, allowGIDs) + go func() { + for { + conn, err := gated.Accept() + if err != nil { + return + } + accepted <- struct{}{} + conn.Close() + } + }() + return addr, accepted +} + +func TestGatedListenerAcceptsCurrentUID(t *testing.T) { + addr, accepted := startGatedListener(t, []int{os.Getuid()}, nil) + + conn, err := net.DialTimeout("unix", addr, 1*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + select { + case <-accepted: + case <-time.After(500 * time.Millisecond): + t.Fatalf("listener did not accept connection from current UID %d", os.Getuid()) + } +} + +func TestGatedListenerRejectsForeignUID(t *testing.T) { + // Allow a UID we are not running as. + addr, accepted := startGatedListener(t, []int{os.Getuid() + 1}, nil) + + conn, err := net.DialTimeout("unix", addr, 1*time.Second) + if err != nil { + t.Fatalf("dial: %v", err) + } + defer conn.Close() + + if err := conn.SetReadDeadline(time.Now().Add(500 * time.Millisecond)); err != nil { + t.Fatalf("set read deadline: %v", err) + } + buf := make([]byte, 1) + if _, err := conn.Read(buf); err == nil { + t.Fatal("expected EOF / closed conn after gate rejection, got data") + } + + select { + case <-accepted: + t.Fatal("Accept handed a connection from a UID outside the allow-list to the application") + case <-time.After(200 * time.Millisecond): + } +} + +func TestSocketModeUmask0117(t *testing.T) { + dir := t.TempDir() + addr := filepath.Join(dir, "auth.sock") + + old := syscall.Umask(0117) + l, err := net.Listen("unix", addr) + syscall.Umask(old) + if err != nil { + t.Fatalf("listen: %v", err) + } + defer l.Close() + + st, err := os.Stat(addr) + if err != nil { + t.Fatalf("stat socket: %v", err) + } + mode := st.Mode().Perm() + if mode&0007 != 0 { + t.Errorf("socket mode = %#o, expected no permissions for `other`", mode) + } +} diff --git a/cmd/auth/peercred_other.go b/cmd/auth/peercred_other.go new file mode 100644 index 0000000..f177606 --- /dev/null +++ b/cmd/auth/peercred_other.go @@ -0,0 +1,16 @@ +//go:build !linux + +package main + +import ( + "log" + "net" +) + +// On non-Linux platforms SO_PEERCRED isn't portable, so we don't gate by +// peer credentials. rdpgw-auth itself depends on PAM and is effectively +// Linux-only; this file just keeps the build green if anyone tries. +func newGatedListener(l net.Listener, _, _ []int) net.Listener { + log.Printf("rdpgw-auth: peer-credential gating is not implemented on this platform; relying on socket file mode for access control") + return l +} diff --git a/docs/pam-authentication.md b/docs/pam-authentication.md index b75c3ba..c18dca3 100644 --- a/docs/pam-authentication.md +++ b/docs/pam-authentication.md @@ -69,6 +69,23 @@ Run the `rdpgw-auth` helper program: systemctl start rdpgw-auth ``` +### 4. Allow the gateway process to call the socket + +`rdpgw-auth` only accepts connections from its own UID by default. If +the rdpgw process runs as a different user, list its UID (or a shared +GID) explicitly: + +```bash +# rdpgw runs as UID 1001 +./rdpgw-auth -s /tmp/rdpgw-auth.sock --allow-uid 1001 + +# rdpgw and rdpgw-auth share group 'rdpgw' (GID 1100) +./rdpgw-auth -s /tmp/rdpgw-auth.sock --allow-gid 1100 +``` + +The socket itself is created with mode `0660`, so listing a shared GID +on the daemon is the typical privilege-separated deployment. + ## Authentication Flow 1. Client connects to gateway with username/password diff --git a/go.mod b/go.mod index 230b8ce..ff18a55 100644 --- a/go.mod +++ b/go.mod @@ -29,6 +29,7 @@ require ( github.com/thought-machine/go-flags v1.6.3 golang.org/x/crypto v0.46.0 golang.org/x/oauth2 v0.34.0 + golang.org/x/sys v0.39.0 google.golang.org/grpc v1.79.3 google.golang.org/protobuf v1.36.10 ) @@ -53,7 +54,6 @@ require ( github.com/prometheus/common v0.50.0 // indirect github.com/prometheus/procfs v0.13.0 // indirect golang.org/x/net v0.48.0 // indirect - golang.org/x/sys v0.39.0 // indirect golang.org/x/text v0.32.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20251202230838-ff82c1b0f217 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect