From 72db2040e863422a1d2b9985fc6d8e67960c0f60 Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Fri, 5 Aug 2022 16:58:10 +0800 Subject: [PATCH 1/4] executor: add privilege check for prepare stmt --- executor/prepared.go | 5 ++++ planner/core/plan_cache.go | 4 +-- privilege/privileges/privileges_test.go | 40 +++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/executor/prepared.go b/executor/prepared.go index 64d2dd1cf321b..1b8017f2d041b 100644 --- a/executor/prepared.go +++ b/executor/prepared.go @@ -241,6 +241,11 @@ func (e *PrepareExec) Next(ctx context.Context, req *chunk.Chunk) error { NormalizedSQL4PC: normalizedSQL4PC, SQLDigest4PC: digest4PC, } + + if err = plannercore.CheckPreparedPriv(ctx, e.ctx, preparedObj, ret.InfoSchema); err != nil { + return err + } + return vars.AddPreparedStmt(e.ID, preparedObj) } diff --git a/planner/core/plan_cache.go b/planner/core/plan_cache.go index 9a4e6b67a564d..98dae0cdf65cc 100644 --- a/planner/core/plan_cache.go +++ b/planner/core/plan_cache.go @@ -142,7 +142,7 @@ func getGeneralPlan(ctx context.Context, sctx sessionctx.Context, cacheKey kvcac stmtCtx := sessVars.StmtCtx if cacheValue, exists := sctx.PreparedPlanCache().Get(cacheKey); exists { - if err := checkPreparedPriv(ctx, sctx, preparedStmt, is); err != nil { + if err := CheckPreparedPriv(ctx, sctx, preparedStmt, is); err != nil { return nil, nil, false, err } cachedVals := cacheValue.([]*PlanCacheValue) @@ -537,7 +537,7 @@ func buildRangeForIndexScan(sctx sessionctx.Context, is *PhysicalIndexScan) (err return } -func checkPreparedPriv(_ context.Context, sctx sessionctx.Context, +func CheckPreparedPriv(_ context.Context, sctx sessionctx.Context, preparedObj *CachedPrepareStmt, is infoschema.InfoSchema) error { if pm := privilege.GetPrivilegeManager(sctx); pm != nil { visitInfo := VisitInfo4PrivCheck(is, preparedObj.PreparedAst.Stmt, preparedObj.VisitInfos) diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 0369e8ee923e6..df4b4d273f1e6 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -2895,3 +2895,43 @@ func TestIssue29823(t *testing.T) { err = tk2.QueryToErr("show tables from test") require.EqualError(t, err, "[executor:1044]Access denied for user 'u1'@'%' to database 'test'") } + +func TestCheckPreparePrivileges(t *testing.T) { + store := createStoreAndPrepareDB(t) + tk := testkit.NewTestKit(t, store) + tk.MustExec("use test") + tk.MustExec("drop table if exists t") + tk.MustExec("create user u1") + tk.MustExec("create table t (a int)") + tk.MustExec("insert into t values(1)") + + tk2 := testkit.NewTestKit(t, store) + require.True(t, tk2.Session().Auth(&auth.UserIdentity{Username: "u1", Hostname: "%"}, nil, nil)) + + // sql + err := tk2.ExecToErr("prepare s from 'select * from test.t'") + require.EqualError(t, err, "[planner:1142]SELECT command denied to user 'u1'@'%' for table 't'") + err = tk2.ExecToErr("execute s") + require.EqualError(t, err, "[planner:8111]Prepared statement not found") + + // binary proto + stmtID, _, _, err := tk2.Session().PrepareStmt("select * from test.t") + require.EqualError(t, err, "[planner:1142]SELECT command denied to user 'u1'@'%' for table 't'") + require.Zero(t, stmtID) + + // grant + tk.MustExec("grant SELECT ON test.t TO 'u1'@'%';") + + // should success after grant + tk2.MustExec("prepare s from 'select * from test.t'") + tk2.MustQuery("execute s").Check(testkit.Rows("1")) + stmtID, _, _, err = tk2.Session().PrepareStmt("select * from test.t") + require.NoError(t, err) + require.NotZero(t, stmtID) + rs, err := tk2.Session().ExecutePreparedStmt(context.TODO(), stmtID, nil) + require.NoError(t, err) + defer func() { + require.NoError(t, rs.Close()) + }() + tk2.ResultSetToResult(rs, "").Check(testkit.Rows("1")) +} From 3017033619f17eaceef36463c81fc3eedd095942 Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Fri, 5 Aug 2022 17:32:40 +0800 Subject: [PATCH 2/4] comments --- planner/core/plan_cache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/plan_cache.go b/planner/core/plan_cache.go index 98dae0cdf65cc..2000694e3fb17 100644 --- a/planner/core/plan_cache.go +++ b/planner/core/plan_cache.go @@ -537,6 +537,7 @@ func buildRangeForIndexScan(sctx sessionctx.Context, is *PhysicalIndexScan) (err return } +// CheckPreparedPriv checks the privilege for prepare stmt func CheckPreparedPriv(_ context.Context, sctx sessionctx.Context, preparedObj *CachedPrepareStmt, is infoschema.InfoSchema) error { if pm := privilege.GetPrivilegeManager(sctx); pm != nil { From a806c6bc973049432e191d2baece714998a65b74 Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Wed, 10 Aug 2022 19:07:17 +0800 Subject: [PATCH 3/4] address comments --- executor/prepared.go | 2 +- planner/core/plan_cache.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/executor/prepared.go b/executor/prepared.go index 8fb352da84911..87c638a858d12 100644 --- a/executor/prepared.go +++ b/executor/prepared.go @@ -242,7 +242,7 @@ func (e *PrepareExec) Next(ctx context.Context, req *chunk.Chunk) error { SQLDigest4PC: digest4PC, } - if err = plannercore.CheckPreparedPriv(ctx, e.ctx, preparedObj, ret.InfoSchema); err != nil { + if err = plannercore.CheckPreparedPriv(e.ctx, preparedObj, ret.InfoSchema); err != nil { return err } diff --git a/planner/core/plan_cache.go b/planner/core/plan_cache.go index 9e3e2a8e1df70..7749304127a9a 100644 --- a/planner/core/plan_cache.go +++ b/planner/core/plan_cache.go @@ -81,7 +81,7 @@ func GetPlanFromSessionPlanCache(ctx context.Context, sctx sessionctx.Context, i } if prepared.UseCache && !ignorePlanCache { // for general plans - if plan, names, ok, err := getGeneralPlan(ctx, sctx, cacheKey, bindSQL, is, preparedStmt, + if plan, names, ok, err := getGeneralPlan(sctx, cacheKey, bindSQL, is, preparedStmt, paramTypes); err != nil || ok { return plan, names, err } @@ -135,14 +135,14 @@ func getPointQueryPlan(prepared *ast.Prepared, sessVars *variable.SessionVars, s return plan, names, true, nil } -func getGeneralPlan(ctx context.Context, sctx sessionctx.Context, cacheKey kvcache.Key, bindSQL string, +func getGeneralPlan(sctx sessionctx.Context, cacheKey kvcache.Key, bindSQL string, is infoschema.InfoSchema, preparedStmt *CachedPrepareStmt, paramTypes []*types.FieldType) (Plan, []*types.FieldName, bool, error) { sessVars := sctx.GetSessionVars() stmtCtx := sessVars.StmtCtx if cacheValue, exists := sctx.PreparedPlanCache().Get(cacheKey); exists { - if err := CheckPreparedPriv(ctx, sctx, preparedStmt, is); err != nil { + if err := CheckPreparedPriv(sctx, preparedStmt, is); err != nil { return nil, nil, false, err } cachedVals := cacheValue.([]*PlanCacheValue) @@ -538,7 +538,7 @@ func buildRangeForIndexScan(sctx sessionctx.Context, is *PhysicalIndexScan) (err } // CheckPreparedPriv checks the privilege for prepare stmt -func CheckPreparedPriv(_ context.Context, sctx sessionctx.Context, +func CheckPreparedPriv(sctx sessionctx.Context, preparedObj *CachedPrepareStmt, is infoschema.InfoSchema) error { if pm := privilege.GetPrivilegeManager(sctx); pm != nil { visitInfo := VisitInfo4PrivCheck(is, preparedObj.PreparedAst.Stmt, preparedObj.VisitInfos) From bb1865cee82f7db674bc7a19ed20d52d3367712c Mon Sep 17 00:00:00 2001 From: Chao Wang Date: Wed, 10 Aug 2022 19:55:28 +0800 Subject: [PATCH 4/4] comments --- planner/core/plan_cache.go | 1 + 1 file changed, 1 insertion(+) diff --git a/planner/core/plan_cache.go b/planner/core/plan_cache.go index 6198d99fc6fe1..da40c5dd70103 100644 --- a/planner/core/plan_cache.go +++ b/planner/core/plan_cache.go @@ -537,6 +537,7 @@ func buildRangeForIndexScan(sctx sessionctx.Context, is *PhysicalIndexScan) (err return } +// CheckPreparedPriv checks the privilege of the prepared statement func CheckPreparedPriv(sctx sessionctx.Context, stmt *PlanCacheStmt, is infoschema.InfoSchema) error { if pm := privilege.GetPrivilegeManager(sctx); pm != nil { visitInfo := VisitInfo4PrivCheck(is, stmt.PreparedAst.Stmt, stmt.VisitInfos)