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

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Yeb Havinga <yebhavinga(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [v9.1] sepgsql - userspace access vector cache
Date: 2011-07-18 20:21:23
Message-ID: CADyhKSVceir9fsqY4M5a9Leuu5LMwcHwWjk_QoCHt9FQz0vabQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Yeb, Thanks for your volunteering.

2011/7/18 Yeb Havinga <yebhavinga(at)gmail(dot)com>:
> Hello KaiGai-san,
>
> I've been preparing to review this patch by reading both pgsql-hackers
> history on sepgsql, and also the RHEL 6 guide on SELinux this weekend, today
> I installed GIT HEAD with --with-selinux on Scientific Linux 6, developer
> installation, so far almost everything looking good.
>
The Scientific Linux 6 is not suitable, because its libselinux version
is a bit older
than this patch expects (libselinux-2.0.99 or later).
My recommendation is Fedora 15, instead.

> 1) the line with for DBNAME in ... do postgres --single etc, lacks a -D
> argument and hence gives the error:
> postgres does not know where to find the server configuration file.
>
OK. I intended users to adjust their own paths (including -D option),
but an explicit "-D /path/to/database" seems to me more helpful as
an example.
I'll submit a patch in a separate thread.

> 2) there is a dependency to objects outside of the postgresql installation
> tree in /etc/selinux/targeted/contexts/sepgsql_contexts, and that file has
> an error that is thrown when contrib/sepgsql is executed:
> /etc/selinux/targeted/contexts/sepgsql_contexts:  line 33 has invalid object
> type db_blobs
> (same for db_language)
> I found your fix for the error on a forum on oss.tresys.com, but IMHO either
> the contrib/sepgsql should mention that the dependency exists and it might
> contain errors for (older) reference policies, or it should include a
> bugfree reference policy for sepgsql to replace the system installed
> refpolicy with (and mention that in the install documentation).
>
It is not an error, but just a notification to inform users that
sepgsql_contexts
file contains invalid lines. It is harmless, so we can ignore them.
I don't think sepgsql.sgml should mention about this noise, because it purely
come from the problem in libselinux and refpolicy; these are external packages
from viewpoint of PostgreSQL.

> 3) sepgsql is currently a bit hard to find in the documentation.
> www.postgresql.org website search doesn't find sepgsql and selinux only
> refers to an old PostgreSQL redhat bug in 2005 on bugzilla.redhat.com. I had
> to manually remember it was a contrib module. Also sepgsql isn't linked to
> at the SECURITY LABEL page. At the moment I'm unsure if I have seen all
> sepgsql related sgml-based documentation.
>
Improvement of documentation is an issue.
The wiki.postgresql.org should be an appropriate place, maybe.

The reason why SECURITY LABEL does not point to sepgsql.sgml is
that it is general purpose infrastructure for all the upcoming label based
security mechanism, not only sepgsql.
It was our consensus in v9.1 development.

> After fixing the refpolicy I proceeded with the contrib/sepgsql manual, with
> the goal to get something easy done, like create a top secret table like
> 'thisyearsbonusses', and a single user 'boss' and configure sepgsql in such
> a way that only the boss can access the top secret table. I've read the the
> contrib documentation, browsed links on the bottom of the page but until now
> I don't even have a clue how to proceed. Until I do so, I don't feel it's
> appropriate for me to review the avc patch.
>
At least, you don't need to fix the policy stuff anything.

The point of this patch is replacement of existing mechanism (that always
asks in-kernel selinux with system-call invocation) by a smart caching
mechanism (it requires minimum number of system-call invocation) without
any user-visible changes.
So, it is not necessary to define a new policy for testing.

> Would you be willing to help me getting a bit started? Specific questions
> are:
>
> 1) The contrib doc under DML permissions talks about 'db_table:select' etc?
> What are these things? They are not labels since I do not see them listed in
> the output of 'select distinct label from pg_seclabel'.
>
The security label is something like user-id or ownership/object-acl in the
default database access controls. It checks a relationship between user-id
and ownership/object-acl of the target object. If this relationship allowed
particular actions like 'select', 'update' or others, it shall be allowed when
user requires these actions.
In similar way, 'db_table:select' is a type of action; 'select' on table object,
not an identifier of user or objects.
SELinux defines a set of allowed actions (such as 'db_table:select') between
a particular pair of security labels (such as 'staff_t' and 'sepgsql_table_t').
The pg_seclabel holds only security label of object being referenced.
So, you should see /selinux/class/db_*/perms to see list of permissions
defined in the security policy (but limited number of them are in use, now).

> 2) The regression test label.sql introduces labels with types
> sepgsql_trusted_proc_exec_t, sepgsql_ro_table_t. My question is: where are
> these defined? What is their meaning? Can I define my own?
>
The system's default security policy (selinux-policy package) defines all the
necessary labeles, and access control rules between them.
So, we never need to modify security policy to run regression test.

The sepgsql_trusted_proc_exec_t means that functions labeled with this label
is a trusted procedure. It switches security label of the user during
execution of
this function. It is a similar mechanism like SetExec or security
definer function.

The sepgsql_ro_table_t means 'read-only' tables that disallow any
writer operations
except for administrative domains.

