Security #145

coderoshi · 2013-10-02T20:17:17Z

Added two new permissions:

yokozuna.search
yokozuna.admin

The first permission is read access for an index. Currently it does not support types, only bucket name which represent an index. After speaking with @Vagabond I'm going to expand this to support a default type index, so you could grant a user search access like this:

riak-admin security grant yokozuna.search ON index indexName TO userName

We may wish to make this a reserved word for bucket types.

The admin permissions are universal per type index or schema. Meaning, you have access to index, you can read, list, create and delete them. For example:

riak-admin security grant yokozuna.admin ON index TO userName
riak-admin security grant yokozuna.admin ON schema TO userName

We may find a need to break these up later into more fine-grained permissions, but that seems a bit overzealous at this point in time, because it would potentially turn yokozuna's 3 permissions into 7.

coderoshi · 2013-09-17T03:12:52Z

The first pass at security is in. Looking at how to incorporate this into yokozuna, whatever that may mean.

basho/riak#355

rzezeski · 2013-10-03T17:19:26Z

After speaking with @Vagabond I'm going to expand this to support a default type index, so you could grant a user search access like this:

What is a "default type index"? Is that a bucket type or security type?

Also, if I'm reading this correctly. Where we currently use the name "bucket" in security it will actually stand for "index" when granted one of the yokozuna.* permissions? Just want to make sure I'm following correctly. Examples might help.

We may wish to make this a reserved word for bucket types.

I'm not sure I understand this fully. Are you saying that, if we add an "index" type to security that we should prevent a user from creating a bucket type with the name "index" because that would just confused things?

coderoshi · 2013-10-03T21:49:39Z

What is a "default type index"? Is that a bucket type or security type?

Bucket type. There's no such thing as a security type.

Also, if I'm reading this correctly. Where we currently use the name "bucket" in security it will actually stand for "index" when granted one of the yokozuna.* permissions? Just want to make sure I'm following correctly. Examples might help.

I'm not sure what other example you're asking for. This is standard command line:

riak-admin security grant <permissions> ON ANY|<type> [bucket] TO <users>

This is an example for adding search permission to the <type> index and [bucket] indexName:

riak-admin security grant yokozuna.search ON index indexName TO userName

Or for all indexes

riak-admin security grant yokozuna.search ON index TO userName

I'm not sure I understand this fully. Are you saying that, if we add an "index" type to security that we should prevent a user from creating a bucket type with the name "index" because that would just confused things?

Yep.

rzezeski · 2013-10-03T21:55:08Z

So "index" is a bucket type? I think I still fundamentally don't understand bucket types. To me a bucket type is a way for many buckets to share the same properties in an efficient manner. I don't see the usefulness or sense in having an index bucket type because that assumes all buckets want to use the same index. I.e. if you had the yz_index property defined on the bucket type "index" then all buckets that are indexed would all go to the same index or they would all have to overwrite this value at the bucket property/ring level. Am I misunderstanding what "type" means here?

In the security example, <type> is a bucket type?

Vagabond · 2013-10-03T22:14:00Z

You're way overthinking this. Security operates on 2-tuples, of
{ThingOne, ThingTwo}, If you grant a user permission on ThingOne, they
get implicit access for any tuple with ThingOne as the first element. If
you Grant a user a permission on a {ThingOne, ThingTwo} they only have
access to the specific element unde ThingOne. This is wildcard access vs
specific access.

riak_core_security doesn't know about bucket types, or anything, it is
just fundamentally providing access control based on a user's grants and
the permissions requested. The API can if the user has a particular
permission globally, matching just the first element of the tuple, or a
complete match.

So if you used something like "yz_index" as 'ThingOne', and made the
specific index as 'ThingTwo' you could do things like this.

GRANT riak_search.get ON yz_index TO andrew
GRANT riak_search.edit_schema ON yz_index myindex TO andrew

So I can read ANY index, but I can only edit the schema on the 'myindex'
index. The first grant has an implicit wildcard in it.

Point is, this actually has nothing to do with bucket types.

Andrew

coderoshi · 2013-10-03T22:35:12Z

Sorry @Vagabond, as you were typing this, we had taken this conversation into mumble. I think everything is getting cleared up.

Vagabond · 2013-10-21T20:32:39Z

src/yz_pb_admin.erl

+        #rpbyokozunaschemaputreq{} ->
+            {ok, Msg, {"yokozuna.schema", ?YZ_SECURITY_THING_ONE}};
+        #rpbyokozunaschemagetreq{} ->
+            {ok, Msg, {"yokozuna.schema", {?YZ_SECURITY_THING_ONE}}};


These should not be wrapped in a tuple. it should be like #rpbyokozunaschemaputreq{} and #rpbyokozunaindexdeletereq{}

See an example:

https://github.com/basho/riak_kv/blob/develop/src/riak_kv_pb_bucket.erl#L71

