Merge pull request #1550 from gravitational/rjones/sec-fix

Don't allow second factor by-pass.

lib/auth/tun.go

@@ -543,7 +543,14 @@ func (s *AuthTunnel) passwordAuth(
 		return perms, nil
 	// user is trying to get in using their password only
 	case AuthWebPasswordWithoutOTP:
-		err := s.authServer.WithUserLock(conn.User(), func() error {
+		ap, err := s.authServer.GetAuthPreference()
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+		if ap.GetSecondFactor() != teleport.OFF {
+			return nil, trace.AccessDenied("missing second factor")
+		}
+		err = s.authServer.WithUserLock(conn.User(), func() error {
 			return s.authServer.CheckPasswordWOToken(conn.User(), ab.Pass)
 		})
 		if err != nil {

lib/auth/tun_test.go

@@ -727,3 +727,75 @@ func (s *TunSuite) TestSync(c *C) {
 	c.Assert(sorted[2].Addr, Equals, "2")
 	c.Assert(sorted[3].Addr, Equals, "3")
 }
+
+// TestEnforceSecondFactor tests that second factor is enforced on the auth server side
+// not just proxy side
+func (s *TunSuite) TestEnforceSecondFactor(c *C) {
+	ap, err := services.NewAuthPreference(services.AuthPreferenceSpecV2{
+		Type:         "local",
+		SecondFactor: "otp",
+	})
+	c.Assert(err, IsNil)
+	err = s.a.SetAuthPreference(ap)
+	c.Assert(err, IsNil)
+
+	c.Assert(s.a.UpsertCertAuthority(
+		suite.NewTestCA(services.UserCA, "localhost")), IsNil)
+
+	user := "foobar"
+	password := "bazqux"
+	mappings := []string{"admin", "db"}
+
+	token, err := s.a.CreateSignupToken(services.UserV1{Name: user, AllowedLogins: mappings}, 0)
+	c.Assert(err, IsNil)
+
+	authMethod, err := NewSignupTokenAuth(token)
+	c.Assert(err, IsNil)
+
+	clt, err := NewTunClient("test",
+		[]utils.NetAddr{{AddrNetwork: "tcp", Addr: s.tsrv.Addr()}}, user, authMethod)
+	c.Assert(err, IsNil)
+	defer clt.Close()
+
+	// check that the usernames are the same
+	userInToken, _, err := clt.GetSignupTokenData(token)
+	c.Assert(err, IsNil)
+	c.Assert(user, Equals, userInToken)
+
+	// get otp token
+	tokenData, err := s.a.Identity.GetSignupToken(token)
+	validToken, err := totp.GenerateCode(tokenData.OTPKey, time.Now())
+	c.Assert(err, IsNil)
+
+	// create a user
+	_, err = clt.CreateUserWithOTP(token, password, validToken)
+	c.Assert(err, IsNil)
+
+	// delete token so we can re-use it without messing with clocks
+	err = s.a.Identity.DeleteUsedTOTPToken(user)
+	c.Assert(err, IsNil)
+
+	// can't authenticate with without second factor
+	authMethod, err = NewWebPasswordWithoutOTPAuth(user, []byte(password))
+	c.Assert(err, IsNil)
+
+	clt, err = NewTunClient("test",
+		[]utils.NetAddr{{AddrNetwork: "tcp", Addr: s.tsrv.Addr()}}, user, authMethod)
+	c.Assert(err, IsNil)
+
+	_, err = clt.SignIn(user, []byte(password))
+	c.Assert(err, NotNil)
+
+	// can authenticate with without second factor
+	authMethod, err = NewWebPasswordAuth(user, []byte(password), validToken)
+	c.Assert(err, IsNil)
+
+	clt, err = NewTunClient("test",
+		[]utils.NetAddr{{AddrNetwork: "tcp", Addr: s.tsrv.Addr()}}, user, authMethod)
+	c.Assert(err, IsNil)
+	defer clt.Close()
+
+	ws, err := clt.SignIn(user, []byte(password))
+	c.Assert(err, IsNil)
+	c.Assert(ws, Not(Equals), "")
+}