> 3) In the examples so far I've seen unconfined_u and system_u? Can I define
> my own?
>
You can define your own policy, however, I intend to run regression test
without any modification of the default security policy.

Thanks,

> Thanks,
> Yeb Havinga
>
>
> On 2011-07-14 21:46, Kohei KaiGai wrote:
>
> Sorry, the syscache part was mixed to contrib/sepgsql part
> in my previous post.
> Please see the attached revision.
>
> Although its functionality is enough simple (it just reduces
> number of system-call invocation), its performance
> improvement is obvious.
> So, I hope someone to volunteer to review these patches.
>
> Thanks,
>
> 2011/7/11 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
>
> I rebased the userspace access vector cache patch to the latest tree.
>
> I'll describe the background of this patch because this thread has not been
> active more than a week.
> The sepgsql asks in-kernel selinux when it needs to make its access control
> decison, so it always causes system call invocations.
> However, access control decision of selinux for a particular pair of
> security
> label is consistent as long as its security policy is not reloaded.
> Thus, it is a good idea to cache access control decisions recently used in
> userspace.
> In addition, current GetSecurityLabel() always open pg_seclabel catalog and
> scan to fetch security label of database objects, although it is a situation
> we
> can utilize syscache mechanism.
>
> The "uavc-syscache" patch adds a new SECLABELOID syscache.
> It also redefine pg_seclabel.provide as Name, instead of Text, according to
> the suggestion from Tom.
> (To avoid collation conscious datatype)
>
> The "uavc-selinux-cache" patch adds cache mechanism of contrib/sepgsql.
> Its internal api to communicate with selinux (sepgsql_check_perms) was
> replaced by newer sepgsql_avc_check_perms that checks cached access
> control decision at first, prior to system call invocations.
>
> The result of performance improvement is obvious.
>
> * Test 2. time to run 50,000 of SELECT from empty tables
>  selinux | SECLABELOID syscache
>  avc   |   without |   with
> ---------+-----------------------
>  without | 185.59[s] | 178.38[s]
> ---------+-----------------------
>  with    |  23.58[s] |  21.79[s]
> ---------+-----------------------
>
> I strongly hope this patch (and security label support on shared objects) to
> get unstreamed  in this commit-fest, because it will perform as a basis of
> other upcoming features.
> Please volunteer the reviewing!
>
> Thanks,
>
> 2011/7/2 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
>
> The attached patch re-defines pg_seclabel.provider as NameData, instead of
> Text,
> and revert changes of catcache.c about collations; to keep consistency with
> the
> security label support on shared objects.
> All the format changes are hidden by (Get|Set)SecurityLabel(), so no
> need to change
> on the patch to contrib/sepgsql.
>
> Thanks,
>
> 2011/6/13 Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>:
>
> 2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
>
> On Mon, Jun 13, 2011 at 7:51 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
> For syscache, length of a typical security label in selinux is
> less than 64 bytes. If we assume an entry consume 128bytes
> including Oid pairs or pointers, its consumption is 128KBytes
> per 1,000 of tables or others.
> (Do we have a way to confirm syscache status?)
>
> I was thinking you might start a new session, SELECT pg_backendd_pid()
> to get the PID, use top/ps to get its memory usage; then do a bunch of
> stuff and see how much it's grown.  The difference between how much it
> grows with and without the patch is the amount of additional memory
> the patch consumes.
>
> I checked memory consumption of the backend with / without
> patches. Because sepgsql_restorecon() tries to reset security
> label of all the schemas, relations, columns and procedures,
> an execution of this function is suitable to emphasize differences
> between two cases in maximum.
>
> The results shows us about 3MB of additional consumption
> in VmRSS, even if it caches all the security label of the objects
> being created in the default (3331 entries).
>
> * without patches before/after sepgsql_restorecon()
>
> VmPeak:   150812 kB -> 170864 kB
> VmSize:   150804 kB -> 154712 kB
> VmLck:         0 kB -> 0kB
> VmHWM:      3800 kB -> 22248 kB
> VmRSS:      3800 kB -> 10620 kB
> VmData:     1940 kB ->  5820 kB
> VmStk:       196 kB -> 196 kB
> VmExe:      5324 kB -> 5324 kB
> VmLib:      2468 kB -> 2468 kB
> VmPTE:       108 kB -> 120 kB
> VmSwap:        0 kB -> 0kB
>
> * with patches before/after sepgsql_restorecon()
> VmPeak:   150816 kB -> 175092 kB
> VmSize:   150808 kB -> 158804 kB
> VmLck:         0 kB -> 0 kB
> VmHWM:      3868 kB -> 25956 kB
> VmRSS:      3868 kB -> 13736 kB
> VmData:     1944 kB -> 9912 kB
> VmStk:       192 kB -> 192 kB
> VmExe:      5324 kB -> 5324 kB
> VmLib:      2472 kB -> 2472 kB
> VmPTE:       100 kB -> 124 kB
> VmSwap:        0 kB -> 0 kB
>
> Thanks,
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
>
>
>
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
>
>
>
> --
> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>
>
>
>
>
>
>
>
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-07-18 20:26:07 Re: [v9.1] sepgsql - userspace access vector cache
Previous Message Tom Lane 2011-07-18 20:19:48 Re: Commitfest Status: Sudden Death Overtime