Thanks, typo

Vagabond · 2013-10-21T21:42:08Z

If you want to just fix the typo (which maybe the test should have caught?) I can live with the permission structure you've chosen.

rzezeski · 2013-10-26T03:38:35Z

The tests pass (I tested against this branch with a fresh merge of develop), that's a good sign. Still need to run benchmarks and run through the code. This will not make the tech preview.

yz_wm_extract_test-bitcask   : pass
yz_stat_test-bitcask         : pass
yz_solr_start_timeout-bitcask: pass
yz_siblings-bitcask          : pass
yz_security-bitcask          : pass
yz_schema_admin-bitcask      : pass
yz_rs_migration-bitcask      : fail
yz_pb-bitcask                : pass
yz_monitor_solr-bitcask      : pass
yz_mapreduce-bitcask         : pass
yz_languages-bitcask         : pass
yz_index_admin-bitcask       : pass
yz_flag_transitions-bitcask  : pass
yz_fallback-bitcask          : pass
yz_errors-bitcask            : pass
yokozuna_essential-bitcask   : pass
aae_test-bitcask             : pass

rzezeski · 2013-10-26T18:40:16Z

Is this output from print-user confusing to anyone else in the context of Yokozuna? The type is index. I think that's probably okay since we don't call it bucket type but the second column is called bucket which has a very specific meaning in Riak. So what is a user to think about a yokozuna.index grant for all buckets? That doesn't even make sense. I think I'm rehashing our old discussion about the fact that the term "bucket" is being overloaded. This is when @Vagabond explained the "thing1 vs thing2" to me. This grant is saying that the user can create indexes. But is that clear from the table below?

Applied permissions

+----------+----------+----------------------------------------+
|   type   |  bucket  |                 grants                 |
+----------+----------+----------------------------------------+
|  index   |    *     |             yokozuna.index             |
| default  |  fruit   |        riak_kv.put, riak_kv.get        |
+----------+----------+----------------------------------------+

Here is after I add the search grant. It seems like maybe that second column should have a more generic title. But I'm not sure what that is.

Applied permissions

+----------+----------+----------------------------------------+
|   type   |  bucket  |                 grants                 |
+----------+----------+----------------------------------------+
|  index   |  fruit   |            yokozuna.search             |
|  index   |    *     |             yokozuna.index             |
| default  |  fruit   |        riak_kv.put, riak_kv.get        |
+----------+----------+----------------------------------------+

Vagabond · 2013-10-26T18:57:50Z

I plan to have something like letting applications register permissions with an associated 'granularity', so kv could use 'buckets' and yz could use 'indexes' and we could print seperate tables for seperate granularities.

rzezeski · 2013-10-26T21:44:05Z

Benchmark results are in. In general indexing performance drops by about 3% with security enabled. Query by about 6%. And the patch itself doesn't seem to add any overhead. I only benchmarked protocol buffers but I would hope HTTP isn't much different. Most of the cost should be in connection establishment. Which, by the way, takes much longer now. Long lived connections will be vital with security enabled.

Indexing Results

Measurement	Security OFF	Security ON	Delta
Mean Throughput	1986 ops/s	1917 ops/s	- ~3.5%
Median Latency	12.1 ms	13.1ms	+ ~8.2%
95th Latency	38.3 ms	40.1 ms	+ ~4.7%
99th Latency	77.1 ms	72.0 ms	- ~6.6%

Query Results

Measurement	Security OFF	Security ON	Delta
Mean Throughput	2191 ops/s	2065 ops/s	- ~5.8%
Median Latency	14.8 ms	15.6 ms	+ ~5.4%
95th Latency	22.6 ms	24.3 ms	+ ~7.5%
99th Latency	27.4 ms	29.1 ms	+ ~6.2%

Before/After Patch

I wanted to make sure that the patch itself didn't incur any overhead. These results show that before the patch and with the patch with security off the results are the same. This is good.

Measurement	No Patch	Security OFF	Delta
Index Thru	1999 ops/s	1986 ops/s	- 0.7%
Query Thru	2184 ops/s	2191 ops/s	+ 0.3%

rzezeski · 2013-10-26T22:27:05Z

So I've been looking at the grants for Yokozuna and I'm not sure they really make sense to me. E.g. granting the permission yokozuna.index on index. What does that even mean? To me it should be more like grant ALL ON index ALL.

Here's the break down of Type (Thing-1) and associated grants that make sense to me. Thing-2 is used as a specific instance of the type. For most grants it probably only makes sense for that to be ALL.

Type	Grants
Index	create, get, list, search, delete
Schema	create, get

Here are some examples that I had in mind.

% grant all permissions on all indexes
grant ALL on index ALL TO user

% grant ability to create indexes, it doesn't make any sense to me to grant a user create privilage for a subset of indexes. either you can create an index or your can't. Same goes for get, list and delete.
grant create ON index ALL TO user
grant get ON index ALL TO user

