satellite/metainfo: do full bucket validation only on create

We are doing full bucket name validation for many requests but
we should do this only while creating bucket. Other requests will be
covered only by basic name length validation. Less strict validation for
other requests will make bucket usable in case of invalid bucket names
in DB (we have such cases from the past).

https://github.com/storj/storj/issues/6044

Change-Id: I3a41050e3637787f788705ef15b5dc4df4d01fc6
This commit is contained in:
Michal Niewrzal 2023-07-17 10:42:03 +02:00
parent 95761908b5
commit 5272fd8497
4 changed files with 39 additions and 24 deletions

View File

@ -71,7 +71,7 @@ func (endpoint *Endpoint) CreateBucket(ctx context.Context, req *pb.BucketCreate
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Name)
err = endpoint.validateBucketName(req.Name)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -180,7 +180,7 @@ func (endpoint *Endpoint) DeleteBucket(ctx context.Context, req *pb.BucketDelete
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Name)
err = endpoint.validateBucketNameLength(req.Name)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}

View File

@ -118,12 +118,6 @@ func TestBucketNameValidation(t *testing.T) {
"test\\", "test%",
}
for _, name := range invalidNames {
_, err = metainfoClient.BeginObject(ctx, metaclient.BeginObjectParams{
Bucket: []byte(name),
EncryptedObjectKey: []byte("123"),
})
require.Error(t, err, "bucket name: %v", name)
require.True(t, errs2.IsRPC(err, rpcstatus.InvalidArgument))
_, err = metainfoClient.CreateBucket(ctx, metaclient.CreateBucketParams{
Name: []byte(name),
@ -131,6 +125,20 @@ func TestBucketNameValidation(t *testing.T) {
require.Error(t, err, "bucket name: %v", name)
require.True(t, errs2.IsRPC(err, rpcstatus.InvalidArgument))
}
invalidNames = []string{
"", "t", "te",
"testbucket-64-0123456789012345678901234567890123456789012345abcd",
}
for _, name := range invalidNames {
// BeginObject validates only bucket name length
_, err = metainfoClient.BeginObject(ctx, metaclient.BeginObjectParams{
Bucket: []byte(name),
EncryptedObjectKey: []byte("123"),
})
require.Error(t, err, "bucket name: %v", name)
require.True(t, errs2.IsRPC(err, rpcstatus.InvalidArgument))
}
})
}

View File

@ -66,7 +66,8 @@ func (endpoint *Endpoint) BeginObject(ctx context.Context, req *pb.ObjectBeginRe
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, "Invalid expiration time")
}
err = endpoint.validateBucket(ctx, req.Bucket)
// we can do just basic name validation because later we are checking bucket in DB
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -310,7 +311,7 @@ func (endpoint *Endpoint) GetObject(ctx context.Context, req *pb.ObjectGetReques
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -409,7 +410,7 @@ func (endpoint *Endpoint) DownloadObject(ctx context.Context, req *pb.ObjectDown
return nil, err
}
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -804,7 +805,7 @@ func (endpoint *Endpoint) ListObjects(ctx context.Context, req *pb.ObjectListReq
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -944,7 +945,7 @@ func (endpoint *Endpoint) ListPendingObjectStreams(ctx context.Context, req *pb.
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1057,7 +1058,7 @@ func (endpoint *Endpoint) BeginDeleteObject(ctx context.Context, req *pb.ObjectB
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1146,7 +1147,7 @@ func (endpoint *Endpoint) GetObjectIPs(ctx context.Context, req *pb.ObjectGetIPs
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1237,7 +1238,7 @@ func (endpoint *Endpoint) UpdateObjectMetadata(ctx context.Context, req *pb.Obje
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.Bucket)
err = endpoint.validateBucketNameLength(req.Bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1615,7 +1616,7 @@ func (endpoint *Endpoint) BeginMoveObject(ctx context.Context, req *pb.ObjectBeg
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
for _, bucket := range [][]byte{req.Bucket, req.NewBucket} {
err = endpoint.validateBucket(ctx, bucket)
err = endpoint.validateBucketNameLength(bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1761,7 +1762,7 @@ func (endpoint *Endpoint) FinishMoveObject(ctx context.Context, req *pb.ObjectFi
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.NewBucket)
err = endpoint.validateBucketNameLength(req.NewBucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1840,7 +1841,7 @@ func (endpoint *Endpoint) BeginCopyObject(ctx context.Context, req *pb.ObjectBeg
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
for _, bucket := range [][]byte{req.Bucket, req.NewBucket} {
err = endpoint.validateBucket(ctx, bucket)
err = endpoint.validateBucketNameLength(bucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}
@ -1952,7 +1953,7 @@ func (endpoint *Endpoint) FinishCopyObject(ctx context.Context, req *pb.ObjectFi
}
endpoint.usageTracking(keyInfo, req.Header, fmt.Sprintf("%T", req))
err = endpoint.validateBucket(ctx, req.NewBucket)
err = endpoint.validateBucketNameLength(req.NewBucket)
if err != nil {
return nil, rpcstatus.Error(rpcstatus.InvalidArgument, err.Error())
}

View File

@ -247,9 +247,7 @@ func (endpoint *Endpoint) checkRate(ctx context.Context, projectID uuid.UUID) (e
return nil
}
func (endpoint *Endpoint) validateBucket(ctx context.Context, bucket []byte) (err error) {
defer mon.Task()(&ctx)(&err)
func (endpoint *Endpoint) validateBucketNameLength(bucket []byte) (err error) {
if len(bucket) == 0 {
return Error.Wrap(buckets.ErrNoBucket.New(""))
}
@ -258,11 +256,19 @@ func (endpoint *Endpoint) validateBucket(ctx context.Context, bucket []byte) (er
return Error.New("bucket name must be at least 3 and no more than 63 characters long")
}
return nil
}
func (endpoint *Endpoint) validateBucketName(bucket []byte) error {
if err := endpoint.validateBucketNameLength(bucket); err != nil {
return err
}
// Regexp not used because benchmark shows it will be slower for valid bucket names
// https://gist.github.com/mniewrzal/49de3af95f36e63e88fac24f565e444c
labels := bytes.Split(bucket, []byte("."))
for _, label := range labels {
err = validateBucketLabel(label)
err := validateBucketLabel(label)
if err != nil {
return err
}