From a22dc3ff382cd52842e7bac888e764ed65625e71 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg Date: Mon, 25 Nov 2019 18:05:45 +0100 Subject: [PATCH] service: verify local connections originate from the same UID (#1764) This prevents other users on the same machine (e.g. a production server) from using dlv under the credentials of another user, which poses a security issue. --- service/rpccommon/sameuser.go | 9 +++ service/rpccommon/sameuser_linux.go | 99 ++++++++++++++++++++++++ service/rpccommon/sameuser_linux_test.go | 64 +++++++++++++++ service/rpccommon/server.go | 6 ++ 4 files changed, 178 insertions(+) create mode 100644 service/rpccommon/sameuser.go create mode 100644 service/rpccommon/sameuser_linux.go create mode 100644 service/rpccommon/sameuser_linux_test.go diff --git a/service/rpccommon/sameuser.go b/service/rpccommon/sameuser.go new file mode 100644 index 00000000..b2d6e1cc --- /dev/null +++ b/service/rpccommon/sameuser.go @@ -0,0 +1,9 @@ +//+build !linux + +package rpccommon + +import "net" + +func canAccept(_, _ net.Addr) bool { + return true +} diff --git a/service/rpccommon/sameuser_linux.go b/service/rpccommon/sameuser_linux.go new file mode 100644 index 00000000..2bfc81e8 --- /dev/null +++ b/service/rpccommon/sameuser_linux.go @@ -0,0 +1,99 @@ +//+build linux + +package rpccommon + +import ( + "bytes" + "encoding/binary" + "fmt" + "io/ioutil" + "log" + "net" + "os" + "strings" +) + +// for testing +var ( + uid = os.Getuid() + readFile = ioutil.ReadFile +) + +func sameUserForHexLocalAddr(filename, hexaddr string) (bool, error) { + b, err := readFile(filename) + if err != nil { + return false, err + } + for _, line := range strings.Split(strings.TrimSpace(string(b)), "\n") { + // The format contains whitespace padding (%4d, %5u), so we use + // fmt.Sscanf instead of splitting on whitespace. + var ( + sl int + localAddr, remoteAddr string + state int + queue, timer string + retransmit int + remoteUID uint + ) + // Note that we must use %d where the kernel format uses %5u: + // %u is not understood by the fmt package (%U is something else), + // %5d cuts off longer uids (e.g. 149098 on gLinux). + n, err := fmt.Sscanf(line, "%4d: %s %s %02X %s %s %08X %d", + &sl, &localAddr, &remoteAddr, &state, &queue, &timer, &retransmit, &remoteUID) + if n != 8 || err != nil { + continue // invalid line (e.g. header line) + } + if localAddr != hexaddr { + continue + } + return uid == int(remoteUID), nil + } + return false, fmt.Errorf("connection not found in %s", filename) +} + +func sameUserForRemoteAddr4(remoteAddr *net.TCPAddr) (bool, error) { + // For details about the format, see the kernel side implementation: + // https://elixir.bootlin.com/linux/v5.2.2/source/net/ipv4/tcp_ipv4.c#L2375 + b := remoteAddr.IP.To4() + hexaddr := fmt.Sprintf("%02X%02X%02X%02X:%04X", b[3], b[2], b[1], b[0], remoteAddr.Port) + return sameUserForHexLocalAddr("/proc/net/tcp", hexaddr) +} + +func sameUserForRemoteAddr6(remoteAddr *net.TCPAddr) (bool, error) { + a16 := remoteAddr.IP.To16() + // For details about the format, see the kernel side implementation: + // https://elixir.bootlin.com/linux/v5.2.2/source/net/ipv6/tcp_ipv6.c#L1792 + words := make([]uint32, 4) + if err := binary.Read(bytes.NewReader(a16), binary.LittleEndian, words); err != nil { + return false, err + } + hexaddr := fmt.Sprintf("%08X%08X%08X%08X:%04X", words[0], words[1], words[2], words[3], remoteAddr.Port) + return sameUserForHexLocalAddr("/proc/net/tcp6", hexaddr) +} + +func sameUserForRemoteAddr(remoteAddr *net.TCPAddr) (bool, error) { + if remoteAddr.IP.To4() == nil { + return sameUserForRemoteAddr6(remoteAddr) + } + return sameUserForRemoteAddr4(remoteAddr) +} + +func canAccept(listenAddr, remoteAddr net.Addr) bool { + laddr, ok := listenAddr.(*net.TCPAddr) + if !ok || !laddr.IP.IsLoopback() { + return true + } + addr, ok := remoteAddr.(*net.TCPAddr) + if !ok { + panic(fmt.Sprintf("BUG: conn.RemoteAddr is %T, want *net.TCPAddr", remoteAddr)) + } + same, err := sameUserForRemoteAddr(addr) + if err != nil { + log.Printf("cannot check remote address: %v", err) + } + if !same { + log.Printf("closing connection from different user (%v): connections to localhost are only accepted from the same UNIX user for security reasons", addr) + return false + } + return true +} diff --git a/service/rpccommon/sameuser_linux_test.go b/service/rpccommon/sameuser_linux_test.go new file mode 100644 index 00000000..9fd2bd4a --- /dev/null +++ b/service/rpccommon/sameuser_linux_test.go @@ -0,0 +1,64 @@ +//+build linux + +package rpccommon + +import ( + "net" + "testing" +) + +func TestSameUserForRemoteAddr(t *testing.T) { + uid = 149098 + var proc string + readFile = func(string) ([]byte, error) { + return []byte(proc), nil + } + for _, tt := range []struct { + name string + proc string + addr *net.TCPAddr + want bool + }{ + { + name: "ipv4-same", + proc: ` sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode + 21: 0100007F:E682 0100007F:0FC8 01 00000000:00000000 00:00000000 00000000 149098 0 8420541 2 0000000000000000 20 0 0 10 -1 `, + addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 59010}, + want: true, + }, + + { + name: "ipv4-not-found", + proc: ` sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode + 21: 0100007F:E682 0100007F:0FC8 01 00000000:00000000 00:00000000 00000000 149098 0 8420541 2 0000000000000000 20 0 0 10 -1 `, + addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 2342}, + want: false, + }, + + { + name: "ipv4-different-uid", + proc: ` sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode + 21: 0100007F:E682 0100007F:0FC8 01 00000000:00000000 00:00000000 00000000 149097 0 8420541 2 0000000000000000 20 0 0 10 -1 `, + addr: &net.TCPAddr{IP: net.ParseIP("127.0.0.1"), Port: 59010}, + want: false, + }, + + { + name: "ipv6-same", + proc: ` sl local_address remote_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode + 5: 00000000000000000000000001000000:D3E4 00000000000000000000000001000000:0FC8 01 00000000:00000000 00:00000000 00000000 149098 0 8425526 2 0000000000000000 20 0 0 10 -1 + 6: 00000000000000000000000001000000:0FC8 00000000000000000000000001000000:D3E4 01 00000000:00000000 00:00000000 00000000 149098 0 8424744 1 0000000000000000 20 0 0 10 -1`, + addr: &net.TCPAddr{IP: net.ParseIP("::1"), Port: 54244}, + want: true, + }, + } { + t.Run(tt.name, func(t *testing.T) { + proc = tt.proc + // The returned error is for reporting only. + same, _ := sameUserForRemoteAddr(tt.addr) + if got, want := same, tt.want; got != want { + t.Errorf("sameUserForRemoteAddr(%v) = %v, want %v", tt.addr, got, want) + } + }) + } +} diff --git a/service/rpccommon/server.go b/service/rpccommon/server.go index 3d624389..67925b4c 100644 --- a/service/rpccommon/server.go +++ b/service/rpccommon/server.go @@ -155,6 +155,12 @@ func (s *ServerImpl) Run() error { panic(err) } } + + if !canAccept(s.listener.Addr(), c.RemoteAddr()) { + c.Close() + continue + } + go s.serveJSONCodec(c) if !s.config.AcceptMulti { break