% The search permission, however, can be limited to a specific index.
grant search ON index fruit TO user

% Or not.
grant search ON index ALL TO god

% Schemas are another item where either a user should have full access or none. Not per-schema permission.
grant create ON schema ALL TO user

I realize that this may not be feasible at the moment given implementation details but I really think it should look like something above. Now, if the permission need to be namespaced, e.g. yokozuna.create, then fine. I can live with that. But I strongly feel grant yokozuna.index ON index TO user is nonsensical.

coderoshi · 2013-10-26T23:03:29Z

After another tea-party gab session, we've agreed to change the index/schema permission names to one permission admin, and instead use types (thing 1's) to dictate what a permission is good for (index or schema). Example:

grant yokozuna.admin ON index TO user
grant yokozuna.admin ON schema TO user
grant yokozuna.admin ON ANY TO user (both index and schema)

yokozuna.search will remain the same.

rzezeski · 2013-10-26T23:05:14Z

After some discussion in backchannel with @coderoshi we came to the following solution.

type	grants
index	admin, search
schema	admin

The admin grant means that you can do all your basic admin stuff like put/get/list/delete.

The search grant means you can search. This only applies to the index type because it is non-sensical to "search" a schema in Yokozuna.

This would produce the following examples.

% grant admin on schemas
grant yokozuna.admin ON schema TO user

% grant admin on indexes
grant yokozuna.admin ON index TO user

% grant search on index fruit
grant yokozuna.search ON index fruit TO user

% grant search on all indexes
grant yokozuna.search ON index TO user

% or maybe ALL, for those of us that like that sorta thing
grant yokozuna.search ON index ALL TO user

Previously, permissions were search,index,schema. Search remains the same, but now admin is a permission for either the index or schema type.

Conflicts: src/yz_app.erl

rzezeski · 2013-11-01T17:26:03Z

+1 to merge.

Security

ghost assigned coderoshi Jul 23, 2013

coderoshi added 2 commits October 2, 2013 13:14

Add security to yokozuna

839a9c8

Cleanup registration

1627e18

coderoshi added 4 commits October 16, 2013 17:24

Change default type to index

88d37b2

Working security test

0619890

Added some HTTP test goodness

046365c

Merge branch 'develop' into feature/er/security

3657353

Vagabond reviewed Oct 21, 2013
View reviewed changes

fix decoding typo

059bec6

rzezeski mentioned this pull request Oct 26, 2013

Add security support to BB driver for PB conns #226

Merged

Reduce permissions to search and admin

3b16ad9

Previously, permissions were search,index,schema. Search remains the same, but now admin is a permission for either the index or schema type.

Merge remote-tracking branch 'origin/develop' into feature/er/security

cbe2a15

Conflicts: src/yz_app.erl

rzezeski added a commit that referenced this pull request Nov 4, 2013

Merge pull request #145 from basho/feature/er/security

3576727

Security

rzezeski merged commit 3576727 into develop Nov 4, 2013

rzezeski deleted the feature/er/security branch December 24, 2013 03:35

rzezeski unassigned coderoshi Feb 4, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Security #145

Security #145

coderoshi commented Oct 2, 2013

coderoshi commented Sep 17, 2013

rzezeski commented Oct 3, 2013

coderoshi commented Oct 3, 2013

rzezeski commented Oct 3, 2013

Vagabond commented Oct 3, 2013

coderoshi commented Oct 3, 2013

Vagabond Oct 21, 2013

coderoshi Oct 21, 2013

Vagabond commented Oct 21, 2013

rzezeski commented Oct 26, 2013

rzezeski commented Oct 26, 2013

Vagabond commented Oct 26, 2013

rzezeski commented Oct 26, 2013

rzezeski commented Oct 26, 2013

coderoshi commented Oct 26, 2013

rzezeski commented Oct 26, 2013

rzezeski commented Nov 1, 2013

Security #145

Security #145

Conversation

coderoshi commented Oct 2, 2013

coderoshi commented Sep 17, 2013

rzezeski commented Oct 3, 2013

coderoshi commented Oct 3, 2013

rzezeski commented Oct 3, 2013

Vagabond commented Oct 3, 2013

coderoshi commented Oct 3, 2013

Vagabond Oct 21, 2013

Choose a reason for hiding this comment

coderoshi Oct 21, 2013

Choose a reason for hiding this comment

Vagabond commented Oct 21, 2013

rzezeski commented Oct 26, 2013

rzezeski commented Oct 26, 2013

Vagabond commented Oct 26, 2013

rzezeski commented Oct 26, 2013

Indexing Results

Query Results

Before/After Patch

rzezeski commented Oct 26, 2013

coderoshi commented Oct 26, 2013

rzezeski commented Oct 26, 2013

rzezeski commented Nov 1, 2013