From: Serge Petrenko <sergepetrenko@tarantool.org>
To: tarantool-patches@freelists.org
Cc: Serge Petrenko <sergepetrenko@tarantool.org>
Subject: [tarantool-patches] [PATCH v2] Add entities user, role to access control.
Date: Fri, 13 Jul 2018 10:05:18 +0300 [thread overview]
Message-ID: <20180713070518.66846-1-sergepetrenko@tarantool.org> (raw)
Previously the only existing entities in access control were space,
funciton and sequence. Added user and role entities, so it is now
possible to create users or roles without create privilege on universe.
Also added all the needed checks and modified tests accordingly.
Closes #3524
---
Sorry for accidentally posting a patch with a syntax error.
Please first see patch related to issue #3516, it is the base
for this patch:
https://github.com/tarantool/tarantool/issues/3516
https://github.com/tarantool/tarantool/tree/sergepetrenko/gh-3516-entity-access-checks
And then current patch:
https://github.com/tarantool/tarantool/issues/3524
https://github.com/tarantool/tarantool/tree/sergepetrenko/gh-3524-entity-access-grants
Changes in v2:
-fixed a syntax error
src/box/alter.cc | 72 +++++++++++++++++++++++++++++++++------------
src/box/lua/schema.lua | 27 ++++++++++-------
src/box/schema.h | 6 ++++
src/box/user.cc | 25 +++++++++++++++-
test/box/access.result | 20 ++++++-------
test/box/access.test.lua | 15 ++++------
test/box/access_misc.result | 2 +-
test/box/role.result | 25 ++++++++++++++--
test/box/role.test.lua | 12 ++++++--
9 files changed, 150 insertions(+), 54 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index ef8b1acff..3c6af0e87 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2173,7 +2173,7 @@ 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);
+ access_check_ddl(user->name, user->owner, user->type, PRIV_C, true);
auto def_guard = make_scoped_guard([=] { free(user); });
(void) user_cache_replace(user);
def_guard.is_active = false;
@@ -2182,7 +2182,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);
+ old_user->def->type, PRIV_D, true);
/* Can't drop guest or super user */
if (uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) {
tnt_raise(ClientError, ER_DROP_USER,
@@ -2208,6 +2208,14 @@ on_replace_dd_user(struct trigger * /* trigger */, void *event)
* correct.
*/
struct user_def *user = user_def_new_from_tuple(new_tuple);
+ /* Do not allow changes for system users and roles. */
+ struct credentials *cr = effective_user();
+ if ((uid <= (uint32_t) BOX_SYSTEM_USER_ID_MAX || uid == SUPER) &&
+ cr->uid != ADMIN) {
+ struct user *current_user = user_find_xc(cr->uid);
+ tnt_raise(AccessDeniedError, "alter", "user or role",
+ old_user->def->name, current_user->def->name);
+ }
access_check_ddl(user->name, user->uid, SC_USER, PRIV_A,
true);
auto def_guard = make_scoped_guard([=] { free(user); });
@@ -2628,26 +2636,49 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
}
case SC_ROLE:
{
- struct user *role = user_by_id(priv->object_id);
- if (role == NULL || role->def->type != SC_ROLE) {
- tnt_raise(ClientError, ER_NO_SUCH_ROLE,
- role ? role->def->name :
- int2str(priv->object_id));
- }
- /*
- * Only the creator of the role can grant or revoke it.
- * Everyone can grant 'PUBLIC' role.
- */
- if (role->def->owner != grantor->def->uid &&
- grantor->def->uid != ADMIN &&
- (role->def->uid != PUBLIC || priv->access != PRIV_X)) {
+ if (priv->object_id != 0) {
+ struct user *role = user_by_id(priv->object_id);
+ if (role == NULL || role->def->type != SC_ROLE) {
+ tnt_raise(ClientError, ER_NO_SUCH_ROLE,
+ role ? role->def->name :
+ int2str(priv->object_id));
+ }
+ /*
+ * Only the creator of the role can grant or revoke it.
+ * Everyone can grant 'PUBLIC' role.
+ */
+ 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);
+ }
+ /* Not necessary to do during revoke, but who cares. */
+ role_check(grantee, role);
+ } else if (grantor->def->uid != ADMIN) {
+ /* only admin may grant privileges on an entire entity. */
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);
+ break;
+ }
+ case SC_USER:
+ {
+ struct user *user = NULL;
+ if (priv->object_id != 0)
+ user = user_by_id(priv->object_id);
+ if ((user == NULL || user->def->owner != grantor->def->uid) &&
+ grantor->def->uid != ADMIN) {
+ tnt_raise(AccessDeniedError,
+ priv_name(priv_type),
+ schema_object_name(SC_USER), name,
+ grantor->def->name);
+ }
+ break;
}
default:
break;
@@ -2668,7 +2699,12 @@ grant_or_revoke(struct priv_def *priv)
struct user *grantee = user_by_id(priv->grantee_id);
if (grantee == NULL)
return;
- if (priv->object_type == SC_ROLE) {
+ /*
+ * Grant a role to a user only when privilege type is 'execute'
+ * and the role is specified.
+ */
+ if (priv->object_type == SC_ROLE && !(priv->access & ~PRIV_X) &&
+ priv->object_id != 0) {
struct user *role = user_by_id(priv->object_id);
if (role == NULL || role->def->type != SC_ROLE)
return;
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index dc18f71b2..3235c49a6 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1737,6 +1737,8 @@ local priv_object_combo = {
box.priv.C, box.priv.D),
["role"] = bit.bor(box.priv.X, box.priv.U,
box.priv.C, box.priv.D),
+ ["user"] = bit.bor(box.priv.C, box.priv.U,
+ box.priv.D),
}
--
@@ -1840,21 +1842,25 @@ local function object_resolve(object_type, object_name)
end
return seq
end
- if object_type == 'role' then
+ if object_type == 'role' or object_type == 'user' then
local _vuser = box.space[box.schema.VUSER_ID]
- local role
+ local role_or_user
+ if object_name == nil or object_name == 0 then
+ return 0
+ end
if type(object_name) == 'string' then
- role = _vuser.index.name:get{object_name}
+ role_or_user = _vuser.index.name:get{object_name}
else
- role = _vuser:get{object_name}
+ role_or_user = _vuser:get{object_name}
end
- if role and role[4] == 'role' then
- return role[1]
- else
+ if role_or_user and role_or_user[4] == object_type then
+ return role_or_user[1]
+ elseif object_type == 'role' then
box.error(box.error.NO_SUCH_ROLE, object_name)
- end
+ else
+ box.error(box.error.NO_SUCH_USER, object_name)
+ end
end
-
box.error(box.error.UNKNOWN_SCHEMA_OBJECT, object_type)
end
@@ -2100,7 +2106,8 @@ local function grant(uid, name, privilege, object_type,
if privilege_hex ~= old_privilege then
_priv:replace{options.grantor, uid, object_type, oid, privilege_hex}
elseif not options.if_not_exists then
- if object_type == 'role' then
+ if object_type == 'role' and object_name ~= nil and
+ object_name ~= 0 then
box.error(box.error.ROLE_GRANTED, name, object_name)
else
box.error(box.error.PRIV_GRANTED, name, privilege,
diff --git a/src/box/schema.h b/src/box/schema.h
index 0822262d0..f78ea43cc 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -240,6 +240,8 @@ struct on_access_denied_ctx {
struct entity_access {
struct access space[BOX_USER_MAX];
struct access function[BOX_USER_MAX];
+ struct access user[BOX_USER_MAX];
+ struct access role[BOX_USER_MAX];
struct access sequence[BOX_USER_MAX];
};
@@ -255,6 +257,10 @@ entity_access_get(enum schema_object_type type)
return entity_access.space;
case SC_FUNCTION:
return entity_access.function;
+ case SC_USER:
+ return entity_access.user;
+ case SC_ROLE:
+ return entity_access.role;
case SC_SEQUENCE:
return entity_access.sequence;
default:
diff --git a/src/box/user.cc b/src/box/user.cc
index fbf06566a..4edef1d5f 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -229,6 +229,29 @@ access_find(struct priv_def *priv)
access = func->access;
break;
}
+ case SC_USER:
+ {
+ /*
+ * user ID 0 is shared between user 'guest' and granting
+ * privileges upon whole entity user. This is not a problem,
+ * since we don't want to grant privileges on any system user,
+ * including 'guest'.
+ */
+ if(priv->object_id == 0) {
+ access = entity_access.user;
+ break;
+ }
+ /* No grants on a single object user yet. */
+ }
+ case SC_ROLE:
+ {
+ /* Tha same remark as in case SC_USER applies. */
+ if (priv->object_id == 0) {
+ access = entity_access.role;
+ break;
+ }
+ /* No grants on a single object role yet. */
+ }
case SC_SEQUENCE:
{
if (priv->object_id == 0) {
@@ -315,7 +338,7 @@ user_reload_privs(struct user *user)
* Skip role grants, we're only
* interested in real objects.
*/
- if (priv.object_type != SC_ROLE)
+ if (priv.object_type != SC_ROLE || !(priv.access & PRIV_X))
user_grant_priv(user, &priv);
}
}
diff --git a/test/box/access.result b/test/box/access.result
index f4669a4a3..31095aec2 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -878,8 +878,6 @@ box.schema.user.create('test')
box.schema.user.grant('test', 'read', 'space', '_collation')
---
...
---box.schema.user.grant('test', 'write', 'space', '_collation')
--- FIXME: granting create on 'collation' only doesn't work
box.schema.user.grant('test', 'create', 'universe')
---
...
@@ -1427,16 +1425,18 @@ box.session.su("admin")
box.schema.user.grant('tester', 'write', 'universe')
---
...
--- no entity user currently, so have to grant create
--- on universe in order to create a user.
-box.schema.user.grant('tester', 'create', 'universe')
+box.schema.user.grant('tester', 'create', 'user')
+---
+...
+box.schema.user.grant('tester', 'create', 'space')
+---
+...
+box.schema.user.grant('tester', 'create', 'function')
+---
+...
+box.schema.user.grant('tester', 'create' , 'sequence')
---
...
--- this should work instead:
---box.schema.user.grant('tester', 'create', 'user')
---box.schema.user.grant('tester', 'create', 'space')
---box.schema.user.grant('tester', 'create', 'function')
---box.schema.user.grant('tester', 'create' , 'sequence')
box.schema.user.grant('tester', 'read', 'space', '_sequence')
---
...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 9ae0e1114..9b7510e64 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -341,8 +341,7 @@ c:close()
session = box.session
box.schema.user.create('test')
box.schema.user.grant('test', 'read', 'space', '_collation')
---box.schema.user.grant('test', 'write', 'space', '_collation')
--- FIXME: granting create on 'collation' only doesn't work
+
box.schema.user.grant('test', 'create', 'universe')
session.su('test')
box.internal.collation.create('test', 'ICU', 'ru_RU')
@@ -538,14 +537,10 @@ box.session.su("admin")
-- tables from ddl
--
box.schema.user.grant('tester', 'write', 'universe')
--- no entity user currently, so have to grant create
--- on universe in order to create a user.
-box.schema.user.grant('tester', 'create', 'universe')
--- this should work instead:
---box.schema.user.grant('tester', 'create', 'user')
---box.schema.user.grant('tester', 'create', 'space')
---box.schema.user.grant('tester', 'create', 'function')
---box.schema.user.grant('tester', 'create' , 'sequence')
+box.schema.user.grant('tester', 'create', 'user')
+box.schema.user.grant('tester', 'create', 'space')
+box.schema.user.grant('tester', 'create', 'function')
+box.schema.user.grant('tester', 'create' , 'sequence')
box.schema.user.grant('tester', 'read', 'space', '_sequence')
box.session.su("tester")
-- successful create
diff --git a/test/box/access_misc.result b/test/box/access_misc.result
index 839b576ac..0f4892533 100644
--- a/test/box/access_misc.result
+++ b/test/box/access_misc.result
@@ -361,7 +361,7 @@ testuser_uid = session.uid()
...
_ = box.space._user:delete(2)
---
-- error: Drop access to user 'public' is denied for user 'testuser'
+- error: Drop access to role 'public' is denied for user 'testuser'
...
box.space._user:select(1)
---
diff --git a/test/box/role.result b/test/box/role.result
index 806cea90b..243c7bc6c 100644
--- a/test/box/role.result
+++ b/test/box/role.result
@@ -214,7 +214,22 @@ box.schema.role.drop('test')
box.schema.user.grant('grantee', 'liaison')
---
...
-box.schema.user.grant('test', 'read,write,create', 'universe')
+box.schema.user.grant('test', 'read,write', 'space', '_priv')
+---
+...
+box.schema.user.grant('test', 'write', 'space', '_schema')
+---
+...
+box.schema.user.grant('test', 'create', 'space')
+---
+...
+box.schema.user.grant('test', 'read,write', 'space', '_space')
+---
+...
+box.schema.user.grant('test', 'write', 'space', '_index')
+---
+...
+box.schema.user.grant('test', 'read', 'space', '_user')
---
...
box.session.su('test')
@@ -635,7 +650,13 @@ box.schema.user.create('user')
box.schema.user.create('grantee')
---
...
-box.schema.user.grant('user', 'read,write,execute,create', 'universe')
+box.schema.user.grant('user', 'read,write', 'space', '_user')
+---
+...
+box.schema.user.grant('user', 'read,write', 'space', '_priv')
+---
+...
+box.schema.user.grant('user', 'create', 'role')
---
...
box.session.su('user')
diff --git a/test/box/role.test.lua b/test/box/role.test.lua
index e97339f49..9845f4c4c 100644
--- a/test/box/role.test.lua
+++ b/test/box/role.test.lua
@@ -69,7 +69,13 @@ box.schema.role.revoke('test', 'liaison')
box.schema.role.drop('test')
box.schema.user.grant('grantee', 'liaison')
-box.schema.user.grant('test', 'read,write,create', 'universe')
+
+box.schema.user.grant('test', 'read,write', 'space', '_priv')
+box.schema.user.grant('test', 'write', 'space', '_schema')
+box.schema.user.grant('test', 'create', 'space')
+box.schema.user.grant('test', 'read,write', 'space', '_space')
+box.schema.user.grant('test', 'write', 'space', '_index')
+box.schema.user.grant('test', 'read', 'space', '_user')
box.session.su('test')
s = box.schema.space.create('test')
_ = s:create_index('i1')
@@ -248,7 +254,9 @@ box.schema.role.drop("role10")
box.schema.user.create('user')
box.schema.user.create('grantee')
-box.schema.user.grant('user', 'read,write,execute,create', 'universe')
+box.schema.user.grant('user', 'read,write', 'space', '_user')
+box.schema.user.grant('user', 'read,write', 'space', '_priv')
+box.schema.user.grant('user', 'create', 'role')
box.session.su('user')
box.schema.role.create('role')
box.session.su('admin')
--
2.15.2 (Apple Git-101.1)
reply other threads:[~2018-07-13 7:05 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180713070518.66846-1-sergepetrenko@tarantool.org \
--to=sergepetrenko@tarantool.org \
--cc=tarantool-patches@freelists.org \
--subject='Re: [tarantool-patches] [PATCH v2] Add entities user, role to access control.' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox