Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
8c6efee
Add local proxy middleware for db cert checking
GavinFrazar Oct 4, 2022
d6608e1
Use tls conversion util instead of inline
GavinFrazar Oct 4, 2022
c664b47
Add middleware to local proxy config
GavinFrazar Oct 4, 2022
072c8fd
Add middleware configuration in tsh
GavinFrazar Oct 4, 2022
58d3bed
Use route to database check and set defaults func
GavinFrazar Oct 4, 2022
6051701
Dont trigger normal db login flow if using local proxy tunnel
GavinFrazar Oct 4, 2022
2dfeafc
Split out adding client creds into helper func for testing
GavinFrazar Oct 4, 2022
fcdf792
Add integration test for local proxy tunnel db cert middleware
GavinFrazar Oct 4, 2022
188beb2
Add unit test for local proxy middleware
GavinFrazar Oct 4, 2022
31108b4
Update comment
GavinFrazar Oct 4, 2022
a14627e
Make middleware on new conn block
GavinFrazar Oct 4, 2022
8291cdd
godoc
GavinFrazar Oct 4, 2022
ff3309f
Make any cert check error trigger cert renewal in local proxy middleware
GavinFrazar Oct 4, 2022
e4c85c2
Move dbcertchecker into lib/client
GavinFrazar Oct 5, 2022
094481d
Remove unneeded mutex in local proxy and unused func in lib/utils
GavinFrazar Oct 5, 2022
284bd96
Make local proxy middleware integration test more robust
GavinFrazar Oct 5, 2022
c525d9a
Print message before mfa prompt in proxy tunnel
GavinFrazar Oct 5, 2022
b6b3643
Add before prompt option to test
GavinFrazar Oct 5, 2022
4a4ffdd
Remove unneeded comment
GavinFrazar Oct 6, 2022
093573c
Change local proxy messages to be more clear
GavinFrazar Oct 6, 2022
6ddcc49
Pass local proxy opts by reference
GavinFrazar Oct 6, 2022
93f979c
Pass certs in opts instead of cert/key file path
GavinFrazar Oct 6, 2022
d8de925
Move db route checking back to tsh
GavinFrazar Oct 6, 2022
3f4c535
Fix lint err
GavinFrazar Oct 7, 2022
cae0be8
Merge branch 'master' into gavinfrazar/proxy_db_tunnel_mfa_access
GavinFrazar Oct 7, 2022
dd4b399
Fix typo and print the hint to same writer as the mfa prompt
GavinFrazar Oct 7, 2022
de66715
Merge branch 'master' into gavinfrazar/proxy_db_tunnel_mfa_access
GavinFrazar Oct 7, 2022
6a8e38f
Merge branch 'master' into gavinfrazar/proxy_db_tunnel_mfa_access
GavinFrazar Oct 7, 2022
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
19 changes: 12 additions & 7 deletions integration/helpers/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ import (
"testing"
"time"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport/api/breaker"
"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/types"
Expand All @@ -46,11 +52,6 @@ import (
"github.com/gravitational/teleport/lib/sshutils"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ssh"
)

const (
Expand Down Expand Up @@ -1214,8 +1215,7 @@ func (i *TeleInstance) NewClientWithCreds(cfg ClientConfig, creds UserCreds) (tc
return clt, nil
}

// NewUnauthenticatedClient returns a fully configured and pre-authenticated client
// (pre-authenticated with server CAs and signed session key)
// NewUnauthenticatedClient returns a fully configured and un-authenticated client
func (i *TeleInstance) NewUnauthenticatedClient(cfg ClientConfig) (tc *client.TeleportClient, err error) {
keyDir, err := os.MkdirTemp(i.Config.DataDir, "tsh")
if err != nil {
Expand Down Expand Up @@ -1274,7 +1274,12 @@ func (i *TeleInstance) NewClient(cfg ClientConfig) (*client.TeleportClient, erro
if err != nil {
return nil, trace.Wrap(err)
}
return i.AddClientCredentials(tc, cfg)
}

// AddClientCredentials adds authenticated credentials to a client.
// (server CAs and signed session key).
func (i *TeleInstance) AddClientCredentials(tc *client.TeleportClient, cfg ClientConfig) (*client.TeleportClient, error) {
// Generate certificates for the user simulating login.
creds, err := GenerateUserCreds(UserCredsRequest{
Process: i.Process,
Expand Down
68 changes: 68 additions & 0 deletions integration/proxy/proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/stretchr/testify/require"
"go.mongodb.org/mongo-driver/bson"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -41,6 +42,7 @@ import (
"github.com/gravitational/teleport/integration/kube"
"github.com/gravitational/teleport/lib"
"github.com/gravitational/teleport/lib/auth/testauthority"
libclient "github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/service"
"github.com/gravitational/teleport/lib/srv/alpnproxy"
Expand Down Expand Up @@ -754,6 +756,72 @@ func TestALPNSNIProxyDatabaseAccess(t *testing.T) {
require.NoError(t, client.Close())
})
})

t.Run("authenticated tunnel with cert renewal", func(t *testing.T) {
// get a teleport client
tc, err := pack.Root.Cluster.NewClient(helpers.ClientConfig{
Login: pack.Root.User.GetName(),
Cluster: pack.Root.Cluster.Secrets.SiteName,
})
require.NoError(t, err)
routeToDatabase := tlsca.RouteToDatabase{
ServiceName: pack.Root.MysqlService.Name,
Protocol: pack.Root.MysqlService.Protocol,
Username: "root",
}
// inject a fake clock into the middleware so we can control when it thinks certs have expired
fakeClock := clockwork.NewFakeClockAt(time.Now())

// configure local proxy without certs but with cert checking/reissuing middleware
// local proxy middleware should fetch a DB cert when the local proxy starts
lp := mustStartALPNLocalProxyWithConfig(t, alpnproxy.LocalProxyConfig{
RemoteProxyAddr: pack.Root.Cluster.SSHProxy,
Protocols: []alpncommon.Protocol{alpncommon.ProtocolMySQL},
InsecureSkipVerify: true,
Middleware: libclient.NewDBCertChecker(tc, routeToDatabase, fakeClock),
})

client, err := mysql.MakeTestClientWithoutTLS(lp.GetAddr(), routeToDatabase)
require.NoError(t, err)

// Execute a query.
result, err := client.Execute("select 1")
require.NoError(t, err)
require.Equal(t, mysql.TestQueryResponse, result)

// Disconnect.
require.NoError(t, client.Close())
certs := lp.GetCerts()
require.NotEmpty(t, certs)
cert1, err := utils.TLSCertToX509(certs[0])
require.NoError(t, err)
// sanity check that cert equality check works
require.Equal(t, cert1, cert1, "cert should be equal to itself")

// mock db cert expiration (as far as the middleware thinks anyway)
// Unfortunately, mocking cert expiration by advancing a fake clock
// does not cause an invalid certificate error even if no cert renewal is done by the middleware,
// because TLS handshakes are done with real system time.
require.Greater(t, cert1.NotAfter, fakeClock.Now())
fakeClock.Advance(cert1.NotAfter.Sub(fakeClock.Now()) + time.Second)

// Open a new connection
client, err = mysql.MakeTestClientWithoutTLS(lp.GetAddr(), routeToDatabase)
require.NoError(t, err)

// Execute a query.
result, err = client.Execute("select 1")
require.NoError(t, err)
require.Equal(t, mysql.TestQueryResponse, result)

// Disconnect.
require.NoError(t, client.Close())
certs = lp.GetCerts()
require.NotEmpty(t, certs)
cert2, err := utils.TLSCertToX509(certs[0])
require.NoError(t, err)
require.NotEqual(t, cert1, cert2, "cert should have been renewed by middleware")
})
}

// TestALPNSNIProxyAppAccess tests application access via ALPN SNI proxy service.
Expand Down
4 changes: 2 additions & 2 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -1720,7 +1720,7 @@ func (tc *TeleportClient) ReissueUserCerts(ctx context.Context, cachePolicy Cert
// (according to RBAC), IssueCertsWithMFA will:
// - for SSH certs, return the existing Key from the keystore.
// - for TLS certs, fall back to ReissueUserCerts.
func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params ReissueParams) (*Key, error) {
func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params ReissueParams, applyOpts func(opts *PromptMFAChallengeOpts)) (*Key, error) {
ctx, span := tc.Tracer.Start(
ctx,
"teleportClient/IssueUserCertsWithMFA",
Expand All @@ -1737,7 +1737,7 @@ func (tc *TeleportClient) IssueUserCertsWithMFA(ctx context.Context, params Reis
return proxyClient.IssueUserCertsWithMFA(
ctx, params,
func(ctx context.Context, proxyAddr string, c *proto.MFAAuthenticateChallenge) (*proto.MFAAuthenticateResponse, error) {
return tc.PromptMFAChallenge(ctx, proxyAddr, c, nil /* applyOpts */)
return tc.PromptMFAChallenge(ctx, proxyAddr, c, applyOpts)
})
}

Expand Down
1 change: 1 addition & 0 deletions lib/client/api_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ func TestTeleportClient_PromptMFAChallenge(t *testing.T) {
challenge := &proto.MFAAuthenticateChallenge{}

customizedOpts := &client.PromptMFAChallengeOpts{
HintBeforePrompt: "some hint explaining the imminent prompt",
PromptDevicePrefix: "llama",
Quiet: true,
AllowStdinHijack: true,
Expand Down
161 changes: 161 additions & 0 deletions lib/client/local_proxy_middleware.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
/*
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 client

import (
"context"
"crypto/tls"
"crypto/x509"
"errors"
"fmt"
"net"
"time"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"

"github.com/gravitational/teleport/api/client/proto"
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/srv/alpnproxy"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
)

// DBCertChecker is a middleware that ensures that the local proxy has valid TLS database certs.
type DBCertChecker struct {
// tc is a TeleportClient used to reissue certificates when necessary.
tc *TeleportClient
// dbRoute contains database routing information.
dbRoute tlsca.RouteToDatabase
// Clock specifies the time provider. Will be used to override the time anchor
// for TLS certificate verification.
// Defaults to real clock if unspecified
clock clockwork.Clock
}

func NewDBCertChecker(tc *TeleportClient, dbRoute tlsca.RouteToDatabase, clock clockwork.Clock) alpnproxy.LocalProxyMiddleware {
if clock == nil {
clock = clockwork.NewRealClock()
}
return &DBCertChecker{
tc: tc,
dbRoute: dbRoute,
clock: clock,
}
}

var _ alpnproxy.LocalProxyMiddleware = (*DBCertChecker)(nil)

// OnNewConnection is a callback triggered when a new downstream connection is
// accepted by the local proxy.
func (c *DBCertChecker) OnNewConnection(ctx context.Context, lp *alpnproxy.LocalProxy, conn net.Conn) error {
return trace.Wrap(c.ensureValidCerts(ctx, lp))
}

// OnStart is a callback triggered when the local proxy starts.
func (c *DBCertChecker) OnStart(ctx context.Context, lp *alpnproxy.LocalProxy) error {
return trace.Wrap(c.ensureValidCerts(ctx, lp))
}

// checkCerts checks if the local proxy TLS certs are configured, not expired, and match the db route.
func (c *DBCertChecker) checkCerts(lp *alpnproxy.LocalProxy) error {
log.Debug("checking local proxy database certs")
certs := lp.GetCerts()
if len(certs) == 0 {
return trace.Wrap(trace.NotFound("local proxy has no TLS certificates configured"))
}
cert, err := utils.TLSCertToX509(certs[0])
if err != nil {
return trace.Wrap(err)
}
err = utils.VerifyCertificateExpiry(cert, c.clock)
if err != nil {
return trace.Wrap(err)
}
identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter)
if err != nil {
return trace.Wrap(err)
}
if c.dbRoute.Username != "" && c.dbRoute.Username != identity.RouteToDatabase.Username {
msg := fmt.Sprintf("certificate subject is for user %s, but need %s", identity.RouteToDatabase.Username, c.dbRoute.Username)
return trace.Wrap(errors.New(msg))
}
if c.dbRoute.Database != "" && c.dbRoute.Database != identity.RouteToDatabase.Database {
msg := fmt.Sprintf("certificate subject is for database name %s, but need %s", identity.RouteToDatabase.Database, c.dbRoute.Database)
return trace.Wrap(errors.New(msg))
}
return nil
}

// ensureValidCerts ensures that the local proxy is configured with valid certs.
func (c *DBCertChecker) ensureValidCerts(ctx context.Context, lp *alpnproxy.LocalProxy) error {
if err := c.checkCerts(lp); err != nil {
log.WithError(err).Debug("local proxy tunnel certificates need to be reissued")
} else {
return nil
}
return trace.Wrap(c.renewCerts(ctx, lp))
}

// renewCerts attempts to renew the database certs for the local proxy.
func (c *DBCertChecker) renewCerts(ctx context.Context, lp *alpnproxy.LocalProxy) error {
var accessRequests []string
if profile, err := StatusCurrent(c.tc.HomePath, c.tc.WebProxyAddr, ""); err != nil {
log.WithError(err).Warn("unable to load profile, requesting database certs without access requests")
} else {
accessRequests = profile.ActiveRequests.AccessRequests
}

hint := fmt.Sprintf("MFA is required to access database %q", c.dbRoute.ServiceName)
var key *Key
if err := RetryWithRelogin(ctx, c.tc, func() error {
newKey, err := c.tc.IssueUserCertsWithMFA(ctx, ReissueParams{
RouteToCluster: c.tc.SiteName,
RouteToDatabase: proto.RouteToDatabase{
ServiceName: c.dbRoute.ServiceName,
Protocol: c.dbRoute.Protocol,
Username: c.dbRoute.Username,
Database: c.dbRoute.Database,
},
AccessRequests: accessRequests,
}, func(opts *PromptMFAChallengeOpts) {
opts.HintBeforePrompt = hint
})
key = newKey
return trace.Wrap(err)
}); err != nil {
return trace.Wrap(err)
}

dbCert, ok := key.DBTLSCerts[c.dbRoute.ServiceName]
if !ok {
return trace.NotFound("database '%v' TLS cert missing", c.dbRoute.ServiceName)
}
tlsCert, err := keys.X509KeyPair(dbCert, key.PrivateKeyPEM())
if err != nil {
return trace.Wrap(err)
}
x509cert, err := x509.ParseCertificate(tlsCert.Certificate[0])
if err != nil {
return trace.Wrap(err)
}
certTTL := x509cert.NotAfter.Sub(c.clock.Now()).Round(time.Minute)
fmt.Printf("Database certificate renewed: valid until %s [valid for %v]\n",
x509cert.NotAfter.Format(time.RFC3339), certTTL)
lp.SetCerts([]tls.Certificate{tlsCert})
return nil
}
12 changes: 10 additions & 2 deletions lib/client/mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ func (p *mfaPrompt) PromptPIN() (string, error) {

// PromptMFAChallengeOpts groups optional settings for PromptMFAChallenge.
type PromptMFAChallengeOpts struct {
// HintBeforePrompt is an optional hint message to print before an MFA prompt.
// It is used to provide context about why the user is being prompted where it may
// not be obvious.
HintBeforePrompt string
// PromptDevicePrefix is an optional prefix printed before "security key" or
// "device". It is used to emphasize between different kinds of devices, like
// registered vs new.
Expand Down Expand Up @@ -105,6 +109,10 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge,
if opts == nil {
opts = &PromptMFAChallengeOpts{}
}
writer := os.Stderr
if opts.HintBeforePrompt != "" {
fmt.Fprintln(writer, opts.HintBeforePrompt)
}
promptDevicePrefix := opts.PromptDevicePrefix
quiet := opts.Quiet

Expand Down Expand Up @@ -175,7 +183,7 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge,
msg = fmt.Sprintf("Enter an OTP code from a %sdevice", promptDevicePrefix)
}

otp, err := prompt.Password(otpCtx, os.Stderr, prompt.Stdin(), msg)
otp, err := prompt.Password(otpCtx, writer, prompt.Stdin(), msg)
if err != nil {
respC <- response{kind: kind, err: err}
return
Expand All @@ -202,7 +210,7 @@ func PromptMFAChallenge(ctx context.Context, c *proto.MFAAuthenticateChallenge,
defer wg.Done()
log.Debugf("WebAuthn: prompting devices with origin %q", origin)

prompt := wancli.NewDefaultPrompt(ctx, os.Stderr)
prompt := wancli.NewDefaultPrompt(ctx, writer)
prompt.SecondTouchMessage = fmt.Sprintf("Tap your %ssecurity key to complete login", promptDevicePrefix)
switch {
case quiet:
Expand Down
Loading