From 05efdb2a176926dd70d635bceacdea4ec574e5be Mon Sep 17 00:00:00 2001 From: Russell Jones Date: Tue, 17 Jan 2017 12:25:39 -0800 Subject: [PATCH] Moved Check* methods into auth package. --- lib/auth/apiserver.go | 9 +- lib/auth/apiserver_test.go | 70 +++++++++---- lib/auth/auth_test.go | 15 +-- lib/auth/auth_with_roles.go | 22 +++- lib/auth/clt.go | 15 +-- lib/auth/new_web_user.go | 65 +++--------- lib/auth/password.go | 142 +++++++++++++++++++++++++ lib/auth/tun_test.go | 73 +++++++------ lib/services/identity.go | 11 +- lib/services/local/services_test.go | 8 -- lib/services/local/users.go | 155 ++-------------------------- lib/services/suite/suite.go | 61 ----------- lib/utils/otp.go | 27 +++++ lib/web/web_test.go | 23 ++--- 14 files changed, 320 insertions(+), 376 deletions(-) create mode 100644 lib/auth/password.go diff --git a/lib/auth/apiserver.go b/lib/auth/apiserver.go index 46d256e7db6..4399f892b50 100644 --- a/lib/auth/apiserver.go +++ b/lib/auth/apiserver.go @@ -454,11 +454,6 @@ type upsertPasswordReq struct { Password string `json:"password"` } -type upsertPasswordResponse struct { - OTPURL string `json:"otp_url"` - OTPQRCode []byte `json:"otp_qr"` -} - func (s *APIServer) upsertPassword(auth ClientI, w http.ResponseWriter, r *http.Request, p httprouter.Params, version string) (interface{}, error) { var req *upsertPasswordReq if err := httplib.ReadJSON(r, &req); err != nil { @@ -466,12 +461,12 @@ func (s *APIServer) upsertPassword(auth ClientI, w http.ResponseWriter, r *http. } user := p.ByName("user") - otpURL, otpQRCode, err := auth.UpsertPassword(user, []byte(req.Password)) + err := auth.UpsertPassword(user, []byte(req.Password)) if err != nil { return nil, trace.Wrap(err) } - return &upsertPasswordResponse{OTPURL: otpURL, OTPQRCode: otpQRCode}, nil + return message(fmt.Sprintf("password for for user %q upserted", user)), nil } type upsertUserRawReq struct { diff --git a/lib/auth/apiserver_test.go b/lib/auth/apiserver_test.go index 3f72d3dc22b..1c2ef712beb 100644 --- a/lib/auth/apiserver_test.go +++ b/lib/auth/apiserver_test.go @@ -38,7 +38,6 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/kylelemons/godebug/diff" - "github.com/pquerna/otp" "github.com/pquerna/otp/totp" "golang.org/x/crypto/ssh" . "gopkg.in/check.v1" @@ -174,9 +173,9 @@ func (s *APISuite) TestGenerateKeysAndCerts(c *C) { user1, _ := createUserAndRole(s.clt, "user1", []string{"user1"}) user2, _ := createUserAndRole(s.clt, "user2", []string{"user2"}) - _, _, err = s.clt.UpsertPassword(user1.GetName(), []byte("abc1231")) + err = s.clt.UpsertPassword(user1.GetName(), []byte("abc1231")) c.Assert(err, IsNil) - _, _, err = s.clt.UpsertPassword(user2.GetName(), []byte("abc1232")) + err = s.clt.UpsertPassword(user2.GetName(), []byte("abc1232")) c.Assert(err, IsNil) newChecker, err := NewAccessChecker(s.AccessS, s.WebS) @@ -221,7 +220,7 @@ func (s *APISuite) TestGenerateKeysAndCerts(c *C) { } func (s *APISuite) TestUserCRUD(c *C) { - _, _, err := s.clt.UpsertPassword("user1", []byte("some pass")) + err := s.clt.UpsertPassword("user1", []byte("some pass")) c.Assert(err, IsNil) users, err := s.WebS.GetUsers() @@ -236,45 +235,78 @@ func (s *APISuite) TestUserCRUD(c *C) { c.Assert(len(users), Equals, 0) } -// TODO(rjones): Can this test be removed? It seems redundant see lib/services/suite/suite.go. func (s *APISuite) TestPasswordCRUD(c *C) { pass := []byte("abc123") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) err := s.clt.CheckPassword("user1", pass, "123456") c.Assert(err, NotNil) - otpURL, _, err := s.clt.UpsertPassword("user1", pass) + err = s.clt.UpsertPassword("user1", pass) + c.Assert(err, IsNil) + + err = s.a.UpsertTOTP("user1", otpSecret) + c.Assert(err, IsNil) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) + c.Assert(err, IsNil) + + err = s.clt.CheckPassword("user1", pass, validToken) + c.Assert(err, IsNil) +} + +func (s *APISuite) TestOTPCRUD(c *C) { + user := "user1" + pass := []byte("abc123") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) + + // upsert a password and totp secret + err := s.clt.UpsertPassword("user1", pass) + c.Assert(err, IsNil) + err = s.a.UpsertTOTP(user, otpSecret) c.Assert(err, IsNil) // make sure the otp url we get back is valid url issued to the correct user + otpURL, _, err := s.a.GetOTPData(user) + c.Assert(err, IsNil) u, err := url.Parse(otpURL) c.Assert(err, IsNil) c.Assert(u.Path, Equals, "/user1") - // extract otp secret key from url, we will use this to create - // tokens to make sure the url (and qr code) the user receives is - // the same as the one stored in the backend - otpKeyMeta, err := otp.NewKeyFromURL(u.String()) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - // a completely invalid token should return access denied err = s.clt.CheckPassword("user1", pass, "123456") c.Assert(err, NotNil) // a invalid token (made 1 minute in the future but from a valid key) // should also return access denied - invalidToken, err := totp.GenerateCode(string(otpKey), time.Now().Add(1*time.Minute)) + invalidToken, err := totp.GenerateCode(otpSecret, time.Now().Add(1*time.Minute)) c.Assert(err, IsNil) - err = s.WebS.CheckPassword("user1", pass, invalidToken) + err = s.clt.CheckPassword("user1", pass, invalidToken) c.Assert(err, NotNil) // a valid token (created right now and from a valid key) should return success - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) - err = s.WebS.CheckPassword("user1", pass, validToken) + + err = s.clt.CheckPassword("user1", pass, validToken) c.Assert(err, IsNil) + + // try the same valid token now it should fail because we don't allow re-use of tokens + err = s.clt.CheckPassword("user1", pass, validToken) + c.Assert(err, NotNil) +} + +func (s *APISuite) PasswordGarbage(c *C) { + garbage := [][]byte{ + nil, + make([]byte, defaults.MaxPasswordLength+1), + make([]byte, defaults.MinPasswordLength-1), + } + for _, g := range garbage { + err := s.clt.CheckPassword("user1", g, "123456") + c.Assert(err, NotNil) + } } func (s *APISuite) TestSessions(c *C) { @@ -290,7 +322,7 @@ func (s *APISuite) TestSessions(c *C) { c.Assert(err, NotNil) c.Assert(ws, IsNil) - _, _, err = s.clt.UpsertPassword(user, pass) + err = s.clt.UpsertPassword(user, pass) ws, err = s.clt.SignIn(user, pass) c.Assert(err, IsNil) diff --git a/lib/auth/auth_test.go b/lib/auth/auth_test.go index e2ca949a959..9006ebfa1c9 100644 --- a/lib/auth/auth_test.go +++ b/lib/auth/auth_test.go @@ -17,7 +17,6 @@ limitations under the License. package auth import ( - "net/url" "testing" "time" @@ -73,14 +72,9 @@ func (s *AuthSuite) TestSessions(c *C) { createUserAndRole(s.a, user, []string{user}) - otpURL, _, err := s.a.UpsertPassword(user, pass) + err = s.a.UpsertPassword(user, pass) c.Assert(err, IsNil) - // make sure label in url is correct - u, err := url.Parse(otpURL) - c.Assert(err, IsNil) - c.Assert(u.Path, Equals, "/user1") - ws, err = s.a.SignIn(user, pass) c.Assert(err, IsNil) c.Assert(ws, NotNil) @@ -108,14 +102,9 @@ func (s *AuthSuite) TestUserLock(c *C) { createUserAndRole(s.a, user, []string{user}) - otpURL, _, err := s.a.UpsertPassword(user, pass) + err = s.a.UpsertPassword(user, pass) c.Assert(err, IsNil) - // make sure label in url is correct - u, err := url.Parse(otpURL) - c.Assert(err, IsNil) - c.Assert(u.Path, Equals, "/user1") - // successfull log in ws, err = s.a.SignIn(user, pass) c.Assert(err, IsNil) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index df7dd771bb9..2ea5f15fc34 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -242,18 +242,32 @@ func (a *AuthWithRoles) UpsertToken(token string, roles teleport.Roles, ttl time return a.authServer.UpsertToken(token, roles, ttl) } -func (a *AuthWithRoles) UpsertPassword(user string, password []byte) (hotpURL string, hotpQR []byte, err error) { +func (a *AuthWithRoles) UpsertPassword(user string, password []byte) error { if err := a.currentUserAction(user); err != nil { - return "", nil, trace.Wrap(err) + return trace.Wrap(err) } return a.authServer.UpsertPassword(user, password) } -func (a *AuthWithRoles) CheckPassword(user string, password []byte, hotpToken string) error { +func (a *AuthWithRoles) CheckPassword(user string, password []byte, otpToken string) error { if err := a.currentUserAction(user); err != nil { return trace.Wrap(err) } - return a.authServer.CheckPassword(user, password, hotpToken) + return a.authServer.CheckPassword(user, password, otpToken) +} + +func (a *AuthWithRoles) UpsertTOTP(user string, otpSecret string) error { + if err := a.currentUserAction(user); err != nil { + return trace.Wrap(err) + } + return a.authServer.UpsertTOTP(user, otpSecret) +} + +func (a *AuthWithRoles) GetOTPData(user string) (string, []byte, error) { + if err := a.currentUserAction(user); err != nil { + return "", nil, trace.Wrap(err) + } + return a.authServer.GetOTPData(user) } func (a *AuthWithRoles) SignIn(user string, password []byte) (*Session, error) { diff --git a/lib/auth/clt.go b/lib/auth/clt.go index 414a3f6e8c3..13ff7419a63 100644 --- a/lib/auth/clt.go +++ b/lib/auth/clt.go @@ -515,22 +515,17 @@ func (c *Client) GetU2FAppID() (string, error) { } // UpsertPassword updates web access password for the user -func (c *Client) UpsertPassword(user string, password []byte) (otpURL string, otpQRCode []byte, err error) { - out, err := c.PostJSON( +func (c *Client) UpsertPassword(user string, password []byte) error { + _, err := c.PostJSON( c.Endpoint("users", user, "web", "password"), upsertPasswordReq{ Password: string(password), }) if err != nil { - return "", nil, trace.Wrap(err) + return trace.Wrap(err) } - var re *upsertPasswordResponse - if err := json.Unmarshal(out.Bytes(), &re); err != nil { - return "", nil, trace.Wrap(err) - } - - return re.OTPURL, re.OTPQRCode, err + return nil } // UpsertUser user updates or inserts user entry @@ -1172,7 +1167,7 @@ type WebService interface { // IdentityService manages identities and userse type IdentityService interface { // UpsertPassword updates web access password for the user - UpsertPassword(user string, password []byte) (otpURL string, otpQRCode []byte, err error) + UpsertPassword(user string, password []byte) error // UpsertOIDCConnector updates or creates OIDC connector UpsertOIDCConnector(connector services.OIDCConnector, ttl time.Duration) error diff --git a/lib/auth/new_web_user.go b/lib/auth/new_web_user.go index 37dc3f54dc9..b470872590f 100644 --- a/lib/auth/new_web_user.go +++ b/lib/auth/new_web_user.go @@ -25,7 +25,6 @@ package auth import ( "bytes" - "crypto/subtle" "image/png" "github.com/gravitational/teleport/lib/defaults" @@ -171,30 +170,28 @@ func (s *AuthServer) CreateUserWithToken(token string, password string, otpToken return nil, trace.Wrap(err) } - err = s.checkAndUpsertTOTP(tokenData.User.Name, otpToken, tokenData.OTPKey) - if err != nil { - return nil, trace.Wrap(err) - } - - _, _, err = s.UpsertPassword(tokenData.User.Name, []byte(password)) - if err != nil { - return nil, trace.Wrap(err) - } - - // TODO(rjones): We have to do this because UpsertPassword above wipes out the stored OTP secret. - // To fix this, we need to update UpsertPassword so it doesn't do that but we need to make sure - // that changing the behavior of UpsertPassword doesn't break things elsewhere. err = s.UpsertTOTP(tokenData.User.Name, tokenData.OTPKey) if err != nil { return nil, trace.Wrap(err) } + err = s.CheckOTP(tokenData.User.Name, otpToken) + if err != nil { + return nil, trace.Wrap(err) + } + + err = s.UpsertPassword(tokenData.User.Name, []byte(password)) + if err != nil { + return nil, trace.Wrap(err) + } + // apply user allowed logins role := services.RoleForUser(tokenData.User.V2()) role.SetLogins(tokenData.User.AllowedLogins) if err := s.UpsertRole(role); err != nil { return nil, trace.Wrap(err) } + // Allowed logins are not going to be used anymore tokenData.User.AllowedLogins = nil tokenData.User.Roles = append(tokenData.User.Roles, role.GetName()) @@ -223,42 +220,6 @@ func (s *AuthServer) CreateUserWithToken(token string, password string, otpToken return sess, nil } -// checkAndUpsertTOTP validates token and if valid, it upserts hotp key. This function -// does not perform a security check but rather a usability check to ensure the -// second factor works. -func (s *AuthServer) checkAndUpsertTOTP(username string, otpToken string, otpKey string) error { - // make sure we have not seen this otp token before - usedToken, err := s.GetUsedTOTPToken(username) - if err != nil { - return trace.Wrap(err) - } - - if subtle.ConstantTimeCompare([]byte(otpToken), []byte(usedToken)) == 1 { - return trace.AccessDenied("previously used totp token") - } - - // totp tokens are only valid during there time window t - valid := totp.Validate(otpToken, otpKey) - if !valid { - return trace.BadParameter("invalid TOTP token") - } - - // if we have gotten here we were able to verify the otp token - // so go ahead and upsert it into the backend - err = s.UpsertTOTP(username, otpKey) - if err != nil { - return trace.Wrap(err) - } - - // we successfully validated this otp token, don't let it be used again for some time t - err = s.UpsertUsedTOTPToken(username, otpToken) - if err != nil { - return trace.Wrap(err) - } - - return nil -} - func (s *AuthServer) CreateUserWithU2FToken(token string, password string, response u2f.RegisterResponse) (*Session, error) { err := s.CheckU2FEnabled() if err != nil { @@ -290,15 +251,17 @@ func (s *AuthServer) CreateUserWithU2FToken(token string, password string, respo return nil, trace.Wrap(err) } - _, _, err = s.UpsertPassword(tokenData.User.Name, []byte(password)) + err = s.UpsertPassword(tokenData.User.Name, []byte(password)) if err != nil { return nil, trace.Wrap(err) } + role := services.RoleForUser(tokenData.User.V2()) role.SetLogins(tokenData.User.AllowedLogins) if err := s.UpsertRole(role); err != nil { return nil, trace.Wrap(err) } + // Allowed logins are not going to be used anymore tokenData.User.AllowedLogins = nil tokenData.User.Roles = append(tokenData.User.Roles, role.GetName()) diff --git a/lib/auth/password.go b/lib/auth/password.go new file mode 100644 index 00000000000..ba7b58139c0 --- /dev/null +++ b/lib/auth/password.go @@ -0,0 +1,142 @@ +package auth + +import ( + "crypto/subtle" + + "golang.org/x/crypto/bcrypt" + + "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/services" + "github.com/gravitational/teleport/lib/utils" + "github.com/gravitational/trace" + + "github.com/pquerna/otp/totp" +) + +// CheckPasswordWOToken checks just password without checking OTP tokens +// used in case of SSH authentication, when token has been validated. +func (s *AuthServer) CheckPasswordWOToken(user string, password []byte) error { + err := services.VerifyPassword(password) + if err != nil { + return trace.Wrap(err) + } + + hash, err := s.GetPasswordHash(user) + if err != nil { + return trace.Wrap(err) + } + + if err = bcrypt.CompareHashAndPassword(hash, password); err != nil { + return trace.BadParameter("passwords do not match") + } + + return nil +} + +// CheckPassword checks the password and OTP token. Called by tsh or lib/web/*. +func (s *AuthServer) CheckPassword(user string, password []byte, otpToken string) error { + err := s.CheckPasswordWOToken(user, password) + if err != nil { + return trace.AccessDenied("invalid password") + } + + err = s.CheckOTP(user, otpToken) + if err != nil { + return trace.AccessDenied("invalid otp token") + } + + return nil +} + +// CheckOTP determines the type of OTP token used (for legacy HOTP support), fetches the +// appropriate type from the backend, and checks if the token is valid. +func (s *AuthServer) CheckOTP(user string, otpToken string) error { + var err error + + otpType, err := s.getOTPType(user) + if err != nil { + return trace.Wrap(err) + } + + switch otpType { + case "hotp": + otp, err := s.GetHOTP(user) + if err != nil { + return trace.Wrap(err) + } + + // look ahead n tokens to see if we can find a matching token + if !otp.Scan(otpToken, defaults.HOTPFirstTokensRange) { + return trace.AccessDenied("bad htop token") + } + + // we need to upsert the hotp state again because the + // counter was incremented + if err := s.UpsertHOTP(user, otp); err != nil { + return trace.Wrap(err) + } + case "totp": + otpSecret, err := s.GetTOTP(user) + if err != nil { + return trace.Wrap(err) + } + + // get the previously used token to mitigate token replay attacks + usedToken, err := s.GetUsedTOTPToken(user) + if err != nil { + return trace.Wrap(err) + } + + // we use a constant time compare function to mitigate timing attacks + if subtle.ConstantTimeCompare([]byte(otpToken), []byte(usedToken)) == 1 { + return trace.AccessDenied("previously used totp token") + } + + valid := totp.Validate(otpToken, otpSecret) + if !valid { + return trace.AccessDenied("invalid totp token") + } + + // if we have a valid token, update the previously used token + err = s.UpsertUsedTOTPToken(user, otpToken) + if err != nil { + return trace.Wrap(err) + } + } + + return nil +} + +// getOTPType returns the type of OTP token used, HOTP or TOTP. +// Deprecated: Remove this method once HOTP support has been removed. +func (s *AuthServer) getOTPType(user string) (string, error) { + _, err := s.GetHOTP(user) + if err != nil { + if trace.IsNotFound(err) { + return "totp", nil + } + return "", trace.Wrap(err) + } + return "hotp", nil +} + +// GetOTPData returns the OTP Key, Key URL, and the QR code. +func (s *AuthServer) GetOTPData(user string) (string, []byte, error) { + // get otp key from backend + otpSecret, err := s.GetTOTP(user) + if err != nil { + return "", nil, trace.Wrap(err) + } + + // create otp url + params := map[string][]byte{"secret": []byte(otpSecret)} + otpURL := utils.GenerateOTPURL("totp", user, params) + + // create the qr code + otpQR, err := utils.GenerateQRCode(otpURL) + if err != nil { + return "", nil, trace.Wrap(err) + } + + return otpURL, otpQR, nil +} diff --git a/lib/auth/tun_test.go b/lib/auth/tun_test.go index 026d3b14745..0e7d82bf252 100644 --- a/lib/auth/tun_test.go +++ b/lib/auth/tun_test.go @@ -36,7 +36,6 @@ import ( "github.com/gravitational/teleport/lib/sshutils" "github.com/gravitational/teleport/lib/utils" - "github.com/pquerna/otp" "github.com/pquerna/otp/totp" "golang.org/x/crypto/ssh" . "gopkg.in/check.v1" @@ -135,13 +134,21 @@ func (s *TunSuite) TestUnixServerClient(c *C) { userName := "test" pass := []byte("pwd123") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) user, role := createUserAndRole(s.a, userName, []string{userName}) role.SetResource(services.KindNode, services.RW()) err = s.a.UpsertRole(role) c.Assert(err, IsNil) - otpURL, _, err := s.a.UpsertPassword(user.GetName(), pass) + err = s.a.UpsertPassword(user.GetName(), pass) + c.Assert(err, IsNil) + + err = s.a.UpsertTOTP(user.GetName(), otpSecret) + c.Assert(err, IsNil) + + otpURL, _, err := s.a.GetOTPData(userName) c.Assert(err, IsNil) // make sure label in url is correct @@ -149,14 +156,8 @@ func (s *TunSuite) TestUnixServerClient(c *C) { c.Assert(err, IsNil) c.Assert(u.Path, Equals, "/test") - // extract otp key from signup url - otpKeyMeta, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - // create a valid otp token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) authMethod, err := NewWebPasswordAuth(user.GetName(), pass, validToken) @@ -177,10 +178,18 @@ func (s *TunSuite) TestSessions(c *C) { user := "ws-test" pass := []byte("ws-abc123") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) createUserAndRole(s.a, user, []string{user}) - otpURL, _, err := s.a.UpsertPassword(user, pass) + err := s.a.UpsertPassword(user, pass) + c.Assert(err, IsNil) + + err = s.a.UpsertTOTP(user, otpSecret) + c.Assert(err, IsNil) + + otpURL, _, err := s.a.GetOTPData(user) c.Assert(err, IsNil) // make sure label in url is correct @@ -188,14 +197,8 @@ func (s *TunSuite) TestSessions(c *C) { c.Assert(err, IsNil) c.Assert(u.Path, Equals, "/ws-test") - // extract otp key from signup url - otpKeyMeta, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - // create a valid otp token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) authMethod, err := NewWebPasswordAuth(user, pass, validToken) @@ -390,10 +393,18 @@ func (s *TunSuite) TestPermissions(c *C) { userName := "ws-test2" pass := []byte("ws-abc1234") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) user, _ := createUserAndRole(s.a, userName, []string{userName}) - otpURL, _, err := s.a.UpsertPassword(user.GetName(), pass) + err := s.a.UpsertPassword(user.GetName(), pass) + c.Assert(err, IsNil) + + err = s.a.UpsertTOTP(user.GetName(), otpSecret) + c.Assert(err, IsNil) + + otpURL, _, err := s.a.GetOTPData(userName) c.Assert(err, IsNil) // make sure label in url is correct @@ -401,14 +412,8 @@ func (s *TunSuite) TestPermissions(c *C) { c.Assert(err, IsNil) c.Assert(u.Path, Equals, "/ws-test2") - // extract otp key from signup url - otpKeyMeta, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - // create a valid otp token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) authMethod, err := NewWebPasswordAuth(user.GetName(), pass, validToken) @@ -466,8 +471,16 @@ func (s *TunSuite) TestSessionsBadPassword(c *C) { user := "system-test" pass := []byte("system-abc123") + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) - otpURL, _, err := s.a.UpsertPassword(user, pass) + err := s.a.UpsertPassword(user, pass) + c.Assert(err, IsNil) + + err = s.a.UpsertTOTP(user, otpSecret) + c.Assert(err, IsNil) + + otpURL, _, err := s.a.GetOTPData(user) c.Assert(err, IsNil) // make sure label in url is correct @@ -475,14 +488,8 @@ func (s *TunSuite) TestSessionsBadPassword(c *C) { c.Assert(err, IsNil) c.Assert(u.Path, Equals, "/system-test") - // extract otp key from signup url - otpKeyMeta, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - // create a valid otp token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) authMethod, err := NewWebPasswordAuth(user, pass, validToken) diff --git a/lib/services/identity.go b/lib/services/identity.go index b02d9965874..22317d9b44d 100644 --- a/lib/services/identity.go +++ b/lib/services/identity.go @@ -15,7 +15,7 @@ limitations under the License. */ // Package services implements API services exposed by Teleport: -// * presence service that takes care of heratbeats +// * presence service that takes care of heartbeats // * web service that takes care of web logins // * ca service - certificate authorities package services @@ -101,14 +101,7 @@ type Identity interface { DeleteWebSession(user, sid string) error // UpsertPassword upserts new password and OTP token - UpsertPassword(user string, password []byte) (otpURL string, otpQRCode []byte, err error) - - // CheckPassword is called on web user or tsh user login using the password and otp token - CheckPassword(user string, password []byte, otpToken string) error - - // CheckPasswordWOToken checks just password without checking HOTP tokens - // used in case of SSH authentication, when token has been validated - CheckPasswordWOToken(user string, password []byte) error + UpsertPassword(user string, password []byte) error // UpsertSignupToken upserts signup token - one time token that lets user to create a user account UpsertSignupToken(token string, tokenData SignupToken, ttl time.Duration) error diff --git a/lib/services/local/services_test.go b/lib/services/local/services_test.go index a8b7bab0999..0761e8e374c 100644 --- a/lib/services/local/services_test.go +++ b/lib/services/local/services_test.go @@ -86,14 +86,6 @@ func (s *ServicesSuite) TestPasswordHashCRUD(c *C) { s.suite.PasswordHashCRUD(c) } -func (s *ServicesSuite) TestPasswordAndHotpCRUD(c *C) { - s.suite.PasswordCRUD(c) -} - -func (s *ServicesSuite) TestPasswordGarbage(c *C) { - s.suite.PasswordGarbage(c) -} - func (s *ServicesSuite) TestWebSessionCRUD(c *C) { s.suite.WebSessionCRUD(c) } diff --git a/lib/services/local/users.go b/lib/services/local/users.go index e7b34bbce28..f8c4b9904e6 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -17,14 +17,11 @@ limitations under the License. package local import ( - "bytes" "crypto/ecdsa" - "crypto/subtle" "crypto/x509" "encoding/json" "errors" "fmt" - "image/png" "sort" "time" @@ -33,14 +30,12 @@ import ( "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/services" - "github.com/gravitational/teleport/lib/utils" log "github.com/Sirupsen/logrus" "github.com/gokyle/hotp" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" "github.com/pborman/uuid" - "github.com/pquerna/otp/totp" "github.com/tstranex/u2f" ) @@ -231,7 +226,7 @@ func (s *IdentityService) GetHOTP(user string) (*hotp.HOTP, error) { return otp, nil } -// UpsertTOTP upserts TOTP secret key for a user that can be used to generate and validate tokens +// UpsertTOTP upserts TOTP secret key for a user that can be used to generate and validate tokens. func (s *IdentityService) UpsertTOTP(user string, secretKey string) error { err := s.backend.UpsertVal([]string{"web", "users", user}, "totp", []byte(secretKey), 0) if err != nil { @@ -366,128 +361,23 @@ func (s *IdentityService) DeleteWebSession(user, sid string) error { return err } -// UpsertPassword upserts new password and OTP token and returns OTP url and QR code. -func (s *IdentityService) UpsertPassword(user string, password []byte) (otpURL string, otpQRCode []byte, err error) { - if err := services.VerifyPassword(password); err != nil { - return "", nil, err +// UpsertPassword upserts new password hash into a backend. +func (s *IdentityService) UpsertPassword(user string, password []byte) error { + err := services.VerifyPassword(password) + if err != nil { + return trace.Wrap(err) } hash, err := bcrypt.GenerateFromPassword(password, bcrypt.DefaultCost) if err != nil { - return "", nil, trace.Wrap(err) + return trace.Wrap(err) } err = s.UpsertPasswordHash(user, hash) - if err != nil { - return "", nil, trace.Wrap(err) - } - - otpURL, otpQRCode, err = s.createAndUpsertTOTP(user) - if err != nil { - return "", nil, trace.Wrap(err) - } - - return otpURL, otpQRCode, nil -} - -// CheckPassword is called on web user or tsh user login using the password and otp token -func (s *IdentityService) CheckPassword(user string, password []byte, otpToken string) error { - var err error - - hash, err := s.GetPasswordHash(user) if err != nil { return trace.Wrap(err) } - if err := bcrypt.CompareHashAndPassword(hash, password); err != nil { - return trace.AccessDenied("passwords do not match") - } - - return s.checkOTP(user, otpToken) -} - -// checkOTP determines the type of OTP token used (for legacy HOTP support), fetches the -// appropriate type from the backend, and checks the token. -func (s *IdentityService) checkOTP(user string, otpToken string) error { - var err error - - otpType, err := s.getOTPType(user) - if err != nil { - return trace.Wrap(err) - } - - switch otpType { - case "hotp": - otp, err := s.GetHOTP(user) - if err != nil { - return trace.Wrap(err) - } - - // look ahead n tokens to see if we can find a matching token - if !otp.Scan(otpToken, defaults.HOTPFirstTokensRange) { - return trace.AccessDenied("bad htop token") - } - - // we need to upsert the hotp state again because the - // counter was incremented - if err := s.UpsertHOTP(user, otp); err != nil { - return trace.Wrap(err) - } - case "totp": - otpSecret, err := s.GetTOTP(user) - if err != nil { - return trace.Wrap(err) - } - - usedToken, err := s.GetUsedTOTPToken(user) - if err != nil { - return trace.Wrap(err) - } - - if subtle.ConstantTimeCompare([]byte(otpToken), []byte(usedToken)) == 1 { - return trace.AccessDenied("previously used totp token") - } - - valid := totp.Validate(otpToken, otpSecret) - if !valid { - return trace.AccessDenied("invalid totp token") - } - - err = s.UpsertUsedTOTPToken(user, otpToken) - if err != nil { - return trace.Wrap(err) - } - } - - return nil -} - -// getOTPType returns the type of OTP token used, HOTP or TOTP. -// Deprecated: Remove this method once HOTP support has been removed. -func (s *IdentityService) getOTPType(user string) (string, error) { - _, err := s.GetHOTP(user) - if err != nil { - if trace.IsNotFound(err) { - return "totp", nil - } - return "", trace.Wrap(err) - } - return "hotp", nil -} - -// CheckPasswordWOToken checks just password without checking HOTP tokens -// used in case of SSH authentication, when token has been validated -func (s *IdentityService) CheckPasswordWOToken(user string, password []byte) error { - if err := services.VerifyPassword(password); err != nil { - return trace.Wrap(err) - } - hash, err := s.GetPasswordHash(user) - if err != nil { - return trace.Wrap(err) - } - if err := bcrypt.CompareHashAndPassword(hash, password); err != nil { - return trace.BadParameter("passwords do not match") - } return nil } @@ -786,34 +676,3 @@ func (s *IdentityService) GetOIDCAuthRequest(stateToken string) (*services.OIDCA } return req, nil } - -func (s *IdentityService) createAndUpsertTOTP(user string) (otpURI string, otpQRCode []byte, err error) { - // create totp key - otpKey, err := totp.Generate(totp.GenerateOpts{ - Issuer: "Teleport", - AccountName: user, - }) - if err != nil { - return "", nil, trace.Wrap(err) - } - - // create QR code - var otpQRBuf bytes.Buffer - otpImage, err := otpKey.Image(456, 456) - if err != nil { - return "", nil, trace.Wrap(err) - } - png.Encode(&otpQRBuf, otpImage) - - // create otp url, only the key is needed - otpKeyBytes := []byte(otpKey.Secret()) - otpURI = utils.GenerateOTPURL("totp", user, map[string][]byte{"secret": otpKeyBytes}) - - // upsert totp key into the backend - err = s.UpsertTOTP(user, otpKey.Secret()) - if err != nil { - return "", nil, trace.Wrap(err) - } - - return otpURI, otpQRBuf.Bytes(), nil -} diff --git a/lib/services/suite/suite.go b/lib/services/suite/suite.go index 3215fb31e95..08f3d28a827 100644 --- a/lib/services/suite/suite.go +++ b/lib/services/suite/suite.go @@ -19,9 +19,7 @@ package suite import ( "crypto/ecdsa" "crypto/x509" - "encoding/base32" "encoding/base64" - "net/url" "sort" "time" @@ -32,8 +30,6 @@ import ( "github.com/gravitational/trace" "github.com/jonboulle/clockwork" - "github.com/pquerna/otp" - "github.com/pquerna/otp/totp" "github.com/tstranex/u2f" "golang.org/x/crypto/ssh" @@ -372,51 +368,6 @@ func (s *ServicesTestSuite) TokenCRUD(c *C) { c.Assert(trace.IsNotFound(err), Equals, true, Commentf("%#v", err)) } -func (s *ServicesTestSuite) PasswordCRUD(c *C) { - pass := []byte("abc123") - - err := s.WebS.CheckPassword("user1", pass, "123456") - c.Assert(err, NotNil) - - // upsert a password which will create a new source of totp tokens - otpURL, _, err := s.WebS.UpsertPassword("user1", pass) - c.Assert(err, IsNil) - - // make sure the otp url we get back is valid url issued to the correct user - u, err := url.Parse(otpURL) - c.Assert(err, IsNil) - c.Assert(u.Path, Equals, "/user1") - - // extract otp secret key from url, we will use this to create - // tokens to make sure the url (and qr code) the user receives is - // the same as the one stored in the backend - otpKeyMeta, err := otp.NewKeyFromURL(u.String()) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) - c.Assert(err, IsNil) - - // a completely invalid token should return access denied - err = s.WebS.CheckPassword("user1", pass, "123456") - c.Assert(trace.IsAccessDenied(err), Equals, true, Commentf("%T", err)) - - // a invalid token (made 1 minute in the future but from a valid key) - // should also return access denied - invalidToken, err := totp.GenerateCode(string(otpKey), time.Now().Add(1*time.Minute)) - c.Assert(err, IsNil) - err = s.WebS.CheckPassword("user1", pass, invalidToken) - c.Assert(trace.IsAccessDenied(err), Equals, true, Commentf("%T", err)) - - // a valid token (created right now and from a valid key) should return success - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) - c.Assert(err, IsNil) - err = s.WebS.CheckPassword("user1", pass, validToken) - c.Assert(err, IsNil) - - // try the same valid token now it should fail because we don't allow re-use of tokens - err = s.WebS.CheckPassword("user1", pass, validToken) - c.Assert(err, NotNil) -} - func (s *ServicesTestSuite) RolesCRUD(c *C) { out, err := s.Access.GetRoles() c.Assert(err, IsNil) @@ -483,18 +434,6 @@ func (s *ServicesTestSuite) NamespacesCRUD(c *C) { c.Assert(trace.IsNotFound(err), Equals, true, Commentf("%T", err)) } -func (s *ServicesTestSuite) PasswordGarbage(c *C) { - garbage := [][]byte{ - nil, - make([]byte, defaults.MaxPasswordLength+1), - make([]byte, defaults.MinPasswordLength-1), - } - for _, g := range garbage { - err := s.WebS.CheckPassword("user1", g, "123456") - c.Assert(err, NotNil) - } -} - func (s *ServicesTestSuite) U2FCRUD(c *C) { token := "tok1" appId := "https://localhost" diff --git a/lib/utils/otp.go b/lib/utils/otp.go index ac984fe0dcf..865f61caaad 100644 --- a/lib/utils/otp.go +++ b/lib/utils/otp.go @@ -1,10 +1,37 @@ package utils import ( + "bytes" "encoding/base32" + "image/png" "net/url" + + "github.com/gravitational/trace" + + "github.com/pquerna/otp" ) +// GenerateQRCode takes in a OTP Key URL and returns a PNG-encoded QR code. +func GenerateQRCode(u string) ([]byte, error) { + otpKey, err := otp.NewKeyFromURL(u) + if err != nil { + return nil, trace.Wrap(err) + } + + otpImage, err := otpKey.Image(450, 450) + if err != nil { + return nil, trace.Wrap(err) + } + + var otpQR bytes.Buffer + err = png.Encode(&otpQR, otpImage) + if err != nil { + return nil, trace.Wrap(err) + } + + return otpQR.Bytes(), nil +} + // GenerateOTPURL returns a OTP Key URL that can be used to construct a HOTP or TOTP key. For more // details see: https://github.com/google/google-authenticator/wiki/Key-Uri-Format // Example: otpauth://totp/foo:bar@baz.com?secret=qux diff --git a/lib/web/web_test.go b/lib/web/web_test.go index 5b7e46c7003..ece9d0626e0 100644 --- a/lib/web/web_test.go +++ b/lib/web/web_test.go @@ -53,7 +53,6 @@ import ( "github.com/gokyle/hotp" "github.com/gravitational/roundtrip" "github.com/gravitational/trace" - "github.com/pquerna/otp" "github.com/pquerna/otp/totp" "github.com/tstranex/u2f" "golang.org/x/crypto/ssh" @@ -374,6 +373,8 @@ func (s *WebSuite) authPackFromResponse(c *C, re *roundtrip.Response) *authPack func (s *WebSuite) authPack(c *C) *authPack { user := s.user pass := "abc123" + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) teleUser, err := services.NewUser(user) c.Assert(err, IsNil) @@ -386,17 +387,14 @@ func (s *WebSuite) authPack(c *C) *authPack { err = s.roleAuth.UpsertUser(teleUser) c.Assert(err, IsNil) - otpURL, _, err := s.roleAuth.UpsertPassword(user, []byte(pass)) + err = s.roleAuth.UpsertPassword(user, []byte(pass)) c.Assert(err, IsNil) - // extract key from otp url - otpKeyMeta, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyMeta.Secret()) + err = s.roleAuth.UpsertTOTP(user, otpSecret) c.Assert(err, IsNil) // create a valid otp token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) clt := s.client() @@ -488,18 +486,17 @@ func (s *WebSuite) TestWebSessionsRenew(c *C) { func (s *WebSuite) TestWebSessionsBadInput(c *C) { user := "bob" pass := "abc123" + rawSecret := "def456" + otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret)) - otpURL, _, err := s.roleAuth.UpsertPassword(user, []byte(pass)) + err := s.roleAuth.UpsertPassword(user, []byte(pass)) c.Assert(err, IsNil) - // extract otp key from url - otpKeyObject, err := otp.NewKeyFromURL(otpURL) - c.Assert(err, IsNil) - otpKey, err := base32.StdEncoding.DecodeString(otpKeyObject.Secret()) + err = s.roleAuth.UpsertTOTP(user, otpSecret) c.Assert(err, IsNil) // create valid token - validToken, err := totp.GenerateCode(string(otpKey), time.Now()) + validToken, err := totp.GenerateCode(otpSecret, time.Now()) c.Assert(err, IsNil) clt := s.client()