[tarantool-patches] Re: [security 2/3] security: Refactor reads from systems spaces

[Konstantin Osipov <kostja@tarantool.org>]

* Ilya Markov <imarkov@tarantool.org> [18/03/29 12:10]:
> +const uint32_t PRIV_WRDA = PRIV_W | PRIV_D | PRIV_A | PRIV_R;
> +
>  static bool
>  vspace_filter(struct space *source, struct tuple *tuple)

What did you need this for?


> +	if (PRIV_WRDA & cr->universal_access)
> +		return true;
> +	if (source->access[cr->auth_token].effective & PRIV_R)
> +		return true;

Stray change.

> -	return uid == cr->uid || owner_id == cr->uid;
> +	return uid == cr->uid || owner_id == cr->uid || uid == PUBLIC;

What is this change for?

> +	/* If user has global alter, drop privilege
> +	 * she may access all privileges
> +	 */
> +	if (PRIV_WRDA & cr->universal_access)
> +		return true;

The comment only talks about global alter/drop, what about global
'write'? Why did you include it into the list?

> +	if (source->access[cr->auth_token].effective & PRIV_R)
> +		return true;
> +	const char *type;
> +	uint32_t obj_id;
> +	if ((type = tuple_field_cstr(tuple, BOX_PRIV_FIELD_OBJECT_TYPE)) == NULL ||
> +		tuple_field_u32(tuple, BOX_PRIV_FIELD_OBJECT_ID, &obj_id) != 0)
> +		return false;
> +	return grantor_id == cr->uid || grantee_id == cr->uid ||
> +		(strncmp(type, "role", 4) == 0 && obj_id == PUBLIC);

What is this for?

> +	uint32_t effective = sequence->access[cr->auth_token].effective;
> +	return sequence->def->uid == cr->uid ||
> +		(PRIV_WRDA & effective);

Stray changes make things harder to follow.


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