From 94808bb4a943ec17d6325ca11d0bd84917549af9 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Fri, 16 Jun 2023 23:18:21 -0700 Subject: [PATCH 1/3] refactor iam service for blind backend store --- backend/auth/acl.go | 11 +- backend/auth/iam.go | 266 ++++++++++++++-------------- backend/posix/posix.go | 118 +++++++++--- cmd/versitygw/main.go | 7 +- cmd/versitygw/posix.go | 2 +- cmd/versitygw/scoutfs.go | 2 +- s3api/controllers/admin.go | 6 +- s3api/controllers/base.go | 10 +- s3api/middlewares/authentication.go | 25 ++- s3api/router_test.go | 2 +- s3api/server.go | 4 +- s3api/server_test.go | 2 +- 12 files changed, 260 insertions(+), 195 deletions(-) diff --git a/backend/auth/acl.go b/backend/auth/acl.go index d9630336..976e10c2 100644 --- a/backend/auth/acl.go +++ b/backend/auth/acl.go @@ -76,7 +76,7 @@ func ParseACLOutput(data []byte) (GetBucketAclOutput, error) { }, nil } -func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMConfig) error { +func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMService) error { if acl.Owner != *input.AccessControlPolicy.Owner.ID { return s3err.GetAPIError(s3err.ErrAccessDenied) } @@ -141,12 +141,15 @@ func UpdateACL(input *s3.PutBucketAclInput, acl ACL, iam IAMConfig) error { return nil } -func checkIfAccountsExist(accs []string, iam IAMConfig) ([]string, error) { +func checkIfAccountsExist(accs []string, iam IAMService) ([]string, error) { result := []string{} for _, acc := range accs { - _, ok := iam.AccessAccounts[acc] - if !ok { + _, err := iam.GetUserAccount(acc) + if err != nil && err != ErrNoSuchUser { + return nil, fmt.Errorf("check user account: %w", err) + } + if err == nil { result = append(result, acc) } } diff --git a/backend/auth/iam.go b/backend/auth/iam.go index a6bf53ec..8e7c7582 100644 --- a/backend/auth/iam.go +++ b/backend/auth/iam.go @@ -16,187 +16,179 @@ package auth import ( "encoding/json" + "errors" "fmt" - "os" + "hash/crc32" "sync" - - "github.com/versity/versitygw/s3err" ) +// Account is an internal IAM account type Account struct { Secret string `json:"secret"` Role string `json:"role"` - Region string `json:"region"` } +// UpdateAcctFunc accepts the current data and returns the new data to be stored +type UpdateAcctFunc func([]byte) ([]byte, error) + +// Storer is the interface to manage the peristent IAM data for the internal +// IAM service +type Storer interface { + InitIAM() error + GetIAM() ([]byte, error) + StoreIAM(UpdateAcctFunc) error +} + +// IAMConfig stores all internal IAM accounts type IAMConfig struct { AccessAccounts map[string]Account `json:"accessAccounts"` } -type AccountsCache struct { - mu sync.Mutex - Accounts map[string]Account -} - -func (c *AccountsCache) getAccount(access string) *Account { - c.mu.Lock() - defer c.mu.Unlock() - - acc, ok := c.Accounts[access] - if !ok { - return nil - } - - return &acc -} - -func (c *AccountsCache) updateAccounts() error { - c.mu.Lock() - defer c.mu.Unlock() - - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("error reading config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return fmt.Errorf("error parsing the data: %w", err) - } - - c.Accounts = data.AccessAccounts - - return nil -} - -func (c *AccountsCache) deleteAccount(access string) { - c.mu.Lock() - defer c.mu.Unlock() - delete(c.Accounts, access) -} - +// IAMService is the interface for all IAM service implementations type IAMService interface { - GetIAMConfig() (*IAMConfig, error) - CreateAccount(access string, account *Account) error - GetUserAccount(access string) *Account + CreateAccount(access string, account Account) error + GetUserAccount(access string) (Account, error) DeleteUserAccount(access string) error } -type IAMServiceUnsupported struct { - accCache *AccountsCache +// IAMServiceInternal manages the internal IAM service +type IAMServiceInternal struct { + storer Storer + + mu sync.RWMutex + accts IAMConfig + serial uint32 } -var _ IAMService = &IAMServiceUnsupported{} +var _ IAMService = &IAMServiceInternal{} -func InitIAM() (IAMService, error) { - _, err := os.ReadFile("users.json") +// NewInternal creates a new instance for the Internal IAM service +func NewInternal(s Storer) (*IAMServiceInternal, error) { + i := &IAMServiceInternal{ + storer: s, + } + + err := i.updateCache() if err != nil { - jsonData, err := json.MarshalIndent(IAMConfig{AccessAccounts: map[string]Account{}}, "", " ") + return nil, fmt.Errorf("refresh iam cache: %w", err) + } + + return i, nil +} + +// CreateAccount creates a new IAM account. Returns an error if the account +// already exists. +func (s *IAMServiceInternal) CreateAccount(access string, account Account) error { + s.mu.Lock() + defer s.mu.Unlock() + + return s.storer.StoreIAM(func(data []byte) ([]byte, error) { + var conf IAMConfig + + if len(data) > 0 { + if err := json.Unmarshal(data, &conf); err != nil { + return nil, fmt.Errorf("failed to parse iam: %w", err) + } + } else { + conf.AccessAccounts = make(map[string]Account) + } + + _, ok := conf.AccessAccounts[access] + if ok { + return nil, fmt.Errorf("account already exists") + } + conf.AccessAccounts[access] = account + + b, err := json.Marshal(s.accts) if err != nil { - return nil, err + return nil, fmt.Errorf("failed to serialize iam: %w", err) } - if err := os.WriteFile("users.json", jsonData, 0644); err != nil { - return nil, err - } - } - return &IAMServiceUnsupported{accCache: &AccountsCache{Accounts: map[string]Account{}}}, nil + return b, nil + }) } -func (IAMServiceUnsupported) GetIAMConfig() (*IAMConfig, error) { - return nil, s3err.GetAPIError(s3err.ErrNotImplemented) -} +var ErrNoSuchUser = errors.New("user not found") -func GetIAMConfig() (*IAMConfig, error) { - var data IAMConfig +// GetUserAccount retrieves account info for the requested user. Returns +// ErrNoSuchUser if the account does not exist. +func (s *IAMServiceInternal) GetUserAccount(access string) (Account, error) { + s.mu.RLock() + defer s.mu.RUnlock() - file, err := os.ReadFile("users.json") + data, err := s.storer.GetIAM() if err != nil { - return nil, fmt.Errorf("unable to read config file: %w", err) + return Account{}, fmt.Errorf("get iam data: %w", err) } - if err := json.Unmarshal(file, &data); err != nil { - return nil, err - } - - return &data, nil -} - -func (s IAMServiceUnsupported) CreateAccount(access string, account *Account) error { - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("unable to read config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return err - } - - _, ok := data.AccessAccounts[access] - if ok { - return fmt.Errorf("user with the given access already exists") - } - - data.AccessAccounts[access] = *account - - updatedJSON, err := json.MarshalIndent(data, "", " ") - if err != nil { - return err - } - - if err := os.WriteFile("users.json", updatedJSON, 0644); err != nil { - return err - } - - return nil -} - -func (s IAMServiceUnsupported) GetUserAccount(access string) *Account { - acc := s.accCache.getAccount(access) - if acc == nil { - err := s.accCache.updateAccounts() + serial := crc32.ChecksumIEEE(data) + if serial != s.serial { + s.mu.RUnlock() + err := s.updateCache() + s.mu.RUnlock() if err != nil { - return nil + return Account{}, fmt.Errorf("refresh iam cache: %w", err) } - - return s.accCache.getAccount(access) } - return acc -} - -func (s IAMServiceUnsupported) DeleteUserAccount(access string) error { - var data IAMConfig - - file, err := os.ReadFile("users.json") - if err != nil { - return fmt.Errorf("unable to read config file: %w", err) - } - - if err := json.Unmarshal(file, &data); err != nil { - return fmt.Errorf("failed to parse the config file: %w", err) - } - - _, ok := data.AccessAccounts[access] + acct, ok := s.accts.AccessAccounts[access] if !ok { - return fmt.Errorf("invalid access for the user: user does not exist") + return Account{}, ErrNoSuchUser } - delete(data.AccessAccounts, access) + return acct, nil +} - updatedJSON, err := json.MarshalIndent(data, "", " ") +// updateCache must be called with no locks held +func (s *IAMServiceInternal) updateCache() error { + s.mu.Lock() + defer s.mu.Unlock() + + data, err := s.storer.GetIAM() if err != nil { - return fmt.Errorf("failed to parse the data: %w", err) + return fmt.Errorf("get iam data: %w", err) } - if err := os.WriteFile("users.json", updatedJSON, 0644); err != nil { - return fmt.Errorf("failed to saved the changes: %w", err) + serial := crc32.ChecksumIEEE(data) + + if len(data) > 0 { + if err := json.Unmarshal(data, &s.accts); err != nil { + return fmt.Errorf("failed to parse the config file: %w", err) + } + } else { + s.accts.AccessAccounts = make(map[string]Account) } - s.accCache.deleteAccount(access) + s.serial = serial return nil } + +// DeleteUserAccount deletes the specified user account. Does not check if +// account exists. +func (s *IAMServiceInternal) DeleteUserAccount(access string) error { + s.mu.Lock() + defer s.mu.Unlock() + + return s.storer.StoreIAM(func(data []byte) ([]byte, error) { + if len(data) == 0 { + // empty config, do nothing + return data, nil + } + + var conf IAMConfig + + if err := json.Unmarshal(data, &conf); err != nil { + return nil, fmt.Errorf("failed to parse iam: %w", err) + } + + delete(conf.AccessAccounts, access) + + b, err := json.Marshal(s.accts) + if err != nil { + return nil, fmt.Errorf("failed to serialize iam: %w", err) + } + + return b, nil + }) +} diff --git a/backend/posix/posix.go b/backend/posix/posix.go index b8ee76b1..70488ae0 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -28,7 +28,9 @@ import ( "sort" "strconv" "strings" + "sync" "syscall" + "time" "github.com/aws/aws-sdk-go-v2/service/s3" "github.com/aws/aws-sdk-go-v2/service/s3/types" @@ -41,19 +43,34 @@ import ( ) type Posix struct { + backend.BackendUnsupported + rootfd *os.File rootdir string - backend.BackendUnsupported + + mu sync.RWMutex + iamcache []byte + iamvalid bool + iamexpire time.Time } var _ backend.Backend = &Posix{} +var ( + cacheDuration = 5 * time.Minute +) + const ( metaTmpDir = ".sgwtmp" metaTmpMultipartDir = metaTmpDir + "/multipart" onameAttr = "user.objname" tagHdr = "X-Amz-Tagging" + contentTypeHdr = "content-type" + contentEncHdr = "content-encoding" emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" + iamkey = "user.iam" + aclkey = "user.acl" + etagkey = "user.etag" ) func New(rootdir string) (*Posix, error) { @@ -140,7 +157,7 @@ func (p *Posix) PutBucket(bucket string, owner string) error { return fmt.Errorf("marshal acl: %w", err) } - if err := xattr.Set(bucket, "user.acl", jsonACL); err != nil { + if err := xattr.Set(bucket, aclkey, jsonACL); err != nil { return fmt.Errorf("set acl: %w", err) } @@ -263,7 +280,7 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ return nil, s3err.GetAPIError(s3err.ErrInvalidPart) } - b, err := xattr.Get(partPath, "user.etag") + b, err := xattr.Get(partPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -319,7 +336,7 @@ func (p *Posix) CompleteMultipartUpload(bucket, object, uploadID string, parts [ // Calculate s3 compatible md5sum for complete multipart. s3MD5 := backend.GetMultipartMD5(parts) - err = xattr.Set(objname, "user.etag", []byte(s3MD5)) + err = xattr.Set(objname, etagkey, []byte(s3MD5)) if err != nil { // cleanup object if returning error os.Remove(objname) @@ -373,22 +390,22 @@ func loadUserMetaData(path string, m map[string]string) (contentType, contentEnc m[strings.TrimPrefix(e, "user.")] = string(b) } - b, err := xattr.Get(path, "user.content-type") + b, err := xattr.Get(path, "user."+contentTypeHdr) contentType = string(b) if err != nil { contentType = "" } if contentType != "" { - m["content-type"] = contentType + m[contentTypeHdr] = contentType } - b, err = xattr.Get(path, "user.content-encoding") + b, err = xattr.Get(path, "user."+contentEncHdr) contentEncoding = string(b) if err != nil { contentEncoding = "" } if contentEncoding != "" { - m["content-encoding"] = contentEncoding + m[contentEncHdr] = contentEncoding } return @@ -626,7 +643,7 @@ func (p *Posix) ListObjectParts(bucket, object, uploadID string, partNumberMarke } partPath := filepath.Join(objdir, uploadID, e.Name()) - b, err := xattr.Get(partPath, "user.etag") + b, err := xattr.Get(partPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -713,7 +730,7 @@ func (p *Posix) PutObjectPart(bucket, object, uploadID string, part int, length dataSum := hash.Sum(nil) etag := hex.EncodeToString(dataSum) - xattr.Set(partPath, "user.etag", []byte(etag)) + xattr.Set(partPath, etagkey, []byte(etag)) return etag, nil } @@ -741,7 +758,7 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { } // set etag attribute to signify this dir was specifically put - xattr.Set(name, "user.etag", []byte(emptyMD5)) + xattr.Set(name, etagkey, []byte(emptyMD5)) return emptyMD5, nil } @@ -779,7 +796,7 @@ func (p *Posix) PutObject(po *s3.PutObjectInput) (string, error) { dataSum := hash.Sum(nil) etag := hex.EncodeToString(dataSum[:]) - xattr.Set(name, "user.etag", []byte(etag)) + xattr.Set(name, etagkey, []byte(etag)) return etag, nil } @@ -819,7 +836,7 @@ func (p *Posix) removeParents(bucket, object string) error { break } - _, err := xattr.Get(parent, "user.etag") + _, err := xattr.Get(parent, etagkey) if err == nil { break } @@ -893,7 +910,7 @@ func (p *Posix) GetObject(bucket, object, acceptRange string, writer io.Writer) contentType, contentEncoding := loadUserMetaData(objPath, userMetaData) - b, err := xattr.Get(objPath, "user.etag") + b, err := xattr.Get(objPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -937,7 +954,7 @@ func (p *Posix) HeadObject(bucket, object string) (*s3.HeadObjectOutput, error) userMetaData := make(map[string]string) contentType, contentEncoding := loadUserMetaData(objPath, userMetaData) - b, err := xattr.Get(objPath, "user.etag") + b, err := xattr.Get(objPath, etagkey) etag := string(b) if err != nil { etag = "" @@ -1010,7 +1027,7 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) ( fileSystem := os.DirFS(bucket) results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, func(path string) (bool, error) { - _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + _, err := xattr.Get(filepath.Join(bucket, path), etagkey) if isNoAttr(err) { return false, nil } @@ -1019,7 +1036,7 @@ func (p *Posix) ListObjects(bucket, prefix, marker, delim string, maxkeys int) ( } return true, nil }, func(path string) (string, error) { - etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + etag, err := xattr.Get(filepath.Join(bucket, path), etagkey) return string(etag), err }, []string{metaTmpDir}) if err != nil { @@ -1051,7 +1068,7 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int) fileSystem := os.DirFS(bucket) results, err := backend.Walk(fileSystem, prefix, delim, marker, maxkeys, func(path string) (bool, error) { - _, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + _, err := xattr.Get(filepath.Join(bucket, path), etagkey) if isNoAttr(err) { return false, nil } @@ -1060,7 +1077,7 @@ func (p *Posix) ListObjectsV2(bucket, prefix, marker, delim string, maxkeys int) } return true, nil }, func(path string) (string, error) { - etag, err := xattr.Get(filepath.Join(bucket, path), "user.etag") + etag, err := xattr.Get(filepath.Join(bucket, path), etagkey) return string(etag), err }, []string{metaTmpDir}) if err != nil { @@ -1089,7 +1106,7 @@ func (p *Posix) PutBucketAcl(bucket string, data []byte) error { return fmt.Errorf("stat bucket: %w", err) } - if err := xattr.Set(bucket, "user.acl", data); err != nil { + if err := xattr.Set(bucket, aclkey, data); err != nil { return fmt.Errorf("set acl: %w", err) } @@ -1105,7 +1122,7 @@ func (p *Posix) GetBucketAcl(bucket string) ([]byte, error) { return nil, fmt.Errorf("stat bucket: %w", err) } - b, err := xattr.Get(bucket, "user.acl") + b, err := xattr.Get(bucket, aclkey) if err != nil { return nil, fmt.Errorf("get acl: %w", err) } @@ -1185,6 +1202,65 @@ func (p *Posix) RemoveTags(bucket, object string) error { return p.SetTags(bucket, object, nil) } +func (p *Posix) GetIAM() ([]byte, error) { + p.mu.RLock() + defer p.mu.Unlock() + + if !p.iamvalid || !p.iamexpire.After(time.Now()) { + p.mu.Unlock() + err := p.refreshIAM() + p.mu.RLock() + if err != nil { + return nil, err + } + } + + return p.iamcache, nil +} + +func (p *Posix) refreshIAM() error { + p.mu.Lock() + defer p.mu.Unlock() + + b, err := xattr.FGet(p.rootfd, iamkey) + if isNoAttr(err) { + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + + return nil +} + +func (p *Posix) StoreIAM(update auth.UpdateAcctFunc) error { + p.mu.Lock() + defer p.mu.Unlock() + + b, err := xattr.FGet(p.rootfd, iamkey) + if isNoAttr(err) { + return err + } + b, err = update(b) + if err != nil { + return err + } + + // TODO: use xattr.FRemove/xattr.FSetWithFlags/xattr.XATTR_CREATE + // to detect racing updates, loop on update race fail + err = xattr.FSet(p.rootfd, iamkey, b) + if err != nil { + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + + return nil +} + func isNoAttr(err error) bool { if err == nil { return false diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index 1c94504a..dd8dc2c1 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -133,7 +133,7 @@ func initFlags() []cli.Flag { } } -func runGateway(be backend.Backend) error { +func runGateway(be backend.Backend, s auth.Storer) error { app := fiber.New(fiber.Config{ AppName: "versitygw", ServerHeader: "VERSITYGW", @@ -161,7 +161,7 @@ func runGateway(be backend.Backend) error { opts = append(opts, s3api.WithDebug()) } - iam, err := auth.InitIAM() + iam, err := auth.NewInternal(s) if err != nil { return err } @@ -169,8 +169,7 @@ func runGateway(be backend.Backend) error { srv, err := s3api.New(app, be, middlewares.RootUserConfig{ Access: rootUserAccess, Secret: rootUserSecret, - Region: region, - }, port, iam, opts...) + }, port, region, iam, opts...) if err != nil { return fmt.Errorf("init gateway: %v", err) } diff --git a/cmd/versitygw/posix.go b/cmd/versitygw/posix.go index a7de618f..ebd8204f 100644 --- a/cmd/versitygw/posix.go +++ b/cmd/versitygw/posix.go @@ -49,5 +49,5 @@ func runPosix(ctx *cli.Context) error { return fmt.Errorf("init posix: %v", err) } - return runGateway(be) + return runGateway(be, be) } diff --git a/cmd/versitygw/scoutfs.go b/cmd/versitygw/scoutfs.go index 1b2d5fab..dd272e95 100644 --- a/cmd/versitygw/scoutfs.go +++ b/cmd/versitygw/scoutfs.go @@ -52,5 +52,5 @@ func runScoutfs(ctx *cli.Context) error { return fmt.Errorf("init scoutfs: %v", err) } - return runGateway(be) + return runGateway(be, be) } diff --git a/s3api/controllers/admin.go b/s3api/controllers/admin.go index a9e82ddf..3a0fdc45 100644 --- a/s3api/controllers/admin.go +++ b/s3api/controllers/admin.go @@ -26,16 +26,16 @@ type AdminController struct { } func (c AdminController) CreateUser(ctx *fiber.Ctx) error { - access, secret, role, region := ctx.Query("access"), ctx.Query("secret"), ctx.Query("role"), ctx.Query("region") + access, secret, role := ctx.Query("access"), ctx.Query("secret"), ctx.Query("role") requesterRole := ctx.Locals("role") if requesterRole != "admin" { return fmt.Errorf("access denied: only admin users have access to this resource") } - user := auth.Account{Secret: secret, Role: role, Region: region} + user := auth.Account{Secret: secret, Role: role} - err := c.IAMService.CreateAccount(access, &user) + err := c.IAMService.CreateAccount(access, user) if err != nil { return fmt.Errorf("failed to create a user: %w", err) } diff --git a/s3api/controllers/base.go b/s3api/controllers/base.go index 9116d1fd..17ee506f 100644 --- a/s3api/controllers/base.go +++ b/s3api/controllers/base.go @@ -36,7 +36,8 @@ import ( ) type S3ApiController struct { - be backend.Backend + be backend.Backend + iam auth.IAMService } func New(be backend.Backend) S3ApiController { @@ -257,12 +258,7 @@ func (c S3ApiController) PutBucketActions(ctx *fiber.Ctx) error { AccessControlPolicy: &types.AccessControlPolicy{Owner: &types.Owner{ID: &access}}, } - iam, err := auth.GetIAMConfig() - if err != nil { - return SendResponse(ctx, err) - } - - err = auth.UpdateACL(input, parsedAcl, *iam) + err = auth.UpdateACL(input, parsedAcl, c.iam) return SendResponse(ctx, err) } diff --git a/s3api/middlewares/authentication.go b/s3api/middlewares/authentication.go index 88589226..a5ab3237 100644 --- a/s3api/middlewares/authentication.go +++ b/s3api/middlewares/authentication.go @@ -38,10 +38,9 @@ const ( type RootUserConfig struct { Access string Secret string - Region string } -func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fiber.Handler { +func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, region string, debug bool) fiber.Handler { acct := accounts{root: root, iam: iam} return func(ctx *fiber.Ctx) error { @@ -74,10 +73,13 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib } signedHdrs := strings.Split(signHdrKv[1], ";") - account := acct.getAccount(creds[0]) - if account == nil { + account, err := acct.getAccount(creds[0]) + if err == auth.ErrNoSuchUser { return controllers.SendResponse(ctx, s3err.GetAPIError(s3err.ErrInvalidAccessKeyID)) } + if err != nil { + return controllers.SendResponse(ctx, err) + } // Check X-Amz-Date header date := ctx.Get("X-Amz-Date") @@ -113,7 +115,7 @@ func VerifyV4Signature(root RootUserConfig, iam auth.IAMService, debug bool) fib signErr := signer.SignHTTP(req.Context(), aws.Credentials{ AccessKeyID: creds[0], SecretAccessKey: account.Secret, - }, req, hexPayload, creds[3], account.Region, tdate, func(options *v4.SignerOptions) { + }, req, hexPayload, creds[3], region, tdate, func(options *v4.SignerOptions) { if debug { options.LogSigning = true options.Logger = logging.NewStandardLogger(os.Stderr) @@ -147,16 +149,13 @@ type accounts struct { iam auth.IAMService } -func (a accounts) getAccount(access string) *auth.Account { - var account *auth.Account +func (a accounts) getAccount(access string) (auth.Account, error) { if access == a.root.Access { - account = &auth.Account{ + return auth.Account{ Secret: a.root.Secret, Role: "admin", - Region: a.root.Region, - } - } else { - account = a.iam.GetUserAccount(access) + }, nil } - return account + + return a.iam.GetUserAccount(access) } diff --git a/s3api/router_test.go b/s3api/router_test.go index 2b14dd93..663c71fa 100644 --- a/s3api/router_test.go +++ b/s3api/router_test.go @@ -39,7 +39,7 @@ func TestS3ApiRouter_Init(t *testing.T) { args: args{ app: fiber.New(), be: backend.BackendUnsupported{}, - iam: auth.IAMServiceUnsupported{}, + iam: &auth.IAMServiceInternal{}, }, }, } diff --git a/s3api/server.go b/s3api/server.go index 1588dabf..09883192 100644 --- a/s3api/server.go +++ b/s3api/server.go @@ -33,7 +33,7 @@ type S3ApiServer struct { debug bool } -func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port string, iam auth.IAMService, opts ...Option) (*S3ApiServer, error) { +func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, port, region string, iam auth.IAMService, opts ...Option) (*S3ApiServer, error) { server := &S3ApiServer{ app: app, backend: be, @@ -45,7 +45,7 @@ func New(app *fiber.App, be backend.Backend, root middlewares.RootUserConfig, po opt(server) } - app.Use(middlewares.VerifyV4Signature(root, iam, server.debug)) + app.Use(middlewares.VerifyV4Signature(root, iam, region, server.debug)) app.Use(logger.New()) app.Use(middlewares.VerifyMD5Body()) server.router.Init(app, be, iam) diff --git a/s3api/server_test.go b/s3api/server_test.go index ea6aa1cf..27ce0bcf 100644 --- a/s3api/server_test.go +++ b/s3api/server_test.go @@ -63,7 +63,7 @@ func TestNew(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { gotS3ApiServer, err := New(tt.args.app, tt.args.be, tt.args.root, - tt.args.port, auth.IAMServiceUnsupported{}) + tt.args.port, "us-east-1", &auth.IAMServiceInternal{}) if (err != nil) != tt.wantErr { t.Errorf("New() error = %v, wantErr %v", err, tt.wantErr) return From d2eab5bce3143694aea4f950f1fafa0125c9ab15 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Sat, 17 Jun 2023 22:58:54 -0700 Subject: [PATCH 2/3] posix: move iam data store to file Storing to a file will allow more than 64k of storage that the xattr would be limited to. This attempts to resolve racing updates between multiple gateways without an explicit coordination between gateways. This wil also setup a default IAM file on init. --- backend/posix/posix.go | 182 +++++++++++++++++++++++++++++++++++------ cmd/versitygw/main.go | 7 +- 2 files changed, 164 insertions(+), 25 deletions(-) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index 70488ae0..f54eabb3 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -68,7 +68,8 @@ const ( contentTypeHdr = "content-type" contentEncHdr = "content-encoding" emptyMD5 = "d41d8cd98f00b204e9800998ecf8427e" - iamkey = "user.iam" + iamFile = "users.json" + iamBackupFile = "users.json.backup" aclkey = "user.acl" etagkey = "user.etag" ) @@ -1202,12 +1203,35 @@ func (p *Posix) RemoveTags(bucket, object string) error { return p.SetTags(bucket, object, nil) } +const ( + iamMode = 0600 +) + +func (p *Posix) InitIAM() error { + p.mu.RLock() + defer p.mu.RUnlock() + + _, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + b, err := json.Marshal(auth.IAMConfig{}) + if err != nil { + return fmt.Errorf("marshal default iam: %w", err) + } + err = os.WriteFile(iamFile, b, iamMode) + if err != nil { + return fmt.Errorf("write default iam: %w", err) + } + } + + return nil +} + func (p *Posix) GetIAM() ([]byte, error) { p.mu.RLock() - defer p.mu.Unlock() + defer p.mu.RUnlock() if !p.iamvalid || !p.iamexpire.After(time.Now()) { - p.mu.Unlock() + p.mu.RUnlock() err := p.refreshIAM() p.mu.RLock() if err != nil { @@ -1218,18 +1242,44 @@ func (p *Posix) GetIAM() ([]byte, error) { return p.iamcache, nil } +const ( + backoff = 100 * time.Millisecond + maxretry = 300 +) + func (p *Posix) refreshIAM() error { p.mu.Lock() defer p.mu.Unlock() - b, err := xattr.FGet(p.rootfd, iamkey) - if isNoAttr(err) { - return err - } + // We are going to be racing with other running gateways without any + // coordination. So we might find the file does not exist at times. + // For this case we need to retry for a while assuming the other gateway + // will eventually write the file. If it doesn't after the max retries, + // then we will return the error. - p.iamcache = b - p.iamvalid = true - p.iamexpire = time.Now().Add(cacheDuration) + retries := 0 + + for { + b, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + retries++ + if retries < maxretry { + time.Sleep(backoff) + continue + } + return fmt.Errorf("read iam file: %w", err) + } + if err != nil { + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + break + } return nil } @@ -1238,25 +1288,109 @@ func (p *Posix) StoreIAM(update auth.UpdateAcctFunc) error { p.mu.Lock() defer p.mu.Unlock() - b, err := xattr.FGet(p.rootfd, iamkey) - if isNoAttr(err) { - return err - } - b, err = update(b) - if err != nil { - return err + // We are going to be racing with other running gateways without any + // coordination. So the strategy here is to read the current file data. + // If the file doesn't exist, then we assume someone else is currently + // updating the file. So we just need to keep retrying. We also need + // to make sure the data is consistent within a single update. So racing + // writes to a file would possibly leave this in some invalid state. + // We can get atomic updates with rename. If we read the data, update + // the data, write to a temp file, then rename the tempfile back to the + // data file. This should always result in a complete data image. + + // There is at least one unsolved failure mode here. + // If a gateway removes the data file and then crashes, all other + // gateways will retry forever thinking that the original will eventually + // write the file. + + retries := 0 + + for { + b, err := os.ReadFile(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + retries++ + if retries < maxretry { + time.Sleep(backoff) + continue + } + + // we have been unsuccessful trying to read the iam file + // so this must be the case where something happened and + // the file did not get updated successfully, and probably + // isn't going to be. The recovery procedure would be to + // copy the backup file into place of the original. + return fmt.Errorf("no iam file, needs backup recovery") + } + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("read iam file: %w", err) + } + + // reset retries on successful read + retries = 0 + + err = os.Remove(iamFile) + if errors.Is(err, fs.ErrNotExist) { + // racing with someone else updating + // keep retrying after backoff + time.Sleep(backoff) + continue + } + if err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove old iam file: %w", err) + } + + // save copy of data + datacopy := make([]byte, len(b)) + copy(datacopy, b) + + // make a backup copy in case we crash before update + // this is after remove, so there is a small window something + // can go wrong, but the remove should barrier other gateways + // from trying to write backup at the same time. Only one + // gateway will successfully remove the file. + os.WriteFile(iamBackupFile, b, iamMode) + + b, err = update(b) + if err != nil { + // update failed, try to write old data back out + os.WriteFile(iamFile, datacopy, iamMode) + return fmt.Errorf("update iam data: %w", err) + } + + err = writeTempFile(b) + if err != nil { + // update failed, try to write old data back out + os.WriteFile(iamFile, datacopy, iamMode) + return err + } + + p.iamcache = b + p.iamvalid = true + p.iamexpire = time.Now().Add(cacheDuration) + break } - // TODO: use xattr.FRemove/xattr.FSetWithFlags/xattr.XATTR_CREATE - // to detect racing updates, loop on update race fail - err = xattr.FSet(p.rootfd, iamkey, b) + return nil +} + +func writeTempFile(b []byte) error { + f, err := os.CreateTemp(".", iamFile) if err != nil { - return err + return fmt.Errorf("create temp file: %w", err) + } + defer os.Remove(f.Name()) + + _, err = f.Write(b) + if err != nil { + return fmt.Errorf("write temp file: %w", err) } - p.iamcache = b - p.iamvalid = true - p.iamexpire = time.Now().Add(cacheDuration) + err = os.Rename(f.Name(), iamFile) + if err != nil { + return fmt.Errorf("rename temp file: %w", err) + } return nil } diff --git a/cmd/versitygw/main.go b/cmd/versitygw/main.go index dd8dc2c1..52997c81 100644 --- a/cmd/versitygw/main.go +++ b/cmd/versitygw/main.go @@ -161,9 +161,14 @@ func runGateway(be backend.Backend, s auth.Storer) error { opts = append(opts, s3api.WithDebug()) } + err := s.InitIAM() + if err != nil { + return fmt.Errorf("init iam: %w", err) + } + iam, err := auth.NewInternal(s) if err != nil { - return err + return fmt.Errorf("setup internal iam service: %w", err) } srv, err := s3api.New(app, be, middlewares.RootUserConfig{ From 33673de160a74347d0e286b7fb1fe37e7e715849 Mon Sep 17 00:00:00 2001 From: Ben McClelland Date: Mon, 19 Jun 2023 10:34:45 -0700 Subject: [PATCH 3/3] fix case where bucket directory is created without acl --- backend/auth/acl.go | 4 ++++ backend/posix/posix.go | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/backend/auth/acl.go b/backend/auth/acl.go index 976e10c2..c1dfc6e6 100644 --- a/backend/auth/acl.go +++ b/backend/auth/acl.go @@ -46,6 +46,10 @@ type AccessControlList struct { } func ParseACL(data []byte) (ACL, error) { + if len(data) == 0 { + return ACL{}, nil + } + var acl ACL if err := json.Unmarshal(data, &acl); err != nil { return acl, fmt.Errorf("parse acl: %w", err) diff --git a/backend/posix/posix.go b/backend/posix/posix.go index f54eabb3..49229eca 100644 --- a/backend/posix/posix.go +++ b/backend/posix/posix.go @@ -834,11 +834,15 @@ func (p *Posix) removeParents(bucket, object string) error { parent := filepath.Dir(objPath) if filepath.Base(parent) == bucket { + // stop removing parents if we hit the bucket directory. break } _, err := xattr.Get(parent, etagkey) if err == nil { + // a directory with a valid etag means this was specifically + // uploaded with a put object, so stop here and leave this + // directory in place. break } @@ -1124,6 +1128,9 @@ func (p *Posix) GetBucketAcl(bucket string) ([]byte, error) { } b, err := xattr.Get(bucket, aclkey) + if isNoAttr(err) { + return []byte{}, nil + } if err != nil { return nil, fmt.Errorf("get acl: %w", err) }