diff --git a/integration/helpers.go b/integration/helpers.go index 6fbe0d912e3..98ecc31bf2e 100644 --- a/integration/helpers.go +++ b/integration/helpers.go @@ -10,6 +10,7 @@ import ( "io/ioutil" "net" "net/http" + "net/url" "os" "os/exec" "os/user" @@ -40,6 +41,7 @@ import ( "github.com/gravitational/teleport/lib/tlsca" "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/roundtrip" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" log "github.com/sirupsen/logrus" @@ -1294,6 +1296,24 @@ func closeAgent(teleAgent *teleagent.AgentServer, socketDirPath string) error { return nil } +// createWebClient builds a *client.WebClient that is used to simulate +// browser requests. +func createWebClient(cluster *TeleInstance, opts ...roundtrip.ClientParam) (*client.WebClient, error) { + // Craft URL to Web UI. + u := &url.URL{ + Scheme: "https", + Host: cluster.Config.Proxy.WebAddr.Addr, + } + + opts = append(opts, roundtrip.HTTPClient(client.NewInsecureWebClient())) + wc, err := client.NewWebClient(u.String(), opts...) + if err != nil { + return nil, trace.Wrap(err) + } + + return wc, nil +} + func fatalIf(err error) { if err != nil { log.Fatalf("%v at %v", string(debug.Stack()), err) diff --git a/integration/integration_test.go b/integration/integration_test.go index ac1a93a625b..d2fcc83c52f 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -3056,6 +3056,66 @@ func (s *IntSuite) TestList(c *check.C) { } } +// TestMultipleSignup makes sure that multiple users can create Teleport accounts. +func (s *IntSuite) TestMultipleSignup(c *check.C) { + type createNewUserReq struct { + InviteToken string `json:"invite_token"` + Pass string `json:"pass"` + } + + // Create and start a Teleport cluster. + makeConfig := func() (*check.C, []string, []*InstanceSecrets, *service.Config) { + clusterConfig, err := services.NewClusterConfig(services.ClusterConfigSpecV3{ + SessionRecording: services.RecordAtNode, + }) + c.Assert(err, check.IsNil) + + tconf := service.MakeDefaultConfig() + tconf.Auth.Preference.SetSecondFactor("off") + tconf.Auth.Enabled = true + tconf.Auth.ClusterConfig = clusterConfig + tconf.Proxy.Enabled = true + tconf.Proxy.DisableWebService = false + tconf.Proxy.DisableWebInterface = true + tconf.SSH.Enabled = true + return c, nil, nil, tconf + } + main := s.newTeleportWithConfig(makeConfig()) + defer main.Stop(true) + + mainAuth := main.Process.GetAuthServer() + + // Create a few users to make sure the proxy uses the correct identity + // when connecting to the auth server. + for i := 0; i < 5; i++ { + // Create a random username. + username, err := utils.CryptoRandomHex(16) + c.Assert(err, check.IsNil) + + // Create signup token, this is like doing "tctl users add foo foo". + token, err := mainAuth.CreateSignupToken(services.UserV1{ + Name: username, + AllowedLogins: []string{username}, + }, backend.Forever) + c.Assert(err, check.IsNil) + + // Create client that will simulate web browser. + clt, err := createWebClient(main) + c.Assert(err, check.IsNil) + + // Render the signup page. + _, err = clt.Get(clt.Endpoint("webapi", "users", "invites", token), url.Values{}) + c.Assert(err, check.IsNil) + + // Make sure signup is successful. + _, err = clt.PostJSON(clt.Endpoint("webapi", "users"), createNewUserReq{ + InviteToken: token, + Pass: "fake-password-123", + }) + c.Assert(err, check.IsNil) + } +} + // runCommand is a shortcut for running SSH command, it creates a client // connected to proxy of the passed in instance, runs the command, and returns // the result. If multiple attempts are requested, a 250 millisecond delay is diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 628f564cb13..92544c34180 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -43,11 +43,11 @@ type AuthWithRoles struct { } func (a *AuthWithRoles) actionWithContext(ctx *services.Context, namespace string, resource string, action string) error { - return a.checker.CheckAccessToRule(ctx, namespace, resource, action) + return a.checker.CheckAccessToRule(ctx, namespace, resource, action, false) } func (a *AuthWithRoles) action(namespace string, resource string, action string) error { - return a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, resource, action) + return a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, resource, action, false) } // currentUserAction is a special checker that allows certain actions for users @@ -58,7 +58,7 @@ func (a *AuthWithRoles) currentUserAction(username string) error { return nil } return a.checker.CheckAccessToRule(&services.Context{User: a.user}, - defaults.Namespace, services.KindUser, services.VerbCreate) + defaults.Namespace, services.KindUser, services.VerbCreate, false) } // authConnectorAction is a special checker that grants access to auth @@ -66,8 +66,8 @@ func (a *AuthWithRoles) currentUserAction(username string) error { // If not, it checks if the requester has the meta KindAuthConnector access // (which grants access to all connectors). func (a *AuthWithRoles) authConnectorAction(namespace string, resource string, verb string) error { - if err := a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, resource, verb); err != nil { - if err := a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, services.KindAuthConnector, verb); err != nil { + if err := a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, resource, verb, false); err != nil { + if err := a.checker.CheckAccessToRule(&services.Context{User: a.user}, namespace, services.KindAuthConnector, verb, false); err != nil { return trace.Wrap(err) } } @@ -601,11 +601,8 @@ func (a *AuthWithRoles) GetUsers() ([]services.User, error) { } func (a *AuthWithRoles) GetUser(name string) (services.User, error) { - // TODO(klizhentas) before merge, check security implications of this change, - // it looks harmless enough, but make sure there is no leakage of secrets here - // make sure that GetUser is safe to call, and it should never leak any secrets - if err := a.action(defaults.Namespace, services.KindUser, services.VerbRead); err != nil { - if err := a.currentUserAction(name); err != nil { + if err := a.currentUserAction(name); err != nil { + if err := a.action(defaults.Namespace, services.KindUser, services.VerbRead); err != nil { return nil, trace.Wrap(err) } } diff --git a/lib/service/service.go b/lib/service/service.go index 4711f7308e9..1b8b73d70c3 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -1762,15 +1762,15 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error { } webHandler, err = web.NewHandler( web.Config{ - Proxy: tsrv, - AuthServers: cfg.AuthServers[0], - DomainName: cfg.Hostname, - ProxyClient: conn.Client, - DisableUI: process.Config.Proxy.DisableWebInterface, - ProxySSHAddr: cfg.Proxy.SSHAddr, - ProxyWebAddr: cfg.Proxy.WebAddr, - ClientTLSConfig: clientTLSConfig, - ProxySettings: proxySettings, + Proxy: tsrv, + AuthServers: cfg.AuthServers[0], + DomainName: cfg.Hostname, + ProxyClient: conn.Client, + DisableUI: process.Config.Proxy.DisableWebInterface, + ProxySSHAddr: cfg.Proxy.SSHAddr, + ProxyWebAddr: cfg.Proxy.WebAddr, + ProxySettings: proxySettings, + CipherSuites: cfg.CipherSuites, }) if err != nil { return trace.Wrap(err) diff --git a/lib/services/role.go b/lib/services/role.go index 68d5f4f77ab..7e0a47af6e9 100644 --- a/lib/services/role.go +++ b/lib/services/role.go @@ -1254,7 +1254,7 @@ type AccessChecker interface { CheckAccessToServer(login string, server Server) error // CheckAccessToRule checks access to a rule within a namespace. - CheckAccessToRule(context RuleContext, namespace string, rule string, verb string) error + CheckAccessToRule(context RuleContext, namespace string, rule string, verb string, silent bool) error // CheckLoginDuration checks if role set can login up to given duration and // returns a combined list of allowed logins. @@ -1684,7 +1684,7 @@ func (set RoleSet) String() string { return fmt.Sprintf("roles %v", strings.Join(roleNames, ",")) } -func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource string, verb string) error { +func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource string, verb string, silent bool) error { whereParser, err := GetWhereParserFn()(ctx) if err != nil { return trace.Wrap(err) @@ -1702,7 +1702,12 @@ func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource return trace.Wrap(err) } if matched { - log.Infof("[RBAC] %s access to %s [namespace %s] denied for role %q: deny rule matched", verb, resource, namespace, role.GetName()) + if !silent { + log.WithFields(log.Fields{ + trace.Component: teleport.ComponentRBAC, + }).Infof("Access to %v %v in namespace %v denied to %v: deny rule matched.", + verb, resource, namespace, role.GetName()) + } return trace.AccessDenied("access denied to perform action '%s' on %s", verb, resource) } } @@ -1722,7 +1727,12 @@ func (set RoleSet) CheckAccessToRule(ctx RuleContext, namespace string, resource } } - log.Infof("[RBAC] %s access to %s [namespace %s] denied for %v: no allow rule matched", verb, resource, namespace, set) + if !silent { + log.WithFields(log.Fields{ + trace.Component: teleport.ComponentRBAC, + }).Infof("Access to %v %v in namespace %v denied to %v: no allow rule matched.", + verb, resource, namespace, set) + } return trace.AccessDenied("access denied to perform action %q on %q", verb, resource) } diff --git a/lib/services/role_test.go b/lib/services/role_test.go index d9ca5f09315..0e3f3aedee1 100644 --- a/lib/services/role_test.go +++ b/lib/services/role_test.go @@ -964,7 +964,7 @@ func (s *RoleSuite) TestCheckRuleAccess(c *C) { } for j, check := range tc.checks { comment := Commentf("test case %v '%v', check %v", i, tc.name, j) - result := set.CheckAccessToRule(&check.context, check.namespace, check.rule, check.verb) + result := set.CheckAccessToRule(&check.context, check.namespace, check.rule, check.verb, false) if check.hasAccess { c.Assert(result, IsNil, comment) } else { diff --git a/lib/web/apiserver.go b/lib/web/apiserver.go index 7e4e14e9997..68bbf1361bc 100644 --- a/lib/web/apiserver.go +++ b/lib/web/apiserver.go @@ -20,7 +20,6 @@ package web import ( "compress/gzip" - "crypto/tls" "encoding/base64" "encoding/json" "fmt" @@ -103,8 +102,8 @@ type Config struct { // ProxyWebAddr points to the web (HTTPS) address of the proxy ProxyWebAddr utils.NetAddr - // ClientTLSConfig is the TLS configuration the client uses. - ClientTLSConfig *tls.Config + // CipherSuites is the list of cipher suites Teleport suppports. + CipherSuites []uint16 // ProxySettings is a settings communicated to proxy ProxySettings client.ProxySettings @@ -126,7 +125,7 @@ func (r *RewritingHandler) Close() error { // NewHandler returns a new instance of web proxy handler func NewHandler(cfg Config, opts ...HandlerOption) (*RewritingHandler, error) { const apiPrefix = "/" + teleport.WebAPIVersion - lauth, err := newSessionCache(cfg.ProxyClient, []utils.NetAddr{cfg.AuthServers}, cfg.ClientTLSConfig) + lauth, err := newSessionCache(cfg.ProxyClient, []utils.NetAddr{cfg.AuthServers}, cfg.CipherSuites) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/web/apiserver_test.go b/lib/web/apiserver_test.go index 94b11259cfd..cd790abfc36 100644 --- a/lib/web/apiserver_test.go +++ b/lib/web/apiserver_test.go @@ -214,16 +214,12 @@ func (s *WebSuite) SetUpTest(c *C) { ) c.Assert(err, IsNil) - tlsConfig := &tls.Config{ - CipherSuites: utils.DefaultCipherSuites(), - } - handler, err := NewHandler(Config{ - Proxy: revTunServer, - AuthServers: utils.FromAddr(s.server.Addr()), - DomainName: s.server.ClusterName(), - ProxyClient: s.proxyClient, - ClientTLSConfig: tlsConfig, + Proxy: revTunServer, + AuthServers: utils.FromAddr(s.server.Addr()), + DomainName: s.server.ClusterName(), + ProxyClient: s.proxyClient, + CipherSuites: utils.DefaultCipherSuites(), }, SetSessionStreamPollPeriod(200*time.Millisecond)) c.Assert(err, IsNil) @@ -1623,7 +1619,7 @@ func removeSpace(in string) string { func newTerminalHandler() TerminalHandler { return TerminalHandler{ - log: logrus.WithFields(logrus.Fields{}), + log: logrus.WithFields(logrus.Fields{}), encoder: unicode.UTF8.NewEncoder(), decoder: unicode.UTF8.NewDecoder(), } diff --git a/lib/web/sessions.go b/lib/web/sessions.go index 6180d0a8f7a..a9b582a9b76 100644 --- a/lib/web/sessions.go +++ b/lib/web/sessions.go @@ -213,7 +213,7 @@ func (c *SessionContext) ClientTLSConfig(clusterName ...string) (*tls.Config, er } } - tlsConfig := c.parent.clientTLSConfig.Clone() + tlsConfig := utils.TLSConfig(c.parent.cipherSuites) tlsCert, err := tls.X509KeyPair(c.sess.GetTLSCert(), c.sess.GetPriv()) if err != nil { return nil, trace.Wrap(err, "failed to parse TLS cert and key") @@ -348,7 +348,7 @@ func (c *SessionContext) Close() error { } // newSessionCache returns new instance of the session cache -func newSessionCache(proxyClient auth.ClientI, servers []utils.NetAddr, clientTLSConfig *tls.Config) (*sessionCache, error) { +func newSessionCache(proxyClient auth.ClientI, servers []utils.NetAddr, cipherSuites []uint16) (*sessionCache, error) { clusterName, err := proxyClient.GetClusterName() if err != nil { return nil, trace.Wrap(err) @@ -358,12 +358,12 @@ func newSessionCache(proxyClient auth.ClientI, servers []utils.NetAddr, clientTL return nil, trace.Wrap(err) } cache := &sessionCache{ - clusterName: clusterName.GetClusterName(), - proxyClient: proxyClient, - contexts: m, - authServers: servers, - closer: utils.NewCloseBroadcaster(), - clientTLSConfig: clientTLSConfig, + clusterName: clusterName.GetClusterName(), + proxyClient: proxyClient, + contexts: m, + authServers: servers, + closer: utils.NewCloseBroadcaster(), + cipherSuites: cipherSuites, } // periodically close expired and unused sessions go cache.expireSessions() @@ -380,8 +380,8 @@ type sessionCache struct { closer *utils.CloseBroadcaster clusterName string - // clientTLSConfig is the TLS configuration the client uses. - clientTLSConfig *tls.Config + // cipherSuites is the list of supported TLS cipher suites. + cipherSuites []uint16 } // Close closes all allocated resources and stops goroutines @@ -618,8 +618,7 @@ func (s *sessionCache) ValidateSession(user, sid string) (*SessionContext, error if err != nil { return nil, trace.Wrap(err) } - - tlsConfig := s.clientTLSConfig.Clone() + tlsConfig := utils.TLSConfig(s.cipherSuites) tlsCert, err := tls.X509KeyPair(sess.GetTLSCert(), sess.GetPriv()) if err != nil { return nil, trace.Wrap(err, "failed to parse TLS cert and key") diff --git a/lib/web/ui/usercontext.go b/lib/web/ui/usercontext.go index 8923a46254e..e56d8131d46 100644 --- a/lib/web/ui/usercontext.go +++ b/lib/web/ui/usercontext.go @@ -85,7 +85,9 @@ func getLogins(roleSet services.RoleSet) []string { func hasAccess(roleSet services.RoleSet, ctx *services.Context, kind string, verbs ...string) bool { for _, verb := range verbs { - err := roleSet.CheckAccessToRule(ctx, defaults.Namespace, kind, verb) + // Since this check occurs often and it does not imply the caller is trying + // to access any resource, silence any logging done on the proxy. + err := roleSet.CheckAccessToRule(ctx, defaults.Namespace, kind, verb, true) if err != nil { return false }