Skip to content

Commit

Permalink
feat: enforce passwords on public share provider
Browse files Browse the repository at this point in the history
  • Loading branch information
@micbar
micbar committed Nov 29, 2023
1 parent ea8d133 commit 8e7c024
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 44 deletions.
96 changes: 85 additions & 11 deletions internal/grpc/services/publicshareprovider/publicshareprovider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
link "github.com/cs3org/go-cs3apis/cs3/sharing/link/v1beta1"
"github.com/cs3org/reva/v2/pkg/password"
"github.com/mitchellh/mapstructure"
"github.com/pkg/errors"
"google.golang.org/grpc"
Expand All @@ -38,6 +39,8 @@ import (
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
)

const getUserCtxErrMsg = "error getting user from context"

func init() {
rgrpc.Register("publicshareprovider", New)
}
Expand All @@ -48,6 +51,17 @@ type config struct {
AllowedPathsForShares []string `mapstructure:"allowed_paths_for_shares"`
EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"`
WriteableShareMustHavePassword bool `mapstructure:"writeable_share_must_have_password"`
PublicShareMustHavePassword bool `mapstructure:"public_share_must_have_password"`
PasswordPolicy map[string]interface{} `mapstructure:"password_policy"`
}

type passwordPolicy struct {
MinCharacters int `mapstructure:"min_characters"`
MinLowerCaseCharacters int `mapstructure:"min_lowercase_characters"`
MinUpperCaseCharacters int `mapstructure:"min_uppercase_characters"`
MinDigits int `mapstructure:"min_digits"`
MinSpecialCharacters int `mapstructure:"min_special_characters"`
BannedPasswordsList map[string]struct{} `mapstructure:"banned_passwords_list"`
}

func (c *config) init() {
Expand All @@ -60,6 +74,7 @@ type service struct {
conf *config
sm publicshare.Manager
allowedPathsForShares []*regexp.Regexp
passwordValidator password.Validator
}

func getShareManager(c *config) (publicshare.Manager, error) {
Expand Down Expand Up @@ -90,13 +105,26 @@ func parseConfig(m map[string]interface{}) (*config, error) {
return c, nil
}

func parsePasswordPolicy(m map[string]interface{}) (*passwordPolicy, error) {
p := &passwordPolicy{}
if err := mapstructure.Decode(m, p); err != nil {
err = errors.Wrap(err, "error decoding conf")
return nil, err
}
return p, nil
}

// New creates a new user share provider svc
func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {

c, err := parseConfig(m)
if err != nil {
return nil, err
}
p, err := parsePasswordPolicy(c.PasswordPolicy)
if err != nil {
return nil, err
}

c.init()

Expand All @@ -118,11 +146,26 @@ func New(m map[string]interface{}, ss *grpc.Server) (rgrpc.Service, error) {
conf: c,
sm: sm,
allowedPathsForShares: allowedPathsForShares,
passwordValidator: passwordPolicies(p),
}

return service, nil
}

func passwordPolicies(c *passwordPolicy) password.Validator {
if c == nil {
return password.NewPasswordPolicy(0, 0, 0, 0, 0, nil)
}
return password.NewPasswordPolicy(
c.MinCharacters,
c.MinLowerCaseCharacters,
c.MinUpperCaseCharacters,
c.MinDigits,
c.MinSpecialCharacters,
c.BannedPasswordsList,
)
}

func (s *service) isPathAllowed(path string) bool {
if len(s.allowedPathsForShares) == 0 {
return true
Expand All @@ -139,6 +182,15 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS
log := appctx.GetLogger(ctx)
log.Info().Str("publicshareprovider", "create").Msg("create public share")

// check that user has share permissions
// TODO Check if we need to stat again, because the client could send fake permissions
if !req.GetResourceInfo().GetPermissionSet().AddGrant {
err := errors.New("no share permission")
return &link.CreatePublicShareResponse{
Status: status.NewPermissionDenied(ctx, err, err.Error()),
}, nil
}

if !conversions.SufficientCS3Permissions(req.GetResourceInfo().GetPermissionSet(), req.GetGrant().GetPermissions().GetPermissions()) {
return &link.CreatePublicShareResponse{
Status: status.NewInvalid(ctx, "insufficient permissions to create that kind of share"),
Expand All @@ -151,17 +203,33 @@ func (s *service) CreatePublicShare(ctx context.Context, req *link.CreatePublicS
}, nil
}

// check that this is a valid share
if req.GetResourceInfo().GetId().GetOpaqueId() == req.GetResourceInfo().GetId().GetSpaceId() &&
req.GetResourceInfo().GetSpace().GetSpaceType() == "personal" {
return &link.CreatePublicShareResponse{
Status: status.NewInvalid(ctx, "cannot create link on space root"),
}, nil
}

grant := req.GetGrant()
if grant != nil && s.conf.WriteableShareMustHavePassword &&
publicshare.IsWriteable(grant.GetPermissions()) && grant.Password == "" {
if enforcePassword(grant, s.conf) {
return &link.CreatePublicShareResponse{
Status: status.NewInvalid(ctx, "writeable shares must have a password protection"),
Status: status.NewInvalid(ctx, "password protection is enforced"),
}, nil
}

setPassword := grant.GetPassword()
if len(setPassword) > 0 {
if err := s.passwordValidator.Validate(setPassword); err != nil {
return &link.CreatePublicShareResponse{
Status: status.NewInvalid(ctx, err.Error()),
}, nil
}
}

u, ok := ctxpkg.ContextGetUser(ctx)
if !ok {
log.Error().Msg("error getting user from context")
log.Error().Msg(getUserCtxErrMsg)
}

res := &link.CreatePublicShareResponse{}
Expand Down Expand Up @@ -227,7 +295,7 @@ func (s *service) GetPublicShare(ctx context.Context, req *link.GetPublicShareRe

u, ok := ctxpkg.ContextGetUser(ctx)
if !ok {
log.Error().Msg("error getting user from context")
log.Error().Msg(getUserCtxErrMsg)
}

ps, err := s.sm.GetPublicShare(ctx, u, req.Ref, req.GetSign())
Expand Down Expand Up @@ -281,16 +349,11 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS

u, ok := ctxpkg.ContextGetUser(ctx)
if !ok {
log.Error().Msg("error getting user from context")
log.Error().Msg(getUserCtxErrMsg)
}

updateR, err := s.sm.UpdatePublicShare(ctx, u, req)
if err != nil {
if errors.Is(err, publicshare.ErrShareNeedsPassword) {
return &link.UpdatePublicShareResponse{
Status: status.NewInvalid(ctx, err.Error()),
}, nil
}
return &link.UpdatePublicShareResponse{
Status: status.NewInternal(ctx, err.Error()),
}, nil
Expand All @@ -302,3 +365,14 @@ func (s *service) UpdatePublicShare(ctx context.Context, req *link.UpdatePublicS
}
return res, nil
}

func enforcePassword(grant *link.Grant, conf *config) bool {
if conf.PublicShareMustHavePassword {
return true
}
isReadOnly := conversions.SufficientCS3Permissions(conversions.NewViewerRole(true).CS3ResourcePermissions(), grant.GetPermissions().GetPermissions())
if !isReadOnly && conf.WriteableShareMustHavePassword {
return true
}
return false
}
39 changes: 14 additions & 25 deletions pkg/publicshare/manager/json/json.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func NewFile(c map[string]interface{}) (publicshare.Manager, error) {
return nil, err
}

return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword)
return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p)
}

// NewMemory returns a new in-memory public shares manager.
Expand All @@ -93,7 +93,7 @@ func NewMemory(c map[string]interface{}) (publicshare.Manager, error) {
return nil, err
}

return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword)
return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p)
}

// NewCS3 returns a new cs3 public shares manager.
Expand All @@ -115,19 +115,18 @@ func NewCS3(c map[string]interface{}) (publicshare.Manager, error) {
return nil, err
}

return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p, conf.WriteableShareMustHavePassword)
return New(conf.GatewayAddr, conf.SharePasswordHashCost, conf.JanitorRunInterval, conf.EnableExpiredSharesCleanup, p)
}

