Skip to content

Commit 3080527

Browse files
committed
server/etcdserver: enforce auth checks for nested txn ops
Signed-off-by: Wei Fu <fuweid89@gmail.com> (cherry picked from commit 204097b) Signed-off-by: Wei Fu <fuweid89@gmail.com>
1 parent db3cc27 commit 3080527

File tree

3 files changed

+163
-0
lines changed

3 files changed

+163
-0
lines changed

server/etcdserver/txn/txn.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,6 +668,10 @@ func IsTxnReadonly(r *pb.TxnRequest) bool {
668668
}
669669

670670
func CheckTxnAuth(as auth.AuthStore, ai *auth.AuthInfo, rt *pb.TxnRequest) error {
671+
return checkTxnPermission(as, ai, rt)
672+
}
673+
674+
func checkTxnPermission(as auth.AuthStore, ai *auth.AuthInfo, rt *pb.TxnRequest) error {
671675
for _, c := range rt.Compare {
672676
if err := as.IsRangePermitted(ai, c.Key, c.RangeEnd); err != nil {
673677
return err
@@ -716,6 +720,15 @@ func checkTxnReqsPermission(as auth.AuthStore, ai *auth.AuthInfo, reqs []*pb.Req
716720
if err != nil {
717721
return err
718722
}
723+
case *pb.RequestOp_RequestTxn:
724+
if tv.RequestTxn == nil {
725+
continue
726+
}
727+
728+
err := checkTxnPermission(as, ai, tv.RequestTxn)
729+
if err != nil {
730+
return err
731+
}
719732
}
720733
}
721734

server/etcdserver/txn/txn_test.go

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -515,6 +515,90 @@ func TestCheckTxnAuth(t *testing.T) {
515515
},
516516
err: auth.ErrPermissionDenied,
517517
},
518+
{
519+
name: "Nested txn request in range is authorized",
520+
txnRequest: &pb.TxnRequest{
521+
Success: []*pb.RequestOp{
522+
{
523+
Request: &pb.RequestOp_RequestTxn{
524+
RequestTxn: &pb.TxnRequest{
525+
Success: []*pb.RequestOp{inRangeRequestRange, inRangeRequestPut},
526+
Failure: []*pb.RequestOp{inRangeRequestDeleteRange},
527+
},
528+
},
529+
},
530+
},
531+
},
532+
err: nil,
533+
},
534+
{
535+
name: "Nested txn request out of range success case is unauthorized",
536+
txnRequest: &pb.TxnRequest{
537+
Success: []*pb.RequestOp{
538+
{
539+
Request: &pb.RequestOp_RequestTxn{
540+
RequestTxn: &pb.TxnRequest{
541+
Success: []*pb.RequestOp{outOfRangeRequestRange},
542+
},
543+
},
544+
},
545+
},
546+
},
547+
err: auth.ErrPermissionDenied,
548+
},
549+
{
550+
name: "Nested txn request out of range failure case is unauthorized",
551+
txnRequest: &pb.TxnRequest{
552+
Failure: []*pb.RequestOp{
553+
{
554+
Request: &pb.RequestOp_RequestTxn{
555+
RequestTxn: &pb.TxnRequest{
556+
Failure: []*pb.RequestOp{outOfRangeRequestPut},
557+
},
558+
},
559+
},
560+
},
561+
},
562+
err: auth.ErrPermissionDenied,
563+
},
564+
{
565+
name: "Nested txn request out of range delete is unauthorized",
566+
txnRequest: &pb.TxnRequest{
567+
Success: []*pb.RequestOp{
568+
{
569+
Request: &pb.RequestOp_RequestTxn{
570+
RequestTxn: &pb.TxnRequest{
571+
Success: []*pb.RequestOp{outOfRangeRequestDeleteRange},
572+
},
573+
},
574+
},
575+
},
576+
},
577+
err: auth.ErrPermissionDenied,
578+
},
579+
{
580+
name: "Two level nested txn request out of range delete is unauthorized",
581+
txnRequest: &pb.TxnRequest{
582+
Success: []*pb.RequestOp{
583+
{
584+
Request: &pb.RequestOp_RequestTxn{
585+
RequestTxn: &pb.TxnRequest{
586+
Failure: []*pb.RequestOp{
587+
{
588+
Request: &pb.RequestOp_RequestTxn{
589+
RequestTxn: &pb.TxnRequest{
590+
Success: []*pb.RequestOp{outOfRangeRequestDeleteRange},
591+
},
592+
},
593+
},
594+
},
595+
},
596+
},
597+
},
598+
},
599+
},
600+
err: auth.ErrPermissionDenied,
601+
},
518602
}
519603

520604
for _, tt := range tests {

tests/integration/v3_auth_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,72 @@ func TestV3AuthNonAuthorizedRPCs(t *testing.T) {
345345
}
346346
}
347347

348+
func TestV3AuthNestedTxnPermissionDenied(t *testing.T) {
349+
integration.BeforeTest(t)
350+
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})
351+
defer clus.Terminate(t)
352+
353+
users := []user{
354+
{
355+
name: "user1",
356+
password: "user1-123",
357+
role: "role1",
358+
key: "foo",
359+
end: "zoo",
360+
},
361+
}
362+
authSetupUsers(t, integration.ToGRPC(clus.Client(0)).Auth, users)
363+
authSetupRoot(t, integration.ToGRPC(clus.Client(0)).Auth)
364+
365+
rootc, err := integration.NewClient(t, clientv3.Config{
366+
Endpoints: clus.Client(0).Endpoints(),
367+
Username: "root",
368+
Password: "123",
369+
})
370+
require.NoError(t, err)
371+
defer rootc.Close()
372+
373+
userc, err := integration.NewClient(t, clientv3.Config{
374+
Endpoints: clus.Client(0).Endpoints(),
375+
Username: "user1",
376+
Password: "user1-123",
377+
})
378+
require.NoError(t, err)
379+
defer userc.Close()
380+
381+
_, err = rootc.Put(t.Context(), "boo", "bar")
382+
require.NoError(t, err)
383+
384+
txn := &pb.TxnRequest{
385+
Success: []*pb.RequestOp{
386+
{
387+
Request: &pb.RequestOp_RequestTxn{
388+
RequestTxn: &pb.TxnRequest{
389+
Success: []*pb.RequestOp{
390+
{
391+
Request: &pb.RequestOp_RequestDeleteRange{
392+
RequestDeleteRange: &pb.DeleteRangeRequest{
393+
Key: []byte("boo"),
394+
},
395+
},
396+
},
397+
},
398+
},
399+
},
400+
},
401+
},
402+
}
403+
404+
_, err = integration.ToGRPC(userc).KV.Txn(t.Context(), txn)
405+
require.Error(t, err)
406+
require.Truef(t, eqErrGRPC(err, rpctypes.ErrGRPCPermissionDenied), "got %v, expected %v", err, rpctypes.ErrGRPCPermissionDenied)
407+
408+
resp, err := rootc.Get(t.Context(), "boo")
409+
require.NoError(t, err)
410+
require.Len(t, resp.Kvs, 1)
411+
require.Equal(t, resp.Kvs[0].Value, []byte("bar"))
412+
}
413+
348414
func TestV3AuthOldRevConcurrent(t *testing.T) {
349415
integration.BeforeTest(t)
350416
clus := integration.NewCluster(t, &integration.ClusterConfig{Size: 1})

0 commit comments

Comments
 (0)