[full-ci][bump reva] fix race in a publicshare manager

This commit is contained in:
Roman Perekhod
2024-01-23 12:44:09 +01:00
parent c9a77eb43f
commit 8dc7420b04
5 changed files with 35 additions and 29 deletions

View File

@@ -0,0 +1,7 @@
Bugfix: Fix concurrent access to a map
We fixed the race condition that led to concurrent map access in a publicshare manager.
https://github.com/owncloud/ocis/pull/8269
https://github.com/cs3org/reva/pull/4472
https://github.com/owncloud/ocis/issues/8255

2
go.mod
View File

@@ -13,7 +13,7 @@ require (
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/coreos/go-oidc/v3 v3.9.0
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781
github.com/cs3org/reva/v2 v2.18.1-0.20240122171534-e8fc07f2395a
github.com/cs3org/reva/v2 v2.18.1-0.20240123151850-049234cc31e0
github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25
github.com/disintegration/imaging v1.6.2
github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e

6
go.sum
View File

@@ -1018,10 +1018,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c=
github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/reva/v2 v2.18.1-0.20240122154113-d79f163775db h1:0nc89DfeCGRr98Z/NZor9fRyQ0pGxq0iQnnUsWFhnek=
github.com/cs3org/reva/v2 v2.18.1-0.20240122154113-d79f163775db/go.mod h1:plMbmaHczZbP+1rtV56YCYs5lkmpdRNpj0KZb9BWLus=
github.com/cs3org/reva/v2 v2.18.1-0.20240122171534-e8fc07f2395a h1:2FLa9HFgDlvXTAdDjqcHsb8Ky2eTBXIIR/ukZeemMKI=
github.com/cs3org/reva/v2 v2.18.1-0.20240122171534-e8fc07f2395a/go.mod h1:GCN3g6uYE0Nvd31dGlhaGGyUviUfbG2NkecPRv5oSc4=
github.com/cs3org/reva/v2 v2.18.1-0.20240123151850-049234cc31e0 h1:btY1QRE5X4bvp7V+PUkQptFFBkB9QoJTObDNrX9G3EU=
github.com/cs3org/reva/v2 v2.18.1-0.20240123151850-049234cc31e0/go.mod h1:GCN3g6uYE0Nvd31dGlhaGGyUviUfbG2NkecPRv5oSc4=
github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=

View File

@@ -219,6 +219,9 @@ func (m *manager) Dump(ctx context.Context, shareChan chan<- *publicshare.WithPa
// Load imports public shares and received shares from channels (e.g. during migration)
func (m *manager) Load(ctx context.Context, shareChan <-chan *publicshare.WithPassword) error {
m.mutex.Lock()
defer m.mutex.Unlock()
db, err := m.persistence.Read(ctx)
if err != nil {
return err
@@ -414,6 +417,9 @@ func (m *manager) UpdatePublicShare(ctx context.Context, u *user.User, req *link
// GetPublicShare gets a public share either by ID or Token.
func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.PublicShareReference, sign bool) (*link.PublicShare, error) {
m.mutex.Lock()
defer m.mutex.Unlock()
if ref.GetToken() != "" {
ps, pw, err := m.getByToken(ctx, ref.GetToken())
if err != nil {
@@ -428,9 +434,6 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu
return ps, nil
}
m.mutex.Lock()
defer m.mutex.Unlock()
db, err := m.persistence.Read(ctx)
if err != nil {
return nil, err
@@ -447,7 +450,7 @@ func (m *manager) GetPublicShare(ctx context.Context, u *user.User, ref *link.Pu
if ref.GetId().GetOpaqueId() == ps.Id.OpaqueId {
if publicshare.IsExpired(ps) {
if err := m.revokeExpiredPublicShare(ctx, &ps, u); err != nil {
if err := m.revokeExpiredPublicShare(ctx, &ps); err != nil {
return nil, err
}
return nil, errtypes.NotFound("no shares found by id:" + ref.GetId().String())
@@ -491,7 +494,7 @@ func (m *manager) ListPublicShares(ctx context.Context, u *user.User, filters []
}
if publicshare.IsExpired(local.PublicShare) {
if err := m.revokeExpiredPublicShare(ctx, &local.PublicShare, u); err != nil {
if err := m.revokeExpiredPublicShare(ctx, &local.PublicShare); err != nil {
log.Error().Err(err).
Str("share_token", local.Token).
Msg("failed to revoke expired public share")
@@ -561,20 +564,18 @@ func (m *manager) cleanupExpiredShares() {
_ = utils.UnmarshalJSONToProtoV1([]byte(d.(string)), &ps)
if publicshare.IsExpired(ps) {
_ = m.revokeExpiredPublicShare(context.Background(), &ps, nil)
_ = m.revokeExpiredPublicShare(context.Background(), &ps)
}
}
}
func (m *manager) revokeExpiredPublicShare(ctx context.Context, s *link.PublicShare, u *user.User) error {
// revokeExpiredPublicShare doesn't have a lock inside, ensure a lock before call
func (m *manager) revokeExpiredPublicShare(ctx context.Context, s *link.PublicShare) error {
if !m.enableExpiredSharesCleanup {
return nil
}
m.mutex.Unlock()
defer m.mutex.Lock()
err := m.RevokePublicShare(ctx, u, &link.PublicShareReference{
err := m.revokePublicShare(ctx, &link.PublicShareReference{
Spec: &link.PublicShareReference_Id{
Id: &link.PublicShareId{
OpaqueId: s.Id.OpaqueId,
@@ -590,13 +591,18 @@ func (m *manager) revokeExpiredPublicShare(ctx context.Context, s *link.PublicSh
}
// RevokePublicShare undocumented.
func (m *manager) RevokePublicShare(ctx context.Context, u *user.User, ref *link.PublicShareReference) error {
func (m *manager) RevokePublicShare(ctx context.Context, _ *user.User, ref *link.PublicShareReference) error {
m.mutex.Lock()
defer m.mutex.Unlock()
return m.revokePublicShare(ctx, ref)
}
// revokePublicShare doesn't have a lock inside, ensure a lock before call
func (m *manager) revokePublicShare(ctx context.Context, ref *link.PublicShareReference) error {
db, err := m.persistence.Read(ctx)
if err != nil {
return err
}
m.mutex.Unlock()
switch {
case ref.GetId() != nil && ref.GetId().OpaqueId != "":
@@ -615,20 +621,16 @@ func (m *manager) RevokePublicShare(ctx context.Context, u *user.User, ref *link
return errors.New("reference does not exist")
}
m.mutex.Lock()
defer m.mutex.Unlock()
return m.persistence.Write(ctx, db)
}
// getByToken doesn't have a lock inside, ensure a lock before call
func (m *manager) getByToken(ctx context.Context, token string) (*link.PublicShare, string, error) {
db, err := m.persistence.Read(ctx)
if err != nil {
return nil, "", err
}
m.mutex.Lock()
defer m.mutex.Unlock()
for _, v := range db {
var local link.PublicShare
if err := utils.UnmarshalJSONToProtoV1([]byte(v.(map[string]interface{})["share"].(string)), &local); err != nil {
@@ -646,14 +648,14 @@ func (m *manager) getByToken(ctx context.Context, token string) (*link.PublicSha
// GetPublicShareByToken gets a public share by its opaque token.
func (m *manager) GetPublicShareByToken(ctx context.Context, token string, auth *link.PublicShareAuthentication, sign bool) (*link.PublicShare, error) {
m.mutex.Lock()
defer m.mutex.Unlock()
db, err := m.persistence.Read(ctx)
if err != nil {
return nil, err
}
m.mutex.Lock()
defer m.mutex.Unlock()
for _, v := range db {
passDB := v.(map[string]interface{})["password"].(string)
var local link.PublicShare
@@ -663,8 +665,7 @@ func (m *manager) GetPublicShareByToken(ctx context.Context, token string, auth
if local.Token == token {
if publicshare.IsExpired(local) {
// TODO user is not needed at all in this API.
if err := m.revokeExpiredPublicShare(ctx, &local, nil); err != nil {
if err := m.revokeExpiredPublicShare(ctx, &local); err != nil {
return nil, err
}
break

2
vendor/modules.txt vendored
View File

@@ -362,7 +362,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1
github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1
github.com/cs3org/go-cs3apis/cs3/tx/v1beta1
github.com/cs3org/go-cs3apis/cs3/types/v1beta1
# github.com/cs3org/reva/v2 v2.18.1-0.20240122171534-e8fc07f2395a
# github.com/cs3org/reva/v2 v2.18.1-0.20240123151850-049234cc31e0
## explicit; go 1.21
github.com/cs3org/reva/v2/cmd/revad/internal/grace
github.com/cs3org/reva/v2/cmd/revad/runtime