Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep App Session secrets secret #39853

Merged
merged 11 commits into from
Mar 29, 2024
7 changes: 7 additions & 0 deletions api/types/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ func (ws *WebSessionV2) GetIdleTimeout() time.Duration {

// WithoutSecrets returns a copy of the WebSession without secrets.
func (ws *WebSessionV2) WithoutSecrets() WebSession {
// With gogoproto, proto.Clone and proto.Merge panic with
// nonnullabe stdtime types unless they are in UTC.
// https://github.com/gogo/protobuf/issues/519
ws.Spec.Expires = ws.Spec.Expires.UTC()
ws.Spec.LoginTime = ws.Spec.LoginTime.UTC()
ws.Spec.BearerTokenExpires = ws.Spec.BearerTokenExpires.UTC()

cp := proto.Clone(ws).(*WebSessionV2)
cp.Spec.Priv = nil
cp.Spec.SAMLSession = nil
Expand Down
29 changes: 23 additions & 6 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -5345,13 +5345,29 @@ func (a *ServerWithRoles) GetAppSession(ctx context.Context, req types.GetAppSes
if err != nil {
return nil, trace.Wrap(err)
}
// Users can only fetch their own app sessions.
if err := a.currentUserAction(session.GetUser()); err != nil {
if err := a.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead); err != nil {
return nil, trace.Wrap(err)

authErr := a.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead)
if authErr == nil {
return session, nil
}

// Users can fetch their own app sessions without secrets.
if err := a.currentUserAction(session.GetUser()); err == nil {
// TODO (Joerger): DELETE IN 17.0.0
// App Session secrets should not be returned to the user. We only do this
// here for backwards compatibility with `tsh proxy azure`, which uses the
// app session key to sign JWT tokens with Azure claims. This check means
// that `tsh proxy azure` will fail for old clients when used with Per-session
// MFA or Hardware Key support, which is planned for release in v16.0.0.
identity := a.context.Identity.GetIdentity()
if !identity.IsMFAVerified() && identity.PrivateKeyPolicy != keys.PrivateKeyPolicyWebSession {
return session, nil
}

return session.WithoutSecrets(), nil
}
return session, nil

return nil, trace.Wrap(authErr)
}

// GetSnowflakeSession gets a Snowflake web session.
Expand Down Expand Up @@ -5445,7 +5461,8 @@ func (a *ServerWithRoles) CreateAppSession(ctx context.Context, req *proto.Creat
if err != nil {
return nil, trace.Wrap(err)
}
return session, nil

return session.WithoutSecrets(), nil
Tener marked this conversation as resolved.
Show resolved Hide resolved
}

// CreateSnowflakeSession creates a Snowflake web session.
Expand Down
4 changes: 2 additions & 2 deletions lib/auth/sessions.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ func (a *Server) CreateAppSession(ctx context.Context, req *proto.CreateAppSessi
Priv: privateKey,
Pub: certs.SSH,
TLSCert: certs.TLS,
LoginTime: a.clock.Now(),
Expires: a.clock.Now().Add(ttl),
LoginTime: a.clock.Now().UTC(),
Expires: a.clock.Now().UTC().Add(ttl),
BearerToken: bearer,
})
if err != nil {
Expand Down
28 changes: 0 additions & 28 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -2603,34 +2603,6 @@ func (tc *TeleportClient) CreateAppSession(ctx context.Context, req *proto.Creat
return ws, nil
}

// GetAppSession returns an existing application access session.
func (tc *TeleportClient) GetAppSession(ctx context.Context, req types.GetAppSessionRequest) (types.WebSession, error) {
ctx, span := tc.Tracer.Start(
ctx,
"teleportClient/GetAppSession",
oteltrace.WithSpanKind(oteltrace.SpanKindClient),
oteltrace.WithAttributes(
attribute.String("session", req.SessionID),
),
)
defer span.End()

clusterClient, err := tc.ConnectToCluster(ctx)
if err != nil {
return nil, trace.Wrap(err)
}
defer clusterClient.Close()

rootAuthClient, err := clusterClient.ConnectToRootCluster(ctx)
if err != nil {
return nil, trace.Wrap(err)
}
defer rootAuthClient.Close()

ws, err := rootAuthClient.GetAppSession(ctx, req)
return ws, trace.Wrap(err)
}

// DeleteAppSession removes the specified application access session.
func (tc *TeleportClient) DeleteAppSession(ctx context.Context, sessionID string) error {
ctx, span := tc.Tracer.Start(
Expand Down
24 changes: 0 additions & 24 deletions lib/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -835,30 +835,6 @@ func (proxy *ProxyClient) CreateAppSession(ctx context.Context, req *proto.Creat
return ws, nil
}

// GetAppSession creates a new application access session.
func (proxy *ProxyClient) GetAppSession(ctx context.Context, req types.GetAppSessionRequest) (types.WebSession, error) {
ctx, span := proxy.Tracer.Start(
ctx,
"proxyClient/GetAppSession",
oteltrace.WithSpanKind(oteltrace.SpanKindClient),
)
defer span.End()

clusterName, err := proxy.RootClusterName(ctx)
if err != nil {
return nil, trace.Wrap(err)
}
authClient, err := proxy.ConnectToCluster(ctx, clusterName)
if err != nil {
return nil, trace.Wrap(err)
}
ws, err := authClient.GetAppSession(ctx, req)
if err != nil {
return nil, trace.Wrap(err)
}
return ws, nil
}

// DeleteAppSession removes the specified application access session.
func (proxy *ProxyClient) DeleteAppSession(ctx context.Context, sessionID string) error {
ctx, span := proxy.Tracer.Start(
Expand Down
49 changes: 26 additions & 23 deletions lib/web/app/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ import (
"github.com/gravitational/teleport/api/types"
apievents "github.com/gravitational/teleport/api/types/events"
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/auth/native"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/reversetunnelclient"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
)
Expand Down Expand Up @@ -80,7 +80,7 @@ func TestAuthPOST(t *testing.T) {
stateValue := fmt.Sprintf("%s_%s", secretToken, cookieID)
appCookieValue := "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b"

fakeClock := clockwork.NewFakeClockAt(time.Date(2017, 05, 10, 18, 53, 0, 0, time.UTC))
fakeClock := clockwork.NewFakeClock()
clusterName := "test-cluster"
publicAddr := "app.example.com"
// Generate CA TLS key and cert with the cluster and application DNS.
Expand Down Expand Up @@ -249,7 +249,7 @@ func TestAuthPOST_Legacy(t *testing.T) {
cookieValue = "5588e2be54a2834b4f152c56bafcd789f53b15477129d2ab4044e9a3c1bf0f3b" // random value we set in the header and expect to get back as a cookie
)

fakeClock := clockwork.NewFakeClockAt(time.Date(2017, 05, 10, 18, 53, 0, 0, time.UTC))
fakeClock := clockwork.NewFakeClock()
clusterName := "test-cluster"
publicAddr := "proxy.goteleport.com:443"

Expand Down Expand Up @@ -681,7 +681,7 @@ func TestHealthCheckAppServer(t *testing.T) {
)
require.NoError(t, err)

fakeClock := clockwork.NewFakeClockAt(time.Date(2017, 05, 10, 18, 53, 0, 0, time.UTC))
fakeClock := clockwork.NewFakeClock()
appSession := createAppSession(t, fakeClock, key, cert, clusterName, tc.publicAddr)
authClient := &mockAuthClient{
clusterName: clusterName,
Expand Down Expand Up @@ -870,7 +870,6 @@ func (r *fakeRemoteListener) Accept() (net.Conn, error) {
}

return conn, nil

}

func (r *fakeRemoteListener) Close() error {
Expand All @@ -883,9 +882,28 @@ func (r *fakeRemoteListener) Addr() net.Addr {

// createAppSession generates a WebSession for an application.
func createAppSession(t *testing.T, clock clockwork.FakeClock, caKey, caCert []byte, clusterName, publicAddr string) types.WebSession {
key, cert := createAppKeyCertPair(t, clock, caKey, caCert, clusterName, publicAddr)
appSession, err := types.NewWebSession(uuid.New().String(), types.KindAppSession, types.WebSessionSpecV2{
User: "testuser",
Priv: key.PrivateKeyPEM(),
Pub: key.MarshalSSHPublicKey(),
TLSCert: cert,
Expires: clock.Now().Add(5 * time.Minute),
BearerToken: "abc123",
})
require.NoError(t, err)

return appSession
}

// createAppKeyCertPair creates and a client key and signed app cert for the client key
func createAppKeyCertPair(t *testing.T, clock clockwork.FakeClock, caKey, caCert []byte, clusterName, publicAddr string) (*keys.PrivateKey, []byte) {
tlsCA, err := tlsca.FromKeys(caCert, caKey)
require.NoError(t, err)

privateKey, err := native.GeneratePrivateKey()
require.NoError(t, err)

// Generate the identity with a `RouteToApp` option.
subj, err := (&tlsca.Identity{
Username: "testuser",
Expand All @@ -898,30 +916,15 @@ func createAppSession(t *testing.T, clock clockwork.FakeClock, caKey, caCert []b
}).Subject()
require.NoError(t, err)

// Generate public and private keys for the application request certificate.
priv, pub, err := testauthority.New().GetNewKeyPairFromPool()
require.NoError(t, err)
cryptoPubKey, err := sshutils.CryptoPublicKey(pub)
require.NoError(t, err)

cert, err := tlsCA.GenerateCertificate(tlsca.CertificateRequest{
Clock: clock,
PublicKey: cryptoPubKey,
PublicKey: privateKey.Public(),
Subject: subj,
NotAfter: clock.Now().Add(5 * time.Minute),
})
require.NoError(t, err)

appSession, err := types.NewWebSession(uuid.New().String(), types.KindAppSession, types.WebSessionSpecV2{
User: "testuser",
Priv: priv,
TLSCert: cert,
Expires: clock.Now().Add(5 * time.Minute),
BearerToken: "abc123",
})
require.NoError(t, err)

return appSession
return privateKey, cert
}

func createAppServer(t *testing.T, publicAddr string) types.AppServer {
Expand Down
1 change: 1 addition & 0 deletions lib/web/app/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ func (h *Handler) newSession(ctx context.Context, ws types.WebSession) (*session
// Create a rewriting transport that will be used to forward requests.
transport, err := newTransport(&transportConfig{
log: h.log,
clock: h.c.Clock,
proxyClient: h.c.ProxyClient,
accessPoint: h.c.AccessPoint,
cipherSuites: h.c.CipherSuites,
Expand Down
Loading
Loading