gravitational/teleport
1
0
Fork 0
mirror of https://github.com/gravitational/teleport.git synced 2026-10-12 02:15:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix remote port forwarding with separate login (#43069)

Browse Source 
This change fixes a bug where remote port forwarding would fail if
the local login the user is SSHing with doesn't match the local user
running the node.
This commit is contained in:
Andrew Burke
2024-06-25 21:34:21 +00:00
committed by GitHub
parent 360d1fde6e
commit 18572bc034
10 changed files with 136 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
integration/hostuser_test.go
+3 -9
View File
@@ -41,21 +41,15 @@ import (
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/local"
"github.com/gravitational/teleport/lib/srv"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/host"
)
const testuser = "teleport-testuser"
const testgroup = "teleport-testgroup"
func requireRoot(t *testing.T) {
t.Helper()
if !isRoot() {
t.Skip("This test will be skipped because tests are not being run as root.")
}
}
func TestRootHostUsersBackend(t *testing.T) {
requireRoot(t)
utils.RequireRoot(t)
sudoersTestDir := t.TempDir()
usersbk := srv.HostUsersProvisioningBackend{}
sudoersbk := srv.HostSudoersProvisioningBackend{
@@ -213,7 +207,7 @@ func cleanupUsersAndGroups(users []string, groups []string) func() {
}
func TestRootHostUsers(t *testing.T) {
requireRoot(t)
utils.RequireRoot(t)
ctx := context.Background()
bk, err := lite.New(ctx, backend.Params{"path": t.TempDir()})
require.NoError(t, err)
integration/port_forwarding_test.go
+1 -12
View File
@@ -25,7 +25,6 @@ import (
"net/http"
"net/http/httptest"
"net/url"
"os/user"
"strconv"
"testing"
"time"
@@ -97,17 +96,7 @@ func testPingLocalServer(t *testing.T, port int, expectSuccess bool) {
}
func testPortForwarding(t *testing.T, suite *integrationTestSuite) {
invalidOSLogin := uuid.NewString()[:12]
notFound := false
for i := 0; i < 10; i++ {
if _, err := user.Lookup(invalidOSLogin); err == nil {
invalidOSLogin = uuid.NewString()[:12]
continue
}
notFound = true
break
}
require.True(t, notFound, "unable to locate invalid os user")
invalidOSLogin := utils.GenerateLocalUsername(t)
// Providing our own logins to Teleport so we can verify that a user
// that exists within Teleport but does not exist on the local node
lib/srv/exec_linux_test.go
+35
View File
@@ -32,7 +32,12 @@ import (
"time"
"github.com/gravitational/trace"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/host"
)
func TestOSCommandPrep(t *testing.T) {
@@ -132,6 +137,36 @@ func TestConfigureCommand(t *testing.T) {
require.NotContains(t, cmd.Env, unexpectedKey+"="+unexpectedValue)
}
func TestRootConfigureCommand(t *testing.T) {
utils.RequireRoot(t)
login := utils.GenerateLocalUsername(t)
_, err := host.UserAdd(login, nil, "", "", "")
require.NoError(t, err)
t.Cleanup(func() {
_, err := host.UserDel(login)
require.NoError(t, err)
})
srv := newMockServer(t)
scx := newExecServerContext(t, srv)
scx.Identity.Login = login
scx.ExecType = teleport.TCPIPForwardRequest
u, err := user.Lookup(login)
require.NoError(t, err)
uid, err := strconv.ParseUint(u.Uid, 10, 32)
require.NoError(t, err)
gid, err := strconv.ParseUint(u.Gid, 10, 32)
require.NoError(t, err)
cmd, err := ConfigureCommand(scx)
require.NoError(t, err)
// Verify that the configured command will run as the expected user.
assert.Equal(t, uint32(uid), cmd.SysProcAttr.Credential.Uid)
assert.Equal(t, uint32(gid), cmd.SysProcAttr.Credential.Gid)
}
// TestContinue tests if the process hangs if a continue signal is not sent
// and makes sure the process continues once it has been sent.
func TestContinue(t *testing.T) {
lib/srv/reexec.go
+24
View File
@@ -25,6 +25,7 @@ import (
"errors"
"fmt"
"io"
"log/slog"
"net"
"os"
"os/exec"
@@ -1106,6 +1107,29 @@ func ConfigureCommand(ctx *ServerContext, extraFiles ...*os.File) (*exec.Cmd, er
cmd.ExtraFiles = append(cmd.ExtraFiles, extraFiles...)
}
// For remote port forwarding, the child needs to run as the user to
// create listeners with the correct permissions.
if subCommand == teleport.RemoteForwardSubCommand {
localUser, err := user.Lookup(ctx.Identity.Login)
if err != nil {
return nil, trace.Wrap(err)
}
// Ensure that the working directory is one that the child has access to.
cmd.Dir = "/"
credential, err := getCmdCredential(localUser)
if err != nil {
return nil, trace.Wrap(err)
}
log := slog.With("uid", credential.Uid, "gid", credential.Gid, "groups", credential.Groups)
if os.Getuid() != int(credential.Uid) || os.Getgid() != int(credential.Gid) {
cmd.SysProcAttr = &syscall.SysProcAttr{Credential: credential}
log.DebugContext(ctx.Context, "Creating process with new credentials.")
} else {
log.DebugContext(ctx.Context, "Creating process with environment credentials.")
}
}
// Perform OS-specific tweaks to the command.
reexecCommandOSTweaks(cmd)
lib/srv/reexec_test.go
+31 -3
View File
@@ -37,6 +37,8 @@ import (
"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/host"
"github.com/gravitational/teleport/lib/utils/uds"
)
@@ -249,20 +251,24 @@ func TestLocalPortForwardCommand(t *testing.T) {
require.Equal(t, "Hello, world", string(body))
}
func TestRemotePortForwardCommand(t *testing.T) {
t.Parallel()
func testRemotePortForwardCommand(t *testing.T, login string) {
srv := newMockServer(t)
scx := newExecServerContext(t, srv)
scx.ExecType = teleport.TCPIPForwardRequest
if login != "" {
scx.Identity.Login = login
}
// Start forwarding subprocess.
controlConn, controlFD := newSocketPair(t)
command, err := ConfigureCommand(scx, controlFD)
require.NoError(t, err)
require.NoError(t, command.Start())
t.Cleanup(func() {
require.NoError(t, command.Process.Kill())
_, err := command.Process.Wait()
require.NoError(t, err)
})
require.NoError(t, command.Start())
// Request a listener from the forwarder.
replyConn, replyFD := newSocketPair(t)
@@ -284,3 +290,25 @@ func TestRemotePortForwardCommand(t *testing.T) {
require.NoError(t, err)
require.Equal(t, "Hello, world", string(body))
}
func TestRemotePortForwardCommand(t *testing.T) {
t.Parallel()
testRemotePortForwardCommand(t, "")
}
// TestRootRemotePortForwardCommand tests that remote port forwarding works
// for a user different than the one running a node (which we need to run
// as root to create).
func TestRootRemotePortForwardCommand(t *testing.T) {
utils.RequireRoot(t)
login := utils.GenerateLocalUsername(t)
_, err := host.UserAdd(login, nil, "", "", "")
require.NoError(t, err)
t.Cleanup(func() {
_, err := host.UserDel(login)
require.NoError(t, err)
})
testRemotePortForwardCommand(t, login)
}
lib/srv/regular/sshserver_linux.go
-96
View File
@@ -1,96 +0,0 @@
//go:build linux
// +build linux
/*
* Teleport
* Copyright (C) 2024 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package regular
import (
"net"
"os"
"os/user"
"strconv"
"github.com/gravitational/trace"
"golang.org/x/sys/unix"
"github.com/gravitational/teleport/lib/srv"
)
// validateListenerSocket checks that the socket and listener file descriptor
// sent from the forwarding process have the expected properties.
func validateListenerSocket(scx *srv.ServerContext, controlConn *net.UnixConn, listenerFD *os.File) error {
// Get the credentials of the client connected to the socket.
var cred *unix.Ucred
var err error
if err := controlSyscallConn(controlConn, func(fd uintptr) error {
cred, err = unix.GetsockoptUcred(int(fd), unix.SOL_SOCKET, unix.SO_PEERCRED)
return err
}); err != nil {
return trace.Wrap(err)
}
// Check that the user connected to the socket is who we expect.
usr, err := user.Lookup(scx.Identity.Login)
if err != nil {
return trace.Wrap(err)
}
if expectedUid, err := strconv.Atoi(usr.Uid); err != nil {
return trace.Wrap(err)
} else if int(cred.Uid) != expectedUid {
return trace.AccessDenied("unexpected user UID for the socket: %v", cred.Uid)
}
if expectedGid, err := strconv.Atoi(usr.Gid); err != nil {
return trace.Wrap(err)
} else if int(cred.Gid) != expectedGid {
return trace.AccessDenied("unexpected user GID for the socket: %v", cred.Gid)
}
if err := controlSyscallConn(listenerFD, func(fd uintptr) error {
// Verify the socket type
if sockType, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_TYPE); err != nil {
return trace.Wrap(err)
} else if sockType != unix.SOCK_STREAM {
return trace.AccessDenied("socket is not of the expected type (STREAM)")
}
// Verify that reuse is not enabled on the socket
if reuseAddr, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR); err != nil {
return trace.Wrap(err)
} else if reuseAddr != 0 {
return trace.AccessDenied("SO_REUSEADDR is enabled on the socket")
}
if reusePort, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEPORT); err != nil {
// Some systems may not support SO_REUSEPORT, so we ignore the error here
} else if reusePort != 0 {
return trace.AccessDenied("SO_REUSEPORT is enabled on the socket")
}
// Verify that the listener is already listening.
if acceptConn, err := unix.GetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_ACCEPTCONN); err != nil {
return trace.Wrap(err)
} else if acceptConn == 0 {
return trace.AccessDenied("SO_ACCEPTCONN is not set, socket is not listening")
}
return nil
}); err != nil {
return trace.Wrap(err)
}
return nil
}
lib/srv/regular/sshserver_test.go
+1 -8
View File
@@ -1046,7 +1046,7 @@ func TestX11Forward(t *testing.T) {
// TestRootX11ForwardPermissions tests that X11 forwarding sessions are set up
// with the connecting user's file permissions (where needed), rather than root.
func TestRootX11ForwardPermissions(t *testing.T) {
requireRoot(t)
utils.RequireRoot(t)
if os.Getenv("TELEPORT_XAUTH_TEST") == "" {
t.Skip("Skipping test as xauth is not enabled")
}
@@ -2963,13 +2963,6 @@ func newCertAuthorityWatcher(ctx context.Context, t *testing.T, client types.Eve
return caWatcher
}
func requireRoot(t *testing.T) {
t.Helper()
if os.Geteuid() != 0 {
t.Skip("This test will be skipped because tests are not being run as root.")
}
}
// newSigner creates a new SSH signer that can be used by the Server.
func newSigner(t *testing.T, ctx context.Context, testServer *auth.TestServer) ssh.Signer {
t.Helper()
lib/srv/regular/sshserver_darwin.go → lib/srv/regular/sshserver_unix.go
+2 -2
View File
@@ -1,5 +1,5 @@
//go:build darwin
// +build darwin
//go:build unix
// +build unix
/*
* Teleport
lib/srv/regular/sshserver_linux_test.go → lib/srv/regular/sshserver_unix_test.go
+3 -22
View File
@@ -1,5 +1,5 @@
//go:build linux
// +build linux
//go:build unix
// +build unix
/*
* Teleport
@@ -23,7 +23,6 @@ package regular
import (
"net"
"os"
"os/user"
"syscall"
"testing"
@@ -63,29 +62,18 @@ func TestValidateListenerSocket(t *testing.T) {
return left, listenerFD
}
u, err := user.Current()
require.NoError(t, err)
tests := []struct {
name string
user string
mutateFiles func(*testing.T, *uds.Conn, *os.File) (*uds.Conn, *os.File)
mutateConn func(*testing.T, *os.File)
assert require.ErrorAssertionFunc
}{
{
name: "ok",
user: u.Username,
assert: require.NoError,
},
{
name: "wrong user",
user: "fake-user",
assert: require.Error,
},
{
name: "socket type not STREAM",
user: u.Username,
mutateFiles: func(t *testing.T, conn *uds.Conn, file *os.File) (*uds.Conn, *os.File) {
left, right, err := uds.NewSocketpair(uds.SocketTypeDatagram)
require.NoError(t, err)
@@ -102,7 +90,6 @@ func TestValidateListenerSocket(t *testing.T) {
},
{
name: "SO_REUSEADDR enabled",
user: u.Username,
mutateConn: func(t *testing.T, file *os.File) {
fd := file.Fd()
err := unix.SetsockoptInt(int(fd), unix.SOL_SOCKET, unix.SO_REUSEADDR, 1)
@@ -112,7 +99,6 @@ func TestValidateListenerSocket(t *testing.T) {
},
{
name: "listener socket is not listening",
user: u.Username,
mutateFiles: func(t *testing.T, conn *uds.Conn, file *os.File) (*uds.Conn, *os.File) {
left, right, err := uds.NewSocketpair(uds.SocketTypeStream)
require.NoError(t, err)
@@ -130,11 +116,6 @@ func TestValidateListenerSocket(t *testing.T) {
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
scx := &srv.ServerContext{
Identity: srv.IdentityContext{
Login: tc.user,
},
}
conn, listenerFD := newSocketFiles(t)
if tc.mutateFiles != nil {
conn, listenerFD = tc.mutateFiles(t, conn, listenerFD)
@@ -142,7 +123,7 @@ func TestValidateListenerSocket(t *testing.T) {
if tc.mutateConn != nil {
tc.mutateConn(t, listenerFD)
}
err := validateListenerSocket(scx, conn.UnixConn, listenerFD)
err := validateListenerSocket(&srv.ServerContext{}, conn.UnixConn, listenerFD)
tc.assert(t, err)
})
}
lib/utils/testhelpers.go
+36
View File
@@ -18,11 +18,16 @@ package utils
import (
"context"
"crypto/rand"
"errors"
"fmt"
"os"
"os/user"
"testing"
"time"
"github.com/gravitational/trace"
"github.com/stretchr/testify/require"
)
// TestBackgroundTask is a task that should be run in the background for the remaining duration of a test and
@@ -87,3 +92,34 @@ func RunTestBackgroundTask(ctx context.Context, t *testing.T, task *TestBackgrou
}
})
}
// RequireRoot skips the current test if it is not running as root.
func RequireRoot(t *testing.T) {
t.Helper()
if os.Geteuid() != 0 {
t.Skip("This test will be skipped because tests are not being run as root.")
}
}
func generateUsername(t *testing.T) string {
suffix := make([]byte, 8)
_, err := rand.Read(suffix)
require.NoError(t, err)
return fmt.Sprintf("teleport-%x", suffix)
}
// GenerateLocalUsername generates the username for a local user that does not
// already exists (but it does not create the user).
func GenerateLocalUsername(t *testing.T) string {
const maxAttempts = 10
for i := 0; i < maxAttempts; i++ {
login := generateUsername(t)
_, err := user.Lookup(login)
if errors.Is(err, user.UnknownUserError(login)) {
return login
}
require.NoError(t, err)
}
t.Fatalf("Unable to generate unused username after %v attempts", maxAttempts)
return ""
}