Tarantool development patches archive
 help / color / mirror / Atom feed
* [tarantool-patches] [PATCH 0/3] Object group privileges
@ 2018-06-08  9:06 Georgy Kirichenko
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua Georgy Kirichenko
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Georgy Kirichenko @ 2018-06-08  9:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

Introduce spaces, functions and sequences privileges with some
minor refactoring.

Georgy Kirichenko :
  Overall review and fixes

imarkov (3):
  box: Add privilleges constants to lua
  security: add limits on object_type-privilege pair
  Introduce privileges for object groups

 src/box/alter.cc             | 18 ++++---
 src/box/call.c               |  2 +
 src/box/errcode.h            |  2 +-
 src/box/lua/schema.lua       | 87 +++++++++++++++++++++++++++------
 src/box/lua/upgrade.lua      | 40 +++++++--------
 src/box/schema.cc            |  8 +++
 src/box/schema.h             | 26 ++++++++++
 src/box/sequence.c           |  1 +
 src/box/space.c              |  2 +
 src/box/sysview_index.c      | 11 +++++
 src/box/user.cc              | 12 +++++
 test/box/access.result       | 94 ++++++++++++++++++++++++++++++++++++
 test/box/access.test.lua     | 38 ++++++++++++++-
 test/box/lua/identifier.lua  |  1 -
 test/box/misc.result         | 47 +++++++++---------
 test/engine/iterator.result  |  2 +-
 test/engine/savepoint.result | 12 ++---
 17 files changed, 330 insertions(+), 73 deletions(-)

-- 
branch: https://github.com/tarantool/tarantool/tree/gh-3249-group-acl
issue: https://github.com/tarantool/tarantool/issues/945

2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua
  2018-06-08  9:06 [tarantool-patches] [PATCH 0/3] Object group privileges Georgy Kirichenko
@ 2018-06-08  9:06 ` Georgy Kirichenko
  2018-06-08 10:31   ` [tarantool-patches] " Vladislav Shpilevoy
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 2/3] security: add limits on object_type-privilege pair Georgy Kirichenko
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 3/3] Introduce privileges for object groups Georgy Kirichenko
  2 siblings, 1 reply; 8+ messages in thread
From: Georgy Kirichenko @ 2018-06-08  9:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: imarkov

From: imarkov <imarkov@tarantool.org>

Add lua bindings of PRIV_XXX constants.

This patch helps to avoid using numerical constants of privilleges
in schema.lua code.

Relates #945
---
 src/box/lua/schema.lua       | 67 ++++++++++++++++++++++++++++--------
 src/box/lua/upgrade.lua      | 40 +++++++++++----------
 test/box/misc.result         |  1 +
 test/engine/iterator.result  |  2 +-
 test/engine/savepoint.result | 12 +++----
 5 files changed, 82 insertions(+), 40 deletions(-)

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index d6d39170f..43c7d4e6b 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -101,8 +101,47 @@ ffi.cdef[[
 
     void password_prepare(const char *password, int len,
                           char *out, int out_len);
+
+    enum priv_type {
+        PRIV_R = 1,
+        PRIV_W = 2,
+        PRIV_X = 4,
+        PRIV_S = 8,
+        PRIV_U = 16,
+        PRIV_C = 32,
+        PRIV_D = 64,
+        PRIV_A = 128,
+        PRIV_REFERENCE = 256,
+        PRIV_TRIGGER = 512,
+        PRIV_INSERT = 1024,
+        PRIV_UPDATE = 2048,
+        PRIV_DELETE = 4096,
+        PRIV_GRANT = 8192,
+        PRIV_REVOKE = 16384,
+        PRIV_ALL  = 4294967295
+    };
+
 ]]
 
+box.priv = {
+    ["R"] = builtin.PRIV_R,
+    ["W"] = builtin.PRIV_W,
+    ["X"] = builtin.PRIV_X,
+    ["S"] = builtin.PRIV_S,
+    ["U"] = builtin.PRIV_U,
+    ["C"] = builtin.PRIV_C,
+    ["D"] = builtin.PRIV_D,
+    ["A"] = builtin.PRIV_A,
+    ["REFERENCE"] = builtin.PRIV_REFERENCE,
+    ["TRIGGER"] = builtin.PRIV_TRIGGER,
+    ["INSERT"] = builtin.PRIV_INSERT,
+    ["UPDATE"] = builtin.PRIV_UPDATE,
+    ["DELETE"] = builtin.PRIV_DELETE,
+    ["GRANT"]= builtin.PRIV_GRANT,
+    ["REVOKE"] = builtin.PRIV_REVOKE,
+    ["ALL"] = builtin.PRIV_ALL
+}
+
 local function user_or_role_resolve(user)
     local _vuser = box.space[box.schema.VUSER_ID]
     local tuple
@@ -1687,7 +1726,7 @@ end
 
 local function checked_privilege(privilege, object_type)
     local priv_hex = privilege_resolve(privilege)
-    if object_type == 'role' and priv_hex ~= 4 then
+    if object_type == 'role' and priv_hex ~= box.priv.X then
         box.error(box.error.UNSUPPORTED_ROLE_PRIV, privilege)
     end
     return priv_hex
@@ -1695,43 +1734,43 @@ end
 
 local function privilege_name(privilege)
     local names = {}
-    if bit.band(privilege, 1) ~= 0 then
+    if bit.band(privilege, box.priv.R) ~= 0 then
         table.insert(names, "read")
     end
-    if bit.band(privilege, 2) ~= 0 then
+    if bit.band(privilege, box.priv.W) ~= 0 then
         table.insert(names, "write")
     end
-    if bit.band(privilege, 4) ~= 0 then
+    if bit.band(privilege, box.priv.X) ~= 0 then
         table.insert(names, "execute")
     end
-    if bit.band(privilege, 8) ~= 0 then
+    if bit.band(privilege, box.priv.S) ~= 0 then
         table.insert(names, "session")
     end
-    if bit.band(privilege, 16) ~= 0 then
+    if bit.band(privilege, box.priv.U) ~= 0 then
         table.insert(names, "usage")
     end
-    if bit.band(privilege, 32) ~= 0 then
+    if bit.band(privilege, box.priv.C) ~= 0 then
         table.insert(names, "create")
     end
-    if bit.band(privilege, 64) ~= 0 then
+    if bit.band(privilege, box.priv.D) ~= 0 then
         table.insert(names, "drop")
     end
-    if bit.band(privilege, 128) ~= 0 then
+    if bit.band(privilege, box.priv.A) ~= 0 then
         table.insert(names, "alter")
     end
-    if bit.band(privilege, 256) ~= 0 then
+    if bit.band(privilege, box.priv.REFERENCE) ~= 0 then
         table.insert(names, "reference")
     end
-    if bit.band(privilege, 512) ~= 0 then
+    if bit.band(privilege, box.priv.TRIGGER) ~= 0 then
         table.insert(names, "trigger")
     end
-    if bit.band(privilege, 1024) ~= 0 then
+    if bit.band(privilege, box.priv.INSERT) ~= 0 then
         table.insert(names, "insert")
     end
-    if bit.band(privilege, 2048) ~= 0 then
+    if bit.band(privilege, box.priv.UPDATE) ~= 0 then
         table.insert(names, "update")
     end
-    if bit.band(privilege, 4096) ~= 0 then
+    if bit.band(privilege, box.priv.DELETE) ~= 0 then
         table.insert(names, "delete")
     end
     return table.concat(names, ",")
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 589161944..0293f6ef8 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -211,11 +211,11 @@ local function initial()
     log.info("create role public")
     _user:insert{PUBLIC, ADMIN, 'public', 'role'}
     log.info("grant read,write,execute on universe to admin")
-    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
+    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
 
     -- grant role 'public' to 'guest'
     log.info("grant role public to guest")
-    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
+    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
 
     log.info("set max_id to box.schema.SYSTEM_ID_MAX")
     _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
@@ -406,7 +406,7 @@ local function create_sysview(source_id, target_id)
     -- public can read system views
     if box.space._priv.index.primary:count({PUBLIC, 'space', target_id}) == 0 then
         log.info("grant read access to 'public' role for %s view", def[3])
-        box.space._priv:insert({1, PUBLIC, 'space', target_id, 1})
+        box.space._priv:insert({1, PUBLIC, 'space', target_id, box.priv.R})
     end
 end
 
@@ -416,16 +416,17 @@ local function upgrade_users_to_1_6_8()
         local RPL_ID = box.space._user:auto_increment{ADMIN, 'replication', 'role'}[1]
         -- replication can read the entire universe
         log.info("grant read on universe to replication")
-        box.space._priv:replace{1, RPL_ID, 'universe', 0, 1}
+        box.space._priv:replace{1, RPL_ID, 'universe', 0, box.priv.R}
         -- replication can append to '_cluster' system space
         log.info("grant write on space _cluster to replication")
-        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, 2}
+        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, box.priv.W}
     end
 
     if box.space._priv.index.primary:count({ADMIN, 'universe', 0}) == 0 then
         -- grant admin access to the universe
         log.info("grant all on universe to admin")
-        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
+        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R +
+                                                        box.priv.W + box.priv.X}
     end
 
     if box.space._func.index.name:count("box.schema.user.info") == 0 then
@@ -435,7 +436,7 @@ local function upgrade_users_to_1_6_8()
 
         -- grant 'public' role access to 'box.schema.user.info' function
         log.info('grant execute on function "box.schema.user.info" to public')
-        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
+        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
     end
 end
 
@@ -555,7 +556,7 @@ local function create_truncate_space()
     box.space._index:insert{_truncate.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}}
 
     local _priv = box.space[box.schema.PRIV_ID]
-    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
+    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
 end
 
 local function update_existing_users_to_1_7_5()
@@ -809,20 +810,20 @@ local function initial_1_7_5()
     -- Create grants
     --
     log.info("grant read,write,execute on universe to admin")
-    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
+    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
 
     -- grant role 'public' to 'guest'
     log.info("grant role public to guest")
-    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
+    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
 
     -- replication can read the entire universe
     log.info("grant read on universe to replication")
-    _priv:replace{ADMIN, REPLICATION, 'universe', 0, 1}
+    _priv:replace{ADMIN, REPLICATION, 'universe', 0, box.priv.R}
     -- replication can append to '_cluster' system space
     log.info("grant write on space _cluster to replication")
-    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, 2}
+    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, box.priv.W}
 
-    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
+    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
 
     -- create "box.schema.user.info" function
     log.info('create function "box.schema.user.info" with setuid')
@@ -830,7 +831,7 @@ local function initial_1_7_5()
 
     -- grant 'public' role access to 'box.schema.user.info' function
     log.info('grant execute on function "box.schema.user.info" to public')
-    _priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
+    _priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
 
     log.info("set max_id to box.schema.SYSTEM_ID_MAX")
     _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
@@ -904,7 +905,7 @@ local function create_collation_space()
     box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}}
 
     local _priv = box.space[box.schema.PRIV_ID]
-    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, 2}
+    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, box.priv.W}
 end
 
 local function upgrade_to_1_7_6()
@@ -924,7 +925,8 @@ local function upgrade_to_1_7_7()
     --
     for _, v in _user:pairs() do
         if v[4] ~= "role" then
-            _priv:upsert({ADMIN, v[1], "universe", 0, 24}, {{"|", 5, 24}})
+            _priv:upsert({ADMIN, v[1], "universe", 0, box.priv.S + box.priv.U},
+                                                {{"|", 5, box.priv.S + box.priv.U}})
         end
     end
     --
@@ -935,14 +937,14 @@ local function upgrade_to_1_7_7()
     --
     for _, v in _priv.index.object:pairs{'universe'} do
         if bit.band(v[5], 1) ~= 0 and bit.band(v[5], 2) ~= 0 then
-            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, 32}})
+            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, box.priv.C}})
         end
     end
     -- grant admin all new privileges (session, usage, grant option,
     -- create, alter, drop and anything that might come up in the future
     --
-    _priv:upsert({ADMIN, ADMIN, 'universe', 0, 4294967295},
-                 {{ "|", 5, 4294967295}})
+    _priv:upsert({ADMIN, ADMIN, 'universe', 0, box.priv.ALL},
+                 {{ "|", 5, box.priv.ALL}})
     --
     -- create role 'super' and grant it all privileges on universe
     --
diff --git a/test/box/misc.result b/test/box/misc.result
index 8f94f5513..c6e4917bf 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -68,6 +68,7 @@ t
   - info
   - internal
   - once
+  - priv
   - rollback
   - rollback_to_savepoint
   - runtime
diff --git a/test/engine/iterator.result b/test/engine/iterator.result
index ae14c4320..1bde10eaf 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:993: usage: next(param, state)'
+- error: 'builtin/box/schema.lua:1032: usage: next(param, state)'
 ...
 value
 ---
diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result
index dc2ad7986..a62a2e135 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:301: Usage: box.rollback_to_savepoint(savepoint)'
+- error: 'builtin/box/schema.lua:340: 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:301: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
 ...
 ok2, errmsg2
 ---
 - false
-- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
 ...
 ok3, errmsg3
 ---
 - false
-- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
 ...
 ok4, errmsg4
 ---
 - false
-- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
 ...
 ok5, errmsg5
 ---
 - false
-- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
+- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
 ...
 s:select{}
 ---
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] [PATCH 2/3] security: add limits on object_type-privilege pair
  2018-06-08  9:06 [tarantool-patches] [PATCH 0/3] Object group privileges Georgy Kirichenko
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua Georgy Kirichenko
@ 2018-06-08  9:06 ` Georgy Kirichenko
  2018-06-08 14:01   ` [tarantool-patches] " Konstantin Osipov
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 3/3] Introduce privileges for object groups Georgy Kirichenko
  2 siblings, 1 reply; 8+ messages in thread
From: Georgy Kirichenko @ 2018-06-08  9:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: imarkov

From: imarkov <imarkov@tarantool.org>

Introduce constraints on object_type-privilege pairs.
These constraints limit senseless grants/revokes, i.e.,
sequence - execute, all space related privileges(insert, delete,
update),
function - alter, all space related privileges,
role - all privileges except create, drop, alter, execute

Prerequisite #945
---
 src/box/errcode.h        |  2 +-
 src/box/lua/schema.lua   | 13 ++++++++++--
 test/box/access.result   | 17 +++++++++++++++
 test/box/access.test.lua |  9 +++++++-
 test/box/misc.result     | 46 ++++++++++++++++++++--------------------
 5 files changed, 60 insertions(+), 27 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index a0759f8f4..d76673be9 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -150,7 +150,7 @@ struct errcode_record {
 	/* 95 */_(ER_UPDATE_INTEGER_OVERFLOW,   "Integer overflow when performing '%c' operation on field %u") \
 	/* 96 */_(ER_GUEST_USER_PASSWORD,       "Setting password for guest user has no effect") \
 	/* 97 */_(ER_TRANSACTION_CONFLICT,      "Transaction has been aborted by conflict") \
-	/* 98 */_(ER_UNSUPPORTED_ROLE_PRIV,     "Unsupported role privilege '%s'") \
+	/* 98 */_(ER_UNSUPPORTED_PRIV,		"Unsupported %s privilege '%s'") \
 	/* 99 */_(ER_LOAD_FUNCTION,		"Failed to dynamically load function '%s': %s") \
 	/*100 */_(ER_FUNCTION_LANGUAGE,		"Unsupported language '%s' specified for function '%s'") \
 	/*101 */_(ER_RTREE_RECT,		"RTree: %s must be an array with %u (point) or %u (rectangle/box) numeric coordinates") \
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 43c7d4e6b..4455b5e42 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1724,10 +1724,19 @@ local function privilege_resolve(privilege)
     return numeric
 end
 
+-- validate privileges
+local forbidden_privileges = {
+    ["universe"] = 0,
+    ["space"] = 0,
+    ["sequence"] = bit.bor(box.priv.X, box.priv.A, box.priv.INSERT, box.priv.UPDATE, box.priv.DELETE),
+    ["function"] = bit.bor(box.priv.A, box.priv.INSERT, box.priv.UPDATE, box.priv.DELETE),
+    ["role"] = bit.bxor(box.priv.ALL, bit.bor(box.priv.C, box.priv.D, box.priv.X)),
+}
+
 local function checked_privilege(privilege, object_type)
     local priv_hex = privilege_resolve(privilege)
-    if object_type == 'role' and priv_hex ~= box.priv.X then
-        box.error(box.error.UNSUPPORTED_ROLE_PRIV, privilege)
+    if bit.band(priv_hex, forbidden_privileges[object_type] or 0) ~= 0 then
+        box.error(box.error.UNSUPPORTED_PRIV, object_type, privilege)
     end
     return priv_hex
 end
diff --git a/test/box/access.result b/test/box/access.result
index 131a21510..72f91173b 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -1645,3 +1645,20 @@ box.space._vsequence.index.name:get{"test"} ~= nil
 box.session.su('admin')
 ---
 ...
+-- prerequisite gh-945
+box.schema.user.grant("guest", "alter", "function")
+---
+- error: Unsupported function privilege 'alter'
+...
+box.schema.user.grant("guest", "execute", "sequence")
+---
+- error: Unsupported sequence privilege 'execute'
+...
+box.schema.user.grant("guest", "read,execute", "sequence")
+---
+- error: Unsupported sequence privilege 'read,execute'
+...
+box.schema.user.grant("guest", "read,write,execute", "role")
+---
+- error: Unsupported role privilege 'read,write,execute'
+...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 4bd34e45d..62691c471 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -586,6 +586,7 @@ box.schema.user.revoke("guest", "read", "universe", "useless name", {if_exists =
 box.schema.user.revoke("guest", "read", "universe", 0, {if_exists = true})
 box.schema.user.revoke("guest", "read", "universe", nil, {if_exists = true})
 box.schema.user.revoke("guest", "read", "universe", {}, {if_exists = true})
+
 --
 -- Check that box.schema.* api is available to non-super user
 -- In scope of gh-3250 "make sure grant/revoke does not require
@@ -632,4 +633,10 @@ box.space._vsequence.index.name:get{"test"} ~= nil
 --
 -- restore
 --
-box.session.su('admin')
\ No newline at end of file
+box.session.su('admin')
+
+-- prerequisite gh-945
+box.schema.user.grant("guest", "alter", "function")
+box.schema.user.grant("guest", "execute", "sequence")
+box.schema.user.grant("guest", "read,execute", "sequence")
+box.schema.user.grant("guest", "read,write,execute", "role")
diff --git a/test/box/misc.result b/test/box/misc.result
index c6e4917bf..7ab3d2fc4 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -345,12 +345,11 @@ t;
   - 'box.error.DROP_USER : 44'
   - 'box.error.MODIFY_INDEX : 14'
   - 'box.error.PASSWORD_MISMATCH : 47'
-  - 'box.error.UNSUPPORTED_ROLE_PRIV : 98'
   - 'box.error.ACCESS_DENIED : 42'
   - 'box.error.CANT_CREATE_COLLATION : 150'
   - 'box.error.USER_EXISTS : 46'
   - 'box.error.WAL_IO : 40'
-  - 'box.error.PROC_RET : 21'
+  - 'box.error.RTREE_RECT : 101'
   - 'box.error.PRIV_GRANTED : 89'
   - 'box.error.CREATE_SPACE : 9'
   - 'box.error.GRANT : 88'
@@ -401,77 +400,78 @@ t;
   - 'box.error.CROSS_ENGINE_TRANSACTION : 81'
   - 'box.error.FORMAT_MISMATCH_INDEX_PART : 27'
   - 'box.error.FUNCTION_TX_ACTIVE : 30'
+  - 'box.error.injection : table: <address>
   - 'box.error.NO_SUCH_ENGINE : 57'
   - 'box.error.COMMIT_IN_SUB_STMT : 122'
-  - 'box.error.injection : table: <address>
   - 'box.error.NULLABLE_MISMATCH : 153'
+  - 'box.error.TUPLE_FORMAT_LIMIT : 16'
   - 'box.error.LAST_DROP : 15'
-  - 'box.error.NO_SUCH_ROLE : 82'
+  - 'box.error.SPACE_FIELD_IS_DUPLICATE : 149'
   - 'box.error.DECOMPRESSION : 124'
   - 'box.error.CREATE_SEQUENCE : 142'
   - 'box.error.CREATE_USER : 43'
-  - 'box.error.SPACE_FIELD_IS_DUPLICATE : 149'
-  - 'box.error.INSTANCE_UUID_MISMATCH : 66'
   - 'box.error.SEQUENCE_OVERFLOW : 147'
+  - 'box.error.INSTANCE_UUID_MISMATCH : 66'
+  - 'box.error.INJECTION : 8'
   - 'box.error.SYSTEM : 115'
   - 'box.error.KEY_PART_IS_TOO_LONG : 118'
-  - 'box.error.TUPLE_FORMAT_LIMIT : 16'
-  - 'box.error.BEFORE_REPLACE_RET : 53'
-  - 'box.error.NO_SUCH_SAVEPOINT : 61'
+  - 'box.error.INVALID_MSGPACK : 20'
   - 'box.error.TRUNCATE_SYSTEM_SPACE : 137'
+  - 'box.error.NO_SUCH_SAVEPOINT : 61'
   - 'box.error.VY_QUOTA_TIMEOUT : 135'
+  - 'box.error.READ_VIEW_ABORTED : 130'
   - 'box.error.WRONG_INDEX_OPTIONS : 108'
   - 'box.error.INVALID_VYLOG_FILE : 133'
   - 'box.error.INDEX_FIELD_COUNT_LIMIT : 127'
-  - 'box.error.READ_VIEW_ABORTED : 130'
-  - 'box.error.USER_MAX : 56'
   - 'box.error.PROTOCOL : 104'
+  - 'box.error.USER_MAX : 56'
+  - 'box.error.BEFORE_REPLACE_RET : 53'
   - 'box.error.TUPLE_NOT_ARRAY : 22'
   - 'box.error.KEY_PART_COUNT : 31'
   - 'box.error.ALTER_SPACE : 12'
   - 'box.error.ACTIVE_TRANSACTION : 79'
   - 'box.error.EXACT_FIELD_COUNT : 38'
   - 'box.error.DROP_SEQUENCE : 144'
-  - 'box.error.INVALID_MSGPACK : 20'
   - 'box.error.MORE_THAN_ONE_TUPLE : 41'
-  - 'box.error.RTREE_RECT : 101'
-  - 'box.error.SUB_STMT_MAX : 121'
+  - 'box.error.INVALID_XLOG_ORDER : 76'
   - 'box.error.UNKNOWN_REQUEST_TYPE : 48'
-  - 'box.error.SPACE_EXISTS : 10'
+  - 'box.error.SUB_STMT_MAX : 121'
   - 'box.error.PROC_LUA : 32'
+  - 'box.error.SPACE_EXISTS : 10'
   - 'box.error.ROLE_NOT_GRANTED : 92'
+  - 'box.error.UNSUPPORTED : 5'
   - 'box.error.NO_SUCH_SPACE : 36'
   - 'box.error.WRONG_INDEX_PARTS : 107'
-  - 'box.error.DROP_SPACE : 11'
   - 'box.error.MIN_FIELD_COUNT : 39'
   - 'box.error.REPLICASET_UUID_MISMATCH : 63'
   - 'box.error.UPDATE_FIELD : 29'
+  - 'box.error.INDEX_EXISTS : 85'
   - 'box.error.COMPRESSION : 119'
   - 'box.error.INVALID_ORDER : 68'
-  - 'box.error.INDEX_EXISTS : 85'
   - 'box.error.SPLICE : 25'
   - 'box.error.UNKNOWN : 0'
+  - 'box.error.IDENTIFIER : 70'
   - 'box.error.DROP_PRIMARY_KEY : 17'
   - 'box.error.NULLABLE_PRIMARY : 152'
   - 'box.error.NO_SUCH_SEQUENCE : 145'
   - 'box.error.RELOAD_CFG : 58'
   - 'box.error.INVALID_UUID : 64'
-  - 'box.error.INJECTION : 8'
+  - 'box.error.DROP_SPACE : 11'
   - 'box.error.TIMEOUT : 78'
-  - 'box.error.IDENTIFIER : 70'
   - 'box.error.ITERATOR_TYPE : 72'
   - 'box.error.REPLICA_MAX : 73'
+  - 'box.error.NO_SUCH_ROLE : 82'
   - 'box.error.MISSING_REQUEST_FIELD : 69'
   - 'box.error.MISSING_SNAPSHOT : 93'
   - 'box.error.WRONG_SPACE_OPTIONS : 111'
   - 'box.error.READONLY : 7'
-  - 'box.error.UNSUPPORTED : 5'
   - 'box.error.UPDATE_INTEGER_OVERFLOW : 95'
-  - 'box.error.NO_CONNECTION : 77'
-  - 'box.error.INVALID_XLOG_ORDER : 76'
   - 'box.error.UPSERT_UNIQUE_SECONDARY_KEY : 105'
-  - 'box.error.ROLLBACK_IN_SUB_STMT : 123'
+  - 'box.error.NO_CONNECTION : 77'
+  - 'box.error.UNSUPPORTED_PRIV : 98'
   - 'box.error.WRONG_SCHEMA_VERSION : 109'
+  - 'box.error.ROLLBACK_IN_SUB_STMT : 123'
+  - 'box.error.PROC_RET : 21'
   - 'box.error.UNSUPPORTED_INDEX_FEATURE : 112'
   - 'box.error.INDEX_PART_TYPE_MISMATCH : 24'
   - 'box.error.INVALID_XLOG_TYPE : 125'
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] [PATCH 3/3] Introduce privileges for object groups
  2018-06-08  9:06 [tarantool-patches] [PATCH 0/3] Object group privileges Georgy Kirichenko
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua Georgy Kirichenko
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 2/3] security: add limits on object_type-privilege pair Georgy Kirichenko
@ 2018-06-08  9:06 ` Georgy Kirichenko
  2018-06-08 17:26   ` [tarantool-patches] " Konstantin Osipov
  2 siblings, 1 reply; 8+ messages in thread
From: Georgy Kirichenko @ 2018-06-08  9:06 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

Allow define access privileges for all spaces, functions and sequences.
Read and write privileges are supported for spaces, execute privilege
for sequences. Privilege granting and revoking might be done through old api
without object identification:
  box.schema.user.grant("guest", "read", "space")

Prerequisite #945
---
 src/box/alter.cc            | 18 ++++++---
 src/box/call.c              |  2 +
 src/box/lua/schema.lua      |  9 +++++
 src/box/schema.cc           |  8 ++++
 src/box/schema.h            | 26 +++++++++++++
 src/box/sequence.c          |  1 +
 src/box/space.c             |  2 +
 src/box/sysview_index.c     | 11 ++++++
 src/box/user.cc             | 12 ++++++
 test/box/access.result      | 77 +++++++++++++++++++++++++++++++++++++
 test/box/access.test.lua    | 29 ++++++++++++++
 test/box/lua/identifier.lua |  1 -
 12 files changed, 189 insertions(+), 7 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 6f6fcb097..48af128f1 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -2523,8 +2523,10 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 		break;
 	case SC_SPACE:
 	{
-		struct space *space = space_cache_find_xc(priv->object_id);
-		if (space->def->uid != grantor->def->uid &&
+		struct space *space = NULL;
+		if (priv->object_id != 0)
+			space = space_cache_find_xc(priv->object_id);
+		if ((space == NULL || space->def->uid != grantor->def->uid) &&
 		    grantor->def->uid != ADMIN) {
 			tnt_raise(AccessDeniedError,
 				  priv_name(priv_type),
@@ -2535,8 +2537,10 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_FUNCTION:
 	{
-		struct func *func = func_cache_find(priv->object_id);
-		if (func->def->uid != grantor->def->uid &&
+		struct func *func = NULL;
+		if (priv->object_id != 0)
+			func = func_cache_find(priv->object_id);
+		if ((func == NULL || func->def->uid != grantor->def->uid) &&
 		    grantor->def->uid != ADMIN) {
 			tnt_raise(AccessDeniedError,
 				  priv_name(priv_type),
@@ -2547,8 +2551,10 @@ priv_def_check(struct priv_def *priv, enum priv_type priv_type)
 	}
 	case SC_SEQUENCE:
 	{
-		struct sequence *seq = sequence_cache_find(priv->object_id);
-		if (seq->def->uid != grantor->def->uid &&
+		struct sequence *seq = NULL;
+		if (priv->object_id != 0)
+			seq = sequence_cache_find(priv->object_id);
+		if ((seq == NULL || seq->def->uid != grantor->def->uid) &&
 		    grantor->def->uid != ADMIN) {
 			tnt_raise(AccessDeniedError,
 				  priv_name(priv_type),
diff --git a/src/box/call.c b/src/box/call.c
index 6388e1e68..8a43db130 100644
--- a/src/box/call.c
+++ b/src/box/call.c
@@ -71,6 +71,8 @@ access_check_func(const char *name, uint32_t name_len, struct func **funcp)
 		return 0;
 	}
 	user_access_t access = PRIV_X | PRIV_U;
+	/* Check access for all functions. */
+	access &= ~get_entity_access(SC_FUNCTION)[credentials->auth_token].effective;
 	user_access_t func_access = access & ~credentials->universal_access;
 	if (func == NULL ||
 	    /* Check for missing Usage access, ignore owner rights. */
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 4455b5e42..9cf980d11 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -1794,6 +1794,9 @@ local function object_resolve(object_type, object_name)
         return 0
     end
     if object_type == 'space' then
+        if object_name == nil or object_name == 0 then
+            return 0
+        end
         local space = box.space[object_name]
         if  space == nil then
             box.error(box.error.NO_SUCH_SPACE, object_name)
@@ -1801,6 +1804,9 @@ local function object_resolve(object_type, object_name)
         return space.id
     end
     if object_type == 'function' then
+        if object_name == nil or object_name == 0 then
+            return 0
+        end
         local _vfunc = box.space[box.schema.VFUNC_ID]
         local func
         if type(object_name) == 'string' then
@@ -1815,6 +1821,9 @@ local function object_resolve(object_type, object_name)
         end
     end
     if object_type == 'sequence' then
+        if object_name == nil or object_name == 0 then
+            return 0
+        end
         local seq = sequence_resolve(object_name)
         if seq == nil then
             box.error(box.error.NO_SUCH_SEQUENCE, object_name)
diff --git a/src/box/schema.cc b/src/box/schema.cc
index 8df4aa73b..32d7791a6 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -69,6 +69,8 @@ struct rlist on_alter_sequence = RLIST_HEAD_INITIALIZER(on_alter_sequence);
  */
 struct latch schema_lock = LATCH_INITIALIZER(schema_lock);
 
+struct entity_access entity_access;
+
 bool
 space_is_system(struct space *space)
 {
@@ -528,6 +530,8 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
 		return "";
 	case SC_SPACE:
 		{
+			if (object_id == 0)
+				return "SPACE";
 			struct space *space = space_by_id(object_id);
 			if (space == NULL)
 				break;
@@ -535,6 +539,8 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
 		}
 	case SC_FUNCTION:
 		{
+			if (object_id == 0)
+				return "FUNCTION";
 			struct func *func = func_by_id(object_id);
 			if (func == NULL)
 				break;
@@ -542,6 +548,8 @@ schema_find_name(enum schema_object_type type, uint32_t object_id)
 		}
 	case SC_SEQUENCE:
 		{
+			if (object_id == 0)
+				return "SEQUENCE";
 			struct sequence *seq = sequence_by_id(object_id);
 			if (seq == NULL)
 				break;
diff --git a/src/box/schema.h b/src/box/schema.h
index 2b87f5f50..d0bb8f200 100644
--- a/src/box/schema.h
+++ b/src/box/schema.h
@@ -235,4 +235,30 @@ struct on_access_denied_ctx {
 	const char *object_name;
 };
 
+/** Global grants to classes of objects. */
+struct entity_access {
+       struct access space[BOX_USER_MAX];
+       struct access function[BOX_USER_MAX];
+       struct access sequence[BOX_USER_MAX];
+};
+
+/** A single instance of the global entities. */
+extern struct entity_access entity_access;
+
+static inline
+struct access *
+get_entity_access(enum schema_object_type type)
+{
+       switch (type) {
+       case SC_SPACE:
+               return entity_access.space;
+       case SC_FUNCTION:
+               return entity_access.function;
+       case SC_SEQUENCE:
+               return entity_access.sequence;
+       default:
+               return NULL;
+       }
+}
+
 #endif /* INCLUDES_TARANTOOL_BOX_SCHEMA_H */
diff --git a/src/box/sequence.c b/src/box/sequence.c
index 162147cde..a56271971 100644
--- a/src/box/sequence.c
+++ b/src/box/sequence.c
@@ -250,6 +250,7 @@ access_check_sequence(struct sequence *seq)
 
 	user_access_t access = PRIV_U | PRIV_W;
 	user_access_t sequence_access = access & ~cr->universal_access;
+	sequence_access &= ~get_entity_access(SC_SEQUENCE)[cr->auth_token].effective;
 	if (sequence_access &&
 	    /* Check for missing Usage access, ignore owner rights. */
 	    (sequence_access & PRIV_U ||
diff --git a/src/box/space.c b/src/box/space.c
index 02a97926e..eae48cdfd 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -42,6 +42,7 @@
 #include "request.h"
 #include "xrow.h"
 #include "iproto_constants.h"
+#include "schema.h"
 
 int
 access_check_space(struct space *space, user_access_t access)
@@ -57,6 +58,7 @@ access_check_space(struct space *space, user_access_t access)
 	 * since ADMIN has universal access.
 	 */
 	user_access_t space_access = access & ~cr->universal_access;
+	space_access &= ~get_entity_access(SC_SPACE)[cr->auth_token].effective;
 
 	if (space_access &&
 	    /* Check for missing Usage access, ignore owner rights. */
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index f1dfbefe5..b9c837441 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -222,6 +222,9 @@ vspace_filter(struct space *source, struct tuple *tuple)
 	 */
 	if (PRIV_WRDA & cr->universal_access)
 		return true;
+	/* Allow access for a user with space privileges. */
+	if (PRIV_WRDA & get_entity_access(SC_SPACE)[cr->auth_token].effective)
+		return true;
 	if (PRIV_R & source->access[cr->auth_token].effective)
 		return true; /* read access to _space space */
 	uint32_t space_id;
@@ -295,6 +298,10 @@ vfunc_filter(struct space *source, struct tuple *tuple)
 	 */
 	if ((PRIV_WRDA | PRIV_X) & cr->universal_access)
 		return true;
+	/* Allow access for a user with function privileges. */
+	if ((PRIV_WRDA | PRIV_X) &
+	    get_entity_access(SC_FUNCTION)[cr->auth_token].effective)
+		return true;
 	if (PRIV_R & source->access[cr->auth_token].effective)
 		return true; /* read access to _func space */
 
@@ -320,6 +327,10 @@ vsequence_filter(struct space *source, struct tuple *tuple)
 	 */
 	if ((PRIV_WRDA | PRIV_X) & cr->universal_access)
 		return true;
+	/* Allow access for a user with sequence privileges. */
+	if ((PRIV_WRDA | PRIV_X) &
+	    get_entity_access(SC_SEQUENCE)[cr->auth_token].effective)
+		return true;
 	if (PRIV_R & source->access[cr->auth_token].effective)
 		return true; /* read access to _sequence space */
 
diff --git a/src/box/user.cc b/src/box/user.cc
index 7fa66da8f..fbf06566a 100644
--- a/src/box/user.cc
+++ b/src/box/user.cc
@@ -209,6 +209,10 @@ access_find(struct priv_def *priv)
 	}
 	case SC_SPACE:
 	{
+		if (priv->object_id == 0) {
+			access = entity_access.space;
+			break;
+		}
 		struct space *space = space_by_id(priv->object_id);
 		if (space)
 			access = space->access;
@@ -216,6 +220,10 @@ access_find(struct priv_def *priv)
 	}
 	case SC_FUNCTION:
 	{
+		if (priv->object_id == 0) {
+			access = entity_access.function;
+			break;
+		}
 		struct func *func = func_by_id(priv->object_id);
 		if (func)
 			access = func->access;
@@ -223,6 +231,10 @@ access_find(struct priv_def *priv)
 	}
 	case SC_SEQUENCE:
 	{
+		if (priv->object_id == 0) {
+			access = entity_access.sequence;
+			break;
+		}
 		struct sequence *seq = sequence_by_id(priv->object_id);
 		if (seq)
 			access = seq->access;
diff --git a/test/box/access.result b/test/box/access.result
index 72f91173b..ccd0b737f 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -1662,3 +1662,80 @@ box.schema.user.grant("guest", "read,write,execute", "role")
 ---
 - error: Unsupported role privilege 'read,write,execute'
 ...
+-- Check entities DML
+box.schema.user.create("tester", { password  = '123' })
+---
+...
+s = box.schema.space.create("test")
+---
+...
+_ = s:create_index("primary", {parts={1, "unsigned"}})
+---
+...
+seq = box.schema.sequence.create("test")
+---
+...
+box.schema.func.create("func")
+---
+...
+c = (require 'net.box').connect(LISTEN.host, LISTEN.service, {user='tester', password = '123'})
+---
+...
+box.session.su("tester", s.select, s)
+---
+- error: Read access to space 'test' is denied for user 'tester'
+...
+box.session.su("tester", seq.set, seq, 1)
+---
+- error: Write access to sequence 'test' is denied for user 'tester'
+...
+c:call("func")
+---
+- error: Execute access to function 'func' is denied for user 'tester'
+...
+box.schema.user.grant("tester", "read", "space")
+---
+...
+box.schema.user.grant("tester", "write", "sequence")
+---
+...
+box.schema.user.grant("tester", "execute", "function")
+---
+...
+box.session.su("tester", s.select, s)
+---
+- []
+...
+box.session.su("tester", seq.next, seq)
+---
+- 1
+...
+c:call("func")
+---
+...
+box.session.su("tester", s.insert, s, {1})
+---
+- error: Write access to space 'test' is denied for user 'tester'
+...
+box.schema.user.grant("tester", "write", "space")
+---
+...
+box.session.su("tester", s.insert, s, {1})
+---
+- [1]
+...
+box.schema.user.drop("tester")
+---
+...
+s:drop()
+---
+...
+seq:drop()
+---
+...
+box.schema.func.drop("func")
+---
+...
+c:close()
+---
+...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index 62691c471..8d90b3813 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -640,3 +640,32 @@ box.schema.user.grant("guest", "alter", "function")
 box.schema.user.grant("guest", "execute", "sequence")
 box.schema.user.grant("guest", "read,execute", "sequence")
 box.schema.user.grant("guest", "read,write,execute", "role")
+
+-- Check entities DML
+box.schema.user.create("tester", { password  = '123' })
+s = box.schema.space.create("test")
+_ = s:create_index("primary", {parts={1, "unsigned"}})
+seq = box.schema.sequence.create("test")
+box.schema.func.create("func")
+c = (require 'net.box').connect(LISTEN.host, LISTEN.service, {user='tester', password = '123'})
+
+box.session.su("tester", s.select, s)
+box.session.su("tester", seq.set, seq, 1)
+c:call("func")
+box.schema.user.grant("tester", "read", "space")
+box.schema.user.grant("tester", "write", "sequence")
+box.schema.user.grant("tester", "execute", "function")
+box.session.su("tester", s.select, s)
+box.session.su("tester", seq.next, seq)
+c:call("func")
+
+box.session.su("tester", s.insert, s, {1})
+box.schema.user.grant("tester", "write", "space")
+box.session.su("tester", s.insert, s, {1})
+
+box.schema.user.drop("tester")
+s:drop()
+seq:drop()
+box.schema.func.drop("func")
+c:close()
+
diff --git a/test/box/lua/identifier.lua b/test/box/lua/identifier.lua
index c3149bce7..0cfb9e722 100644
--- a/test/box/lua/identifier.lua
+++ b/test/box/lua/identifier.lua
@@ -36,7 +36,6 @@ invalid_testcases = {
 
 function run_test(create_func, cleanup_func)
     local json = require("json")
-    print("loosadlalsd")
     local bad_tests = {}
     for i, identifier in ipairs(valid_testcases) do
         local ok, res = pcall(create_func,identifier)
-- 
2.17.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] Re: [PATCH 1/3] box: Add privilleges constants to lua
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua Georgy Kirichenko
@ 2018-06-08 10:31   ` Vladislav Shpilevoy
  2018-06-08 13:20     ` Konstantin Osipov
  0 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy @ 2018-06-08 10:31 UTC (permalink / raw)
  To: tarantool-patches, Georgy Kirichenko; +Cc: imarkov

Hello. Please, do not change older upgrade functions in
upgrade.lua. They are not executed on new versions, and
so _priv is not updated.

Because of such updates now 1.8.2 and 1.8.4 upgrade to
2.1.0 does not work.

On 08/06/2018 12:06, Georgy Kirichenko wrote:
> From: imarkov <imarkov@tarantool.org>
> 
> Add lua bindings of PRIV_XXX constants.
> 
> This patch helps to avoid using numerical constants of privilleges
> in schema.lua code.
> 
> Relates #945
> ---
>   src/box/lua/schema.lua       | 67 ++++++++++++++++++++++++++++--------
>   src/box/lua/upgrade.lua      | 40 +++++++++++----------
>   test/box/misc.result         |  1 +
>   test/engine/iterator.result  |  2 +-
>   test/engine/savepoint.result | 12 +++----
>   5 files changed, 82 insertions(+), 40 deletions(-)
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index d6d39170f..43c7d4e6b 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -101,8 +101,47 @@ ffi.cdef[[
>   
>       void password_prepare(const char *password, int len,
>                             char *out, int out_len);
> +
> +    enum priv_type {
> +        PRIV_R = 1,
> +        PRIV_W = 2,
> +        PRIV_X = 4,
> +        PRIV_S = 8,
> +        PRIV_U = 16,
> +        PRIV_C = 32,
> +        PRIV_D = 64,
> +        PRIV_A = 128,
> +        PRIV_REFERENCE = 256,
> +        PRIV_TRIGGER = 512,
> +        PRIV_INSERT = 1024,
> +        PRIV_UPDATE = 2048,
> +        PRIV_DELETE = 4096,
> +        PRIV_GRANT = 8192,
> +        PRIV_REVOKE = 16384,
> +        PRIV_ALL  = 4294967295
> +    };
> +
>   ]]
>   
> +box.priv = {
> +    ["R"] = builtin.PRIV_R,
> +    ["W"] = builtin.PRIV_W,
> +    ["X"] = builtin.PRIV_X,
> +    ["S"] = builtin.PRIV_S,
> +    ["U"] = builtin.PRIV_U,
> +    ["C"] = builtin.PRIV_C,
> +    ["D"] = builtin.PRIV_D,
> +    ["A"] = builtin.PRIV_A,
> +    ["REFERENCE"] = builtin.PRIV_REFERENCE,
> +    ["TRIGGER"] = builtin.PRIV_TRIGGER,
> +    ["INSERT"] = builtin.PRIV_INSERT,
> +    ["UPDATE"] = builtin.PRIV_UPDATE,
> +    ["DELETE"] = builtin.PRIV_DELETE,
> +    ["GRANT"]= builtin.PRIV_GRANT,
> +    ["REVOKE"] = builtin.PRIV_REVOKE,
> +    ["ALL"] = builtin.PRIV_ALL
> +}
> +
>   local function user_or_role_resolve(user)
>       local _vuser = box.space[box.schema.VUSER_ID]
>       local tuple
> @@ -1687,7 +1726,7 @@ end
>   
>   local function checked_privilege(privilege, object_type)
>       local priv_hex = privilege_resolve(privilege)
> -    if object_type == 'role' and priv_hex ~= 4 then
> +    if object_type == 'role' and priv_hex ~= box.priv.X then
>           box.error(box.error.UNSUPPORTED_ROLE_PRIV, privilege)
>       end
>       return priv_hex
> @@ -1695,43 +1734,43 @@ end
>   
>   local function privilege_name(privilege)
>       local names = {}
> -    if bit.band(privilege, 1) ~= 0 then
> +    if bit.band(privilege, box.priv.R) ~= 0 then
>           table.insert(names, "read")
>       end
> -    if bit.band(privilege, 2) ~= 0 then
> +    if bit.band(privilege, box.priv.W) ~= 0 then
>           table.insert(names, "write")
>       end
> -    if bit.band(privilege, 4) ~= 0 then
> +    if bit.band(privilege, box.priv.X) ~= 0 then
>           table.insert(names, "execute")
>       end
> -    if bit.band(privilege, 8) ~= 0 then
> +    if bit.band(privilege, box.priv.S) ~= 0 then
>           table.insert(names, "session")
>       end
> -    if bit.band(privilege, 16) ~= 0 then
> +    if bit.band(privilege, box.priv.U) ~= 0 then
>           table.insert(names, "usage")
>       end
> -    if bit.band(privilege, 32) ~= 0 then
> +    if bit.band(privilege, box.priv.C) ~= 0 then
>           table.insert(names, "create")
>       end
> -    if bit.band(privilege, 64) ~= 0 then
> +    if bit.band(privilege, box.priv.D) ~= 0 then
>           table.insert(names, "drop")
>       end
> -    if bit.band(privilege, 128) ~= 0 then
> +    if bit.band(privilege, box.priv.A) ~= 0 then
>           table.insert(names, "alter")
>       end
> -    if bit.band(privilege, 256) ~= 0 then
> +    if bit.band(privilege, box.priv.REFERENCE) ~= 0 then
>           table.insert(names, "reference")
>       end
> -    if bit.band(privilege, 512) ~= 0 then
> +    if bit.band(privilege, box.priv.TRIGGER) ~= 0 then
>           table.insert(names, "trigger")
>       end
> -    if bit.band(privilege, 1024) ~= 0 then
> +    if bit.band(privilege, box.priv.INSERT) ~= 0 then
>           table.insert(names, "insert")
>       end
> -    if bit.band(privilege, 2048) ~= 0 then
> +    if bit.band(privilege, box.priv.UPDATE) ~= 0 then
>           table.insert(names, "update")
>       end
> -    if bit.band(privilege, 4096) ~= 0 then
> +    if bit.band(privilege, box.priv.DELETE) ~= 0 then
>           table.insert(names, "delete")
>       end
>       return table.concat(names, ",")
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index 589161944..0293f6ef8 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -211,11 +211,11 @@ local function initial()
>       log.info("create role public")
>       _user:insert{PUBLIC, ADMIN, 'public', 'role'}
>       log.info("grant read,write,execute on universe to admin")
> -    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
>   
>       -- grant role 'public' to 'guest'
>       log.info("grant role public to guest")
> -    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
> +    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
>   
>       log.info("set max_id to box.schema.SYSTEM_ID_MAX")
>       _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
> @@ -406,7 +406,7 @@ local function create_sysview(source_id, target_id)
>       -- public can read system views
>       if box.space._priv.index.primary:count({PUBLIC, 'space', target_id}) == 0 then
>           log.info("grant read access to 'public' role for %s view", def[3])
> -        box.space._priv:insert({1, PUBLIC, 'space', target_id, 1})
> +        box.space._priv:insert({1, PUBLIC, 'space', target_id, box.priv.R})
>       end
>   end
>   
> @@ -416,16 +416,17 @@ local function upgrade_users_to_1_6_8()
>           local RPL_ID = box.space._user:auto_increment{ADMIN, 'replication', 'role'}[1]
>           -- replication can read the entire universe
>           log.info("grant read on universe to replication")
> -        box.space._priv:replace{1, RPL_ID, 'universe', 0, 1}
> +        box.space._priv:replace{1, RPL_ID, 'universe', 0, box.priv.R}
>           -- replication can append to '_cluster' system space
>           log.info("grant write on space _cluster to replication")
> -        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, 2}
> +        box.space._priv:replace{1, RPL_ID, 'space', box.space._cluster.id, box.priv.W}
>       end
>   
>       if box.space._priv.index.primary:count({ADMIN, 'universe', 0}) == 0 then
>           -- grant admin access to the universe
>           log.info("grant all on universe to admin")
> -        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +        box.space._priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R +
> +                                                        box.priv.W + box.priv.X}
>       end
>   
>       if box.space._func.index.name:count("box.schema.user.info") == 0 then
> @@ -435,7 +436,7 @@ local function upgrade_users_to_1_6_8()
>   
>           -- grant 'public' role access to 'box.schema.user.info' function
>           log.info('grant execute on function "box.schema.user.info" to public')
> -        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
> +        box.space._priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
>       end
>   end
>   
> @@ -555,7 +556,7 @@ local function create_truncate_space()
>       box.space._index:insert{_truncate.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}}
>   
>       local _priv = box.space[box.schema.PRIV_ID]
> -    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>   end
>   
>   local function update_existing_users_to_1_7_5()
> @@ -809,20 +810,20 @@ local function initial_1_7_5()
>       -- Create grants
>       --
>       log.info("grant read,write,execute on universe to admin")
> -    _priv:insert{ADMIN, ADMIN, 'universe', 0, 7}
> +    _priv:insert{ADMIN, ADMIN, 'universe', 0, box.priv.R + box.priv.W + box.priv.X}
>   
>       -- grant role 'public' to 'guest'
>       log.info("grant role public to guest")
> -    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, 4}
> +    _priv:insert{ADMIN, GUEST, 'role', PUBLIC, box.priv.X}
>   
>       -- replication can read the entire universe
>       log.info("grant read on universe to replication")
> -    _priv:replace{ADMIN, REPLICATION, 'universe', 0, 1}
> +    _priv:replace{ADMIN, REPLICATION, 'universe', 0, box.priv.R}
>       -- replication can append to '_cluster' system space
>       log.info("grant write on space _cluster to replication")
> -    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, 2}
> +    _priv:replace{ADMIN, REPLICATION, 'space', _cluster.id, box.priv.W}
>   
> -    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>   
>       -- create "box.schema.user.info" function
>       log.info('create function "box.schema.user.info" with setuid')
> @@ -830,7 +831,7 @@ local function initial_1_7_5()
>   
>       -- grant 'public' role access to 'box.schema.user.info' function
>       log.info('grant execute on function "box.schema.user.info" to public')
> -    _priv:replace{ADMIN, PUBLIC, 'function', 1, 4}
> +    _priv:replace{ADMIN, PUBLIC, 'function', 1, box.priv.X}
>   
>       log.info("set max_id to box.schema.SYSTEM_ID_MAX")
>       _schema:insert{'max_id', box.schema.SYSTEM_ID_MAX}
> @@ -904,7 +905,7 @@ local function create_collation_space()
>       box.space._collation:replace{2, "unicode_ci", ADMIN, "ICU", "", {strength='primary'}}
>   
>       local _priv = box.space[box.schema.PRIV_ID]
> -    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, 2}
> +    _priv:insert{ADMIN, PUBLIC, 'space', _collation.id, box.priv.W}
>   end
>   
>   local function upgrade_to_1_7_6()
> @@ -924,7 +925,8 @@ local function upgrade_to_1_7_7()
>       --
>       for _, v in _user:pairs() do
>           if v[4] ~= "role" then
> -            _priv:upsert({ADMIN, v[1], "universe", 0, 24}, {{"|", 5, 24}})
> +            _priv:upsert({ADMIN, v[1], "universe", 0, box.priv.S + box.priv.U},
> +                                                {{"|", 5, box.priv.S + box.priv.U}})
>           end
>       end
>       --
> @@ -935,14 +937,14 @@ local function upgrade_to_1_7_7()
>       --
>       for _, v in _priv.index.object:pairs{'universe'} do
>           if bit.band(v[5], 1) ~= 0 and bit.band(v[5], 2) ~= 0 then
> -            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, 32}})
> +            _priv:update({v[2], v[3], v[4]}, {{ "|", 5, box.priv.C}})
>           end
>       end
>       -- grant admin all new privileges (session, usage, grant option,
>       -- create, alter, drop and anything that might come up in the future
>       --
> -    _priv:upsert({ADMIN, ADMIN, 'universe', 0, 4294967295},
> -                 {{ "|", 5, 4294967295}})
> +    _priv:upsert({ADMIN, ADMIN, 'universe', 0, box.priv.ALL},
> +                 {{ "|", 5, box.priv.ALL}})
>       --
>       -- create role 'super' and grant it all privileges on universe
>       --
> diff --git a/test/box/misc.result b/test/box/misc.result
> index 8f94f5513..c6e4917bf 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -68,6 +68,7 @@ t
>     - info
>     - internal
>     - once
> +  - priv
>     - rollback
>     - rollback_to_savepoint
>     - runtime
> diff --git a/test/engine/iterator.result b/test/engine/iterator.result
> index ae14c4320..1bde10eaf 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:993: usage: next(param, state)'
> +- error: 'builtin/box/schema.lua:1032: usage: next(param, state)'
>   ...
>   value
>   ---
> diff --git a/test/engine/savepoint.result b/test/engine/savepoint.result
> index dc2ad7986..a62a2e135 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:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- error: 'builtin/box/schema.lua:340: 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:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok2, errmsg2
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok3, errmsg3
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok4, errmsg4
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   ok5, errmsg5
>   ---
>   - false
> -- 'builtin/box/schema.lua:301: Usage: box.rollback_to_savepoint(savepoint)'
> +- 'builtin/box/schema.lua:340: Usage: box.rollback_to_savepoint(savepoint)'
>   ...
>   s:select{}
>   ---
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] Re: [PATCH 1/3] box: Add privilleges constants to lua
  2018-06-08 10:31   ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-06-08 13:20     ` Konstantin Osipov
  0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Osipov @ 2018-06-08 13:20 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko, imarkov

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/06/08 14:43]:
> Hello. Please, do not change older upgrade functions in
> upgrade.lua. They are not executed on new versions, and
> so _priv is not updated.

Vlad, this change is cosmetic, it doesn't change what the function
does.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] Re: [PATCH 2/3] security: add limits on object_type-privilege pair
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 2/3] security: add limits on object_type-privilege pair Georgy Kirichenko
@ 2018-06-08 14:01   ` Konstantin Osipov
  0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Osipov @ 2018-06-08 14:01 UTC (permalink / raw)
  To: tarantool-patches; +Cc: imarkov

* Georgy Kirichenko <georgy@tarantool.org> [18/06/08 12:11]:
> From: imarkov <imarkov@tarantool.org>
> 
> Introduce constraints on object_type-privilege pairs.
> These constraints limit senseless grants/revokes, i.e.,
> sequence - execute, all space related privileges(insert, delete,
> update),
> function - alter, all space related privileges,
> role - all privileges except create, drop, alter, execute

Sorry for nitpicking, but wouldn't it be better to 
list allowed privileges rather than forbidden ones?

Perhaps making a plain C array which would map object type to the
list of allowed bits and exporting it to Lua make things even
simpler?

> 

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [tarantool-patches] Re: [PATCH 3/3] Introduce privileges for object groups
  2018-06-08  9:06 ` [tarantool-patches] [PATCH 3/3] Introduce privileges for object groups Georgy Kirichenko
@ 2018-06-08 17:26   ` Konstantin Osipov
  0 siblings, 0 replies; 8+ messages in thread
From: Konstantin Osipov @ 2018-06-08 17:26 UTC (permalink / raw)
  To: tarantool-patches; +Cc: Georgy Kirichenko

* Georgy Kirichenko <georgy@tarantool.org> [18/06/08 12:11]:
> Allow define access privileges for all spaces, functions and sequences.
> Read and write privileges are supported for spaces, execute privilege
> for sequences. Privilege granting and revoking might be done through old api
> without object identification:
>   box.schema.user.grant("guest", "read", "space")

The patch looks good, I don't understand though why we need it - I
never asked for one.

Does it have a documentation request?

A few minor comments below.
> 
> Prerequisite #945
> ---
>  src/box/alter.cc            | 18 ++++++---
>  src/box/call.c              |  2 +
>  src/box/lua/schema.lua      |  9 +++++
>  src/box/schema.cc           |  8 ++++
>  src/box/schema.h            | 26 +++++++++++++
>  src/box/sequence.c          |  1 +
>  src/box/space.c             |  2 +
>  src/box/sysview_index.c     | 11 ++++++
>  src/box/user.cc             | 12 ++++++
>  test/box/access.result      | 77 +++++++++++++++++++++++++++++++++++++
>  test/box/access.test.lua    | 29 ++++++++++++++
>  test/box/lua/identifier.lua |  1 -
>  12 files changed, 189 insertions(+), 7 deletions(-)
> 
> diff --git a/src/box/call.c b/src/box/call.c
> index 6388e1e68..8a43db130 100644
> --- a/src/box/call.c
> +++ b/src/box/call.c
> @@ -71,6 +71,8 @@ access_check_func(const char *name, uint32_t name_len, struct func **funcp)
>  		return 0;
>  	}
>  	user_access_t access = PRIV_X | PRIV_U;
> +	/* Check access for all functions. */
> +	access &= ~get_entity_access(SC_FUNCTION)[credentials->auth_token].effective;

1) Are you sure get_entity_access() is inlined? This is a hot
path I would try to make it as fast as possible.

2) the name should be entity_access_get() or entity_access_find().

> @@ -295,6 +298,10 @@ vfunc_filter(struct space *source, struct tuple *tuple)
>  	 */
>  	if ((PRIV_WRDA | PRIV_X) & cr->universal_access)
>  		return true;
> +	/* Allow access for a user with function privileges. */
> +	if ((PRIV_WRDA | PRIV_X) &
> +	    get_entity_access(SC_FUNCTION)[cr->auth_token].effective)
> +		return true;

Perhaps we should have a variable for PRIV_WRDA | PRIV_X


Please add a test case checking that granting entity access via
role works.

We also need a test case that revoking entity access works. The
same is for revoking entity access granted via role.

Thank you for working on it,

-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-06-08 17:26 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-08  9:06 [tarantool-patches] [PATCH 0/3] Object group privileges Georgy Kirichenko
2018-06-08  9:06 ` [tarantool-patches] [PATCH 1/3] box: Add privilleges constants to lua Georgy Kirichenko
2018-06-08 10:31   ` [tarantool-patches] " Vladislav Shpilevoy
2018-06-08 13:20     ` Konstantin Osipov
2018-06-08  9:06 ` [tarantool-patches] [PATCH 2/3] security: add limits on object_type-privilege pair Georgy Kirichenko
2018-06-08 14:01   ` [tarantool-patches] " Konstantin Osipov
2018-06-08  9:06 ` [tarantool-patches] [PATCH 3/3] Introduce privileges for object groups Georgy Kirichenko
2018-06-08 17:26   ` [tarantool-patches] " Konstantin Osipov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox