Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/types/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"github.com/gravitational/trace"

"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/utils/keys"
)

// WebSessionsGetter provides access to web sessions
Expand Down Expand Up @@ -645,6 +646,14 @@ type NewWebSessionRequest struct {
AccessRequests []string
// RequestedResourceIDs optionally lists requested resources
RequestedResourceIDs []ResourceID
// AttestWebSession optionally attests the web session to meet private key policy requirements.
// This should only be set to true for web sessions that are purely in the purview of the Proxy
// and Auth services. Users should never have direct access to attested web sessions.
AttestWebSession bool
// PrivateKey is a specific private key to use when generating the web sessions' certificates.
// This should be provided when extending an attested web session in order to maintain the
// session attested status.
PrivateKey *keys.PrivateKey
}

// Check validates the request.
Expand Down
50 changes: 33 additions & 17 deletions api/utils/keys/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,25 @@ const (
// hardware key to generate and store their private keys securely, and
// this key must require touch and pin to be accessed and used.
PrivateKeyPolicyHardwareKeyTouchAndPIN PrivateKeyPolicy = "hardware_key_touch_and_pin"
// PrivateKeyPolicyWebSession is a special case used for Web Sessions. This policy
// implies that the client private key and certificate are stored in the Proxy
// Process Memory and Auth Storage. These certs do not leave the Proxy/Auth
// services, but the Web Client receives a Web Cookie which can be used to
// make requests with the server-side client key+cert.
//
// This policy does not provide the same hardware key guarantee as the above policies.
// Instead, this policy must be accompanied by WebAuthn prompts for important operations
// in order to pass hardware key policy requirements.
PrivateKeyPolicyWebSession PrivateKeyPolicy = "web_session"
)

// IsSatisfiedBy returns whether this key policy is satisfied by the given key policy.
func (requiredPolicy PrivateKeyPolicy) IsSatisfiedBy(keyPolicy PrivateKeyPolicy) bool {
// Web sessions are treated as a special case that meets all private key policy requirements.
if keyPolicy == PrivateKeyPolicyWebSession {
return true
}

switch requiredPolicy {
case PrivateKeyPolicyNone:
return true
Expand All @@ -62,6 +77,22 @@ func (requiredPolicy PrivateKeyPolicy) IsSatisfiedBy(keyPolicy PrivateKeyPolicy)
return false
}

func (p PrivateKeyPolicy) isHardwareKeyTouchVerified() bool {
switch p {
case PrivateKeyPolicyHardwareKeyTouch, PrivateKeyPolicyHardwareKeyTouchAndPIN:
return true
}
return false
}

func (p PrivateKeyPolicy) isHardwareKeyPINVerified() bool {
switch p {
case PrivateKeyPolicyHardwareKeyPIN, PrivateKeyPolicyHardwareKeyTouchAndPIN:
return true
}
return false
}

// Deprecated in favor of IsSatisfiedBy.
// TODO(Joerger): delete once reference in /e is replaced.
func (requiredPolicy PrivateKeyPolicy) VerifyPolicy(keyPolicy PrivateKeyPolicy) error {
Expand Down Expand Up @@ -89,29 +120,14 @@ func (p PrivateKeyPolicy) MFAVerified() bool {
return p.isHardwareKeyTouchVerified() || p.isHardwareKeyPINVerified()
}

func (p PrivateKeyPolicy) isHardwareKeyTouchVerified() bool {
switch p {
case PrivateKeyPolicyHardwareKeyTouch, PrivateKeyPolicyHardwareKeyTouchAndPIN:
return true
}
return false
}

func (p PrivateKeyPolicy) isHardwareKeyPINVerified() bool {
switch p {
case PrivateKeyPolicyHardwareKeyPIN, PrivateKeyPolicyHardwareKeyTouchAndPIN:
return true
}
return false
}

func (p PrivateKeyPolicy) validate() error {
switch p {
case PrivateKeyPolicyNone,
PrivateKeyPolicyHardwareKey,
PrivateKeyPolicyHardwareKeyTouch,
PrivateKeyPolicyHardwareKeyPIN,
PrivateKeyPolicyHardwareKeyTouchAndPIN:
PrivateKeyPolicyHardwareKeyTouchAndPIN,
PrivateKeyPolicyWebSession:
return nil
}
return trace.BadParameter("%q is not a valid key policy", p)
Expand Down
15 changes: 13 additions & 2 deletions api/utils/keys/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,28 @@ var (
keys.PrivateKeyPolicyHardwareKeyTouch,
keys.PrivateKeyPolicyHardwareKeyPIN,
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
keys.PrivateKeyPolicyWebSession,
}
hardwareKeyPolicies = []keys.PrivateKeyPolicy{
keys.PrivateKeyPolicyHardwareKey,
keys.PrivateKeyPolicyHardwareKeyTouch,
keys.PrivateKeyPolicyHardwareKeyPIN,
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
keys.PrivateKeyPolicyWebSession,
}
hardwareKeyTouchPolicies = []keys.PrivateKeyPolicy{
keys.PrivateKeyPolicyHardwareKeyTouch,
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
keys.PrivateKeyPolicyWebSession,
}
hardwareKeyPINPolicies = []keys.PrivateKeyPolicy{
keys.PrivateKeyPolicyHardwareKeyPIN,
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
keys.PrivateKeyPolicyWebSession,
}
hardwareKeyTouchAndPINPolicies = []keys.PrivateKeyPolicy{
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
keys.PrivateKeyPolicyWebSession,
}
)

Comment thread
jentfoo marked this conversation as resolved.
Outdated
Expand Down Expand Up @@ -141,8 +146,14 @@ func TestGetPolicyFromSet(t *testing.T) {
},
wantPolicy: keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
}, {
name: "touch and pin policy",
policySet: privateKeyPolicies,
name: "touch and pin policy",
policySet: []keys.PrivateKeyPolicy{
keys.PrivateKeyPolicyNone,
keys.PrivateKeyPolicyHardwareKey,
keys.PrivateKeyPolicyHardwareKeyTouch,
keys.PrivateKeyPolicyHardwareKeyPIN,
keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
},
wantPolicy: keys.PrivateKeyPolicyHardwareKeyTouchAndPIN,
},
}
Expand Down
35 changes: 30 additions & 5 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -3493,6 +3493,14 @@ func (a *Server) ExtendWebSession(ctx context.Context, req WebSessionReq, identi
accessRequests = nil
}

