Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change hashing algorithm from SHA-512 to bcrypt #638

Merged
merged 2 commits into from
Nov 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
kulmann marked this conversation as resolved.
Show resolved Hide resolved
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