Skip to content

Commit

Permalink
hash share passwords (#1462)
Browse files Browse the repository at this point in the history
Passwords should never be stored in plaintext. The manager stored them base64 encoded which is very easily decoded to plaintext. Now the passwords are hashed using the bcrypt algorithm with a default cost of 11. The cost can be configured.
  • Loading branch information
C0rby authored Feb 9, 2021
1 parent 990ed1b commit 345a281
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 13 deletions.
5 changes: 5 additions & 0 deletions changelog/unreleased/hash-public-share-password.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: Hash public share passwords

The share passwords were only base64 encoded. Added hashing using bcrypt with configurable hash cost.

https://github.com/cs3org/reva/pull/1462
39 changes: 26 additions & 13 deletions pkg/publicshare/manager/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ package filesystem
import (
"bytes"
"context"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"math/rand"
Expand All @@ -35,6 +33,7 @@ import (
"time"

"github.com/rs/zerolog/log"
"golang.org/x/crypto/bcrypt"

user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
Expand All @@ -46,6 +45,7 @@ import (
"github.com/cs3org/reva/pkg/publicshare/manager/registry"
"github.com/golang/protobuf/jsonpb"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"go.opencensus.io/trace"
)

Expand Down Expand Up @@ -87,10 +87,11 @@ func New(c map[string]interface{}) (publicshare.Manager, error) {
conf.init()

m := manager{
mutex: &sync.Mutex{},
marshaler: jsonpb.Marshaler{},
unmarshaler: jsonpb.Unmarshaler{},
file: conf.File,
mutex: &sync.Mutex{},
marshaler: jsonpb.Marshaler{},
unmarshaler: jsonpb.Unmarshaler{},
file: conf.File,
passwordHashCost: conf.SharePasswordHashCost,
}

// attempt to create the db file
Expand Down Expand Up @@ -120,21 +121,26 @@ func New(c map[string]interface{}) (publicshare.Manager, error) {
}

type config struct {
File string `mapstructure:"file"`
File string `mapstructure:"file"`
SharePasswordHashCost int `mapstructure:"password_hash_cost"`
}

func (c *config) init() {
if c.File == "" {
c.File = "/var/tmp/reva/publicshares"
}
if c.SharePasswordHashCost == 0 {
c.SharePasswordHashCost = 11
}
}

type manager struct {
mutex *sync.Mutex
file string

marshaler jsonpb.Marshaler
unmarshaler jsonpb.Unmarshaler
marshaler jsonpb.Marshaler
unmarshaler jsonpb.Unmarshaler
passwordHashCost int
}

// CreatePublicShare adds a new entry to manager.shares
Expand All @@ -154,7 +160,11 @@ func (m *manager) CreatePublicShare(ctx context.Context, u *user.User, rInfo *pr
var passwordProtected bool
password := g.Password
if len(password) > 0 {
password = base64.StdEncoding.EncodeToString([]byte(password))
h, err := bcrypt.GenerateFromPassword([]byte(password), m.passwordHashCost)
if err != nil {
return nil, errors.Wrap(err, "could not hash share password")
}
password = string(h)
passwordProtected = true
}

Expand Down Expand Up @@ -249,7 +259,11 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link
share.PasswordProtected = false
newPasswordEncoded = ""
} else {
newPasswordEncoded = base64.StdEncoding.EncodeToString([]byte(req.Update.GetGrant().Password))
h, err := bcrypt.GenerateFromPassword([]byte(req.Update.GetGrant().Password), m.passwordHashCost)
if err != nil {
return nil, errors.Wrap(err, "could not hash share password")
}
newPasswordEncoded = string(h)
share.PasswordProtected = true
}
default:
Expand Down Expand Up @@ -516,8 +530,7 @@ func (m *manager) GetPublicShareByToken(ctx context.Context, token, password str
}

if local.PasswordProtected {
password = base64.StdEncoding.EncodeToString([]byte(password))
if passDB == password {
if err := bcrypt.CompareHashAndPassword([]byte(passDB), []byte(password)); err == nil {
return local, nil
}

Expand Down

0 comments on commit 345a281

Please sign in to comment.