// Create a new web session with the same private key. This way, if the
// original session was an attested web session, the extended session will
// also be an attested web session.
prevKey, err := keys.ParsePrivateKey(prevSession.GetPriv())
if err != nil {
return nil, trace.Wrap(err)
}

sessionTTL := utils.ToTTL(a.clock, expiresAt)
sess, err := a.NewWebSession(ctx, types.NewWebSessionRequest{
User: req.User,
Expand All @@ -3502,6 +3510,7 @@ func (a *Server) ExtendWebSession(ctx context.Context, req WebSessionReq, identi
SessionTTL: sessionTTL,
AccessRequests: accessRequests,
RequestedResourceIDs: allowedResourceIDs,
PrivateKey: prevKey,
})
if err != nil {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -4128,19 +4137,35 @@ func (a *Server) NewWebSession(ctx context.Context, req types.NewWebSessionReque
return nil, trace.Wrap(err)
}

priv, pub, err := native.GenerateKeyPair()
if err != nil {
return nil, trace.Wrap(err)
if req.PrivateKey == nil {
req.PrivateKey, err = native.GeneratePrivateKey()
if err != nil {
return nil, trace.Wrap(err)
}
}

sessionTTL := req.SessionTTL
if sessionTTL == 0 {
sessionTTL = checker.AdjustSessionTTL(apidefaults.CertDuration)
}

if req.AttestWebSession {
// Upsert web session attestation data so that this key's certs
// will be marked with the web session private key policy.
webAttData, err := services.NewWebSessionAttestationData(req.PrivateKey.Public())
if err != nil {
return nil, trace.Wrap(err)
}
if err = a.UpsertKeyAttestationData(ctx, webAttData, sessionTTL); err != nil {
return nil, trace.Wrap(err)
}
}

certs, err := a.generateUserCert(certRequest{
user: userState,
loginIP: req.LoginIP,
ttl: sessionTTL,
publicKey: pub,
publicKey: req.PrivateKey.MarshalSSHPublicKey(),
checker: checker,
traits: req.Traits,
activeRequests: services.RequestIDs{AccessRequests: req.AccessRequests},
Expand All @@ -4165,7 +4190,7 @@ func (a *Server) NewWebSession(ctx context.Context, req types.NewWebSessionReque

sessionSpec := types.WebSessionSpecV2{
User: req.User,
Priv: priv,
Priv: req.PrivateKey.PrivateKeyPEM(),
Pub: certs.SSH,
TLSCert: certs.TLS,
Expires: startTime.UTC().Add(sessionTTL),
Expand Down
6 changes: 2 additions & 4 deletions lib/auth/auth_with_roles.go
Original file line number Diff line number Diff line change
Expand Up @@ -2351,10 +2351,8 @@ func (a *ServerWithRoles) WebSessions() types.WebSessionInterface {

// Get returns the web session specified with req.
func (r *webSessionsWithRoles) Get(ctx context.Context, req types.GetWebSessionRequest) (types.WebSession, error) {
if err := r.c.currentUserAction(req.User); err != nil {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this check not needed?

Copy link
Copy Markdown
Contributor Author

@Joerger Joerger Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the Proxy Role needs to retrieve web sessions for the WebUI to function correctly, so yes the check can be removed.

Previously, this would allow users to retrieve their own web session (and secret private key / certs) directly using the /webapi. With this PR, those secrets could have been used to bypass hardware key requirements.

if err := r.c.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}
if err := r.c.action(apidefaults.Namespace, types.KindWebSession, types.VerbRead); err != nil {
return nil, trace.Wrap(err)
}
return r.ws.Get(ctx, req)
}
Expand Down
9 changes: 8 additions & 1 deletion lib/auth/methods.go
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,14 @@ func (s *Server) AuthenticateWebUser(ctx context.Context, req AuthenticateUserRe
}
}

sess, err := s.createUserWebSession(ctx, user, loginIP)
sess, err := s.CreateWebSessionFromReq(ctx, types.NewWebSessionRequest{
User: user.GetName(),
LoginIP: loginIP,
Roles: user.GetRoles(),
Traits: user.GetTraits(),
LoginTime: s.clock.Now().UTC(),
AttestWebSession: true,
})
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down
18 changes: 18 additions & 0 deletions lib/services/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package services
import (
"context"
"crypto"
"crypto/x509"
"time"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -406,3 +407,20 @@ func LastFailed(x int, attempts []LoginAttempt) bool {
}
return false
}

// NewWebSessionAttestationData creates attestation data for a web session key.
// Inserting data to the Auth server will allow certificates generated for the
// web session key to pass private key policies that are unobtainable in the web
// (hardware key policies). In exchange, these keys must be kept strictly in the
// Auth and Proxy processes and Auth storage. These keys and certs can only be
// retrieved by users in the form of web session cookies.
func NewWebSessionAttestationData(pub crypto.PublicKey) (*keys.AttestationData, error) {
pubDER, err := x509.MarshalPKIXPublicKey(pub)
if err != nil {
return nil, trace.Wrap(err)
}
return &keys.AttestationData{
PublicKeyDER: pubDER,
PrivateKeyPolicy: keys.PrivateKeyPolicyWebSession,
}, nil
}
11 changes: 8 additions & 3 deletions lib/web/files.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/auth"
wantypes "github.com/gravitational/teleport/lib/auth/webauthntypes"
"github.com/gravitational/teleport/lib/client"
Expand Down Expand Up @@ -219,11 +220,17 @@ func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request
return trace.Wrap(err)
}

key, err := client.GenerateRSAKey()
pk, err := keys.ParsePrivateKey(f.sctx.cfg.Session.GetPriv())
if err != nil {
return trace.Wrap(err)
}

key := &client.Key{
PrivateKey: pk,
Cert: f.sctx.cfg.Session.GetPub(),
TLSCert: f.sctx.cfg.Session.GetTLSCert(),
}

// Always acquire certs from the root cluster, that is where both the user and their devices are registered.
cert, err := f.sctx.cfg.RootClient.GenerateUserCerts(httpReq.Context(), proto.UserCertsRequest{
PublicKey: key.MarshalSSHPublicKey(),
Expand All @@ -240,13 +247,11 @@ func (f *fileTransfer) issueSingleUseCert(webauthn string, httpReq *http.Request
}

key.Cert = cert.SSH

am, err := key.AsAuthMethod()
if err != nil {
return trace.Wrap(err)
}

tc.AuthMethods = []ssh.AuthMethod{am}

return nil
}
27 changes: 19 additions & 8 deletions rfd/0080-hardware-key-support.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,24 @@ In this RFD we'll explore both options together, since they are not mutually exc

Hardware key private keys can also be configured to require pin to be used in cryptographical operations. When combined with touch, requiring pin provides a level of authentication security similar to passwordless, as both user presence and a user secret are verified.

#### Web Sessions

Unlike WebAuthn, PIV does not have any native browser support. This means that the WebUI is incompatible with Hardware Key support. We could create custom browser extensions for some of the most commonly used browsers, but this induces too large of a development and maintenance cost to justify currently.

Instead, web sessions will be treated as an exception from Hardware Key support. This exception will only apply to web sessions created through the auth http endpoint `POST /:version/users/:user/web/authenticate`. This is the endpoint used by the WebUI login flow. Web Session created through this endpoint can only be accessed by the Auth and Proxy services. This will result in similar security properties to hardware private keys since the user, or an attacker, has no way to extract web session secrets without direct access to the Proxy/Auth services or Auth storage.

Web sessions created by user-authorized endpoints like the auth http endpoint `POST /:version/users/:user/web/sessions` will still be subject to hardware key restrictions to prevent abuse.

##### Web Session Access

Currently, the auth grpc endpoint `GetWebSession` can be used by a user to retrieve a specific web session, including secrets. This endpoint will be restricted to require `read` permissions for `KindWebSession`, similar to `GetWebSessions`. Users will still be able to retrieve non-secret web session info with the auth http endpoint `GET /:version/users/:user/web/sessions/:sid`.

##### Web Session cookies

Although we can guarantee that web session private key material is safely stored, web session cookies are easy to obtain from a user's browser. Web session cookies can only be used with the HTTP web API (`/webapi`), which provides a subset of functionality provided by the main Auth API to web sessions. Essentially, any functionality available in the WebUI is available through the `/webapi`.

Since MFA will still be required for sessions and admin actions, this is an acceptable tradeoff.

### Server changes

#### Private Key Policy
Expand All @@ -104,6 +122,7 @@ We will start with the following private key policies:
* Unlike touch, pin is not cached explicitly. However, the pin is cached for the duration of a single PIV transaction. PIV transactions take a few seconds to close and can be reclaimed by subsequent PIV connections during the closing period. In this case, when multiple `tsh` commands are run in quick succession, it is as if the pin is cached.
* This policy is intended for rare circumstances where a touch policy can not be configured due to the use of external PIV tools. However, since pin alone does not verify user presence, this option opens the door for remote attacks. When possible, `hardware_key_touch_and_pin` should be used instead of this option.
* `hardware_key_touch_and_pin`: combination of `hardware_key_touch` and `hardware_key_pin`.
* `web_session`: private key stored as a web session in the Auth service storage. This key is only accessible by the Auth and Proxy services. Keys with this policy meet all other key policy requirements.

In the future, we could choose to enforce more things, such as requiring a specific key algorithm.

Expand Down Expand Up @@ -381,14 +400,6 @@ slot=<slot>

`tsh` and Teleport Connect will both support hardware private key login, and `tctl` will be able to use resulting login sessions.

#### Unsupported clients

The WebUI will not be able to support PIV login, since it is browser-based and cannot connect directly to the user's PIV device. If a user with `require_session_mfa: hardware_key` attempts to login on the WebUI, or use an existing login session, it will fail. However, WebUI user registration and password reset logic must still work, regardless of the user's private key policy requirement. After initial registration/reset flow, the user should be directed to a page which notifies them that `tsh` or Teleport Connect must be used.

It may be possible to work around this limitation by introducing a local proxy to connect to the hardware key, or by supporting a hardware key solution which doesn't need a direct connection, but this is out of scope and will not be explored in this PR.

In cases where WebUI access is needed or desired, cluster admins should only apply hardware key policies selectively to roles which warrant more protection. Teleport Connect will also serve as a great UI alternative.

### UX

#### Hardware key login
Expand Down