Skip to content

Commit

Permalink
Merge pull request #638 from owncloud/accounts-change-hashing-algorithm
Browse files Browse the repository at this point in the history
change hashing algorithm from SHA-512 to bcrypt
  • Loading branch information
C0rby authored Nov 11, 2020
2 parents ec8a45a + 1406350 commit efe3702
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 21 deletions.
2 changes: 2 additions & 0 deletions .drone.star
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,8 @@ def ocisServer(storage):
'KONNECTD_IDENTIFIER_REGISTRATION_CONF': '/drone/src/ocis/tests/config/drone/identifier-registration.yml',
'KONNECTD_ISS': 'https://ocis-server:9200',
'KONNECTD_TLS': 'true',
# 4 is the lowest possible value. ONLY FOR TESTS
'ACCOUNTS_HASH_DIFFICULTY': 4,
},
'commands': [
'apk add mailcap', # install /etc/mime.types
Expand Down
1 change: 1 addition & 0 deletions accounts/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type Server struct {
Version string
Name string
AccountsDataPath string
HashDifficulty int
}

// Asset defines the available asset configuration.
Expand Down
7 changes: 7 additions & 0 deletions accounts/pkg/flagset/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ func ServerWithConfig(cfg *config.Config) []cli.Flag {
EnvVars: []string{"ACCOUNTS_DATA_PATH"},
Destination: &cfg.Server.AccountsDataPath,
},
&cli.IntFlag{
Name: "accounts-hash-difficulty",
Value: 11,
Usage: "accounts password hash difficulty",
EnvVars: []string{"ACCOUNTS_HASH_DIFFICULTY"},
Destination: &cfg.Server.HashDifficulty,
},
&cli.StringFlag{
Name: "asset-path",
Value: "",
Expand Down
23 changes: 9 additions & 14 deletions accounts/pkg/service/v0/accounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package service
import (
"context"
"fmt"
"golang.org/x/crypto/bcrypt"
"path"
"regexp"
"strconv"
Expand All @@ -24,15 +25,8 @@ 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"
)

// accLock mutually exclude readers from writers on account files
Expand Down Expand Up @@ -317,11 +311,13 @@ func (s Service) CreateAccount(ctx context.Context, in *proto.CreateAccountReque
if out.PasswordProfile != nil {
if out.PasswordProfile.Password != "" {
// encrypt password
c := crypt.New(crypt.SHA512)
if out.PasswordProfile.Password, err = c.Generate([]byte(out.PasswordProfile.Password), nil); err != nil {
hashed, err := bcrypt.GenerateFromPassword([]byte(in.Account.PasswordProfile.Password), s.Config.Server.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())
}
out.PasswordProfile.Password = string(hashed)
in.Account.PasswordProfile.Password = ""
}

if err := passwordPoliciesValid(out.PasswordProfile.PasswordPolicies); err != nil {
Expand Down Expand Up @@ -499,13 +495,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), s.Config.Server.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 = ""
}

Expand Down Expand Up @@ -805,6 +801,5 @@ func isPasswordValid(logger log.Logger, 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
}
14 changes: 7 additions & 7 deletions accounts/pkg/service/v0/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 20000,
GidNumber: 30000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=35210$sa1u5Pmfo4cr23Vw$RJNGElaDB1D3xorWkfTEGm2Ko.o2QL3E0cimKx23MNxVWVFSkUUeRoC7FqC4RzYDNQBD6cKzovTEaDD.8TDkD.",
Password: "$2a$11$4WNffzgU/WrIRiDnwu8OnOwgOIIUqR/2Ptvp7WJAQCTSgSrylyuvC",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -172,7 +172,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 20001,
GidNumber: 30000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=81434$sa1u5Pmfo4cr23Vw$W78cyL884GmuvDpxYPvSRBVzEj02T5QhTTcI8Dv4IKvMooDFGv4bwaWMkH9HfJ0wgpEBW7Lp.4Cad0xE/MYSg1",
Password: "$2a$11$Wu2XcDnE6G2No8C88FVWluNHyXuQQi0cHzSe82Vni8AdwIO12fphC",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -191,7 +191,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 20002,
GidNumber: 30000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=5524$sa1u5Pmfo4cr23Vw$58bQVL/JeUlwM0RY21YKAFMvKvwKLLysGllYXox.vwKT5dHMwdzJjCxwTDMnB2o2pwexC8o/iOXyP2zrhALS40",
Password: "$2a$11$6Lak4zh1xUkpObg2rrOotOTdQYGj2Uu/sowcVLhub.8qYIr.CxzEW",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -211,7 +211,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 20003,
GidNumber: 30000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=47068$lhw6odzXW0LTk/ao$GgxS.pIgP8jawLJBAiyNor2FrWzrULF95PwspRkli2W3VF.4HEwTYlQfRXbNQBMjNCEcEYlgZo3a.kRz2k2N0/",
Password: "$2a$11$jvI6PHuvrimpcCHzL2Q2WOqfm1FGdYAuSYZBDahr/B48fpiFxyDy2",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -227,7 +227,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 20004,
GidNumber: 30000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=95551$/bdqsmiGleA20kAS$rCAvHV7wjaHVF5nEVAnpW7mugRqcnPmdU4UPqhSroE74gXFxNGZflCF.ZyHwocDwgAw3uLkqsCzB1h5bXBjYB0",
Password: "$2a$11$En9VIUtqOdDyUl.LuUq2KeuBb5A2n8zE0lkJ2v6IDRSaOamhNq6Uu",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -244,7 +244,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 10000,
GidNumber: 15000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=9746$sa1u5Pmfo4cr23Vw$2hnwpkTvUkWX0v6mh8Aw1pbzEXa9EUJzmrey4g2W/8arwWCwhteqU//3aWnA3S0d5T21fOKYteoqlsN1IbTcN.",
Password: "$2a$11$ntoTP2W/kyQIuoYpH5mRBuNzaEERYWSwn/zCsY5rtffen4d41y9.6",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand All @@ -260,7 +260,7 @@ func (s Service) createDefaultAccounts() (err error) {
UidNumber: 10001,
GidNumber: 15000,
PasswordProfile: &proto.PasswordProfile{
Password: "$6$rounds=91087$sa1u5Pmfo4cr23Vw$wPC3BbMTbP/ytlo0p.f99zJifyO70AUCdKIK9hkhwutBKGCirLmZs/MsWAG6xHjVvmnmHN5NoON7FUGv5pPaN.",
Password: "$2a$11$40xzy3rO8Tq4j2VkFbKz8Ow19BRaqaixEjAR0IbvQXxtOvMtkjwzy",
},
AccountEnabled: true,
MemberOf: []*proto.Group{
Expand Down
7 changes: 7 additions & 0 deletions changelog/unreleased/change-hashing-algorithm.md
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit efe3702

Please sign in to comment.