Thread

Re: [v9.1] sepgsql - userspace access vector cache

Kohei KaiGai <kaigai@kaigai.gr.jp> — 2011-08-26T09:32:59Z

Robert, Thanks for your reviewing.

> For me, the line you removed from dml.out causes the regression tests to fail.
>
Fixed. Why did I removed this line??

> I don't understand what this is going for:
>
> +       /*
> +        * To boost up trusted procedure checks on db_procedure object
> +        * class, we also confirm the decision when user calls a procedure
> +        * labeled as 'tcontext'.
> +        */
>
> Can you explain?
>
Yes. It also caches an expected security label when a client being
labeled as "scontext" tries to execute a procedure being labeled as
"tcontext", to reduce number of system call invocations on fmgr_hook
and needs_fmgr_hook.
If the expected security label is not same with "scontext", it means
the procedure performs as a trusted procedure that switches security
label of the client during its execution; like a security invoker
function.
A pair of security labels are the only factor to determine whether the
procedure is a trusted-procedure, or not. Thus, it is suitable to
cache in userspace avc.

As an aside, the reason why we don't cache the default security label
being assigned on newly created named objects (such as tables, ...) is
that selinux allows to set up exceptional default security label on a
particular name, so it does not suitable for avc structure.
(I'm waiting for getting included this interface into libselinux.)

> sepgsql_avc_check_perms_label has a formatting error on the line that
> says "result = false".  It's not indented correctly.
>
OK, I fixed it.

> Several functions do this: sepgsql_avc_check_valid(); do { ... } while
> (!sepgsql_avc_check_valid);  I don't understand why we need a loop
> there.
>
It enables to prevent inconsistent access control decision from
concurrent security policy reloading.
I want the following steps being executed in atomic.
 1) Lookup object class number in kernel-side
 2) Lookup permission bits in kernel-side
 3) Ask kernel-side its access control decision.

The selinux_status_update returns 1, if any status of selinux in
kernel side (that requires to flush userspace caches) had been changed
since the last invocation.
In this case, we retry whole of the process from the beginning to
ensure whole of access control decision being made by either old or
new policy.
Thus, I enclosed these blocks by do {...} while() loop.

> The comment for sepgql_avc_check_perms_label uses the word "elsewhere"
> when it really means "otherwise".
>
OK, I fixed it.

> Changing the calling sequence of sepgsql_get_label() would perhaps be
> better separated out into its own patch.
>
OK, I reverted it.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>