mirror of
https://github.com/bolkedebruin/rdpgw.git
synced 2026-05-12 19:30:04 +00:00
Restrict the rdpgw-auth socket to its own UID by default (#190)
The auth daemon's gRPC socket was world-writable and accepted any local UID that could connect to it. On a multi-tenant host any user on the box could speak the gRPC API and run an arbitrary username/ password through PAM -- effectively an unauthenticated PAM oracle. Create the socket with mode 0660 (Umask(0117)) and gate Accept on SO_PEERCRED: only the daemon's own UID is allowed by default, plus any operator-supplied --allow-uid / --allow-gid. Privilege-separated deployments (rdpgw and rdpgw-auth as different users) need to list the gateway's UID, or share a group; the existing path otherwise would have been permissive. The peer-credentials check is Linux-only; the non-Linux build keeps the listener as-is and logs a warning, since rdpgw-auth itself requires libpam and is effectively Linux-only in practice.
This commit is contained in:
@@ -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`).
|
||||
|
||||
21
UPGRADING.md
21
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
|
||||
|
||||
@@ -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)
|
||||
|
||||
89
cmd/auth/peercred_linux.go
Normal file
89
cmd/auth/peercred_linux.go
Normal file
@@ -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
|
||||
}
|
||||
103
cmd/auth/peercred_linux_test.go
Normal file
103
cmd/auth/peercred_linux_test.go
Normal file
@@ -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)
|
||||
}
|
||||
}
|
||||
16
cmd/auth/peercred_other.go
Normal file
16
cmd/auth/peercred_other.go
Normal file
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
|
||||
2
go.mod
2
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
|
||||
|
||||
Reference in New Issue
Block a user