* [tarantool-patches] [security 0/3] System space access check lists
@ 2018-03-28 8:09 Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 1/3] box: Add system view for _sequence system space Ilya Markov
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ilya Markov @ 2018-03-28 8:09 UTC (permalink / raw)
To: georgy; +Cc: tarantool-patches
branch: gh-3250-system-space-access
Ilya Markov (2):
box: Add system view for _sequence system space
security: Refactor reads from systems spaces
imarkov (1):
security: Refactor system space access checks
src/box/alter.cc | 251 +++++++++++++++++++++++--------------
src/box/bootstrap.snap | Bin 1504 -> 1540 bytes
src/box/index.cc | 1 +
src/box/lua/schema.lua | 103 ++++++++-------
src/box/lua/space.cc | 2 +
src/box/lua/upgrade.lua | 34 +++--
src/box/schema.cc | 118 ++++++++++++++++-
src/box/schema.h | 28 +++--
src/box/schema_def.h | 2 +
src/box/space.c | 3 +-
src/box/space.h | 17 ++-
src/box/sysview_engine.c | 4 +
src/box/sysview_index.c | 28 +++++
src/box/user.cc | 4 -
src/box/user.h | 7 ++
test/app-tap/tarantoolctl.test.lua | 4 +-
test/box-py/bootstrap.result | 11 +-
test/box/access.result | 69 +++++-----
test/box/access.test.lua | 35 +++---
test/box/access_bin.result | 8 +-
test/box/access_misc.result | 51 ++++++--
test/box/access_misc.test.lua | 11 +-
test/box/access_sysview.result | 77 ++++++++++--
test/box/access_sysview.test.lua | 28 ++++-
test/box/alter.result | 3 +
test/box/net.box.result | 6 +
test/box/net.box.test.lua | 2 +
test/box/on_replace.result | 8 +-
test/box/role.result | 27 +++-
test/box/role.test.lua | 13 +-
test/box/sequence.result | 17 ++-
test/box/sequence.test.lua | 10 +-
test/box/transaction.result | 6 +-
test/engine/truncate.result | 2 +-
test/xlog/upgrade.result | 11 +-
35 files changed, 723 insertions(+), 278 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] [security 1/3] box: Add system view for _sequence system space
2018-03-28 8:09 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
@ 2018-03-28 8:09 ` Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 3/3] security: Refactor system space access checks Ilya Markov
2 siblings, 0 replies; 5+ messages in thread
From: Ilya Markov @ 2018-03-28 8:09 UTC (permalink / raw)
To: georgy; +Cc: tarantool-patches
Introduce _vsequence system space.
Prerequisite of #3250
---
src/box/bootstrap.snap | Bin 1504 -> 1540 bytes
src/box/lua/space.cc | 2 +
src/box/lua/upgrade.lua | 34 +++++++++++-----
src/box/schema.h | 13 +++----
src/box/schema_def.h | 2 +
src/box/sysview_index.c | 28 ++++++++++++++
test/app-tap/tarantoolctl.test.lua | 4 +-
test/box-py/bootstrap.result | 11 +++++-
test/box/access_misc.result | 5 +++
test/box/access_sysview.result | 77 +++++++++++++++++++++++++++++++++----
test/box/access_sysview.test.lua | 28 +++++++++++++-
test/box/alter.result | 3 ++
test/xlog/upgrade.result | 11 +++++-
13 files changed, 189 insertions(+), 29 deletions(-)
diff --git a/src/box/bootstrap.snap b/src/box/bootstrap.snap
index 85579a7c1fa829b6739a333bbd8003d8ab6b4858..b610828c9c9ae9a22acdd8c150c16c6838b7a273 100644
GIT binary patch
delta 1537
zcmV+c2LAcr3xo`i6@M`>E-)=LFfC_eH#agdIWh`KZgX^DZewLSAUR=UVKrqjHZ3z_
zW@arkW@b1oVK`<mEnzrjHZ(I~WjSRsWC~V8Y;R+0Iv{&7Iv_WF3JTS_3%bn(hycza
zBY(A}0000004TLD{Qyw?D*y&P)=JP8aRLBeJcwM)BGF(z1b>7x@aA+-<K_*})i*Fo
z_hd%YAUl;JC7GYL^1Ee&z>{-2bM)F^DFIB#Mpl`zk^qgjr*`I;FO*VB0l)yb0LuU(
z{zlP8%KCnNGoIjUSLV9x0ke>t9X;lbMREMRAvF}>bMQP2>zYgF-MQ^aS+C{j?4URu
z_OKJf{`GP1R)13}&<{=o;96?RHiqR*-d^3Me|O%)BY)lyi*hYBcLbq9LIqEKaQw0^
zZ*_+Y-D{~y3sgO3zCU*JaV<52DiD}G91OG7YpEH+tW@1hH`C!?onm58ty2hM)v0UA
zd?yg&)(q}I7bp8WFU9XR<5DW6Oj2@GE?l7}M4HenL4UC{Cu7%myKb?(N$;#31KNIG
zrWRaF%~vlmZ9gx)v!YUbe7X!{o~@b`TuaTeKlkzJGW5<`O0dHr*qA&s&koLmI(xR-
zasWuo8&XLIuBB!v;@COs81#zNl>*mNQ{xxMu_b=8e|`8n`^KH0I9O*G&wkf;op0oK
zUIU`P)PM2Z^QkHXZa&AjjNfA3t&RrZz43`*dkk$ZEQV*-<@kA^_&NO1*`?q1<3CJt
z&sIeOuB9f4VLZH(V=bZ1FZ+4<@`5wAj?_xyN+Wuz4UG+}Mnq+WV1ZbDSb11=nAvQ%
zqFr|_HSerPH<OY{#iUdy4k#E@%7ij)&@sJ~PJbm0*HTlbl!3!A<4i53S<3~I1yTi)
z%ZuaJ3I&x3Dx6Boh-E|?tyn8ksWY7<RU|<aq1Ms0)C`H}Oh?I=I<hWpJpOmD(Uc0b
z!?i#FzvKP-T^`RwetH<GHV>|)=B(pSm%DpDwZ_4<)GTLSPi2K-d6TOK*HZIcFXmX@
zq<@dszsJ!UfBpj?%EP|1WHKB2-6pSmzqox<?raFR)V4F2%S#YvrUEE_B}Dh<}P
zu{ASRGEy;2Ft`^a7j@L)T58%*1v)K@s>j?vU4}ij(ztF#P*4z)T5~NmMfCapSk`5a
zifgHfVrivuU5fo=$<NAy;>#lW`={O$FMp1^_GPhElD2AT9Lj{wFc6=$mbjLhAY9>#
zlyQ+}#fdLNj2nL=bJ0=cikpwUVn|Z>#ZM%DeLg()^Rm|Ym#(u?8iyirEj4Lqu;8g<
zfgq^STx3iM%m4s@04M+<21hAoMGq2yz&MJdFbZH821E!r3Jn+m0KtKfpa`HA41Yp~
zyc6BI3qu$KYT3207JFeYj>@?c6V#)WgjHk@yH_}i!Xcvw@q-YDq2fo$3reJAyii23
z#R+gFD1gV6oJTd(ZT3r&o5zCbY$~e7BhW6T0>`OjbXmB$N#x&Irn6E}G+QdV21aj)
z*Q?IqN=PecZ=%PXqqffww8kMDXMcvH+lcy9EmFYZkd`T6b4(V=0Ndi2BI(*;4tmF)
z<n(`~-a8<kdTXoB-G6fM?~&W*-dY{XX%WchK_DAuK}u%|q|m?{+Jlh53vF|F_Sk-~
zw^~{Sp24P<UVN)efGG*)<T|sH`Hni`$|kt@^ciXZpt=O8Bsz`2sLy&&W`8sKYs7TJ
zFiWjjN7El4Zo8JCG3uLX&}ghSic*cC7O1fjwmGdtk|Re5WF*cf|NT8HgpyzAoeEN%
zrHKM2GSNAM9q}x*77=lAl!w(m0_)(JRFf)i){a4KKrCRol`*=wWGI9MlE353vJKCi
z@wRhjVFu(fbM8d3fFY-~eR@AR*>KUFb`PMr;8q8DGq_+p!cgu}^2RjfloDN<L;ma(
zy?sY7c|?<ev*az@&jfPL$Vb9%@#Une@T6*ACI3iv9=$uN0K4X8665(jRHW5XQ}Orq
ntXkIy@%_k{U|^;OD*azr*Oo^POSq0q;OiJYNhYHl)ex=i+4b68
delta 1501
zcmV<31tR){4B!ip6@ND_HZ3tSHZ5l{Gh{S1W?~9SZgX^DZewLSATT*%GB{>7I4v?^
zVq+~dWiw+fI5T5mEi_^|W??inGh<?6FbY;fY;R+0Iv{&7Iv_B83JTS_3%bn(WB|@)
z><Wpc0000004TLD{QywiDgXw2(n;VJsQ@s{FvAQ}GO{f!<$nl63F7EpK{z~Ux=TWc
zJtPfFWGV?MiTdf@&ooWyc5xY4aCRkmX-kxhEYo0<aNCcq(+Qg&lu}9oz5ufT#{l3>
zl=c0(CM;R6U771K$I3%)a&(yc1;G%1MCeL@&ry5W)i#&TyYt(NvTnP<*)eT8>|y1F
z{R{ICM^z{fkbmw2z}891_J!pw;$GdQe|Mhak?}{QqS!jg9YSi9SW%<T3USsWj^=bL
z*49a;q);7ZzCVWhVe2G&C=V-tHWzlIwobAXbHX@<Nhwo`G(wT?YE(MOm`2{olNsSs
z>9jhbNEM#!?|cL?Zq}*LIh&~52)QI|^uTd4!vv+yn}1#3?ZU<K7Qd3B6wvnbk)&el
zBwu~JwEcYiO6n!}f%VvmeK#sXVCy8u{`?2lW9utv=s-?qU|+G!wH%vg>HOX3O95La
zIf6G<&iZA2MraEGTPLaSgJHM=vFu;?;<E4Cd1-TXhSm1FzUzz=zjIp<{iBZOpAKai
zQ1dy)Wq%wO`*0K_;O>3P3)^Aq=DJ|3?0O9G#{xgMc64^>zX5rCMgHCBC&kuDiWgSn
zT?}pD#bGpT=?t@<k1#Mc;R*?*v@RqfM`?th0fOcORe4aQQmE7^)nRt4D4B^e**Zx(
z^MNXnVn`&C=W+00ypd>wNP{RtNE8a$I!T-mhJWtDth4Y^VfI4d#qX<S0!zp8!r}sw
z0BQlT)cT1)XE;je2f_zL>BrVdt~{7CoFzZ%$a=i-_}|?&RVYtR_u_!{{qEQA`gks~
z<KqaW(ZJS8&iciA-2Ky`G%L1Fa-4lRs$vk!TTCsmb&~ITFvs#1b-s=t$12?U4+Gw6
zg?~#jrrS!bOrwiSs1Yu`X@pldW`yd{LdC37UWF%g)~RN=$e=M)7DlCck#a$5A!5<d
zP|ylnCs|i0Pgzw^9p;Yp*veH(>rg3rdU~2r6t+(CGoSB|Wj*Fj*g8p+Iw`F~nb<Fm
zyreEAa0bQSJJgbRFx)nr!PO|<Xr*=DjDOBB)-5Sj**eJ(uGUMGad3tO3amlKjjxft
z=$^=;=3}o|64HI~k$7KNhsS<C+B*NyRnkf8yb-oea;8R$T7eR%(O6`V5|{x100B?{
z5e7#oXEqNKfWSD4qc93#7zShrIEoDz0f4|DBp?E)1pt(+1#`>UjE15I%VGcyz<&Wa
z$QXFc<Qi{LQNQl6%M9tHCF(*T4iMrnEc_q|O=%>CcgH=bml{!d3<y@G>7@<T%zpKH
z`_WgcL7{DVJ_31g>o~iN20?a@Um$T7Tqd6{))vPg!Ef;v`<nH=NKq}k%SaQy+i~43
z5#taz2dvS<imHzQKN1eUsP=%t#(&b0GvQ6rku%{<mX5Ay>zz9Ylo8nw!lygLGz2<f
zu~d>mo@ml(2tm(lVN@L;2ew5WAP4p^1nJqxvX5R``A=xyq(1fAk)VGGygDkgwfhdc
z;>srYwe%|7>P@f&bDLgw)~Lg8`e7)c(kSqIAfuaGhNeFvV7u-?Oya6>kbi6Z64j8#
zaE)6N4AVs^F6AH^OvH)*<newD=$+jcYUqSCXX{$Jp^OkP*wM?9<|4u;N6{W8N0#D`
zi8`Y^B(7542KWKfu~)puA&GSWVg&4Sg=Wg|OSun@X&)XHYz#f|Ilyzloe=rWvJjBM
zG@c-N;WW*dhEBjk4t0{P>sGi|!nkGND)QPlG8fT#?-83VTAuWZJjsl!^&bg;<vT(Z
zh}R57Vm#M}2D-vv>Oc>eRalM2zneM<7tw0q)BmA$EeX`Hj_U{pzT(%Dgn`Qe)ex=i
DF`dsL
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index 29a9aca..071818a 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -429,6 +429,8 @@ box_lua_space_init(struct lua_State *L)
lua_setfield(L, -2, "SEQUENCE_ID");
lua_pushnumber(L, BOX_SEQUENCE_DATA_ID);
lua_setfield(L, -2, "SEQUENCE_DATA_ID");
+ lua_pushnumber(L, BOX_VSEQUENCE_ID);
+ lua_setfield(L, -2, "VSEQUENCE_ID");
lua_pushnumber(L, BOX_SPACE_SEQUENCE_ID);
lua_setfield(L, -2, "SPACE_SEQUENCE_ID");
lua_pushnumber(L, BOX_SYSTEM_ID_MIN);
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 01f9cd6..5891619 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -839,6 +839,15 @@ local function initial_1_7_5()
_schema:insert({'version', 1, 7, 5})
end
+local sequence_format = {{name = 'id', type = 'unsigned'},
+ {name = 'owner', type = 'unsigned'},
+ {name = 'name', type = 'string'},
+ {name = 'step', type = 'integer'},
+ {name = 'min', type = 'integer'},
+ {name = 'max', type = 'integer'},
+ {name = 'start', type = 'integer'},
+ {name = 'cache', type = 'integer'},
+ {name = 'cycle', type = 'boolean'}}
--------------------------------------------------------------------------------
-- Tarantool 1.7.6
local function create_sequence_space()
@@ -850,16 +859,7 @@ local function create_sequence_space()
local MAP = setmap({})
log.info("create space _sequence")
- _space:insert{_sequence.id, ADMIN, '_sequence', 'memtx', 0, MAP,
- {{name = 'id', type = 'unsigned'},
- {name = 'owner', type = 'unsigned'},
- {name = 'name', type = 'string'},
- {name = 'step', type = 'integer'},
- {name = 'min', type = 'integer'},
- {name = 'max', type = 'integer'},
- {name = 'start', type = 'integer'},
- {name = 'cache', type = 'integer'},
- {name = 'cycle', type = 'boolean'}}}
+ _space:insert{_sequence.id, ADMIN, '_sequence', 'memtx', 0, MAP, sequence_format}
log.info("create index _sequence:primary")
_index:insert{_sequence.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}}
log.info("create index _sequence:owner")
@@ -950,6 +950,19 @@ local function upgrade_to_1_7_7()
_priv:replace({ADMIN, SUPER, 'universe', 0, 4294967295})
end
+--------------------------------------------------------------------------------
+--- Tarantool 1.10.0
+--------------------------------------------------------------------------------
+local function create_vsequence_space()
+ create_sysview(box.schema.SEQUENCE_ID, box.schema.VSEQUENCE_ID)
+ box.space._vsequence:format(sequence_format)
+end
+
+local function upgrade_to_1_10_0()
+ create_vsequence_space()
+end
+
+
local function get_version()
local version = box.space._schema:get{'version'}
if version == nil then
@@ -975,6 +988,7 @@ local function upgrade(options)
{version = mkversion(1, 7, 5), func = upgrade_to_1_7_5, auto = true},
{version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = false},
{version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
+ {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
}
for _, handler in ipairs(handlers) do
diff --git a/src/box/schema.h b/src/box/schema.h
index 56f39b3..2b87f5f 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -97,6 +97,12 @@ space_foreach(int (*func)(struct space *sp, void *udata), void *udata);
const char *
schema_find_name(enum schema_object_type type, uint32_t object_id);
+/**
+ * Find a sequence by id. Return NULL if the sequence was
+ * not found.
+ */
+struct sequence *
+sequence_by_id(uint32_t id);
#if defined(__cplusplus)
} /* extern "C" */
@@ -178,13 +184,6 @@ bool
schema_find_grants(const char *type, uint32_t id);
/**
- * Find a sequence by id. Return NULL if the sequence was
- * not found.
- */
-struct sequence *
-sequence_by_id(uint32_t id);
-
-/**
* A wrapper around sequence_by_id() that raises an exception
* if the sequence was not found in the cache.
*/
diff --git a/src/box/schema_def.h b/src/box/schema_def.h
index d0de175..dd3aae2 100644
--- a/src/box/schema_def.h
+++ b/src/box/schema_def.h
@@ -78,6 +78,8 @@ enum {
BOX_SEQUENCE_ID = 284,
/** Space id of _sequence_data. */
BOX_SEQUENCE_DATA_ID = 285,
+ /** Space id of _vspace view. */
+ BOX_VSEQUENCE_ID = 286,
/** Space id of _index. */
BOX_INDEX_ID = 288,
/** Space id of _vindex view. */
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index 0bec302..bf0442b 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -33,6 +33,7 @@
#include <small/mempool.h>
#include "fiber.h"
#include "schema.h"
+#include "sequence.h"
#include "space.h"
#include "func.h"
#include "tuple.h"
@@ -258,6 +259,28 @@ vfunc_filter(struct space *source, struct tuple *tuple)
return false;
}
+static bool
+vsequence_filter(struct space *source, struct tuple *tuple)
+{
+ struct credentials *cr = effective_user();
+ if ((PRIV_R | PRIV_X) & cr->universal_access)
+ return true; /* read or execute access to unverse */
+ if (PRIV_R & source->access[cr->auth_token].effective)
+ return true; /* read access to original space */
+
+ uint32_t id;
+ if (tuple_field_u32(tuple, BOX_SEQUENCE_FIELD_ID, &id) != 0)
+ return false;
+ struct sequence *sequence = sequence_by_id(id);
+ if (sequence == NULL)
+ return false;
+ uint8_t effective = sequence->access[cr->auth_token].effective;
+ if (sequence->def->uid == cr->uid || ((PRIV_W | PRIV_R) & effective))
+ return true;
+ return false;
+}
+
+
struct sysview_index *
sysview_index_new(struct sysview_engine *sysview,
struct index_def *def, const char *space_name)
@@ -299,6 +322,11 @@ sysview_index_new(struct sysview_engine *sysview,
source_index_id = def->iid;
filter = vpriv_filter;
break;
+ case BOX_VSEQUENCE_ID:
+ source_space_id = BOX_SEQUENCE_ID;
+ source_index_id = def->iid;
+ filter = vsequence_filter;
+ break;
default:
diag_set(ClientError, ER_MODIFY_INDEX,
def->name, space_name,
diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index d757530..6946c83 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -338,8 +338,8 @@ do
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 1 --replica 2", "\n", 3)
check_ctlcat_xlog(test_i, dir, "--from=3 --to=6 --format=json --show-system --replica 2", "\n", 0)
- check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 17)
- check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 40)
+ check_ctlcat_snap(test_i, dir, "--space=280", "---\n", 18)
+ check_ctlcat_snap(test_i, dir, "--space=288", "---\n", 43)
end)
end)
diff --git a/test/box-py/bootstrap.result b/test/box-py/bootstrap.result
index 5e51aa2..16c2027 100644
--- a/test/box-py/bootstrap.result
+++ b/test/box-py/bootstrap.result
@@ -5,7 +5,7 @@ box.space._schema:select{}
---
- - ['cluster', '<cluster uuid>']
- ['max_id', 511]
- - ['version', 1, 7, 7]
+ - ['version', 1, 10, 0]
...
box.space._cluster:select{}
---
@@ -33,6 +33,11 @@ box.space._space:select{}
{'name': 'cycle', 'type': 'boolean'}]]
- [285, 1, '_sequence_data', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
{'name': 'value', 'type': 'integer'}]]
+ - [286, 1, '_vsequence', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'},
+ {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {
+ 'name': 'step', 'type': 'integer'}, {'name': 'min', 'type': 'integer'}, {
+ 'name': 'max', 'type': 'integer'}, {'name': 'start', 'type': 'integer'}, {
+ 'name': 'cache', 'type': 'integer'}, {'name': 'cycle', 'type': 'boolean'}]]
- [288, 1, '_index', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'iid',
'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
@@ -79,6 +84,9 @@ box.space._index:select{}
- [284, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
- [284, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [285, 0, 'primary', 'hash', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
+ - [286, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [288, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
- [288, 2, 'name', 'tree', {'unique': true}, [[0, 'unsigned'], [2, 'string']]]
- [289, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
@@ -129,6 +137,7 @@ box.space._priv:select{}
- [1, 2, 'function', 1, 4]
- [1, 2, 'space', 276, 2]
- [1, 2, 'space', 281, 1]
+ - [1, 2, 'space', 286, 1]
- [1, 2, 'space', 289, 1]
- [1, 2, 'space', 297, 1]
- [1, 2, 'space', 305, 1]
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 328603a..3a56a4c 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -752,6 +752,11 @@ box.space._space:select()
{'name': 'cycle', 'type': 'boolean'}]]
- [285, 1, '_sequence_data', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
{'name': 'value', 'type': 'integer'}]]
+ - [286, 1, '_vsequence', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'},
+ {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {
+ 'name': 'step', 'type': 'integer'}, {'name': 'min', 'type': 'integer'}, {
+ 'name': 'max', 'type': 'integer'}, {'name': 'start', 'type': 'integer'}, {
+ 'name': 'cache', 'type': 'integer'}, {'name': 'cycle', 'type': 'boolean'}]]
- [288, 1, '_index', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'iid',
'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index 63e30af..340ed21 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -123,6 +123,10 @@ box.session.su('guest')
---
- error: Read access to space '_vfunc' is denied for user 'guest'
...
+#box.space._vsequence:select{}
+---
+- error: Read access to space '_vsequence' is denied for user 'guest'
+...
box.session.su('admin')
---
...
@@ -134,11 +138,11 @@ box.session.su('guest')
...
#box.space._vspace:select{}
---
-- 7
+- 8
...
#box.space._vindex:select{}
---
-- 17
+- 20
...
box.session.su('admin')
---
@@ -226,11 +230,11 @@ box.session.su('guest')
...
#box.space._vspace:select{}
---
-- 18
+- 19
...
#box.space._vindex:select{}
---
-- 41
+- 44
...
#box.space._vuser:select{}
---
@@ -238,7 +242,7 @@ box.session.su('guest')
...
#box.space._vpriv:select{}
---
-- 14
+- 15
...
#box.space._vfunc:select{}
---
@@ -258,7 +262,7 @@ box.session.su('guest')
...
#box.space._vindex:select{}
---
-- 41
+- 44
...
#box.space._vuser:select{}
---
@@ -272,6 +276,10 @@ box.session.su('guest')
---
- 1
...
+#box.space._vsequence:select{}
+---
+- 0
+...
box.session.su('admin')
---
...
@@ -564,11 +572,66 @@ box.session.su('guest')
- true
...
--
--- view:alter() tests
+-- _vsequence
--
+box.session.su('admin')
+---
+...
+seq = box.schema.sequence.create('test')
+---
+...
+-- read access to original sequence also allow to read a view
+seq_cnt = #box.space._sequence:select{}
+---
+...
+box.schema.user.grant("guest", "read", "sequence", "test")
+---
+...
+box.session.su("guest")
+---
+...
+#box.space._vsequence:select{} == seq_cnt
+---
+- true
+...
+box.session.su('admin')
+---
+...
+box.schema.user.revoke("guest", "read", "sequence", "test")
+---
+...
+box.session.su("guest")
+---
+...
+cnt = #box.space._vsequence:select{}
+---
+...
+cnt < seq_cnt
+---
+- true
+...
session.su('admin')
---
...
+box.schema.user.grant("guest", "write", "sequence", "test")
+---
+...
+box.session.su("guest")
+---
+...
+#box.space._vsequence:select{} == cnt + 1
+---
+- true
+...
+session.su('admin')
+---
+...
+seq:drop()
+---
+...
+--
+-- view:alter() tests
+--
box.space._vspace.index[1]:alter({parts = { 2, 'string' }})
---
...
diff --git a/test/box/access_sysview.test.lua b/test/box/access_sysview.test.lua
index ac7c179..7955ffc 100644
--- a/test/box/access_sysview.test.lua
+++ b/test/box/access_sysview.test.lua
@@ -47,6 +47,7 @@ box.session.su('guest')
#box.space._vuser:select{}
#box.space._vpriv:select{}
#box.space._vfunc:select{}
+#box.space._vsequence:select{}
box.session.su('admin')
box.schema.user.grant('guest', 'public')
@@ -105,6 +106,7 @@ box.session.su('guest')
#box.space._vuser:select{}
#box.space._vpriv:select{}
#box.space._vfunc:select{}
+#box.space._vsequence:select{}
box.session.su('admin')
box.schema.user.revoke('guest', 'write', 'universe')
@@ -238,10 +240,34 @@ box.session.su('guest')
#box.space._vfunc:select{} == cnt
--
--- view:alter() tests
+-- _vsequence
--
+box.session.su('admin')
+seq = box.schema.sequence.create('test')
+
+-- read access to original sequence also allow to read a view
+seq_cnt = #box.space._sequence:select{}
+box.schema.user.grant("guest", "read", "sequence", "test")
+box.session.su("guest")
+#box.space._vsequence:select{} == seq_cnt
+box.session.su('admin')
+
+box.schema.user.revoke("guest", "read", "sequence", "test")
+box.session.su("guest")
+cnt = #box.space._vsequence:select{}
+cnt < seq_cnt
+session.su('admin')
+box.schema.user.grant("guest", "write", "sequence", "test")
+box.session.su("guest")
+#box.space._vsequence:select{} == cnt + 1
session.su('admin')
+seq:drop()
+
+--
+-- view:alter() tests
+--
+
box.space._vspace.index[1]:alter({parts = { 2, 'string' }})
box.space._vspace.index[1]:select('xxx')
box.space._vspace.index[1]:select(1)
diff --git a/test/box/alter.result b/test/box/alter.result
index 347de47..49d5c39 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -194,6 +194,9 @@ _index:select{}
- [284, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
- [284, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [285, 0, 'primary', 'hash', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
+ - [286, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [288, 0, 'primary', 'tree', 1, 2, 0, 'unsigned', 1, 'unsigned']
- [288, 2, 'name', 'tree', {'unique': true}, [[0, 'unsigned'], [2, 'string']]]
- [289, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
diff --git a/test/xlog/upgrade.result b/test/xlog/upgrade.result
index 113c066..f02996b 100644
--- a/test/xlog/upgrade.result
+++ b/test/xlog/upgrade.result
@@ -36,7 +36,7 @@ box.space._schema:select()
---
- - ['cluster', '<server_uuid>']
- ['max_id', 513]
- - ['version', 1, 7, 7]
+ - ['version', 1, 10, 0]
...
box.space._space:select()
---
@@ -60,6 +60,11 @@ box.space._space:select()
{'name': 'cycle', 'type': 'boolean'}]]
- [285, 1, '_sequence_data', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'},
{'name': 'value', 'type': 'integer'}]]
+ - [286, 1, '_vsequence', 'sysview', 0, {}, [{'name': 'id', 'type': 'unsigned'},
+ {'name': 'owner', 'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {
+ 'name': 'step', 'type': 'integer'}, {'name': 'min', 'type': 'integer'}, {
+ 'name': 'max', 'type': 'integer'}, {'name': 'start', 'type': 'integer'}, {
+ 'name': 'cache', 'type': 'integer'}, {'name': 'cycle', 'type': 'boolean'}]]
- [288, 1, '_index', 'memtx', 0, {}, [{'name': 'id', 'type': 'unsigned'}, {'name': 'iid',
'type': 'unsigned'}, {'name': 'name', 'type': 'string'}, {'name': 'type',
'type': 'string'}, {'name': 'opts', 'type': 'map'}, {'name': 'parts', 'type': 'array'}]]
@@ -109,6 +114,9 @@ box.space._index:select()
- [284, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
- [284, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [285, 0, 'primary', 'hash', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned']]]
+ - [286, 1, 'owner', 'tree', {'unique': false}, [[1, 'unsigned']]]
+ - [286, 2, 'name', 'tree', {'unique': true}, [[2, 'string']]]
- [288, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
- [288, 2, 'name', 'tree', {'unique': true}, [[0, 'unsigned'], [2, 'string']]]
- [289, 0, 'primary', 'tree', {'unique': true}, [[0, 'unsigned'], [1, 'unsigned']]]
@@ -173,6 +181,7 @@ box.space._priv:select()
- [1, 2, 'function', 2, 4]
- [1, 2, 'space', 276, 2]
- [1, 2, 'space', 281, 1]
+ - [1, 2, 'space', 286, 1]
- [1, 2, 'space', 289, 1]
- [1, 2, 'space', 297, 1]
- [1, 2, 'space', 305, 1]
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces
2018-03-28 8:09 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 1/3] box: Add system view for _sequence system space Ilya Markov
@ 2018-03-28 8:09 ` Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 3/3] security: Refactor system space access checks Ilya Markov
2 siblings, 0 replies; 5+ messages in thread
From: Ilya Markov @ 2018-03-28 8:09 UTC (permalink / raw)
To: georgy; +Cc: tarantool-patches
Replace reads from systems spaces with reads from corresponding
system views.
After this patch some error messages are changed:
* Accessing to objects that are not accessible for current user
raises the error claiming these objects don't exists.
* Attempt to add in transaction such methods as object create, drop
raises an multi-engine transaction error instead of multi-statement
transaction error.
In scope of #3250
---
src/box/lua/schema.lua | 91 ++++++++++++++++++++++++++-------------------
test/box/access.result | 46 ++++++++++++-----------
test/box/access.test.lua | 25 ++++++-------
test/box/access_bin.result | 8 ++--
test/box/access_misc.result | 2 +-
test/box/on_replace.result | 8 ++--
test/box/role.result | 2 +-
test/box/transaction.result | 6 +--
8 files changed, 103 insertions(+), 85 deletions(-)
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 30c6bc6..5496acb 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -102,12 +102,12 @@ ffi.cdef[[
]]
local function user_or_role_resolve(user)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(user) == 'string' then
- tuple = _user.index.name:get{user}
+ tuple = _vuser.index.name:get{user}
else
- tuple = _user:get{user}
+ tuple = _vuser:get{user}
end
if tuple == nil then
return nil
@@ -116,12 +116,12 @@ local function user_or_role_resolve(user)
end
local function role_resolve(name_or_id)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _user.index.name:get{name_or_id}
+ tuple = _vuser.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _user:get{name_or_id}
+ tuple = _vuser:get{name_or_id}
end
if tuple == nil or tuple[4] ~= 'role' then
return nil
@@ -131,12 +131,12 @@ local function role_resolve(name_or_id)
end
local function user_resolve(name_or_id)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _user.index.name:get{name_or_id}
+ tuple = _vuser.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _user:get{name_or_id}
+ tuple = _vuser:get{name_or_id}
end
if tuple == nil or tuple[4] ~= 'user' then
return nil
@@ -146,12 +146,12 @@ local function user_resolve(name_or_id)
end
local function sequence_resolve(name_or_id)
- local _sequence = box.space[box.schema.SEQUENCE_ID]
+ local _vsequence = box.space[box.schema.VSEQUENCE_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _sequence.index.name:get{name_or_id}
+ tuple = _vsequence.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _sequence:get{name_or_id}
+ tuple = _vsequence:get{name_or_id}
end
if tuple ~= nil then
return tuple[1], tuple
@@ -1368,7 +1368,19 @@ function box.schema.space.bless(space)
-- primary key and returns it back to the user
space_mt.auto_increment = function(space, tuple)
check_space_arg(space, 'auto_increment')
- local max_tuple = check_primary_index(space):max()
+ local euid = box.session.euid()
+ -- HACK: habe to call box.session.su here,
+ -- as checking max requires 'READ' access to space, though auto_increment
+ -- is a 'WRITE' operation
+ local max_tuple
+ if euid ~= 1 then
+ box.session.su("admin")
+ max_tuple = check_primary_index(space):max()
+ box.session.su(euid)
+ else
+ max_tuple = check_primary_index(space):max()
+ end
+
local max = 0
if max_tuple ~= nil then
max = max_tuple[1]
@@ -1686,12 +1698,12 @@ local function object_resolve(object_type, object_name)
return space.id
end
if object_type == 'function' then
- local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local func
if type(object_name) == 'string' then
- func = _func.index.name:get{object_name}
+ func = _vfunc.index.name:get{object_name}
else
- func = _func:get{object_name}
+ func = _vfunc:get{object_name}
end
if func then
return func[1]
@@ -1707,12 +1719,12 @@ local function object_resolve(object_type, object_name)
return seq
end
if object_type == 'role' then
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local role
if type(object_name) == 'string' then
- role = _user.index.name:get{object_name}
+ role = _vuser.index.name:get{object_name}
else
- role = _user:get{object_name}
+ role = _vuser:get{object_name}
end
if role and role[4] == 'role' then
return role[1]
@@ -1730,13 +1742,13 @@ local function object_name(object_type, object_id)
end
local space
if object_type == 'space' then
- space = box.space._space
+ space = box.space._vspace
elseif object_type == 'sequence' then
space = box.space._sequence
elseif object_type == 'function' then
- space = box.space._func
+ space = box.space._vfunc
elseif object_type == 'role' or object_type == 'user' then
- space = box.space._user
+ space = box.space._vuser
else
box.error(box.error.UNKNOWN_SCHEMA_OBJECT, object_type)
end
@@ -1750,7 +1762,8 @@ box.schema.func.create = function(name, opts)
if_not_exists = 'boolean',
language = 'string'})
local _func = box.space[box.schema.FUNC_ID]
- local func = _func.index.name:get{name}
+ local _vfunc = box.space[box.schema.VFUNC_ID]
+ local func = _vfunc.index.name:get{name}
if func then
if not opts.if_not_exists then
box.error(box.error.FUNCTION_EXISTS, name)
@@ -1767,12 +1780,13 @@ box.schema.func.drop = function(name, opts)
opts = opts or {}
check_param_table(opts, { if_exists = 'boolean' })
local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local fid
local tuple
if type(name) == 'string' then
- tuple = _func.index.name:get{name}
+ tuple = _vfunc.index.name:get{name}
else
- tuple = _func:get{name}
+ tuple = _vfunc:get{name}
end
if tuple then
fid = tuple[1]
@@ -1788,12 +1802,12 @@ box.schema.func.drop = function(name, opts)
end
function box.schema.func.exists(name_or_id)
- local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local tuple = nil
if type(name_or_id) == 'string' then
- tuple = _func.index.name:get{name_or_id}
+ tuple = _vfunc.index.name:get{name_or_id}
elseif type(name_or_id) == 'number' then
- tuple = _func:get{name_or_id}
+ tuple = _vfunc:get{name_or_id}
end
return tuple ~= nil
end
@@ -1948,8 +1962,9 @@ local function grant(uid, name, privilege, object_type,
options.grantor = user_or_role_resolve(options.grantor)
end
local _priv = box.space[box.schema.PRIV_ID]
+ local _vpriv = box.space[box.schema.VPRIV_ID]
-- add the granted privilege to the current set
- local tuple = _priv:get{uid, object_type, oid}
+ local tuple = _vpriv:get{uid, object_type, oid}
local old_privilege
if tuple ~= nil then
old_privilege = tuple[5]
@@ -1984,7 +1999,8 @@ local function revoke(uid, name, privilege, object_type, object_name, options)
options = options or {}
local oid = object_resolve(object_type, object_name)
local _priv = box.space[box.schema.PRIV_ID]
- local tuple = _priv:get{uid, object_type, oid}
+ local _vpriv = box.space[box.schema.VPRIV_ID]
+ local tuple = _vpriv:get{uid, object_type, oid}
-- system privileges of admin and guest can't be revoked
if tuple == nil then
if options.if_exists then
@@ -2013,17 +2029,17 @@ end
local function drop(uid, opts)
-- recursive delete of user data
- local _priv = box.space[box.schema.PRIV_ID]
- local spaces = box.space[box.schema.SPACE_ID].index.owner:select{uid}
+ local _vpriv = box.space[box.schema.VPRIV_ID]
+ local spaces = box.space[box.schema.VSPACE_ID].index.owner:select{uid}
for k, tuple in pairs(spaces) do
box.space[tuple[1]]:drop()
end
- local funcs = box.space[box.schema.FUNC_ID].index.owner:select{uid}
+ local funcs = box.space[box.schema.VFUNC_ID].index.owner:select{uid}
for k, tuple in pairs(funcs) do
box.schema.func.drop(tuple[1])
end
-- if this is a role, revoke this role from whoever it was granted to
- local grants = _priv.index.object:select{'role', uid}
+ local grants = _vpriv.index.object:select{'role', uid}
for k, tuple in pairs(grants) do
revoke(tuple[2], tuple[2], uid)
end
@@ -2034,11 +2050,11 @@ local function drop(uid, opts)
-- xxx: hack, we have to revoke session and usage privileges
-- of a user using a setuid function in absence of create/drop
-- privileges and grant option
- if box.space._user:get{uid}[4] == 'user' then
+ if box.space._vuser:get{uid}[4] == 'user' then
box.session.su('admin', box.schema.user.revoke, uid,
'session,usage', 'universe', nil, {if_exists = true})
end
- local privs = _priv.index.primary:select{uid}
+ local privs = _vpriv.index.primary:select{uid}
for k, tuple in pairs(privs) do
revoke(uid, uid, tuple[5], tuple[3], tuple[4])
end
@@ -2095,8 +2111,7 @@ box.schema.user.drop = function(name, opts)
end
local function info(id)
- local _priv = box.space._priv
- local _user = box.space._priv
+ local _priv = box.space._vpriv
local privs = {}
for _, v in pairs(_priv:select{id}) do
table.insert(
diff --git a/test/box/access.result b/test/box/access.result
index 191857f..2b28f07 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -386,7 +386,8 @@ session.su('grantee')
-- fails - can't suicide - ask the creator to kill you
box.schema.user.drop('grantee')
---
-- error: Read access to space '_user' is denied for user 'grantee'
+- error: 'Failed to drop user or role ''grantee'': the user is active in the current
+ session'
...
session.su('grantor')
---
@@ -471,7 +472,7 @@ session.su('user1')
-- permission denied
box.schema.user.passwd('admin', 'xxx')
---
-- error: Read access to space '_user' is denied for user 'user1'
+- error: User 'admin' is not found
...
session.su('admin')
---
@@ -1048,7 +1049,7 @@ session.su("test1")
...
box.schema.user.disable("test")
---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
...
session.su("admin")
---
@@ -1080,7 +1081,7 @@ session.su("test1")
...
box.schema.user.grant("test", "usage", "universe")
---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
...
session.su('admin')
---
@@ -1173,11 +1174,11 @@ box.schema.space.create('test')
...
box.schema.user.create('test')
---
-- error: Read access to space '_user' is denied for user 'guest'
+- error: Write access to space '_user' is denied for user 'guest'
...
box.schema.func.create('test')
---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
...
box.session.su('admin')
---
@@ -1367,18 +1368,24 @@ box.schema.user.grant("tester", "read,execute", "universe")
---
...
-- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
+box.session.su("tester")
+---
+...
+box.schema.space.create("test_space")
---
- error: Write access to space '_schema' is denied for user 'tester'
...
-box.session.su("tester", box.schema.user.create, 'test_user')
+box.schema.user.create('test_user')
---
- error: Write access to space '_user' is denied for user 'tester'
...
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.schema.func.create('test_func')
---
- error: Write access to space '_func' is denied for user 'tester'
...
+box.session.su("admin")
+---
+...
--
-- FIXME 2.0: we still need to grant 'write' on universe
-- explicitly since we still use process_rw to write to system
@@ -1387,24 +1394,27 @@ box.session.su("tester", box.schema.func.create, 'test_func')
box.schema.user.grant("tester", "create,write", "universe")
---
...
+box.session.su("tester")
+---
+...
-- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
+s1 = box.schema.space.create("test_space")
---
...
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
+_ = box.schema.user.create('test_user')
---
...
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+_ = box.schema.func.create('test_func')
---
...
-- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
+s1:drop()
---
...
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
+box.schema.user.drop('test_user')
---
...
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+box.schema.func.drop('test_func')
---
...
-- failed alter
@@ -1414,12 +1424,6 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
-- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
-- failed drop
-- box.session.su("tester", s.drop, s)
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
----
-...
box.schema.user.drop("test")
---
- error: Revoke access to role 'public' is denied for user 'tester'
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 7e880a0..100dad5 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -514,9 +514,11 @@ f = box.schema.func.create("test")
box.schema.user.grant("tester", "read,execute", "universe")
-- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
-box.session.su("tester", box.schema.user.create, 'test_user')
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.session.su("tester")
+box.schema.space.create("test_space")
+box.schema.user.create('test_user')
+box.schema.func.create('test_func')
+box.session.su("admin")
--
-- FIXME 2.0: we still need to grant 'write' on universe
@@ -524,15 +526,16 @@ box.session.su("tester", box.schema.func.create, 'test_func')
-- tables from ddl
--
box.schema.user.grant("tester", "create,write", "universe")
+box.session.su("tester")
-- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+s1 = box.schema.space.create("test_space")
+_ = box.schema.user.create('test_user')
+_ = box.schema.func.create('test_func')
-- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+s1:drop()
+box.schema.user.drop('test_user')
+box.schema.func.drop('test_func')
-- failed alter
-- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
@@ -544,10 +547,6 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
-- failed drop
-- box.session.su("tester", s.drop, s)
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
box.schema.user.drop("test")
box.session.su("admin")
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index b81279c..d093c75 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -55,14 +55,14 @@ c = remote.connect(box.cfg.listen)
...
c:call("setuid_func")
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('guest')
---
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
@@ -85,7 +85,7 @@ session.su('guest')
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
@@ -122,7 +122,7 @@ session.su('guest')
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 3a56a4c..abc0be7 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -177,7 +177,7 @@ gs = box.schema.space.create('guest_space')
...
box.schema.func.create('guest_func')
---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
...
session.su('admin')
---
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index d6158dc..095a0e2 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -478,14 +478,14 @@ t = s:on_replace(function () box.schema.user.create('newu') end, t)
...
s:replace({3, 4})
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.role.create('newr') end, t)
---
...
s:replace({4, 5})
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () s:drop() end, t)
---
@@ -499,14 +499,14 @@ t = s:on_replace(function () box.schema.func.create('newf') end, t)
...
s:replace({6, 7})
---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.user.grant('guest', 'read,write', 'space', 'test_on_repl_ddl') end, t)
---
...
s:replace({7, 8})
---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () s:rename('newname') end, t)
---
diff --git a/test/box/role.result b/test/box/role.result
index 736ec85..806cea9 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -662,7 +662,7 @@ box.session.su('john')
-- error
box.schema.user.grant('grantee', 'role')
---
-- error: Read access to space '_user' is denied for user 'john'
+- error: User 'grantee' is not found
...
--
box.session.su('admin')
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 2a4b3b2..3ab5205 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -69,21 +69,21 @@ box.rollback();
...
box.begin() box.schema.func.create('test');
---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
...
box.begin() box.schema.user.create('test');
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
...
box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] [security 3/3] security: Refactor system space access checks
2018-03-28 8:09 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 1/3] box: Add system view for _sequence system space Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces Ilya Markov
@ 2018-03-28 8:09 ` Ilya Markov
2 siblings, 0 replies; 5+ messages in thread
From: Ilya Markov @ 2018-03-28 8:09 UTC (permalink / raw)
To: georgy; +Cc: tarantool-patches
From: imarkov <imarkov@tarantool.org>
Processes of creating, dropping objects, granting contain work with
system spaces which so far must be available only to admin or user who has
write access to universe.
This patch removes the need of special accesses to universe.
Introduce virtual access_check function in space object and
separate its implementation for user spaces and system ones.
Move checks on write to system spaces to on_replace triggers.
Follow-up #945
Closes #3250
---
src/box/alter.cc | 251 ++++++++++++++++++++++++++----------------
src/box/index.cc | 1 +
src/box/lua/schema.lua | 12 +-
src/box/schema.cc | 118 +++++++++++++++++++-
src/box/schema.h | 15 +++
src/box/space.c | 3 +-
src/box/space.h | 17 ++-
src/box/sysview_engine.c | 4 +
src/box/user.cc | 4 -
src/box/user.h | 7 ++
test/box/access.result | 27 ++---
test/box/access.test.lua | 10 +-
test/box/access_misc.result | 46 ++++++--
test/box/access_misc.test.lua | 11 +-
test/box/net.box.result | 6 +
test/box/net.box.test.lua | 2 +
test/box/role.result | 25 ++++-
test/box/role.test.lua | 13 ++-
test/box/sequence.result | 17 ++-
test/box/sequence.test.lua | 10 +-
test/engine/truncate.result | 2 +-
21 files changed, 434 insertions(+), 167 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 8455373..cc5578a 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -64,8 +64,7 @@
static void
access_check_ddl(const char *name, uint32_t owner_uid,
enum schema_object_type type,
- enum priv_type priv_type,
- bool is_17_compat_mode)
+ enum priv_type priv_type, uint32_t system_space_id)
{
struct credentials *cr = effective_user();
user_access_t has_access = cr->universal_access;
@@ -79,11 +78,15 @@ access_check_ddl(const char *name, uint32_t owner_uid,
* The legacy fix does not affect sequences since they
* were added in 1.7.7 only.
*/
- if (is_17_compat_mode && has_access & PRIV_R && has_access & PRIV_W)
- has_access |= PRIV_C | PRIV_A;
+ struct space *space = space_by_id(system_space_id);
+ if (space != NULL) {
+ has_access |= space->access[cr->auth_token].effective;
+ }
+
+ if (has_access & PRIV_W)
+ has_access |= PRIV_C | PRIV_A | PRIV_D;
- user_access_t access = ((PRIV_U | (user_access_t) priv_type) &
- ~has_access);
+ user_access_t access = ((user_access_t) priv_type & ~has_access);
bool is_owner = owner_uid == cr->uid || cr->uid == ADMIN;
/*
* Only the owner of the object or someone who has
@@ -91,23 +94,15 @@ access_check_ddl(const char *name, uint32_t owner_uid,
* DDL. If a user has no USAGE access and is owner,
* deny access as well.
*/
- if (access == 0 || (is_owner && !(access & PRIV_U)))
+ if (access == 0 || is_owner)
return; /* Access granted. */
struct user *user = user_find_xc(cr->uid);
- if (is_owner) {
- tnt_raise(AccessDeniedError,
- priv_name(PRIV_U),
- schema_object_name(SC_UNIVERSE),
- "",
- user->def->name);
- } else {
- tnt_raise(AccessDeniedError,
- priv_name(access),
- schema_object_name(type),
- name,
- user->def->name);
- }
+ tnt_raise(AccessDeniedError,
+ priv_name(access),
+ schema_object_name(type),
+ name,
+ user->def->name);
}
/**
@@ -1496,7 +1491,9 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct space_def *def =
space_def_new_from_tuple(new_tuple, ER_CREATE_SPACE,
region);
- access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_C, true);
+ /* When object is not created, we assume ADMIN owns it */
+ access_check_ddl(def->name, ADMIN, SC_SPACE, PRIV_C,
+ BOX_SPACE_ID);
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
RLIST_HEAD(empty_list);
@@ -1523,7 +1520,7 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
txn_on_rollback(txn, on_rollback);
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_space->def->name, old_space->def->uid,
- SC_SPACE, PRIV_D, true);
+ SC_SPACE, PRIV_D, BOX_SPACE_ID);
/* Verify that the space is empty (has no indexes) */
if (old_space->index_count) {
tnt_raise(ClientError, ER_DROP_SPACE,
@@ -1562,7 +1559,8 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
struct space_def *def =
space_def_new_from_tuple(new_tuple, ER_ALTER_SPACE,
region);
- access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_A, true);
+ access_check_ddl(def->name, def->uid, SC_SPACE, PRIV_A,
+ BOX_SPACE_ID);
auto def_guard =
make_scoped_guard([=] { space_def_delete(def); });
/*
@@ -1660,7 +1658,7 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
if (old_tuple && new_tuple)
priv_type = PRIV_A;
access_check_ddl(old_space->def->name, old_space->def->uid, SC_SPACE,
- priv_type, true);
+ priv_type, BOX_SPACE_ID);
struct index *old_index = space_index(old_space, iid);
/*
@@ -1894,10 +1892,16 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
space_name(old_space));
/*
- * Check if a write privilege was given, raise an error if not.
+ * Perform checks specific for space. Then perform usual check_ddl.
*/
- access_check_space_xc(old_space, PRIV_W);
-
+ credentials *cr = effective_user();
+ uint32_t has_access = cr->universal_access |
+ old_space->access[cr->auth_token].effective;
+ if (!(has_access & (PRIV_W | PRIV_D))) {
+ access_check_ddl(old_space->def->name, old_space->def->uid,
+ SC_SPACE,
+ PRIV_D, BOX_SPACE_ID);
+ }
/*
* Truncate counter is updated - truncate the space.
*/
@@ -2133,7 +2137,9 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
struct user *old_user = user_by_id(uid);
if (new_tuple != NULL && old_user == NULL) { /* INSERT */
struct user_def *user = user_def_new_from_tuple(new_tuple);
- access_check_ddl(user->name, user->owner, SC_USER, PRIV_C, true);
+ /* When object is not created, we assume ADMIN owns it */
+ access_check_ddl(user->name, ADMIN, SC_USER, PRIV_C,
+ BOX_USER_ID);
auto def_guard = make_scoped_guard([=] { free(user); });
(void) user_cache_replace(user);
def_guard.is_active = false;
@@ -2142,7 +2148,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
txn_on_rollback(txn, on_rollback);
} else if (new_tuple == NULL) { /* DELETE */
access_check_ddl(old_user->def->name, old_user->def->owner,
- SC_USER, PRIV_D, true);
+ SC_USER, PRIV_D, BOX_USER_ID);
/* Can't drop guest or super user */
if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) {
tnt_raise(ClientError, ER_DROP_USER,
@@ -2169,7 +2175,7 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
*/
struct user_def *user = user_def_new_from_tuple(new_tuple);
access_check_ddl(user->name, user->uid, SC_USER, PRIV_A,
- true);
+ BOX_USER_ID);
auto def_guard = make_scoped_guard([=] { free(user); });
struct trigger *on_commit =
txn_alter_trigger_new(user_cache_alter_user, NULL);
@@ -2271,7 +2277,9 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
struct func *old_func = func_by_id(fid);
if (new_tuple != NULL && old_func == NULL) { /* INSERT */
struct func_def *def = func_def_new_from_tuple(new_tuple);
- access_check_ddl(def->name, def->uid, SC_FUNCTION, PRIV_C, true);
+ /* When object is not created, we assume ADMIN owns it */
+ access_check_ddl(def->name, ADMIN, SC_FUNCTION, PRIV_C,
+ BOX_FUNC_ID);
auto def_guard = make_scoped_guard([=] { free(def); });
func_cache_replace(def);
def_guard.is_active = false;
@@ -2285,8 +2293,8 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
* Can only delete func if you're the one
* who created it or a superuser.
*/
- access_check_ddl(old_func->def->name, uid, SC_FUNCTION,
- PRIV_D, true);
+ access_check_ddl(old_func->def->name, uid, SC_FUNCTION, PRIV_D,
+ BOX_FUNC_ID);
/* Can only delete func if it has no grants. */
if (schema_find_grants("function", old_func->def->fid)) {
tnt_raise(ClientError, ER_DROP_FUNCTION,
@@ -2300,7 +2308,7 @@ on_replace_dd_func(struct trigger * /* trigger */, void *event)
struct func_def *def = func_def_new_from_tuple(new_tuple);
auto def_guard = make_scoped_guard([=] { free(def); });
access_check_ddl(def->name, def->uid, SC_FUNCTION, PRIV_A,
- true);
+ BOX_FUNC_ID);
struct trigger *on_commit =
txn_alter_trigger_new(func_cache_replace_func, NULL);
txn_on_commit(txn, on_commit);
@@ -2442,10 +2450,6 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
BOX_COLLATION_FIELD_ID);
old_coll = coll_by_id(old_id);
assert(old_coll != NULL);
- access_check_ddl(old_coll->name, old_coll->owner_id,
- SC_COLLATION,
- new_tuple == NULL ? PRIV_D: PRIV_A,
- false);
struct trigger *on_commit =
txn_alter_trigger_new(coll_cache_delete_coll, old_coll);
@@ -2465,8 +2469,6 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
struct coll_def new_def;
coll_def_new_from_tuple(new_tuple, &new_def);
- access_check_ddl(new_def.name, new_def.owner_id, SC_COLLATION,
- old_tuple == NULL ? PRIV_C : PRIV_A, false);
struct coll *new_coll = coll_new(&new_def);
if (new_coll == NULL)
diag_raise();
@@ -2509,6 +2511,35 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
priv->access = tuple_field_u32_xc(tuple, BOX_PRIV_FIELD_ACCESS);
}
+/**
+ * Handles case, when current user is not an owner of object and not ADMIN
+ * and wants to add or modify privileges.
+ *
+ * The strategy is following:
+ * When user has write, create, drop privileges in hierarchy over object,
+ * she is able to pass privileges.
+ * This case is required for creating, dropping users.
+ * TODO: we have to diffirentiate granting, revoking options. As so far,
+ * this is security breach, user who has create privilege may delete privileges
+ * of other users. Moreover, we have to differentiate promotion/demotion in altering
+ * privillege.
+ */
+static inline void
+priv_check_not_owner(struct user *grantor, struct priv_def *priv,
+ const char *name, enum priv_type priv_type)
+{
+ uint32_t access = schema_find_access(priv->object_type,
+ priv->object_id,
+ grantor);
+ if (priv->grantee_id == ADMIN || !(access & (PRIV_W | PRIV_C | PRIV_D))) {
+ tnt_raise(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(priv->object_type),
+ name,
+ grantor->def->name);
+ }
+}
+
/*
* This function checks that:
* - a privilege is granted from an existing user to an existing
@@ -2522,35 +2553,33 @@ priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple)
static void
priv_def_check(struct priv_def *priv, enum priv_type priv_type)
{
- struct user *grantor = user_find_xc(priv->grantor_id);
/* May be a role */
struct user *grantee = user_by_id(priv->grantee_id);
+ credentials *cr = effective_user();
+ struct user *grantor = user_find_xc(cr->uid);
+
if (grantee == NULL) {
tnt_raise(ClientError, ER_NO_SUCH_USER,
int2str(priv->grantee_id));
}
const char *name = schema_find_name(priv->object_type, priv->object_id);
- access_check_ddl(name, grantor->def->uid, priv->object_type, priv_type,
- false);
+
switch (priv->object_type) {
case SC_UNIVERSE:
+ {
if (grantor->def->uid != ADMIN) {
- tnt_raise(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_UNIVERSE),
- name,
- grantor->def->name);
+ priv_check_not_owner(grantor, priv, name, priv_type);
}
break;
+ }
case SC_SPACE:
{
- struct space *space = space_cache_find_xc(priv->object_id);
+ struct space *space = space_cache_find_xc(
+ priv->object_id);
if (space->def->uid != grantor->def->uid &&
grantor->def->uid != ADMIN) {
- tnt_raise(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_SPACE), name,
- grantor->def->name);
+ priv_check_not_owner(grantor, priv, name,
+ priv_type);
}
break;
}
@@ -2559,22 +2588,19 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
struct func *func = func_cache_find(priv->object_id);
if (func->def->uid != grantor->def->uid &&
grantor->def->uid != ADMIN) {
- tnt_raise(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_FUNCTION), name,
- grantor->def->name);
+ priv_check_not_owner(grantor, priv, name,
+ priv_type);
}
break;
}
case SC_SEQUENCE:
{
- struct sequence *seq = sequence_cache_find(priv->object_id);
+ struct sequence *seq = sequence_cache_find(
+ priv->object_id);
if (seq->def->uid != grantor->def->uid &&
grantor->def->uid != ADMIN) {
- tnt_raise(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_SEQUENCE), name,
- grantor->def->name);
+ priv_check_not_owner(grantor, priv, name,
+ priv_type);
}
break;
}
@@ -2587,16 +2613,23 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
int2str(priv->object_id));
}
/*
- * Only the creator of the role can grant or revoke it.
- * Everyone can grant 'PUBLIC' role.
+ * Only the creator or owner of grantee of the role
+ * or user with appropriate privileges can grant or revoke it.
+ * If role is 'PUBLIC' it is allowed to grant for anyone,
+ * Revoke 'PUBLIC' is allowed only to users who has drop access.
+ * The latter case is require for dropping users.
*/
if (role->def->owner != grantor->def->uid &&
- grantor->def->uid != ADMIN &&
- (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
- tnt_raise(AccessDeniedError,
- priv_name(priv_type),
- schema_object_name(SC_ROLE), name,
- grantor->def->name);
+ grantor->def->uid != grantee->def->owner &&
+ !((role->def->uid == PUBLIC) &&
+ (((cr->universal_access & PRIV_D)
+ || priv_type == PRIV_GRANT)
+ && priv->access == PRIV_X))) {
+ tnt_raise(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_ROLE),
+ name,
+ grantor->def->name);
}
/* Not necessary to do during revoke, but who cares. */
role_check(grantee, role);
@@ -2713,6 +2746,7 @@ on_replace_dd_priv(struct trigger * /* trigger */, void *event)
} else { /* modify */
priv_def_create_from_tuple(&priv, new_tuple);
priv_def_check(&priv, PRIV_GRANT);
+ /*TODO: check that modified by owner of object or ADMIN or grantor of privillege*/
struct trigger *on_commit =
txn_alter_trigger_new(modify_priv, NULL);
txn_on_commit(txn, on_commit);
@@ -2742,31 +2776,54 @@ on_replace_dd_schema(struct trigger * /* trigger */, void *event)
struct tuple *new_tuple = stmt->new_tuple;
const char *key = tuple_field_cstr_xc(new_tuple ? new_tuple : old_tuple,
BOX_SCHEMA_FIELD_KEY);
- if (strcmp(key, "cluster") == 0) {
- if (new_tuple == NULL)
- tnt_raise(ClientError, ER_REPLICASET_UUID_IS_RO);
- tt_uuid uu;
- tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID, &uu);
- REPLICASET_UUID = uu;
- } else if (strcmp(key, "version") == 0) {
- if (new_tuple != NULL) {
- uint32_t major, minor, patch;
- if (tuple_field_u32(new_tuple, 1, &major) != 0 ||
- tuple_field_u32(new_tuple, 2, &minor) != 0)
- tnt_raise(ClientError, ER_WRONG_DD_VERSION);
- /* Version can be major.minor with no patch. */
- if (tuple_field_u32(new_tuple, 3, &patch) != 0)
- patch = 0;
- dd_version_id = version_id(major, minor, patch);
- } else {
- assert(old_tuple != NULL);
- /*
- * _schema:delete({'version'}) for
- * example, for box.internal.bootstrap().
- */
- dd_version_id = tarantool_version_id();
+ credentials *cr = effective_user();
+ if (strncmp(key, "max_id", 6) == 0) {
+ if (!(cr->universal_access & (PRIV_W | PRIV_C)))
+ goto error;
+ } else {
+ uint32_t access = cr->universal_access;
+ struct space *schema_space = space_by_id(BOX_SCHEMA_ID);
+ access |= schema_space->access[cr->auth_token].effective;
+ if (~access & PRIV_W)
+ goto error;
+ else if (strcmp(key, "cluster") == 0) {
+ if (new_tuple == NULL)
+ tnt_raise(ClientError,
+ ER_REPLICASET_UUID_IS_RO);
+ tt_uuid uu;
+ tuple_field_uuid_xc(new_tuple, BOX_CLUSTER_FIELD_UUID,
+ &uu);
+ REPLICASET_UUID = uu;
+ } else if (strcmp(key, "version") == 0) {
+ if (new_tuple != NULL) {
+ uint32_t major, minor, patch;
+ if (tuple_field_u32(new_tuple, 1, &major) !=
+ 0 ||
+ tuple_field_u32(new_tuple, 2, &minor) != 0)
+ tnt_raise(ClientError,
+ ER_WRONG_DD_VERSION);
+ /* Version can be major.minor with no patch. */
+ if (tuple_field_u32(new_tuple, 3, &patch) != 0)
+ patch = 0;
+ dd_version_id = version_id(major, minor, patch);
+ } else {
+ assert(old_tuple != NULL);
+ /*
+ * _schema:delete({'version'}) for
+ * example, for box.internal.bootstrap().
+ */
+ dd_version_id = tarantool_version_id();
+ }
}
}
+ return;
+error:
+ struct user *user = user_find_xc(cr->uid);
+ tnt_raise(AccessDeniedError,
+ priv_name(PRIV_W),
+ schema_object_name(SC_SPACE),
+ "_schema",
+ user->def->name);
}
/**
@@ -3001,7 +3058,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
struct sequence *seq = sequence_by_id(id);
assert(seq != NULL);
access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
- PRIV_D, false);
+ PRIV_D, BOX_SEQUENCE_ID);
if (space_has_data(BOX_SEQUENCE_DATA_ID, 0, id))
tnt_raise(ClientError, ER_DROP_SEQUENCE,
seq->def->name, "the sequence has data");
@@ -3018,7 +3075,7 @@ on_replace_dd_sequence(struct trigger * /* trigger */, void *event)
struct sequence *seq = sequence_by_id(new_def->id);
assert(seq != NULL);
access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE,
- PRIV_A, false);
+ PRIV_A, BOX_SEQUENCE_ID);
alter->old_def = seq->def;
alter->new_def = new_def;
}
@@ -3098,10 +3155,10 @@ on_replace_dd_space_sequence(struct trigger * /* trigger */, void *event)
/* Check we have the correct access type on the sequence. * */
access_check_ddl(seq->def->name, seq->def->uid, SC_SEQUENCE, priv_type,
- false);
+ BOX_SEQUENCE_ID);
/** Check we have alter access on space. */
access_check_ddl(space->def->name, space->def->uid, SC_SPACE, PRIV_A,
- false);
+ BOX_SPACE_ID);
struct trigger *on_commit =
txn_alter_trigger_new(on_commit_dd_space_sequence, space);
diff --git a/src/box/index.cc b/src/box/index.cc
index 69fc761..a58e172 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -38,6 +38,7 @@
#include "txn.h"
#include "rmean.h"
#include "info.h"
+#include "xrow.h"
/* {{{ Utilities. **********************************************/
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 5496acb..0cacbb3 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1926,10 +1926,7 @@ box.schema.user.create = function(name, opts)
uid = _user:auto_increment{session.euid(), name, 'user', auth_mech_list}[1]
-- grant role 'public' to the user
box.schema.user.grant(uid, 'public')
- -- we have to grant global privileges from setuid function, since
- -- only admin has the ownership over universe and we don't have
- -- grant option
- box.session.su('admin', box.schema.user.grant, uid, 'session,usage', 'universe',
+ box.schema.user.grant(uid, 'session,usage', 'universe',
nil, {if_not_exists=true})
end
@@ -2047,12 +2044,9 @@ local function drop(uid, opts)
for k, tuple in pairs(sequences) do
box.schema.sequence.drop(tuple[1])
end
- -- xxx: hack, we have to revoke session and usage privileges
- -- of a user using a setuid function in absence of create/drop
- -- privileges and grant option
if box.space._vuser:get{uid}[4] == 'user' then
- box.session.su('admin', box.schema.user.revoke, uid,
- 'session,usage', 'universe', nil, {if_exists = true})
+ box.schema.user.revoke(uid, 'session,usage', 'universe',
+ nil, {if_exists = true})
end
local privs = _vpriv.index.primary:select{uid}
for k, tuple in pairs(privs) do
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 2e8533b..663d5e7 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -37,6 +37,9 @@
#include "scoped_guard.h"
#include "version.h"
#include "user.h"
+#include "session.h"
+#include "xrow.h"
+#include "memtx_tuple.h"
#include <stdio.h>
/**
* @module Data Dictionary
@@ -251,6 +254,15 @@ schema_find_id(uint32_t system_space_id, uint32_t index_id,
}
/**
+ * Access checks for system spaces, which can be modified only by admin
+ * _collation, _cluster
+ */
+static int
+sys_read_write_access_check(MAYBE_UNUSED struct space *space,
+ user_access_t access);
+
+
+/**
* Initialize a prototype for the two mandatory data
* dictionary spaces and create a cache entry for them.
* When restoring data from the snapshot these spaces
@@ -308,7 +320,9 @@ schema_init()
sc_space_new(BOX_SEQUENCE_DATA_ID, "_sequence_data", key_def,
&on_replace_sequence_data, NULL);
- /* _space_seq - association space <-> sequence. */
+ /* _space_seq - association space <-> sequence.
+ *
+ */
sc_space_new(BOX_SPACE_SEQUENCE_ID, "_space_sequence", key_def,
&on_replace_space_sequence, NULL);
@@ -520,6 +534,69 @@ sequence_cache_delete(uint32_t id)
}
}
+
+/**
+ * This function runs simple read, usage checks. Returns -1, 0 in
+ * case this checks are performed.
+ * All other checks are postponed. Returns 1 in thi
+ */
+static int
+sys_read_access_check(struct space *space, user_access_t access)
+{
+ credentials *cr = effective_user();
+ if (~cr->universal_access & PRIV_U) {
+ struct user *user = user_find(cr->uid);
+ if (user != NULL)
+ diag_set(AccessDeniedError,
+ priv_name(PRIV_U),
+ schema_object_name(SC_UNIVERSE),
+ "",
+ user->def->name);
+ return -1;
+ }
+ if (access == PRIV_R) {
+ uint32_t access = cr->universal_access |
+ space->access[cr->auth_token].effective;
+ if (~access & PRIV_R) {
+ struct user *user = user_find(cr->uid);
+ if (user != NULL)
+ diag_set(AccessDeniedError,
+ priv_name(PRIV_R),
+ schema_object_name(SC_SPACE),
+ space->def->name,
+ user->def->name);
+ return -1;
+ }
+ }
+ /* Other checks are postponed to trigger*/
+ return 0;
+}
+
+static int
+sys_read_write_access_check(struct space *space,
+ user_access_t access)
+{
+ credentials *cr = effective_user();
+ if (access == PRIV_W) {
+ uint32_t has_access = space->access[cr->auth_token].effective
+ | cr->universal_access;
+ if (~has_access & access) {
+ struct user *user = user_find(cr->uid);
+ if (user != NULL) {
+ diag_set(AccessDeniedError,
+ priv_name(access),
+ schema_object_name(SC_SPACE),
+ space->def->name,
+ user->def->name);
+ }
+ return -1;
+ }
+ return 0;
+ }
+ return sys_read_access_check(space, access);
+}
+
+
const char *
schema_find_name(enum schema_object_type type, uint32_t object_id)
{
@@ -562,3 +639,42 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
return "(nil)";
}
+uint32_t
+schema_find_access(enum schema_object_type type, uint32_t object_id,
+ struct user *grantor)
+{
+ struct priv_def def;
+ def.object_type = type;
+ def.object_id = object_id;
+ def.grantor_id = grantor->def->uid;
+ uint32_t access = universe.access[grantor->auth_token].effective;
+ struct access *obj_access = access_find(&def);
+ if (obj_access != NULL)
+ access |= obj_access->effective;
+ return access;
+}
+
+access_check_func_t
+get_access_check_func(uint32_t space_id)
+{
+ switch (space_id) {
+ case BOX_COLLATION_ID:
+ case BOX_CLUSTER_ID:
+ return sys_read_write_access_check;
+ case BOX_SCHEMA_ID:
+ case BOX_SPACE_ID:
+ case BOX_TRUNCATE_ID:
+ case BOX_INDEX_ID:
+ case BOX_FUNC_ID:
+ case BOX_USER_ID:
+ case BOX_SEQUENCE_ID:
+ case BOX_SEQUENCE_DATA_ID:
+ case BOX_SPACE_SEQUENCE_ID:
+ case BOX_PRIV_ID:
+ /* Specialized access checks will be performed
+ * in trigger. */
+ return sys_read_access_check;
+ default:
+ return access_check_user_space;
+ }
+}
diff --git a/src/box/schema.h b/src/box/schema.h
index 2b87f5f..63b81f4 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -36,6 +36,8 @@
#include "error.h"
#include "space.h"
#include "latch.h"
+#include "user.h"
+#include "iproto_constants.h"
#if defined(__cplusplus)
extern "C" {
@@ -103,6 +105,13 @@ schema_find_name(enum schema_object_type type, uint32_t object_id);
*/
struct sequence *
sequence_by_id(uint32_t id);
+
+uint32_t
+schema_find_access(enum schema_object_type type, uint32_t object_id,
+ struct user *grantor);
+
+int
+access_check_user_space(struct space *space, user_access_t access);
#if defined(__cplusplus)
} /* extern "C" */
@@ -205,6 +214,12 @@ sequence_cache_delete(uint32_t id);
#endif /* defined(__cplusplus) */
/**
+ * Utility function used to specify access_check function for system space.
+ */
+access_check_func_t
+get_access_check_func(uint32_t space_id);
+
+/**
* Triggers fired after committing a change in space definition.
* The space is passed to the trigger callback in the event
* argument. It is the new space in case of create/update or
diff --git a/src/box/space.c b/src/box/space.c
index cc6cbed..a7a0930 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -44,7 +44,7 @@
#include "iproto_constants.h"
int
-access_check_space(struct space *space, user_access_t access)
+access_check_user_space(struct space *space, user_access_t access)
{
struct credentials *cr = effective_user();
/* Any space access also requires global USAGE privilege. */
@@ -132,6 +132,7 @@ space_create(struct space *space, struct engine *engine,
rlist_create(&space->on_replace);
rlist_create(&space->on_stmt_begin);
space->run_triggers = true;
+ space->access_check = get_access_check_func(def->id);
space->format = format;
if (format != NULL)
diff --git a/src/box/space.h b/src/box/space.h
index 6408eed..5335b31 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -49,6 +49,8 @@ struct request;
struct port;
struct tuple;
+typedef int (*access_check_func_t)(struct space *space, user_access_t access);
+
struct space_vtab {
/** Free a space instance. */
void (*destroy)(struct space *);
@@ -139,6 +141,7 @@ struct space_vtab {
struct space {
/** Virtual function table. */
const struct space_vtab *vtab;
+ access_check_func_t access_check;
/** Cached runtime access information. */
struct access access[BOX_USER_MAX];
/** Engine used by this space. */
@@ -291,8 +294,11 @@ index_name_by_id(struct space *space, uint32_t id);
* Check whether or not the current user can be granted
* the requested access to the space.
*/
-int
-access_check_space(struct space *space, user_access_t access);
+static inline int
+access_check_space(struct space *space, user_access_t access)
+{
+ return space->access_check(space, access);
+}
static inline int
space_apply_initial_join_row(struct space *space, struct request *request)
@@ -424,6 +430,12 @@ space_swap_index(struct space *lhs, struct space *rhs,
void
space_fill_index_map(struct space *space);
+/*
+ * Utility function used to specify access_check function for system space.
+ */
+access_check_func_t
+get_access_check_func(uint32_t space_id);
+
#if defined(__cplusplus)
} /* extern "C" */
@@ -540,4 +552,5 @@ space_prepare_alter_xc(struct space *old_space, struct space *new_space)
#endif /* defined(__cplusplus) */
+
#endif /* TARANTOOL_BOX_SPACE_H_INCLUDED */
diff --git a/src/box/sysview_engine.c b/src/box/sysview_engine.c
index b478e78..0f126d2 100644
--- a/src/box/sysview_engine.c
+++ b/src/box/sysview_engine.c
@@ -32,6 +32,9 @@
#include "sysview_index.h"
#include "schema.h"
#include "space.h"
+#include "session.h"
+#include "xrow.h"
+#include "iproto_constants.h"
static void
sysview_space_destroy(struct space *space)
@@ -230,6 +233,7 @@ sysview_engine_create_space(struct engine *engine, struct space_def *def,
free(space);
return NULL;
}
+ space->access_check = access_check_user_space;
return space;
}
diff --git a/src/box/user.cc b/src/box/user.cc
index 7fa66da..3576efe 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -193,10 +193,6 @@ user_grant_priv(struct user *user, struct priv_def *def)
}
}
-/**
- * Find the corresponding access structure
- * given object type and object id.
- */
struct access *
access_find(struct priv_def *priv)
{
diff --git a/src/box/user.h b/src/box/user.h
index 07c4dc5..254f0c9 100644
--- a/src/box/user.h
+++ b/src/box/user.h
@@ -199,6 +199,13 @@ priv_grant(struct user *grantee, struct priv_def *priv);
void
priv_def_create_from_tuple(struct priv_def *priv, struct tuple *tuple);
+/**
+ * Find the corresponding access structure
+ * given object type and object id.
+ */
+struct access *
+access_find(struct priv_def *priv);
+
/* }}} */
#endif /* defined(__cplusplus) */
diff --git a/test/box/access.result b/test/box/access.result
index 2b28f07..53816ea 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -378,7 +378,6 @@ box.schema.user.create('grantee')
...
box.schema.user.grant('grantee', 'read, write, execute', 'universe')
---
-- error: Grant access to universe '' is denied for user 'grantor'
...
session.su('grantee')
---
@@ -898,7 +897,6 @@ session.su('test')
...
box.internal.collation.drop('test') -- fail
---
-- error: Drop access to collation 'test' is denied for user 'test'
...
session.su('admin')
---
@@ -1174,11 +1172,11 @@ box.schema.space.create('test')
...
box.schema.user.create('test')
---
-- error: Write access to space '_user' is denied for user 'guest'
+- error: Create access to user 'test' is denied for user 'guest'
...
box.schema.func.create('test')
---
-- error: Write access to space '_func' is denied for user 'guest'
+- error: Create access to function 'test' is denied for user 'guest'
...
box.session.su('admin')
---
@@ -1377,21 +1375,16 @@ box.schema.space.create("test_space")
...
box.schema.user.create('test_user')
---
-- error: Write access to space '_user' is denied for user 'tester'
+- error: Create access to user 'test_user' is denied for user 'tester'
...
box.schema.func.create('test_func')
---
-- error: Write access to space '_func' is denied for user 'tester'
+- error: Create access to function 'test_func' is denied for user 'tester'
...
box.session.su("admin")
---
...
---
--- FIXME 2.0: we still need to grant 'write' on universe
--- explicitly since we still use process_rw to write to system
--- tables from ddl
---
-box.schema.user.grant("tester", "create,write", "universe")
+box.schema.user.grant("tester", "create", "universe")
---
...
box.session.su("tester")
@@ -1428,12 +1421,16 @@ box.schema.user.drop("test")
---
- error: Revoke access to role 'public' is denied for user 'tester'
...
-box.session.su("admin")
+box.schema.func.drop("test")
---
+- error: Drop access to function 'test' is denied for user 'tester'
...
-box.session.su("tester", box.schema.func.drop, "test")
+s:drop()
+---
+- error: Drop access to space 'test' is denied for user 'tester'
+...
+box.session.su("admin")
---
-- error: Drop access to function 'test' is denied for user 'tester'
...
box.schema.user.grant("tester", "drop", "universe")
---
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 100dad5..6fbbaf6 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -520,12 +520,7 @@ box.schema.user.create('test_user')
box.schema.func.create('test_func')
box.session.su("admin")
---
--- FIXME 2.0: we still need to grant 'write' on universe
--- explicitly since we still use process_rw to write to system
--- tables from ddl
---
-box.schema.user.grant("tester", "create,write", "universe")
+box.schema.user.grant("tester", "create", "universe")
box.session.su("tester")
-- successful create
s1 = box.schema.space.create("test_space")
@@ -548,9 +543,10 @@ box.schema.func.drop('test_func')
-- box.session.su("tester", s.drop, s)
box.schema.user.drop("test")
+box.schema.func.drop("test")
+s:drop()
box.session.su("admin")
-box.session.su("tester", box.schema.func.drop, "test")
box.schema.user.grant("tester", "drop", "universe")
-- successful drop
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index abc0be7..2f94b63 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -61,6 +61,9 @@ box.schema.user.grant('testus', 'read', 'space', 'admin_space')
---
- error: User 'testus' already has read access on space 'admin_space'
...
+box.schema.user.grant('testus', 'read', 'universe')
+---
+...
session.su('testus')
---
...
@@ -78,7 +81,7 @@ s:delete(1)
...
s:drop()
---
-- error: Write access to space '_space_sequence' is denied for user 'testus'
+- error: Drop access to space 'admin_space' is denied for user 'testus'
...
--
-- Check double revoke
@@ -93,6 +96,9 @@ box.schema.user.revoke('testus', 'read', 'space', 'admin_space')
---
- error: User 'testus' does not have read access on space 'admin_space'
...
+box.schema.user.revoke('testus', 'read', 'universe')
+---
+...
session.su('testus')
---
...
@@ -124,9 +130,18 @@ s:insert({3})
---
- [3]
...
+session.su('admin')
+---
+...
+box.schema.user.grant('testus', 'read', 'universe')
+---
+...
+session.su('testus')
+---
+...
s:drop()
---
-- error: Write access to space '_space_sequence' is denied for user 'testus'
+- error: Drop access to space 'admin_space' is denied for user 'testus'
...
session.su('admin')
---
@@ -167,9 +182,18 @@ s:delete({3})
---
- error: Write access to space 'admin_space' is denied for user 'guest'
...
+session.su('admin')
+---
+...
+box.schema.user.grant('guest', 'read', 'universe')
+---
+...
+session.su('guest')
+---
+...
s:drop()
---
-- error: Write access to space '_space_sequence' is denied for user 'guest'
+- error: Drop access to space 'admin_space' is denied for user 'guest'
...
gs = box.schema.space.create('guest_space')
---
@@ -177,11 +201,14 @@ gs = box.schema.space.create('guest_space')
...
box.schema.func.create('guest_func')
---
-- error: Write access to space '_func' is denied for user 'guest'
+- error: Create access to function 'guest_func' is denied for user 'guest'
...
session.su('admin')
---
...
+box.schema.user.revoke('guest', 'read', 'universe')
+---
+...
s:select()
---
- - [2]
@@ -277,7 +304,7 @@ session.su('admin')
box.schema.user.create('someuser')
---
...
-box.schema.user.grant('someuser', 'read, write, execute', 'universe')
+box.schema.user.grant('someuser', 'read', 'universe')
---
...
session.su('someuser')
@@ -344,7 +371,7 @@ testuser_uid = session.uid()
...
_ = box.space._user:delete(2)
---
-- error: Drop access to user 'public' is denied for user 'testuser'
+- error: 'Failed to drop user or role ''public'': the user or the role is a system'
...
box.space._user:select(1)
---
@@ -381,7 +408,7 @@ session.su('testuser')
...
_ = box.space._user:delete(2)
---
-- error: Write access to space '_user' is denied for user 'testuser'
+- error: Drop access to user 'public' is denied for user 'testuser'
...
box.space._user:select(1)
---
@@ -389,7 +416,8 @@ box.space._user:select(1)
...
box.space._user:insert{uid, session.uid(), 'someone2', 'user'}
---
-- error: Write access to space '_user' is denied for user 'testuser'
+- error: Tuple field count 4 is less than required by space format or defined indexes
+ (expected at least 5)
...
session.su('admin')
---
@@ -409,7 +437,7 @@ box.space._index:select(272)
...
box.space._index:insert{512, 1,'owner','tree', 1, 1, 0,'unsigned'}
---
-- error: Write access to space '_index' is denied for user 'testuser'
+- error: 'Tuple field 5 type does not match one required by operation: expected map'
...
session.su('admin')
---
diff --git a/test/box/access_misc.test.lua b/test/box/access_misc.test.lua
index cf6447e..e78058a 100644
--- a/test/box/access_misc.test.lua
+++ b/test/box/access_misc.test.lua
@@ -27,6 +27,7 @@ s:insert({2})
--
box.schema.user.grant('testus', 'read', 'space', 'admin_space')
box.schema.user.grant('testus', 'read', 'space', 'admin_space')
+box.schema.user.grant('testus', 'read', 'universe')
session.su('testus')
s:select(1)
@@ -39,6 +40,7 @@ s:drop()
session.su('admin')
box.schema.user.revoke('testus', 'read', 'space', 'admin_space')
box.schema.user.revoke('testus', 'read', 'space', 'admin_space')
+box.schema.user.revoke('testus', 'read', 'universe')
session.su('testus')
s:select(1)
@@ -52,6 +54,9 @@ session.su('testus')
s:select(1)
s:delete(1)
s:insert({3})
+session.su('admin')
+box.schema.user.grant('testus', 'read', 'universe')
+session.su('testus')
s:drop()
session.su('admin')
--
@@ -68,11 +73,15 @@ box.space._user:select(1)
s:select(1)
s:insert({4})
s:delete({3})
+session.su('admin')
+box.schema.user.grant('guest', 'read', 'universe')
+session.su('guest')
s:drop()
gs = box.schema.space.create('guest_space')
box.schema.func.create('guest_func')
session.su('admin')
+box.schema.user.revoke('guest', 'read', 'universe')
s:select()
--
-- Create user with universe read&write grants
@@ -116,7 +125,7 @@ box.schema.func.create('uniuser_func')
session.su('admin')
box.schema.user.create('someuser')
-box.schema.user.grant('someuser', 'read, write, execute', 'universe')
+box.schema.user.grant('someuser', 'read', 'universe')
session.su('someuser')
--
-- Check drop objects of another user
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 46d85b3..1b62584 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -2140,6 +2140,9 @@ box.schema.user.grant('guest', 'write', 'space', '_space')
box.schema.user.grant('guest', 'write', 'space', '_schema')
---
...
+box.schema.user.grant('guest', 'create', 'universe')
+---
+...
count = 0
---
...
@@ -2188,6 +2191,9 @@ box.schema.user.revoke('guest', 'write', 'space', '_space')
box.schema.user.revoke('guest', 'write', 'space', '_schema')
---
...
+box.schema.user.revoke('guest', 'create', 'universe')
+---
+...
c:close()
---
...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 87e26f8..f9272f2 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -877,6 +877,7 @@ box.session.on_disconnect(nil, on_disconnect)
--
box.schema.user.grant('guest', 'write', 'space', '_space')
box.schema.user.grant('guest', 'write', 'space', '_schema')
+box.schema.user.grant('guest', 'create', 'universe')
count = 0
function create_space(name) count = count + 1 box.schema.create_space(name) return true end
c = net.connect(box.cfg.listen)
@@ -891,6 +892,7 @@ box.space.test2:drop()
box.space.test3:drop()
box.schema.user.revoke('guest', 'write', 'space', '_space')
box.schema.user.revoke('guest', 'write', 'space', '_schema')
+box.schema.user.revoke('guest', 'create', 'universe')
c:close()
--
diff --git a/test/box/role.result b/test/box/role.result
index 806cea9..b8055ff 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -671,7 +671,7 @@ box.session.su('admin')
_ = box.schema.space.create('test')
---
...
-box.schema.user.grant('john', 'read,write,execute', 'universe')
+box.schema.user.grant('john', 'read', 'universe')
---
...
box.session.su('john')
@@ -695,14 +695,33 @@ box.schema.user.grant('grantee', 'public')
- error: User 'grantee' already has role 'public'
...
--
--- revoking role 'public' is another deal - only the
--- superuser can do that, and even that would be useless,
+-- revoking role 'public' is another deal - the
+-- superuser or creator of user can do that, and even that would be useless,
-- since one can still re-grant it back to oneself.
--
box.schema.user.revoke('grantee', 'public')
---
- error: Revoke access to role 'public' is denied for user 'john'
...
+box.session.su("admin")
+---
+...
+box.schema.user.grant("john", "create", 'universe')
+---
+...
+box.session.su('john')
+---
+...
+box.schema.user.create("grantee2")
+---
+...
+-- must be ok
+box.schema.user.revoke('grantee2', 'public')
+---
+...
+box.schema.user.drop('grantee2')
+---
+...
box.session.su('admin')
---
...
diff --git a/test/box/role.test.lua b/test/box/role.test.lua
index e97339f..a1d6808 100644
--- a/test/box/role.test.lua
+++ b/test/box/role.test.lua
@@ -261,7 +261,7 @@ box.schema.user.grant('grantee', 'role')
--
box.session.su('admin')
_ = box.schema.space.create('test')
-box.schema.user.grant('john', 'read,write,execute', 'universe')
+box.schema.user.grant('john', 'read', 'universe')
box.session.su('john')
box.schema.user.grant('grantee', 'role')
box.schema.user.grant('grantee', 'read', 'space', 'test')
@@ -272,11 +272,18 @@ box.schema.user.grant('grantee', 'read', 'space', 'test')
--
box.schema.user.grant('grantee', 'public')
--
--- revoking role 'public' is another deal - only the
--- superuser can do that, and even that would be useless,
+-- revoking role 'public' is another deal - the
+-- superuser or creator of user can do that, and even that would be useless,
-- since one can still re-grant it back to oneself.
--
box.schema.user.revoke('grantee', 'public')
+box.session.su("admin")
+box.schema.user.grant("john", "create", 'universe')
+box.session.su('john')
+box.schema.user.create("grantee2")
+-- must be ok
+box.schema.user.revoke('grantee2', 'public')
+box.schema.user.drop('grantee2')
box.session.su('admin')
box.schema.user.drop('john')
diff --git a/test/box/sequence.result b/test/box/sequence.result
index a509207..631a8dd 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -1447,7 +1447,7 @@ box.session.su('admin')
---
...
-- A user cannot alter sequences created by other users.
-box.schema.user.grant('user', 'read,write', 'universe')
+box.schema.user.grant('user', 'read', 'universe')
---
...
box.session.su('user')
@@ -1518,7 +1518,7 @@ s1 = box.schema.space.create('space1')
_ = s1:create_index('pk')
---
...
-box.schema.user.grant('user', 'read,write', 'universe')
+box.schema.user.grant('user', 'read, create', 'universe')
---
...
box.session.su('user')
@@ -1532,15 +1532,14 @@ s2 = box.schema.space.create('space2')
...
_ = s2:create_index('pk', {sequence = 'seq1'}) -- error
---
-- error: Create access to sequence 'seq1' is denied for user 'user'
...
s1.index.pk:alter({sequence = 'seq1'}) -- error
---
-- error: Create access to sequence 'seq1' is denied for user 'user'
+- error: Alter access to space 'space1' is denied for user 'user'
...
box.space._space_sequence:replace{s1.id, sq1.id, false} -- error
---
-- error: Create access to sequence 'seq1' is denied for user 'user'
+- error: Alter access to space 'space1' is denied for user 'user'
...
box.space._space_sequence:replace{s1.id, sq2.id, false} -- error
---
@@ -1548,7 +1547,7 @@ box.space._space_sequence:replace{s1.id, sq2.id, false} -- error
...
box.space._space_sequence:replace{s2.id, sq1.id, false} -- error
---
-- error: Create access to sequence 'seq1' is denied for user 'user'
+- error: Alter access to sequence 'seq1' is denied for user 'user'
...
s2.index.pk:alter({sequence = 'seq2'}) -- ok
---
@@ -1559,7 +1558,7 @@ box.session.su('admin')
-- If the user owns a sequence attached to a space,
-- it can use it for auto increment, otherwise it
-- needs privileges.
-box.schema.user.revoke('user', 'read,write', 'universe')
+box.schema.user.revoke('user', 'read,create', 'universe')
---
...
box.session.su('user')
@@ -1697,10 +1696,10 @@ box.schema.user.create('user1')
box.schema.user.create('user2')
---
...
-box.schema.user.grant('user1', 'read,write', 'universe')
+box.schema.user.grant('user1', 'read, create', 'universe')
---
...
-box.schema.user.grant('user2', 'read,write', 'universe')
+box.schema.user.grant('user2', 'read', 'universe')
---
...
box.session.su('user1')
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index 26147bb..f6d4c29 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -481,7 +481,7 @@ sq:reset() -- error
box.session.su('admin')
-- A user cannot alter sequences created by other users.
-box.schema.user.grant('user', 'read,write', 'universe')
+box.schema.user.grant('user', 'read', 'universe')
box.session.su('user')
sq:alter{step = 2} -- error
sq:drop() -- error
@@ -507,7 +507,7 @@ sq:drop()
sq1 = box.schema.sequence.create('seq1')
s1 = box.schema.space.create('space1')
_ = s1:create_index('pk')
-box.schema.user.grant('user', 'read,write', 'universe')
+box.schema.user.grant('user', 'read, create', 'universe')
box.session.su('user')
sq2 = box.schema.sequence.create('seq2')
s2 = box.schema.space.create('space2')
@@ -522,7 +522,7 @@ box.session.su('admin')
-- If the user owns a sequence attached to a space,
-- it can use it for auto increment, otherwise it
-- needs privileges.
-box.schema.user.revoke('user', 'read,write', 'universe')
+box.schema.user.revoke('user', 'read,create', 'universe')
box.session.su('user')
s2:insert{nil, 1} -- ok: {1, 1}
box.session.su('admin')
@@ -570,8 +570,8 @@ box.sequence
-- to a sequence.
box.schema.user.create('user1')
box.schema.user.create('user2')
-box.schema.user.grant('user1', 'read,write', 'universe')
-box.schema.user.grant('user2', 'read,write', 'universe')
+box.schema.user.grant('user1', 'read, create', 'universe')
+box.schema.user.grant('user2', 'read', 'universe')
box.session.su('user1')
sq = box.schema.sequence.create('test')
box.session.su('user2')
diff --git a/test/engine/truncate.result b/test/engine/truncate.result
index b6e1a99..1547b8c 100644
--- a/test/engine/truncate.result
+++ b/test/engine/truncate.result
@@ -506,7 +506,7 @@ con = require('net.box').connect(box.cfg.listen)
...
con:eval([[box.space.access_truncate:truncate()]])
---
-- error: Write access to space 'access_truncate' is denied for user 'guest'
+- error: Drop access to space 'access_truncate' is denied for user 'guest'
...
con.space.access_truncate:select()
---
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
* [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces
2018-03-29 7:36 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
@ 2018-03-29 7:37 ` Ilya Markov
0 siblings, 0 replies; 5+ messages in thread
From: Ilya Markov @ 2018-03-29 7:37 UTC (permalink / raw)
To: georgy; +Cc: tarantool-patches
Replace reads from systems spaces with reads from corresponding
system views.
After this patch some error messages are changed:
* Accessing to objects that are not accessible for current user
raises the error claiming these objects don't exists.
* Attempt to add in transaction such methods as object create, drop
raises an multi-engine transaction error instead of multi-statement
transaction error.
In scope of #3250
---
src/box/lua/schema.lua | 111 +++++++++++++++++++++++----------------
src/box/sysview_index.c | 68 +++++++++++++-----------
test/box/access.result | 75 +++++++++++++++++---------
test/box/access.test.lua | 40 +++++++-------
test/box/access_bin.result | 8 +--
test/box/access_misc.result | 2 +-
test/box/access_sysview.result | 11 ++--
test/box/access_sysview.test.lua | 6 +--
test/box/on_replace.result | 12 ++---
test/box/role.result | 2 +-
test/box/transaction.result | 8 +--
test/engine/iterator.result | 2 +-
test/engine/savepoint.result | 12 ++---
13 files changed, 210 insertions(+), 147 deletions(-)
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 30c6bc6..1d245e3 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -102,12 +102,12 @@ ffi.cdef[[
]]
local function user_or_role_resolve(user)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(user) == 'string' then
- tuple = _user.index.name:get{user}
+ tuple = _vuser.index.name:get{user}
else
- tuple = _user:get{user}
+ tuple = _vuser:get{user}
end
if tuple == nil then
return nil
@@ -116,12 +116,12 @@ local function user_or_role_resolve(user)
end
local function role_resolve(name_or_id)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _user.index.name:get{name_or_id}
+ tuple = _vuser.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _user:get{name_or_id}
+ tuple = _vuser:get{name_or_id}
end
if tuple == nil or tuple[4] ~= 'role' then
return nil
@@ -131,12 +131,12 @@ local function role_resolve(name_or_id)
end
local function user_resolve(name_or_id)
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _user.index.name:get{name_or_id}
+ tuple = _vuser.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _user:get{name_or_id}
+ tuple = _vuser:get{name_or_id}
end
if tuple == nil or tuple[4] ~= 'user' then
return nil
@@ -146,12 +146,12 @@ local function user_resolve(name_or_id)
end
local function sequence_resolve(name_or_id)
- local _sequence = box.space[box.schema.SEQUENCE_ID]
+ local _vsequence = box.space[box.schema.VSEQUENCE_ID]
local tuple
if type(name_or_id) == 'string' then
- tuple = _sequence.index.name:get{name_or_id}
+ tuple = _vsequence.index.name:get{name_or_id}
elseif type(name_or_id) ~= 'nil' then
- tuple = _sequence:get{name_or_id}
+ tuple = _vsequence:get{name_or_id}
end
if tuple ~= nil then
return tuple[1], tuple
@@ -162,8 +162,9 @@ end
-- Revoke all privileges associated with the given object.
local function revoke_object_privs(object_type, object_id)
+ local _vpriv = box.space[box.schema.VPRIV_ID]
local _priv = box.space[box.schema.PRIV_ID]
- local privs = _priv.index.object:select{object_type, object_id}
+ local privs = _vpriv.index.object:select{object_type, object_id}
for k, tuple in pairs(privs) do
local uid = tuple[2]
_priv:delete{uid, object_type, object_id}
@@ -429,10 +430,15 @@ end
-- space format - the metadata about space fields
function box.schema.space.format(id, format)
local _space = box.space._space
+ local _vspace = box.space._vspace
check_param(id, 'id', 'number')
if format == nil then
- return _space:get(id)[7]
+ local tuple = _vspace:get(id)
+ if tuple == nil then
+ box.error(box.error.NO_SUCH_SPACE, '#' .. tostring(id))
+ end
+ return tuple[7]
else
check_param(format, 'format', 'table')
format = update_format(format)
@@ -448,6 +454,7 @@ box.schema.space.drop = function(space_id, space_name, opts)
check_param_table(opts, { if_exists = 'boolean' })
local _space = box.space[box.schema.SPACE_ID]
local _index = box.space[box.schema.INDEX_ID]
+ local _vindex = box.space[box.schema.VINDEX_ID]
local _truncate = box.space[box.schema.TRUNCATE_ID]
local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
local sequence_tuple = _space_sequence:delete{space_id}
@@ -455,7 +462,7 @@ box.schema.space.drop = function(space_id, space_name, opts)
-- Delete automatically generated sequence.
box.schema.sequence.drop(sequence_tuple[2])
end
- local keys = _index:select(space_id)
+ local keys = _vindex:select(space_id)
for i = #keys, 1, -1 do
local v = keys[i]
_index:delete{v[1], v[2]}
@@ -693,7 +700,8 @@ box.schema.index.create = function(space_id, name, options)
options = update_param_table(options, options_defaults)
local _index = box.space[box.schema.INDEX_ID]
- if _index.index.name:get{space_id, name} then
+ local _vindex = box.space[box.schema.VINDEX_ID]
+ if _vindex.index.name:get{space_id, name} then
if options.if_not_exists then
return space.index[name], "not created"
else
@@ -706,7 +714,7 @@ box.schema.index.create = function(space_id, name, options)
iid = options.id
else
-- max
- local tuple = _index.index[0]
+ local tuple = _vindex.index[0]
:select(space_id, { limit = 1, iterator = 'LE' })[1]
if tuple then
local id = tuple[1]
@@ -1368,7 +1376,19 @@ function box.schema.space.bless(space)
-- primary key and returns it back to the user
space_mt.auto_increment = function(space, tuple)
check_space_arg(space, 'auto_increment')
- local max_tuple = check_primary_index(space):max()
+ local euid = box.session.euid()
+ -- HACK: habe to call box.session.su here,
+ -- as checking max requires 'READ' access to space, though auto_increment
+ -- is a 'WRITE' operation
+ local max_tuple
+ if euid ~= 1 then
+ box.session.su("admin")
+ max_tuple = check_primary_index(space):max()
+ box.session.su(euid)
+ else
+ max_tuple = check_primary_index(space):max()
+ end
+
local max = 0
if max_tuple ~= nil then
max = max_tuple[1]
@@ -1686,12 +1706,12 @@ local function object_resolve(object_type, object_name)
return space.id
end
if object_type == 'function' then
- local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local func
if type(object_name) == 'string' then
- func = _func.index.name:get{object_name}
+ func = _vfunc.index.name:get{object_name}
else
- func = _func:get{object_name}
+ func = _vfunc:get{object_name}
end
if func then
return func[1]
@@ -1707,12 +1727,12 @@ local function object_resolve(object_type, object_name)
return seq
end
if object_type == 'role' then
- local _user = box.space[box.schema.USER_ID]
+ local _vuser = box.space[box.schema.VUSER_ID]
local role
if type(object_name) == 'string' then
- role = _user.index.name:get{object_name}
+ role = _vuser.index.name:get{object_name}
else
- role = _user:get{object_name}
+ role = _vuser:get{object_name}
end
if role and role[4] == 'role' then
return role[1]
@@ -1730,13 +1750,13 @@ local function object_name(object_type, object_id)
end
local space
if object_type == 'space' then
- space = box.space._space
+ space = box.space._vspace
elseif object_type == 'sequence' then
space = box.space._sequence
elseif object_type == 'function' then
- space = box.space._func
+ space = box.space._vfunc
elseif object_type == 'role' or object_type == 'user' then
- space = box.space._user
+ space = box.space._vuser
else
box.error(box.error.UNKNOWN_SCHEMA_OBJECT, object_type)
end
@@ -1750,7 +1770,8 @@ box.schema.func.create = function(name, opts)
if_not_exists = 'boolean',
language = 'string'})
local _func = box.space[box.schema.FUNC_ID]
- local func = _func.index.name:get{name}
+ local _vfunc = box.space[box.schema.VFUNC_ID]
+ local func = _vfunc.index.name:get{name}
if func then
if not opts.if_not_exists then
box.error(box.error.FUNCTION_EXISTS, name)
@@ -1767,12 +1788,13 @@ box.schema.func.drop = function(name, opts)
opts = opts or {}
check_param_table(opts, { if_exists = 'boolean' })
local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local fid
local tuple
if type(name) == 'string' then
- tuple = _func.index.name:get{name}
+ tuple = _vfunc.index.name:get{name}
else
- tuple = _func:get{name}
+ tuple = _vfunc:get{name}
end
if tuple then
fid = tuple[1]
@@ -1788,12 +1810,12 @@ box.schema.func.drop = function(name, opts)
end
function box.schema.func.exists(name_or_id)
- local _func = box.space[box.schema.FUNC_ID]
+ local _vfunc = box.space[box.schema.VFUNC_ID]
local tuple = nil
if type(name_or_id) == 'string' then
- tuple = _func.index.name:get{name_or_id}
+ tuple = _vfunc.index.name:get{name_or_id}
elseif type(name_or_id) == 'number' then
- tuple = _func:get{name_or_id}
+ tuple = _vfunc:get{name_or_id}
end
return tuple ~= nil
end
@@ -1948,8 +1970,9 @@ local function grant(uid, name, privilege, object_type,
options.grantor = user_or_role_resolve(options.grantor)
end
local _priv = box.space[box.schema.PRIV_ID]
+ local _vpriv = box.space[box.schema.VPRIV_ID]
-- add the granted privilege to the current set
- local tuple = _priv:get{uid, object_type, oid}
+ local tuple = _vpriv:get{uid, object_type, oid}
local old_privilege
if tuple ~= nil then
old_privilege = tuple[5]
@@ -1984,7 +2007,8 @@ local function revoke(uid, name, privilege, object_type, object_name, options)
options = options or {}
local oid = object_resolve(object_type, object_name)
local _priv = box.space[box.schema.PRIV_ID]
- local tuple = _priv:get{uid, object_type, oid}
+ local _vpriv = box.space[box.schema.VPRIV_ID]
+ local tuple = _vpriv:get{uid, object_type, oid}
-- system privileges of admin and guest can't be revoked
if tuple == nil then
if options.if_exists then
@@ -2013,32 +2037,32 @@ end
local function drop(uid, opts)
-- recursive delete of user data
- local _priv = box.space[box.schema.PRIV_ID]
- local spaces = box.space[box.schema.SPACE_ID].index.owner:select{uid}
+ local _vpriv = box.space[box.schema.VPRIV_ID]
+ local spaces = box.space[box.schema.VSPACE_ID].index.owner:select{uid}
for k, tuple in pairs(spaces) do
box.space[tuple[1]]:drop()
end
- local funcs = box.space[box.schema.FUNC_ID].index.owner:select{uid}
+ local funcs = box.space[box.schema.VFUNC_ID].index.owner:select{uid}
for k, tuple in pairs(funcs) do
box.schema.func.drop(tuple[1])
end
-- if this is a role, revoke this role from whoever it was granted to
- local grants = _priv.index.object:select{'role', uid}
+ local grants = _vpriv.index.object:select{'role', uid}
for k, tuple in pairs(grants) do
revoke(tuple[2], tuple[2], uid)
end
- local sequences = box.space[box.schema.SEQUENCE_ID].index.owner:select{uid}
+ local sequences = box.space[box.schema.VSEQUENCE_ID].index.owner:select{uid}
for k, tuple in pairs(sequences) do
box.schema.sequence.drop(tuple[1])
end
-- xxx: hack, we have to revoke session and usage privileges
-- of a user using a setuid function in absence of create/drop
-- privileges and grant option
- if box.space._user:get{uid}[4] == 'user' then
+ if box.space._vuser:get{uid}[4] == 'user' then
box.session.su('admin', box.schema.user.revoke, uid,
'session,usage', 'universe', nil, {if_exists = true})
end
- local privs = _priv.index.primary:select{uid}
+ local privs = _vpriv.index.primary:select{uid}
for k, tuple in pairs(privs) do
revoke(uid, uid, tuple[5], tuple[3], tuple[4])
end
@@ -2095,8 +2119,7 @@ box.schema.user.drop = function(name, opts)
end
local function info(id)
- local _priv = box.space._priv
- local _user = box.space._priv
+ local _priv = box.space._vpriv
local privs = {}
for _, v in pairs(_priv:select{id}) do
table.insert(
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index bf0442b..c18bad3 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -182,23 +182,24 @@ static const struct index_vtab sysview_index_vtab = {
/* .end_build = */ generic_index_end_build,
};
+const uint32_t PRIV_WRDA = PRIV_W | PRIV_D | PRIV_A | PRIV_R;
+
static bool
vspace_filter(struct space *source, struct tuple *tuple)
{
struct credentials *cr = effective_user();
- if (PRIV_R & cr->universal_access)
- return true; /* read access to unverse */
- if (PRIV_R & source->access[cr->auth_token].effective)
- return true; /* read access to original space */
-
+ if (PRIV_WRDA & cr->universal_access)
+ return true;
+ if (source->access[cr->auth_token].effective & PRIV_R)
+ return true;
uint32_t space_id;
if (tuple_field_u32(tuple, BOX_SPACE_FIELD_ID, &space_id) != 0)
return false;
struct space *space = space_cache_find(space_id);
if (space == NULL)
return false;
- uint8_t effective = space->access[cr->auth_token].effective;
- return ((PRIV_R | PRIV_W) & (cr->universal_access | effective) ||
+ uint32_t effective = space->access[cr->auth_token].effective;
+ return ((PRIV_R | PRIV_W | PRIV_D | PRIV_A) & effective ||
space->def->uid == cr->uid);
}
@@ -206,10 +207,11 @@ static bool
vuser_filter(struct space *source, struct tuple *tuple)
{
struct credentials *cr = effective_user();
- if (PRIV_R & cr->universal_access)
- return true; /* read access to unverse */
- if (PRIV_R & source->access[cr->auth_token].effective)
- return true; /* read access to original space */
+ /* If user has global alter, drop privilege she may access all users */
+ if (PRIV_WRDA & cr->universal_access)
+ return true;
+ if (source->access[cr->auth_token].effective & PRIV_R)
+ return true;
uint32_t uid;
if (tuple_field_u32(tuple, BOX_USER_FIELD_ID, &uid) != 0)
@@ -217,17 +219,20 @@ vuser_filter(struct space *source, struct tuple *tuple)
uint32_t owner_id;
if (tuple_field_u32(tuple, BOX_USER_FIELD_UID, &owner_id) != 0)
return false;
- return uid == cr->uid || owner_id == cr->uid;
+ return uid == cr->uid || owner_id == cr->uid || uid == PUBLIC;
}
static bool
vpriv_filter(struct space *source, struct tuple *tuple)
{
struct credentials *cr = effective_user();
- if (PRIV_R & cr->universal_access)
- return true; /* read access to unverse */
- if (PRIV_R & source->access[cr->auth_token].effective)
- return true; /* read access to original space */
+ /* If user has global alter, drop privilege
+ * she may access all privileges
+ */
+ if (PRIV_WRDA & cr->universal_access)
+ return true;
+ if (source->access[cr->auth_token].effective & PRIV_R)
+ return true;
uint32_t grantor_id;
if (tuple_field_u32(tuple, BOX_PRIV_FIELD_ID, &grantor_id) != 0)
@@ -235,15 +240,21 @@ vpriv_filter(struct space *source, struct tuple *tuple)
uint32_t grantee_id;
if (tuple_field_u32(tuple, BOX_PRIV_FIELD_UID, &grantee_id) != 0)
return false;
- return grantor_id == cr->uid || grantee_id == cr->uid;
+ const char *type;
+ uint32_t obj_id;
+ if ((type = tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE)) == NULL ||
+ tuple_field_u32(tuple, BOX_PRIV_FIELD_OBJECT_ID, &obj_id) != 0)
+ return false;
+ return grantor_id == cr->uid || grantee_id == cr->uid ||
+ (strncmp(type, "role", 4) == 0 && obj_id == PUBLIC);
}
static bool
vfunc_filter(struct space *source, struct tuple *tuple)
{
struct credentials *cr = effective_user();
- if ((PRIV_R | PRIV_X) & cr->universal_access)
- return true; /* read or execute access to unverse */
+ if ((PRIV_WRDA | PRIV_X) & cr->universal_access)
+ return true;
if (PRIV_R & source->access[cr->auth_token].effective)
return true; /* read access to original space */
@@ -253,18 +264,17 @@ vfunc_filter(struct space *source, struct tuple *tuple)
uint32_t name_len = strlen(name);
struct func *func = func_by_name(name, name_len);
assert(func != NULL);
- uint8_t effective = func->access[cr->auth_token].effective;
- if (func->def->uid == cr->uid || (PRIV_X & effective))
- return true;
- return false;
+ uint32_t effective = func->access[cr->auth_token].effective;
+ return func->def->uid == cr->uid ||
+ ((PRIV_WRDA | PRIV_X) & effective);
}
static bool
vsequence_filter(struct space *source, struct tuple *tuple)
{
struct credentials *cr = effective_user();
- if ((PRIV_R | PRIV_X) & cr->universal_access)
- return true; /* read or execute access to unverse */
+ if (PRIV_WRDA & cr->universal_access)
+ return true;
if (PRIV_R & source->access[cr->auth_token].effective)
return true; /* read access to original space */
@@ -274,13 +284,11 @@ vsequence_filter(struct space *source, struct tuple *tuple)
struct sequence *sequence = sequence_by_id(id);
if (sequence == NULL)
return false;
- uint8_t effective = sequence->access[cr->auth_token].effective;
- if (sequence->def->uid == cr->uid || ((PRIV_W | PRIV_R) & effective))
- return true;
- return false;
+ uint32_t effective = sequence->access[cr->auth_token].effective;
+ return sequence->def->uid == cr->uid ||
+ (PRIV_WRDA & effective);
}
-
struct sysview_index *
sysview_index_new(struct sysview_engine *sysview,
struct index_def *def, const char *space_name)
diff --git a/test/box/access.result b/test/box/access.result
index 191857f..4354b05 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -386,7 +386,8 @@ session.su('grantee')
-- fails - can't suicide - ask the creator to kill you
box.schema.user.drop('grantee')
---
-- error: Read access to space '_user' is denied for user 'grantee'
+- error: 'Failed to drop user or role ''grantee'': the user is active in the current
+ session'
...
session.su('grantor')
---
@@ -471,7 +472,7 @@ session.su('user1')
-- permission denied
box.schema.user.passwd('admin', 'xxx')
---
-- error: Read access to space '_user' is denied for user 'user1'
+- error: User 'admin' is not found
...
session.su('admin')
---
@@ -1048,7 +1049,7 @@ session.su("test1")
...
box.schema.user.disable("test")
---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
...
session.su("admin")
---
@@ -1080,7 +1081,7 @@ session.su("test1")
...
box.schema.user.grant("test", "usage", "universe")
---
-- error: Read access to space '_user' is denied for user 'test1'
+- error: User 'test' is not found
...
session.su('admin')
---
@@ -1173,11 +1174,11 @@ box.schema.space.create('test')
...
box.schema.user.create('test')
---
-- error: Read access to space '_user' is denied for user 'guest'
+- error: Write access to space '_user' is denied for user 'guest'
...
box.schema.func.create('test')
---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
...
box.session.su('admin')
---
@@ -1357,28 +1358,37 @@ box.schema.user.create("tester")
s = box.schema.space.create("test")
---
...
+_ = s:create_index("primary")
+---
+...
+seq = box.schema.sequence.create("test")
+---
+...
u = box.schema.user.create("test")
---
...
f = box.schema.func.create("test")
---
...
-box.schema.user.grant("tester", "read,execute", "universe")
+-- failed create
+box.session.su("tester")
---
...
--- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
+box.schema.space.create("test_space")
---
- error: Write access to space '_schema' is denied for user 'tester'
...
-box.session.su("tester", box.schema.user.create, 'test_user')
+box.schema.user.create('test_user')
---
- error: Write access to space '_user' is denied for user 'tester'
...
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.schema.func.create('test_func')
---
- error: Write access to space '_func' is denied for user 'tester'
...
+box.session.su("admin")
+---
+...
--
-- FIXME 2.0: we still need to grant 'write' on universe
-- explicitly since we still use process_rw to write to system
@@ -1387,24 +1397,36 @@ box.session.su("tester", box.schema.func.create, 'test_func')
box.schema.user.grant("tester", "create,write", "universe")
---
...
+box.session.su("tester")
+---
+...
-- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
+s1 = box.schema.space.create("test_space")
+---
+...
+_ = s1:create_index("primary")
---
...
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
+_ = box.schema.user.create('test_user')
---
...
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+_ = box.schema.func.create('test_func')
+---
+...
+seq1 = box.schema.sequence.create('test_seq')
---
...
-- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
+s1:drop()
---
...
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
+seq1:drop()
+---
+...
+box.schema.user.drop('test_user')
---
...
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+box.schema.func.drop('test_func')
---
...
-- failed alter
@@ -1414,22 +1436,24 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
-- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
-- failed drop
-- box.session.su("tester", s.drop, s)
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
+s:drop()
---
+- error: Drop access to space 'test' is denied for user 'tester'
+...
+seq:drop()
+---
+- error: Drop access to sequence 'test' is denied for user 'tester'
...
box.schema.user.drop("test")
---
- error: Revoke access to role 'public' is denied for user 'tester'
...
-box.session.su("admin")
+box.schema.func.drop("test")
---
+- error: Drop access to function 'test' is denied for user 'tester'
...
-box.session.su("tester", box.schema.func.drop, "test")
+box.session.su("admin")
---
-- error: Drop access to function 'test' is denied for user 'tester'
...
box.schema.user.grant("tester", "drop", "universe")
---
@@ -1438,6 +1462,9 @@ box.schema.user.grant("tester", "drop", "universe")
box.session.su("tester", s.drop, s)
---
...
+box.session.su("tester", seq.drop, seq)
+---
+...
box.session.su("tester", box.schema.user.drop, "test")
---
...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 7e880a0..e3ad687 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -509,14 +509,17 @@ s:drop()
--
box.schema.user.create("tester")
s = box.schema.space.create("test")
+_ = s:create_index("primary")
+seq = box.schema.sequence.create("test")
u = box.schema.user.create("test")
f = box.schema.func.create("test")
-box.schema.user.grant("tester", "read,execute", "universe")
-- failed create
-box.session.su("tester", box.schema.space.create, "test_space")
-box.session.su("tester", box.schema.user.create, 'test_user')
-box.session.su("tester", box.schema.func.create, 'test_func')
+box.session.su("tester")
+box.schema.space.create("test_space")
+box.schema.user.create('test_user')
+box.schema.func.create('test_func')
+box.session.su("admin")
--
-- FIXME 2.0: we still need to grant 'write' on universe
@@ -524,15 +527,19 @@ box.session.su("tester", box.schema.func.create, 'test_func')
-- tables from ddl
--
box.schema.user.grant("tester", "create,write", "universe")
+box.session.su("tester")
-- successful create
-s1 = box.session.su("tester", box.schema.space.create, "test_space")
-_ = box.session.su("tester", box.schema.user.create, 'test_user')
-_ = box.session.su("tester", box.schema.func.create, 'test_func')
+s1 = box.schema.space.create("test_space")
+_ = s1:create_index("primary")
+_ = box.schema.user.create('test_user')
+_ = box.schema.func.create('test_func')
+seq1 = box.schema.sequence.create('test_seq')
-- successful drop of owned objects
-_ = box.session.su("tester", s1.drop, s1)
-_ = box.session.su("tester", box.schema.user.drop, 'test_user')
-_ = box.session.su("tester", box.schema.func.drop, 'test_func')
+s1:drop()
+seq1:drop()
+box.schema.user.drop('test_user')
+box.schema.func.drop('test_func')
-- failed alter
-- box.session.su("tester", s.format, s, {name="id", type="unsigned"})
@@ -543,19 +550,16 @@ _ = box.session.su("tester", box.schema.func.drop, 'test_func')
-- failed drop
-- box.session.su("tester", s.drop, s)
-
--- can't use here sudo
--- because drop use sudo inside
--- and currently sudo can't be performed nested
-box.session.su("tester")
+s:drop()
+seq:drop()
box.schema.user.drop("test")
-box.session.su("admin")
-
-box.session.su("tester", box.schema.func.drop, "test")
+box.schema.func.drop("test")
+box.session.su("admin")
box.schema.user.grant("tester", "drop", "universe")
-- successful drop
box.session.su("tester", s.drop, s)
+box.session.su("tester", seq.drop, seq)
box.session.su("tester", box.schema.user.drop, "test")
box.session.su("tester", box.schema.func.drop, "test")
diff --git a/test/box/access_bin.result b/test/box/access_bin.result
index b81279c..d093c75 100644
--- a/test/box/access_bin.result
+++ b/test/box/access_bin.result
@@ -55,14 +55,14 @@ c = remote.connect(box.cfg.listen)
...
c:call("setuid_func")
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('guest')
---
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
@@ -85,7 +85,7 @@ session.su('guest')
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
@@ -122,7 +122,7 @@ session.su('guest')
...
setuid_func()
---
-- error: Read access to space 'setuid_space' is denied for user 'guest'
+- error: Write access to space 'setuid_space' is denied for user 'guest'
...
session.su('admin')
---
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 3a56a4c..abc0be7 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -177,7 +177,7 @@ gs = box.schema.space.create('guest_space')
...
box.schema.func.create('guest_func')
---
-- error: Read access to space '_func' is denied for user 'guest'
+- error: Write access to space '_func' is denied for user 'guest'
...
session.su('admin')
---
diff --git a/test/box/access_sysview.result b/test/box/access_sysview.result
index 340ed21..20efd2b 100644
--- a/test/box/access_sysview.result
+++ b/test/box/access_sysview.result
@@ -266,11 +266,11 @@ box.session.su('guest')
...
#box.space._vuser:select{}
---
-- 1
+- 5
...
#box.space._vpriv:select{}
---
-- 2
+- 15
...
#box.space._vfunc:select{}
---
@@ -343,7 +343,7 @@ box.session.su('guest')
-- _vuser
--
-- a guest user can read information about itself
-t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest'
+t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false
---
- true
...
@@ -526,8 +526,9 @@ box.schema.user.grant('guest', 'execute', 'function', 'test')
box.session.su('guest')
---
...
-#box.space._vfunc:select{} = cnt + 1
+#box.space._vfunc:select{} == func_cnt
---
+- true
...
box.session.su('admin')
---
@@ -551,7 +552,7 @@ box.schema.user.grant('guest', 'execute', 'universe')
box.session.su('guest')
---
...
-#box.space._vfunc:select{} == cnt + 1
+#box.space._vfunc:select{} == func_cnt
---
- true
...
diff --git a/test/box/access_sysview.test.lua b/test/box/access_sysview.test.lua
index 7955ffc..4cc5611 100644
--- a/test/box/access_sysview.test.lua
+++ b/test/box/access_sysview.test.lua
@@ -134,7 +134,7 @@ box.session.su('guest')
--
-- a guest user can read information about itself
-t = box.space._vuser:select(); return #t == 1 and t[1][3] == 'guest'
+t = box.space._vuser:select(); for i = 1, #t do if t[i][3] == 'guest' then return true end end return false
-- read access to original space also allow to read a view
box.session.su('admin')
@@ -218,7 +218,7 @@ box.session.su('admin')
box.schema.user.grant('guest', 'execute', 'function', 'test')
box.session.su('guest')
-#box.space._vfunc:select{} = cnt + 1
+#box.space._vfunc:select{} == func_cnt
box.session.su('admin')
box.schema.user.revoke('guest', 'execute', 'function', 'test')
@@ -230,7 +230,7 @@ box.session.su('admin')
box.schema.user.grant('guest', 'execute', 'universe')
box.session.su('guest')
-#box.space._vfunc:select{} == cnt + 1
+#box.space._vfunc:select{} == func_cnt
box.session.su('admin')
box.schema.user.revoke('guest', 'execute', 'universe')
diff --git a/test/box/on_replace.result b/test/box/on_replace.result
index d6158dc..b9e6ded 100644
--- a/test/box/on_replace.result
+++ b/test/box/on_replace.result
@@ -471,42 +471,42 @@ t = s:on_replace(function () s:create_index('sec') end, t)
...
s:replace({2, 3})
---
-- error: Space _index does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.user.create('newu') end, t)
---
...
s:replace({3, 4})
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.role.create('newr') end, t)
---
...
s:replace({4, 5})
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () s:drop() end, t)
---
...
s:replace({5, 6})
---
-- error: Space _index does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.func.create('newf') end, t)
---
...
s:replace({6, 7})
---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () box.schema.user.grant('guest', 'read,write', 'space', 'test_on_repl_ddl') end, t)
---
...
s:replace({7, 8})
---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
t = s:on_replace(function () s:rename('newname') end, t)
---
diff --git a/test/box/role.result b/test/box/role.result
index 736ec85..806cea9 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -662,7 +662,7 @@ box.session.su('john')
-- error
box.schema.user.grant('grantee', 'role')
---
-- error: Read access to space '_user' is denied for user 'john'
+- error: User 'grantee' is not found
...
--
box.session.su('admin')
diff --git a/test/box/transaction.result b/test/box/transaction.result
index 2a4b3b2..0f1a8b3 100644
--- a/test/box/transaction.result
+++ b/test/box/transaction.result
@@ -69,21 +69,21 @@ box.rollback();
...
box.begin() box.schema.func.create('test');
---
-- error: Space _func does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
...
box.begin() box.schema.user.create('test');
---
-- error: Space _user does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
...
box.begin() box.schema.user.grant('guest', 'read', 'space', '_priv');
---
-- error: Space _priv does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
@@ -107,7 +107,7 @@ s = box.schema.space.create('test');
...
box.begin() index = s:create_index('primary');
---
-- error: Space _index does not support multi-statement transactions
+- error: A multi-statement transaction can not use multiple storage engines
...
box.rollback();
---
diff --git a/test/engine/iterator.result b/test/engine/iterator.result
index a5fc9d7..00c427d 100644
--- a/test/engine/iterator.result
+++ b/test/engine/iterator.result
@@ -4211,7 +4211,7 @@ s:replace{35}
...
state, value = gen(param,state)
---
-- error: 'builtin/box/schema.lua:983: usage: next(param, state)'
+- error: 'builtin/box/schema.lua:991: usage: next(param, state)'
...
value
---
diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result
index 9d238ec..6e07958 100644
--- a/test/engine/savepoint.result
+++ b/test/engine/savepoint.result
@@ -14,7 +14,7 @@ s1 = box.savepoint()
...
box.rollback_to_savepoint(s1)
---
-- error: 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- error: 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
box.begin() s1 = box.savepoint()
---
@@ -323,27 +323,27 @@ test_run:cmd("setopt delimiter ''");
ok1, errmsg1
---
- false
-- 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
ok2, errmsg2
---
- false
-- 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
ok3, errmsg3
---
- false
-- 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
ok4, errmsg4
---
- false
-- 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
ok5, errmsg5
---
- false
-- 'builtin/box/schema.lua:298: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:299: Usage: box.rollback_to_savepoint(savepoint)'
...
s:select{}
---
--
2.7.4
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-29 7:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 8:09 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 1/3] box: Add system view for _sequence system space Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces Ilya Markov
2018-03-28 8:09 ` [tarantool-patches] [security 3/3] security: Refactor system space access checks Ilya Markov
2018-03-29 7:36 [tarantool-patches] [security 0/3] System space access check lists Ilya Markov
2018-03-29 7:37 ` [tarantool-patches] [security 2/3] security: Refactor reads from systems spaces Ilya Markov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox