Skip to content

Commit

Permalink
access: update credentials without reconnect
Browse files Browse the repository at this point in the history
Credentials is a cache of user universal privileges. And that
cache can become outdated in case user privs were changed after
creation of the cache.

The patch makes user update all its credentials caches with new
privileges, via a list of all creds.

That solves a couple of real life problems:

- If a user managed to connect after box.cfg started listening
port, but before access was granted, then he needed a reconnect;

- Even if access was granted, a user may connect after box.cfg
listen, but before access *is recovered* from _priv space. It
was not possible to fix without a reconnect. And this problem
affected replication.

Closes #2763
Part of #4535
Part of #4536

@TarantoolBot document
Title: User privileges update affects existing sessions and objects
Previously if user privileges were updated (via
`box.schema.user.grant/revoke`), it was not reflected in already
existing sessions and objects like functions. Now it is.

For example:
```
        box.cfg{listen = 3313}
        box.schema.user.create('test_user', {password = '1'})
        function test1() return 'success' end

        c = require('net.box').connect(box.cfg.listen, {
                user = 'test_user', password = '1'
        })
        -- Error, no access for this connection.
        c:call('test1')

        box.schema.user.grant('test_user', 'execute', 'universe')
        -- Now works, even though access was granted after
        -- connection.
        c:call('test1')
```

A similar thing happens now with `box.session.su` and functions
created via `box.schema.func.create` with `setuid` flag.

In other words, now user privileges update is reflected
everywhere immediately.

(cherry picked from commit 06dbcec)
(cherry picked from commit e5149e3)
  • Loading branch information
Gerold103 authored and kyukhin committed Oct 30, 2019
1 parent 8f6cc05 commit 8f96947
Show file tree
Hide file tree
Showing 8 changed files with 118 additions and 17 deletions.
19 changes: 11 additions & 8 deletions src/box/user.cc
Expand Up @@ -35,7 +35,7 @@
#include "func.h"
#include "index.h"
#include "bit/bit.h"
#include "session.h"
#include "fiber.h"
#include "scoped_guard.h"
#include "sequence.h"

Expand Down Expand Up @@ -159,11 +159,14 @@ user_create(struct user *user, uint8_t auth_token)
user->auth_token = auth_token;
privset_new(&user->privs);
region_create(&user->pool, &cord()->slabc);
rlist_create(&user->credentials_list);
}

static void
user_destroy(struct user *user)
{
while (!rlist_empty(&user->credentials_list))
rlist_shift(&user->credentials_list);
/*
* Sic: we don't have to remove a deleted
* user from users set of roles, since
Expand Down Expand Up @@ -281,7 +284,6 @@ access_find(enum schema_object_type object_type, uint32_t object_id)
static void
user_set_effective_access(struct user *user)
{
struct credentials *cr = effective_user();
struct privset_iterator it;
privset_ifirst(&user->privs, &it);
struct priv_def *priv;
Expand All @@ -293,11 +295,6 @@ user_set_effective_access(struct user *user)
continue;
struct access *access = &object[user->auth_token];
access->effective = access->granted | priv->access;
/** Update global access in the current session. */
if (priv->object_type == SC_UNIVERSE &&
user->def->uid == cr->uid) {
cr->universal_access = access->effective;
}
}
}

Expand Down Expand Up @@ -363,6 +360,10 @@ user_reload_privs(struct user *user)
}
user_set_effective_access(user);
user->is_dirty = false;
struct credentials *creds;
user_access_t new_access = universe.access[user->auth_token].effective;
rlist_foreach_entry(creds, &user->credentials_list, in_user)
creds->universal_access = new_access;
}

/** }}} */
Expand Down Expand Up @@ -721,6 +722,7 @@ credentials_create(struct credentials *cr, struct user *user)
cr->auth_token = user->auth_token;
cr->universal_access = universe.access[user->auth_token].effective;
cr->uid = user->def->uid;
rlist_add_entry(&user->credentials_list, cr, in_user);
}

void
Expand All @@ -729,10 +731,11 @@ credentials_create_empty(struct credentials *cr)
cr->auth_token = BOX_USER_MAX;
cr->universal_access = 0;
cr->uid = BOX_USER_MAX;
rlist_create(&cr->in_user);
}

void
credentials_destroy(struct credentials *cr)
{
(void) cr;
rlist_del_entry(cr, in_user);
}
6 changes: 6 additions & 0 deletions src/box/user.h
Expand Up @@ -88,6 +88,12 @@ struct user
bool is_dirty;
/** Memory pool for privs */
struct region pool;
/**
* List of all currently existing credentials caches of
* the user. Any update of user privileges is applied to
* them.
*/
struct rlist credentials_list;
/** Cached runtime access imformation. */
struct access access[BOX_USER_MAX];
};
Expand Down
7 changes: 7 additions & 0 deletions src/box/user_def.h
Expand Up @@ -34,6 +34,7 @@
#include "scramble.h" /* for SCRAMBLE_SIZE */
#define RB_COMPACT 1
#include "small/rb.h"
#include "small/rlist.h"

