mirror of
https://github.com/gravitational/teleport.git
synced 2026-10-24 12:30:12 +08:00
Security fixes (#13298)
* Add CSRF mitigations This commit includes two fixes: 1. Enforce an application/json Content-Type server-side. 2. When checking the bearer token, verify that the user associated with the token matches the user associated with the cookie. * Fix TEL-Q122-13: Access Requests Denial Of Service Via Request Reason (#125) (#127) * Ignore input when data flow is off in TermManager When data flow is disabled in TermManager (at the beginning or when TermManager.Off was called) we should ignore all input we receive (currently we buffer it) * Agent forwarding socket security fix. Co-authored-by: Lisa Kim <lisa@goteleport.com> Co-authored-by: Joel <jwejdenstal@icloud.com> Co-authored-by: Przemko Robakowski <przemko@przemko-robakowski.pl>
This commit is contained in:
co-authored by
Lisa Kim
Joel
Przemko Robakowski
parent
ec6d110f21
commit
2717c1d2e0
+11
-3
@@ -781,14 +781,22 @@ func (c *Client) GetBotUsers(ctx context.Context) ([]types.User, error) {
|
||||
|
||||
// GetAccessRequests retrieves a list of all access requests matching the provided filter.
|
||||
func (c *Client) GetAccessRequests(ctx context.Context, filter types.AccessRequestFilter) ([]types.AccessRequest, error) {
|
||||
rsp, err := c.grpc.GetAccessRequests(ctx, &filter, c.callOpts...)
|
||||
stream, err := c.grpc.GetAccessRequestsV2(ctx, &filter, c.callOpts...)
|
||||
if err != nil {
|
||||
return nil, trail.FromGRPC(err)
|
||||
}
|
||||
reqs := make([]types.AccessRequest, 0, len(rsp.AccessRequests))
|
||||
for _, req := range rsp.AccessRequests {
|
||||
var reqs []types.AccessRequest
|
||||
for {
|
||||
req, err := stream.Recv()
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
return nil, trail.FromGRPC(err)
|
||||
}
|
||||
reqs = append(reqs, req)
|
||||
}
|
||||
|
||||
return reqs, nil
|
||||
}
|
||||
|
||||
|
||||
+432
-364
File diff suppressed because it is too large
Load Diff
@@ -1796,7 +1796,10 @@ service AuthService {
|
||||
rpc IsMFARequired(IsMFARequiredRequest) returns (IsMFARequiredResponse);
|
||||
|
||||
// GetAccessRequests gets all pending access requests.
|
||||
// DEPRECATED, DELETE IN 11.0.0: Use GetAccessRequestsV2 instead.
|
||||
rpc GetAccessRequests(types.AccessRequestFilter) returns (AccessRequests);
|
||||
// GetAccessRequestsV2 gets all pending access requests.
|
||||
rpc GetAccessRequestsV2(types.AccessRequestFilter) returns (stream types.AccessRequestV3);
|
||||
// CreateAccessRequest creates a new access request.
|
||||
rpc CreateAccessRequest(types.AccessRequestV3) returns (google.protobuf.Empty);
|
||||
// DeleteAccessRequest deletes an access request.
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
/*
|
||||
Copyright 2022 Gravitational, Inc.
|
||||
|
||||
Licensed under the Apache License, Version 2.0 (the "License");
|
||||
you may not use this file except in compliance with the License.
|
||||
You may obtain a copy of the License at
|
||||
|
||||
http://www.apache.org/licenses/LICENSE-2.0
|
||||
|
||||
Unless required by applicable law or agreed to in writing, software
|
||||
distributed under the License is distributed on an "AS IS" BASIS,
|
||||
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
See the License for the specific language governing permissions and
|
||||
limitations under the License.
|
||||
*/
|
||||
|
||||
package integration
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/user"
|
||||
"runtime"
|
||||
"syscall"
|
||||
"testing"
|
||||
|
||||
"github.com/gravitational/teleport/lib/teleagent"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/sys/unix"
|
||||
)
|
||||
|
||||
func TestAgentSocketPermissions(t *testing.T) {
|
||||
if !isRoot() {
|
||||
t.Skip("This test will be skipped because tests are not being run as root.")
|
||||
}
|
||||
|
||||
agentServer := teleagent.NewServer(nil)
|
||||
|
||||
agentServer.SetTestPermissions(func() {
|
||||
// ListenUnixSocket should not have its uid changed from root
|
||||
require.True(t, isRoot())
|
||||
|
||||
done := make(chan struct{})
|
||||
|
||||
// Start goroutine to attempt privilege escalation during
|
||||
// permission updates on the unix socket.
|
||||
//
|
||||
// For each step of permission updating, it should be impossible
|
||||
// for the user to unlink/remove the socket. If they can unlink
|
||||
// or remove the socket, then it could be replaced with a symlink
|
||||
// which can be used to acquire the permissions of the original socket.
|
||||
go func() {
|
||||
defer close(done)
|
||||
|
||||
// Update uid to nonroot
|
||||
_, _, serr := syscall.Syscall(syscall.SYS_SETUID, 1000, 0, 0)
|
||||
require.Zero(t, serr)
|
||||
require.True(t, !isRoot())
|
||||
|
||||
err := unix.Unlink(agentServer.Path)
|
||||
require.Error(t, err)
|
||||
err = os.Remove(agentServer.Path)
|
||||
require.Error(t, err)
|
||||
err = os.Rename(agentServer.Path, agentServer.Path)
|
||||
require.Error(t, err)
|
||||
}()
|
||||
<-done
|
||||
|
||||
// ListenUnixSocket should not have its uid changed from root
|
||||
require.True(t, isRoot())
|
||||
})
|
||||
|
||||
nonRoot, err := user.LookupId("1000")
|
||||
require.NoError(t, err)
|
||||
|
||||
// lock goroutine to root so that ListenUnixSocket doesn't
|
||||
// pick up the syscall in the testPermissions func
|
||||
runtime.LockOSThread()
|
||||
defer runtime.UnlockOSThread()
|
||||
|
||||
err = agentServer.ListenUnixSocket("test", "sock.agent", nonRoot)
|
||||
require.NoError(t, err)
|
||||
}
|
||||
@@ -1292,6 +1292,7 @@ func (p *pack) makeWebapiRequest(method, endpoint string, payload []byte) (int,
|
||||
Value: p.webCookie,
|
||||
})
|
||||
req.Header.Add("Authorization", fmt.Sprintf("Bearer %v", p.webToken))
|
||||
req.Header.Add("Content-Type", "application/json")
|
||||
|
||||
statusCode, body, err := p.sendRequest(req, nil)
|
||||
return statusCode, []byte(body), trace.Wrap(err)
|
||||
|
||||
+4
-16
@@ -1708,20 +1708,8 @@ func externalSSHCommand(o commandOptions) (*exec.Cmd, error) {
|
||||
// clobber your system agent.
|
||||
func createAgent(me *user.User, privateKeyByte []byte, certificateBytes []byte) (*teleagent.AgentServer, string, string, error) {
|
||||
// create a path to the unix socket
|
||||
sockDir, err := os.MkdirTemp("", "int-test")
|
||||
if err != nil {
|
||||
return nil, "", "", trace.Wrap(err)
|
||||
}
|
||||
sockPath := filepath.Join(sockDir, "agent.sock")
|
||||
|
||||
uid, err := strconv.Atoi(me.Uid)
|
||||
if err != nil {
|
||||
return nil, "", "", trace.Wrap(err)
|
||||
}
|
||||
gid, err := strconv.Atoi(me.Gid)
|
||||
if err != nil {
|
||||
return nil, "", "", trace.Wrap(err)
|
||||
}
|
||||
sockDirName := "int-test"
|
||||
sockName := "agent.sock"
|
||||
|
||||
// transform the key and certificate bytes into something the agent can understand
|
||||
publicKey, _, _, _, err := ssh.ParseAuthorizedKey(certificateBytes)
|
||||
@@ -1750,13 +1738,13 @@ func createAgent(me *user.User, privateKeyByte []byte, certificateBytes []byte)
|
||||
})
|
||||
|
||||
// start the SSH agent
|
||||
err = teleAgent.ListenUnixSocket(sockPath, uid, gid, 0o600)
|
||||
err = teleAgent.ListenUnixSocket(sockDirName, sockName, me)
|
||||
if err != nil {
|
||||
return nil, "", "", trace.Wrap(err)
|
||||
}
|
||||
go teleAgent.Serve()
|
||||
|
||||
return teleAgent, sockDir, sockPath, nil
|
||||
return teleAgent, teleAgent.Dir, teleAgent.Path, nil
|
||||
}
|
||||
|
||||
func closeAgent(teleAgent *teleagent.AgentServer, socketDirPath string) error {
|
||||
|
||||
@@ -117,6 +117,7 @@ func TestUpsertServer(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
req := httptest.NewRequest(http.MethodPost, "http://localhost", bytes.NewReader(body))
|
||||
req.RemoteAddr = remoteAddr
|
||||
req.Header.Add("Content-Type", "application/json")
|
||||
|
||||
_, err = new(APIServer).upsertServer(s, tt.role, req, httprouter.Params{httprouter.Param{Key: "namespace", Value: apidefaults.Namespace}})
|
||||
tt.assertErr(t, err)
|
||||
|
||||
@@ -543,6 +543,7 @@ func (g *GRPCServer) GetUsers(req *proto.GetUsersRequest, stream proto.AuthServi
|
||||
return nil
|
||||
}
|
||||
|
||||
// DEPRECATED, DELETE IN 11.0.0: Use GetAccessRequestsV2 instead.
|
||||
func (g *GRPCServer) GetAccessRequests(ctx context.Context, f *types.AccessRequestFilter) (*proto.AccessRequests, error) {
|
||||
auth, err := g.authenticate(ctx)
|
||||
if err != nil {
|
||||
@@ -570,6 +571,34 @@ func (g *GRPCServer) GetAccessRequests(ctx context.Context, f *types.AccessReque
|
||||
}, nil
|
||||
}
|
||||
|
||||
func (g *GRPCServer) GetAccessRequestsV2(f *types.AccessRequestFilter, stream proto.AuthService_GetAccessRequestsV2Server) error {
|
||||
ctx := stream.Context()
|
||||
auth, err := g.authenticate(ctx)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
var filter types.AccessRequestFilter
|
||||
if f != nil {
|
||||
filter = *f
|
||||
}
|
||||
reqs, err := auth.ServerWithRoles.GetAccessRequests(ctx, filter)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
for _, req := range reqs {
|
||||
r, ok := req.(*types.AccessRequestV3)
|
||||
if !ok {
|
||||
err = trace.BadParameter("unexpected access request type %T", req)
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
if err := stream.Send(r); err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (g *GRPCServer) CreateAccessRequest(ctx context.Context, req *types.AccessRequestV3) (*empty.Empty, error) {
|
||||
auth, err := g.authenticate(ctx)
|
||||
if err != nil {
|
||||
|
||||
@@ -21,6 +21,7 @@ package httplib
|
||||
import (
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"mime"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -127,6 +128,18 @@ func WithCSRFProtection(fn HandlerFunc) httprouter.Handle {
|
||||
// ReadJSON reads HTTP json request and unmarshals it
|
||||
// into passed interface{} obj
|
||||
func ReadJSON(r *http.Request, val interface{}) error {
|
||||
// Check content type to mitigate CSRF attack.
|
||||
contentType, _, err := mime.ParseMediaType(r.Header.Get("Content-Type"))
|
||||
if err != nil {
|
||||
log.Warningf("Error parsing media type for reading JSON: %v", err)
|
||||
return trace.BadParameter("invalid request")
|
||||
}
|
||||
|
||||
if contentType != "application/json" {
|
||||
log.Warningf("Invalid HTTP request header content-type %q for reading JSON", contentType)
|
||||
return trace.BadParameter("invalid request")
|
||||
}
|
||||
|
||||
data, err := utils.ReadAtMost(r.Body, teleport.MaxHTTPRequestSize)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
|
||||
@@ -17,11 +17,15 @@ limitations under the License.
|
||||
package httplib
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/julienschmidt/httprouter"
|
||||
"github.com/stretchr/testify/require"
|
||||
. "gopkg.in/check.v1"
|
||||
)
|
||||
|
||||
@@ -77,3 +81,59 @@ func (h *testHandler) postSessionChunkNamespace(w http.ResponseWriter, r *http.R
|
||||
h.capturedID = p.ByName("id")
|
||||
return "ok", nil
|
||||
}
|
||||
|
||||
func TestReadJSON_ContentType(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
type TestJSON struct {
|
||||
Name string `json:"name"`
|
||||
Age int `json:"age"`
|
||||
}
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
contentType string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "empty value",
|
||||
contentType: "",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "invalid type",
|
||||
contentType: "multipart/form-data",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "just type/subtype",
|
||||
contentType: "application/json",
|
||||
},
|
||||
{
|
||||
name: "type/subtype with params",
|
||||
contentType: "application/json; charset=utf-8",
|
||||
},
|
||||
}
|
||||
|
||||
body := TestJSON{Name: "foo", Age: 60}
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
payloadBuf := new(bytes.Buffer)
|
||||
require.NoError(t, json.NewEncoder(payloadBuf).Encode(body))
|
||||
|
||||
httpReq, err := http.NewRequest("", "", payloadBuf)
|
||||
require.NoError(t, err)
|
||||
httpReq.Header.Add("Content-Type", tc.contentType)
|
||||
|
||||
output := TestJSON{}
|
||||
err = ReadJSON(httpReq, &output)
|
||||
if tc.wantErr {
|
||||
require.True(t, strings.Contains(err.Error(), "invalid request"))
|
||||
require.Empty(t, output)
|
||||
} else {
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, body, output)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -32,6 +32,8 @@ import (
|
||||
"github.com/vulcand/predicate"
|
||||
)
|
||||
|
||||
const maxAccessRequestReasonSize = 4096
|
||||
|
||||
// ValidateAccessRequest validates the AccessRequest and sets default values
|
||||
func ValidateAccessRequest(ar types.AccessRequest) error {
|
||||
if err := ar.CheckAndSetDefaults(); err != nil {
|
||||
@@ -41,6 +43,12 @@ func ValidateAccessRequest(ar types.AccessRequest) error {
|
||||
if err != nil {
|
||||
return trace.BadParameter("invalid access request id %q", ar.GetName())
|
||||
}
|
||||
if len(ar.GetRequestReason()) > maxAccessRequestReasonSize {
|
||||
return trace.BadParameter("access request reason is too long, max %v bytes", maxAccessRequestReasonSize)
|
||||
}
|
||||
if len(ar.GetResolveReason()) > maxAccessRequestReasonSize {
|
||||
return trace.BadParameter("access request resolve reason is too long, max %v bytes", maxAccessRequestReasonSize)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -519,6 +519,20 @@ func TestReviewThresholds(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestMaxLength tests that we reject too large access requests.
|
||||
func TestMaxLength(t *testing.T) {
|
||||
req, err := types.NewAccessRequest("some-id", "dave", "dictator", "never")
|
||||
require.NoError(t, err)
|
||||
|
||||
var s []byte
|
||||
for i := 0; i <= maxAccessRequestReasonSize; i++ {
|
||||
s = append(s, 'a')
|
||||
}
|
||||
|
||||
req.SetRequestReason(string(s))
|
||||
require.Error(t, ValidateAccessRequest(req))
|
||||
}
|
||||
|
||||
// TestThresholdReviewFilter verifies basic filter syntax.
|
||||
func TestThresholdReviewFilter(t *testing.T) {
|
||||
|
||||
|
||||
@@ -26,7 +26,6 @@ import (
|
||||
"net"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
@@ -969,42 +968,24 @@ func (s *Server) serveAgent(ctx *srv.ServerContext) error {
|
||||
if err != nil {
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
uid, err := strconv.Atoi(systemUser.Uid)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
gid, err := strconv.Atoi(systemUser.Gid)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
pid := os.Getpid()
|
||||
|
||||
// build the socket path and set permissions
|
||||
socketDir, err := os.MkdirTemp(os.TempDir(), "teleport-")
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
dirCloser := &utils.RemoveDirCloser{Path: socketDir}
|
||||
socketPath := filepath.Join(socketDir, fmt.Sprintf("teleport-%v.socket", pid))
|
||||
if err := os.Chown(socketDir, uid, gid); err != nil {
|
||||
if err := dirCloser.Close(); err != nil {
|
||||
log.Warnf("failed to remove directory: %v", err)
|
||||
}
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
socketDir := "teleport"
|
||||
socketName := fmt.Sprintf("teleport-%v.socket", pid)
|
||||
|
||||
// start an agent server on a unix socket. each incoming connection
|
||||
// will result in a separate agent request.
|
||||
agentServer := teleagent.NewServer(ctx.Parent().StartAgentChannel)
|
||||
err = agentServer.ListenUnixSocket(socketPath, uid, gid, 0600)
|
||||
err = agentServer.ListenUnixSocket(socketDir, socketName, systemUser)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
ctx.Parent().SetEnv(teleport.SSHAuthSock, socketPath)
|
||||
|
||||
ctx.Parent().SetEnv(teleport.SSHAuthSock, agentServer.Path)
|
||||
ctx.Parent().SetEnv(teleport.SSHAgentPID, fmt.Sprintf("%v", pid))
|
||||
ctx.Parent().AddCloser(agentServer)
|
||||
ctx.Parent().AddCloser(dirCloser)
|
||||
ctx.Debugf("Starting agent server for Teleport user %v and socket %v.", ctx.Identity.TeleportUser, socketPath)
|
||||
ctx.Debugf("Starting agent server for Teleport user %v and socket %v.", ctx.Identity.TeleportUser, agentServer.Path)
|
||||
go func() {
|
||||
if err := agentServer.Serve(); err != nil {
|
||||
ctx.Errorf("agent server for user %q stopped: %v", ctx.Identity.TeleportUser, err)
|
||||
|
||||
@@ -243,8 +243,13 @@ func (g *TermManager) AddReader(name string, r io.Reader) {
|
||||
}
|
||||
}
|
||||
|
||||
g.incoming <- buf[:n]
|
||||
g.mu.Lock()
|
||||
|
||||
if g.on {
|
||||
g.mu.Unlock()
|
||||
g.incoming <- buf[:n]
|
||||
g.mu.Lock()
|
||||
}
|
||||
if g.isClosed() || g.readerState[name] {
|
||||
g.mu.Unlock()
|
||||
return
|
||||
|
||||
@@ -17,12 +17,14 @@ limitations under the License.
|
||||
package srv
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/rand"
|
||||
"io"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/sync/semaphore"
|
||||
)
|
||||
|
||||
func TestCTRLCPassthrough(t *testing.T) {
|
||||
@@ -86,3 +88,42 @@ func TestBufferedKept(t *testing.T) {
|
||||
kept := data[len(data)-maxPausedHistoryBytes:]
|
||||
require.Equal(t, m.buffer, kept)
|
||||
}
|
||||
|
||||
type funcReader func(p []byte) (n int, err error)
|
||||
|
||||
func (f funcReader) Read(p []byte) (n int, err error) {
|
||||
return f(p)
|
||||
}
|
||||
|
||||
func TestNoReadWhenOff(t *testing.T) {
|
||||
m := NewTermManager()
|
||||
|
||||
r, w := io.Pipe()
|
||||
data := make([]byte, 90)
|
||||
n, err := rand.Read(data)
|
||||
require.NoError(t, err)
|
||||
|
||||
s := semaphore.NewWeighted(1)
|
||||
require.NoError(t, s.Acquire(context.Background(), 1))
|
||||
var fr funcReader = func(p []byte) (n int, err error) {
|
||||
n, err = r.Read(p)
|
||||
s.Release(1)
|
||||
return
|
||||
}
|
||||
|
||||
m.AddReader("reader", fr)
|
||||
|
||||
_, err = w.Write(data[:n])
|
||||
require.NoError(t, err)
|
||||
|
||||
require.NoError(t, s.Acquire(context.Background(), 1))
|
||||
m.On()
|
||||
|
||||
_, err = w.Write([]byte{1, 2, 3})
|
||||
require.NoError(t, err)
|
||||
|
||||
n, err = m.Read(data)
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Equal(t, []byte{1, 2, 3}, data[:n])
|
||||
}
|
||||
|
||||
+86
-14
@@ -18,8 +18,12 @@ import (
|
||||
"io"
|
||||
"net"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"strconv"
|
||||
"time"
|
||||
|
||||
"github.com/gravitational/teleport"
|
||||
"github.com/gravitational/teleport/lib/utils"
|
||||
|
||||
"github.com/gravitational/trace"
|
||||
@@ -57,7 +61,12 @@ type Getter func() (Agent, error)
|
||||
type AgentServer struct {
|
||||
getAgent Getter
|
||||
listener net.Listener
|
||||
path string
|
||||
Path string
|
||||
Dir string
|
||||
// testPermissions is a test provided function used to test
|
||||
// the permissions of the agent server during potentially
|
||||
// vulnerable moments in permission changes.
|
||||
testPermissions func()
|
||||
}
|
||||
|
||||
// NewServer returns new instance of agent server
|
||||
@@ -65,25 +74,83 @@ func NewServer(getter Getter) *AgentServer {
|
||||
return &AgentServer{getAgent: getter}
|
||||
}
|
||||
|
||||
// ListenUnixSocket starts listening and serving agent assuming that
|
||||
func (a *AgentServer) ListenUnixSocket(path string, uid, gid int, mode os.FileMode) error {
|
||||
l, err := net.Listen("unix", path)
|
||||
// ListenUnixSocket starts listening on a new unix socket.
|
||||
func (a *AgentServer) ListenUnixSocket(sockDir, sockName string, user *user.User) error {
|
||||
// Create a temp directory to hold the agent socket.
|
||||
sockDir, err := os.MkdirTemp(os.TempDir(), sockDir+"-")
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
if err := os.Chown(path, uid, gid); err != nil {
|
||||
l.Close()
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
if err := os.Chmod(path, mode); err != nil {
|
||||
l.Close()
|
||||
return trace.ConvertSystemError(err)
|
||||
a.Dir = sockDir
|
||||
|
||||
sockPath := filepath.Join(sockDir, sockName)
|
||||
l, err := net.Listen("unix", sockPath)
|
||||
if err != nil {
|
||||
a.Close()
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
a.listener = l
|
||||
a.path = path
|
||||
a.Path = sockPath
|
||||
|
||||
if err := a.updatePermissions(user); err != nil {
|
||||
a.Close()
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Update the agent server permissions to give the user sole ownership
|
||||
// of the socket path and prevent other users from accessing or seeing it.
|
||||
func (a *AgentServer) updatePermissions(user *user.User) error {
|
||||
// Tests may provide a testPermissions function to test potentially
|
||||
// vulnerable moments during permission updating.
|
||||
testPermissions := func() {
|
||||
if a.testPermissions != nil {
|
||||
a.testPermissions()
|
||||
}
|
||||
}
|
||||
|
||||
testPermissions()
|
||||
|
||||
uid, err := strconv.Atoi(user.Uid)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
gid, err := strconv.Atoi(user.Gid)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
testPermissions()
|
||||
|
||||
if err := os.Chmod(a.Path, teleport.FileMaskOwnerOnly); err != nil {
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
|
||||
testPermissions()
|
||||
|
||||
if err := os.Chown(a.Path, uid, gid); err != nil {
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
|
||||
testPermissions()
|
||||
|
||||
// To prevent a privilege escalation attack, this must occur
|
||||
// after the socket permissions are updated.
|
||||
if err := os.Chown(a.Dir, uid, gid); err != nil {
|
||||
return trace.ConvertSystemError(err)
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// SetTestPermissions can be used by tests to test agent socket permissions.
|
||||
func (a *AgentServer) SetTestPermissions(testPermissions func()) {
|
||||
a.testPermissions = testPermissions
|
||||
}
|
||||
|
||||
// Serve starts serving on the listener, assumes that Listen was called before
|
||||
func (a *AgentServer) Serve() error {
|
||||
if a.listener == nil {
|
||||
@@ -157,8 +224,13 @@ func (a *AgentServer) Close() error {
|
||||
errors = append(errors, trace.ConvertSystemError(err))
|
||||
}
|
||||
}
|
||||
if a.path != "" {
|
||||
if err := os.Remove(a.path); err != nil {
|
||||
if a.Path != "" {
|
||||
if err := os.Remove(a.Path); err != nil {
|
||||
errors = append(errors, trace.ConvertSystemError(err))
|
||||
}
|
||||
}
|
||||
if a.Dir != "" {
|
||||
if err := os.RemoveAll(a.Dir); err != nil {
|
||||
errors = append(errors, trace.ConvertSystemError(err))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -65,17 +65,6 @@ func MkdirAll(targetDirectory string, mode os.FileMode) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// RemoveDirCloser removes directory and all it's contents
|
||||
// when Close is called
|
||||
type RemoveDirCloser struct {
|
||||
Path string
|
||||
}
|
||||
|
||||
// Close removes directory and all it's contents
|
||||
func (r *RemoveDirCloser) Close() error {
|
||||
return trace.ConvertSystemError(os.RemoveAll(r.Path))
|
||||
}
|
||||
|
||||
// IsDir is a helper function to quickly check if a given path is a valid directory
|
||||
func IsDir(path string) bool {
|
||||
fi, err := os.Stat(path)
|
||||
|
||||
@@ -739,6 +739,28 @@ func TestPasswordChange(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
}
|
||||
|
||||
// TestValidateBearerToken tests that the bearer token's user name
|
||||
// matches the user name on the cookie.
|
||||
func TestValidateBearerToken(t *testing.T) {
|
||||
t.Parallel()
|
||||
env := newWebPack(t, 1)
|
||||
proxy := env.proxies[0]
|
||||
pack1 := proxy.authPack(t, "user1")
|
||||
pack2 := proxy.authPack(t, "user2")
|
||||
|
||||
// Swap pack1's session token with pack2's sessionToken
|
||||
jar, err := cookiejar.New(nil)
|
||||
require.NoError(t, err)
|
||||
pack1.clt = proxy.newClient(t, roundtrip.BearerAuth(pack2.session.Token), roundtrip.CookieJar(jar))
|
||||
jar.SetCookies(&proxy.webURL, pack1.cookies)
|
||||
|
||||
// Auth protected endpoint.
|
||||
req := changePasswordReq{}
|
||||
_, err = pack1.clt.PutJSON(context.Background(), pack1.clt.Endpoint("webapi", "users", "password"), req)
|
||||
require.True(t, trace.IsAccessDenied(err))
|
||||
require.True(t, strings.Contains(err.Error(), "bad bearer token"))
|
||||
}
|
||||
|
||||
func TestWebSessionsBadInput(t *testing.T) {
|
||||
t.Parallel()
|
||||
s := newWebSuite(t)
|
||||
|
||||
+12
-2
@@ -111,11 +111,21 @@ func (c *SessionContext) Invalidate() error {
|
||||
}
|
||||
|
||||
func (c *SessionContext) validateBearerToken(ctx context.Context, token string) error {
|
||||
_, err := c.parent.readBearerToken(ctx, types.GetWebTokenRequest{
|
||||
fetchedToken, err := c.parent.readBearerToken(ctx, types.GetWebTokenRequest{
|
||||
User: c.user,
|
||||
Token: token,
|
||||
})
|
||||
return trace.Wrap(err)
|
||||
if err != nil {
|
||||
return trace.Wrap(err)
|
||||
}
|
||||
|
||||
if fetchedToken.GetUser() != c.user {
|
||||
c.log.Warnf("Failed validating bearer token: the user[%s] in bearer token[%s] did not match the user[%s] for session[%s]",
|
||||
fetchedToken.GetUser(), token, c.user, c.GetSessionID())
|
||||
return trace.AccessDenied("access denied")
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func (c *SessionContext) addRemoteClient(siteName string, remoteClient auth.ClientI) {
|
||||
|
||||
@@ -184,6 +184,7 @@ func newRequest(t *testing.T, body interface{}) *http.Request {
|
||||
|
||||
req, err := http.NewRequest("", "", bytes.NewBuffer(reqBody))
|
||||
require.Nil(t, err)
|
||||
req.Header.Add("Content-Type", "application/json")
|
||||
|
||||
return req
|
||||
}
|
||||
|
||||
+8
-18
@@ -29,6 +29,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gravitational/teleport"
|
||||
"github.com/gravitational/trace"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/crypto/ssh/agent"
|
||||
@@ -472,22 +473,8 @@ func createAgent(t *testing.T) string {
|
||||
user, err := user.Current()
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create own tmp dir instead of using t.TmpDir
|
||||
// because net.Listen("unix", path) has dir path length limitation and
|
||||
// the t.TmpDir calls creates tmp dir with test name.
|
||||
sockDir, err := os.MkdirTemp("", "test")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(func() {
|
||||
os.RemoveAll(sockDir)
|
||||
})
|
||||
|
||||
sockPath := filepath.Join(sockDir, "agent.sock")
|
||||
t.Setenv("SSH_AUTH_SOCK", sockPath)
|
||||
|
||||
uid, err := strconv.Atoi(user.Uid)
|
||||
require.NoError(t, err)
|
||||
gid, err := strconv.Atoi(user.Gid)
|
||||
require.NoError(t, err)
|
||||
sockDir := "test"
|
||||
sockName := "agent.sock"
|
||||
|
||||
keyring := agent.NewKeyring()
|
||||
teleAgent := teleagent.NewServer(func() (teleagent.Agent, error) {
|
||||
@@ -495,13 +482,16 @@ func createAgent(t *testing.T) string {
|
||||
})
|
||||
|
||||
// Start the SSH agent.
|
||||
err = teleAgent.ListenUnixSocket(sockPath, uid, gid, 0600)
|
||||
err = teleAgent.ListenUnixSocket(sockDir, sockName, user)
|
||||
require.NoError(t, err)
|
||||
go teleAgent.Serve()
|
||||
t.Cleanup(func() {
|
||||
teleAgent.Close()
|
||||
})
|
||||
return sockPath
|
||||
|
||||
t.Setenv(teleport.SSHAuthSock, teleAgent.Path)
|
||||
|
||||
return teleAgent.Path
|
||||
}
|
||||
|
||||
func mustLogin(t *testing.T, s *suite) {
|
||||
|
||||
Reference in New Issue
Block a user