Don't pass and clone client *tls.Config, instead pass cipher suites and

create new *tls.Config. Add test coverage for this.

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)

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

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)
 		}
 	}

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)

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)
 }
 

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 {

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)
 	}

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(),
 	}

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")

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
 		}