// New returns a new public share manager instance
func New(gwAddr string, pwHashCost, janitorRunInterval int, enableCleanup bool, p persistence.Persistence, writeableShareMustHavePassword bool) (publicshare.Manager, error) {
func New(gwAddr string, pwHashCost, janitorRunInterval int, enableCleanup bool, p persistence.Persistence) (publicshare.Manager, error) {
m := &manager{
gatewayAddr: gwAddr,
mutex: &sync.Mutex{},
passwordHashCost: pwHashCost,
janitorRunInterval: janitorRunInterval,
enableExpiredSharesCleanup: enableCleanup,
persistence: p,
writeableShareMustHavePassword: writeableShareMustHavePassword,
gatewayAddr: gwAddr,
mutex: &sync.Mutex{},
passwordHashCost: pwHashCost,
janitorRunInterval: janitorRunInterval,
enableExpiredSharesCleanup: enableCleanup,
persistence: p,
}

go m.startJanitorRun()
Expand All @@ -140,6 +139,7 @@ type commonConfig struct {
JanitorRunInterval int `mapstructure:"janitor_run_interval"`
EnableExpiredSharesCleanup bool `mapstructure:"enable_expired_shares_cleanup"`
WriteableShareMustHavePassword bool `mapstructure:"writeable_share_must_have_password"`
PublicShareMustHavePassword bool `mapstructure:"public_share_must_have_password"`
}

type fileConfig struct {
Expand Down Expand Up @@ -171,10 +171,9 @@ type manager struct {
mutex *sync.Mutex
persistence persistence.Persistence

passwordHashCost int
janitorRunInterval int
enableExpiredSharesCleanup bool
writeableShareMustHavePassword bool
passwordHashCost int
janitorRunInterval int
enableExpiredSharesCleanup bool
}

func (m *manager) startJanitorRun() {
Expand Down Expand Up @@ -343,12 +342,6 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link
old, _ := json.Marshal(share.Permissions)
new, _ := json.Marshal(req.Update.GetGrant().Permissions)

if m.writeableShareMustHavePassword &&
publicshare.IsWriteable(req.GetUpdate().GetGrant().GetPermissions()) &&
(!share.PasswordProtected && req.GetUpdate().GetGrant().GetPassword() == "") {
return nil, publicshare.ErrShareNeedsPassword
}

if req.GetUpdate().GetGrant().GetPassword() != "" {
passwordChanged = true
h, err := bcrypt.GenerateFromPassword([]byte(req.Update.GetGrant().Password), m.passwordHashCost)
Expand All @@ -369,10 +362,6 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link
case link.UpdatePublicShareRequest_Update_TYPE_PASSWORD:
passwordChanged = true
if req.Update.GetGrant().Password == "" {
if m.writeableShareMustHavePassword && publicshare.IsWriteable(share.Permissions) {
return nil, publicshare.ErrShareNeedsPassword
}

share.PasswordProtected = false
newPasswordEncoded = ""
} else {
Expand Down
4 changes: 2 additions & 2 deletions pkg/publicshare/manager/json/json_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ var _ = Describe("Json", func() {
persistence := cs3.New(storage)
Expect(persistence.Init(context.Background())).To(Succeed())

m, err = json.New("https://localhost:9200", 11, 60, false, persistence, false)
m, err = json.New("https://localhost:9200", 11, 60, false, persistence)
Expect(err).ToNot(HaveOccurred())

ctx = ctxpkg.ContextSetUser(context.Background(), user1)
Expand Down Expand Up @@ -228,7 +228,7 @@ var _ = Describe("Json", func() {
p := cs3.New(storage)
Expect(p.Init(context.Background())).To(Succeed())

m, err = json.New("https://localhost:9200", 11, 60, false, p, false)
m, err = json.New("https://localhost:9200", 11, 60, false, p)
Expect(err).ToNot(HaveOccurred())

ps, err := m.ListPublicShares(ctx, user1, []*link.ListPublicSharesRequest_Filter{}, false)
Expand Down
6 changes: 0 additions & 6 deletions pkg/publicshare/publicshare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (
"crypto/sha256"
"crypto/sha512"
"encoding/hex"
"errors"
"time"

user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
Expand All @@ -41,11 +40,6 @@ const (
StorageIDFilterType link.ListPublicSharesRequest_Filter_Type = 4
)

var (
// ErrShareNeedsPassword is an error which is returned when a public share must have a password.
ErrShareNeedsPassword = errors.New("the public share needs to have a password")
)

// Manager manipulates public shares.
type Manager interface {
CreatePublicShare(ctx context.Context, u *user.User, md *provider.ResourceInfo, g *link.Grant) (*link.PublicShare, error)
Expand Down

0 comments on commit 8e7c024

Please sign in to comment.