change hashing algorithm from SHA-512 to bcrypt

Even though SHA-512 is currently considered a secure algorithm it is not the best choice for password hashing. As this change introduces a breaking change it is beast to introduce it as early as possible to prevent us from having to implement a migration strategy Signed-off-by: David Christofas <[email protected]>
owncloud · Oct 1, 2020 · 20ada19 · 20ada19
1 parent f899ada
commit 20ada19
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 19 deletions.
diff --git a/accounts/changelog/unreleased/change-hashing-algorithm.md b/accounts/changelog/unreleased/change-hashing-algorithm.md
@@ -0,0 +1,7 @@
+Change: Use bcrypt to hash the user passwords 
+
+Change the hashing algorithm from SHA-512 to bcrypt since the latter is better suitable for password hashing.
+This is a breaking change. Existing deployments need to regenerate the accounts folder.
+
+
+https://github.com/owncloud/ocis/issues/510
diff --git a/accounts/pkg/service/v0/accounts.go b/accounts/pkg/service/v0/accounts.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"golang.org/x/crypto/bcrypt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -23,15 +24,12 @@ import (
 	settings "github.com/owncloud/ocis/settings/pkg/proto/v0"
 	settings_svc "github.com/owncloud/ocis/settings/pkg/service/v0"
 	"github.com/rs/zerolog"
-	"github.com/tredoe/osutil/user/crypt"
 	"google.golang.org/genproto/protobuf/field_mask"
 	"google.golang.org/protobuf/types/known/timestamppb"
+)
 
-	// register crypt functions
-	_ "github.com/tredoe/osutil/user/crypt/apr1_crypt"
-	_ "github.com/tredoe/osutil/user/crypt/md5_crypt"
-	_ "github.com/tredoe/osutil/user/crypt/sha256_crypt"
-	_ "github.com/tredoe/osutil/user/crypt/sha512_crypt"
+const (
+	_hashDifficulty = 12
 )
 
 // accLock mutually exclude readers from writers on account files
@@ -153,8 +151,7 @@ func (s Service) passwordIsValid(hash string, pwd string) (ok bool) {
 		}
 	}()
 
-	c := crypt.NewFromHash(hash)
-	return c.Verify(hash, []byte(pwd)) == nil
+	return bcrypt.CompareHashAndPassword([]byte(hash), []byte(pwd)) == nil
 }
 
 func (s Service) hasAccountManagementPermissions(ctx context.Context) bool {
@@ -330,11 +327,12 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
 	if acc.PasswordProfile != nil {
 		if acc.PasswordProfile.Password != "" {
 			// encrypt password
-			c := crypt.New(crypt.SHA512)
-			if acc.PasswordProfile.Password, err = c.Generate([]byte(acc.PasswordProfile.Password), nil); err != nil {
+			hashed, err := bcrypt.GenerateFromPassword([]byte(acc.PasswordProfile.Password), _hashDifficulty)
+			if err != nil {
 				s.log.Error().Err(err).Str("id", id).Msg("could not hash password")
 				return merrors.InternalServerError(s.id, "could not hash password: %v", err.Error())
 			}
+			acc.PasswordProfile.Password = string(hashed)
 		}
 
 		if err := passwordPoliciesValid(acc.PasswordProfile.PasswordPolicies); err != nil {
@@ -433,13 +431,13 @@ func (s Service) UpdateAccount(ctx context.Context, in *proto.UpdateAccountReque
 		}
 		if in.Account.PasswordProfile.Password != "" {
 			// encrypt password
-			c := crypt.New(crypt.SHA512)
-			if out.PasswordProfile.Password, err = c.Generate([]byte(in.Account.PasswordProfile.Password), nil); err != nil {
+			hashed, err := bcrypt.GenerateFromPassword([]byte(in.Account.PasswordProfile.Password), _hashDifficulty)
+			if err != nil {
 				in.Account.PasswordProfile.Password = ""
 				s.log.Error().Err(err).Str("id", id).Msg("could not hash password")
 				return merrors.InternalServerError(s.id, "could not hash password: %v", err.Error())
 			}
-
+			out.PasswordProfile.Password = string(hashed)
 			in.Account.PasswordProfile.Password = ""
 		}
 

diff --git a/accounts/pkg/service/v0/service.go b/accounts/pkg/service/v0/service.go
@@ -187,7 +187,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                20000,
 					GidNumber:                30000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=35210$sa1u5Pmfo4cr23Vw$RJNGElaDB1D3xorWkfTEGm2Ko.o2QL3E0cimKx23MNxVWVFSkUUeRoC7FqC4RzYDNQBD6cKzovTEaDD.8TDkD.",
+						Password: "$2a$12$qGho4QmaDn4HifisABQ2ROBc41TUCIBNrqwOg3zPEYednDWkXHOKG",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{
@@ -206,7 +206,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                20001,
 					GidNumber:                30000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=81434$sa1u5Pmfo4cr23Vw$W78cyL884GmuvDpxYPvSRBVzEj02T5QhTTcI8Dv4IKvMooDFGv4bwaWMkH9HfJ0wgpEBW7Lp.4Cad0xE/MYSg1",
+						Password: "$2a$12$JzfOtyRiGL25w1UpmiPZ1uU7KJURoTMPSRlZQvMB90/1zqvjwEWgO",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{
@@ -225,7 +225,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                20002,
 					GidNumber:                30000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=5524$sa1u5Pmfo4cr23Vw$58bQVL/JeUlwM0RY21YKAFMvKvwKLLysGllYXox.vwKT5dHMwdzJjCxwTDMnB2o2pwexC8o/iOXyP2zrhALS40",
+						Password: "$2a$12$bWMHNfc92rDForMapNle/eJ1fOY0eTeRzpk1EQFUdKGI6UdTktp/a",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{
@@ -245,7 +245,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                20003,
 					GidNumber:                30000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=47068$lhw6odzXW0LTk/ao$GgxS.pIgP8jawLJBAiyNor2FrWzrULF95PwspRkli2W3VF.4HEwTYlQfRXbNQBMjNCEcEYlgZo3a.kRz2k2N0/",
+						Password: "$2a$12$TY7jDd1PNbZXzZJMvIFm3eXAL4wjzOl.QXJZ6GKGDpAmUcvHBNc66",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{
@@ -262,7 +262,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                10000,
 					GidNumber:                15000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=9746$sa1u5Pmfo4cr23Vw$2hnwpkTvUkWX0v6mh8Aw1pbzEXa9EUJzmrey4g2W/8arwWCwhteqU//3aWnA3S0d5T21fOKYteoqlsN1IbTcN.",
+						Password: "$2a$12$pV8JlGlpM9oo1RaC7kSg/uBTFoTOJ8XfjibFL2U4gOpPKVfbD6pUG",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{
@@ -278,7 +278,7 @@ func (s Service) createDefaultAccounts(accountsDir string) (err error) {
 					UidNumber:                10001,
 					GidNumber:                15000,
 					PasswordProfile: &proto.PasswordProfile{
-						Password: "$6$rounds=91087$sa1u5Pmfo4cr23Vw$wPC3BbMTbP/ytlo0p.f99zJifyO70AUCdKIK9hkhwutBKGCirLmZs/MsWAG6xHjVvmnmHN5NoON7FUGv5pPaN.",
+						Password: "$2a$12$CSM.vkX9o7lO/uvid3XieOVtmq5nh91MFZHvHIsfRms3hLzTa2W6.",
 					},
 					AccountEnabled: true,
 					MemberOf: []*proto.Group{