#if defined(__cplusplus)
extern "C" {
Expand Down Expand Up @@ -61,6 +62,12 @@ struct credentials {
user_access_t universal_access;
/** User id of the authenticated user. */
uint32_t uid;
/**
* Member of credentials list of the source user. The list
* is used to collect privilege updates to keep the
* credentials up to date.
*/
struct rlist in_user;
};

enum priv_type {
Expand Down
9 changes: 4 additions & 5 deletions test/box/access_bin.result
Expand Up @@ -213,9 +213,8 @@ box.space._user:replace(u)
- [1, 1, 'admin', 'user', {}]
...
--
-- Roles: test that universal access of an authenticated
-- session is not updated if grant is made from another
-- session
-- gh-2763: test that universal access of an authenticated session
-- is updated if grant is made from another session.
--
test = box.schema.space.create('test')
---
Expand Down Expand Up @@ -251,7 +250,7 @@ box.schema.role.grant('public', 'read', 'universe')
...
c.space.test:select{}
---
- error: Read access to space 'test' is denied for user 'test'
- - [1]
...
c:close()
---
Expand All @@ -268,7 +267,7 @@ box.schema.role.revoke('public', 'read', 'universe')
...
c.space.test:select{}
---
- - [1]
- error: Read access to space 'test' is denied for user 'test'
...
box.session.su('test')
---
Expand Down
5 changes: 2 additions & 3 deletions test/box/access_bin.test.lua
Expand Up @@ -81,9 +81,8 @@ c:call('dostring', { 'return 2 + 2' })
c:close()
box.space._user:replace(u)
--
-- Roles: test that universal access of an authenticated
-- session is not updated if grant is made from another
-- session
-- gh-2763: test that universal access of an authenticated session
-- is updated if grant is made from another session.
--
test = box.schema.space.create('test')
_ = test:create_index('primary')
Expand Down
2 changes: 1 addition & 1 deletion test/box/access_misc.result
Expand Up @@ -188,7 +188,7 @@ session.su('admin', box.schema.user.grant, "guest", "read", "universe")
...
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')
---
Expand Down
54 changes: 54 additions & 0 deletions test/box/gh-2763-session-credentials-update.result
@@ -0,0 +1,54 @@
-- test-run result file version 2
netbox = require('net.box')
| ---
| ...
--
-- gh-2763: when credentials of a user are updated, it should be
-- reflected in all his sessions and objects.
--

box.schema.user.create('test_user', {password = '1'})
| ---
| ...
function test1() return 'success' end
| ---
| ...

conns = {}
| ---
| ...
for i = 1, 10 do \
local c \
if i % 2 == 0 then \
c = netbox.connect( \
box.cfg.listen, {user = 'test_user', password = '1'}) \
else \
c = netbox.connect(box.cfg.listen) \
end \
local ok, err = pcall(c.call, c, 'test1') \
assert(not ok and err.code == box.error.ACCESS_DENIED) \
table.insert(conns, c) \
end
| ---
| ...

box.schema.user.grant('test_user', 'execute', 'universe')
| ---
| ...
box.schema.user.grant('guest', 'execute', 'universe')
| ---
| ...
-- Succeeds without a reconnect.
for _, c in pairs(conns) do \
assert(c:call('test1') == 'success') \
c:close() \
end
| ---
| ...

box.schema.user.revoke('guest', 'execute', 'universe')
| ---
| ...
box.schema.user.drop('test_user')
| ---
| ...
33 changes: 33 additions & 0 deletions test/box/gh-2763-session-credentials-update.test.lua
@@ -0,0 +1,33 @@
netbox = require('net.box')
--
-- gh-2763: when credentials of a user are updated, it should be
-- reflected in all his sessions and objects.
--

box.schema.user.create('test_user', {password = '1'})
function test1() return 'success' end

conns = {}
for i = 1, 10 do \
local c \
if i % 2 == 0 then \
c = netbox.connect( \
box.cfg.listen, {user = 'test_user', password = '1'}) \
else \
c = netbox.connect(box.cfg.listen) \
end \
local ok, err = pcall(c.call, c, 'test1') \
assert(not ok and err.code == box.error.ACCESS_DENIED) \
table.insert(conns, c) \
end

box.schema.user.grant('test_user', 'execute', 'universe')
box.schema.user.grant('guest', 'execute', 'universe')
-- Succeeds without a reconnect.
for _, c in pairs(conns) do \
assert(c:call('test1') == 'success') \
c:close() \
end

box.schema.user.revoke('guest', 'execute', 'universe')
box.schema.user.drop('test_user')

0 comments on commit 8f96947

Please sign in to comment.