subject:"\[HACKERS\] \[v9.1\] sepgsql \- userspace access vector cache"

Kohei KaiGai kai...@kaigai.gr.jp writes:
 2011/8/18 Robert Haas robertmh...@gmail.com:
 Actually, as I look at this more, I think this build system is
 completely mis-designed.  Given that you want to build sepgsql,
 selinux is not an optional feature.  So the stuff in
 contrib/sepgsql/Makefile that is intended to link against libselinux
 only if --with-selinux was specified at configure time is nonsense.

What stuff is that?

 So, it seems to me we also need to revise configure script, not only
 Makefile of sepgsql.

This patch seems unnecessary to me.  The way it works now appears to be
quite parallel to the way that contrib/xml2 works, and has worked for
years.  I don't think that sepgsql should behave differently from that.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 9:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Kohei KaiGai kai...@kaigai.gr.jp writes:
 2011/8/18 Robert Haas robertmh...@gmail.com:
 Actually, as I look at this more, I think this build system is
 completely mis-designed.  Given that you want to build sepgsql,
 selinux is not an optional feature.  So the stuff in
 contrib/sepgsql/Makefile that is intended to link against libselinux
 only if --with-selinux was specified at configure time is nonsense.

 What stuff is that?

SHLIB_LINK += $(filter -lselinux, $(LIBS))

 This patch seems unnecessary to me.  The way it works now appears to be
 quite parallel to the way that contrib/xml2 works, and has worked for
 years.  I don't think that sepgsql should behave differently from that.

Hmm.  I see now that it's parallel, but I find it pretty confusing
that building sepgsql without specifying --with-selinux results in a
shared library that seems to compile OK but won't load.   Why not
just:

SHLIB_LINK = -lselinux

Similarly, in the case of xml2 we have:

SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))

For xslt, it probably makes sense to filter it out if it wasn't found,
because the code has ifdefs for USE_XSLT that do something sensible if
the library is not there.  But I fail to see what the point is of
filtering out xml2, because surely we're doomed if that's not there...
or am I confused?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Robert Haas robertmh...@gmail.com writes:
 On Fri, Aug 19, 2011 at 9:59 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 This patch seems unnecessary to me.

 Hmm.  I see now that it's parallel, but I find it pretty confusing
 that building sepgsql without specifying --with-selinux results in a
 shared library that seems to compile OK but won't load.

Well, that's a fair point, but the same happens in contrib/xml2 (if you
have a setup that doesn't need a special -I switch, or you provide that
some other way), and nobody has ever complained about it.

 Why not just:

 SHLIB_LINK = -lselinux

I wouldn't have any particular objection to that (although I think it's
supposed to be += here).  I don't see that any of the other changes
Kaigai proposed are helpful, though.

 Similarly, in the case of xml2 we have:

 SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))

 For xslt, it probably makes sense to filter it out if it wasn't found,
 because the code has ifdefs for USE_XSLT that do something sensible if
 the library is not there.  But I fail to see what the point is of
 filtering out xml2, because surely we're doomed if that's not there...
 or am I confused?

Hmm.  I think it's just that way to make the code look parallel for both
libraries.  But I can see potential value in making -lxml2 unconditional
--- as you say, that would result in a link failure instead of a
silently broken library.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Why not just:

 SHLIB_LINK = -lselinux

 I wouldn't have any particular objection to that (although I think it's
 supposed to be += here).

Oh, right.

 I don't see that any of the other changes
 Kaigai proposed are helpful, though.

I was coming to the same conclusion.  I sort of liked his idea of
sticking a conditional #error directive in the header files to make it
more clear why it was failing.  But on closer examination there's
really no benefit: it gets lost in a sea of other failures, and if you
have to look through the failure messages anyway you may as well
notice that the #include of selinux/selinux.h failed as anything
else.  So I think changing that line to link with libselinux
unconditionally is about as well as we can do.

 Similarly, in the case of xml2 we have:

 SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))

 For xslt, it probably makes sense to filter it out if it wasn't found,
 because the code has ifdefs for USE_XSLT that do something sensible if
 the library is not there.  But I fail to see what the point is of
 filtering out xml2, because surely we're doomed if that's not there...
 or am I confused?

 Hmm.  I think it's just that way to make the code look parallel for both
 libraries.  But I can see potential value in making -lxml2 unconditional
 --- as you say, that would result in a link failure instead of a
 silently broken library.

Right.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 10:31 AM, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Aug 19, 2011 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Why not just:

 SHLIB_LINK = -lselinux

 I wouldn't have any particular objection to that (although I think it's
 supposed to be += here).

 Oh, right.

 I don't see that any of the other changes
 Kaigai proposed are helpful, though.

 I was coming to the same conclusion.  I sort of liked his idea of
 sticking a conditional #error directive in the header files to make it
 more clear why it was failing.  But on closer examination there's
 really no benefit: it gets lost in a sea of other failures, and if you
 have to look through the failure messages anyway you may as well
 notice that the #include of selinux/selinux.h failed as anything
 else.  So I think changing that line to link with libselinux
 unconditionally is about as well as we can do.

 Similarly, in the case of xml2 we have:

 SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))

 For xslt, it probably makes sense to filter it out if it wasn't found,
 because the code has ifdefs for USE_XSLT that do something sensible if
 the library is not there.  But I fail to see what the point is of
 filtering out xml2, because surely we're doomed if that's not there...
 or am I confused?

 Hmm.  I think it's just that way to make the code look parallel for both
 libraries.  But I can see potential value in making -lxml2 unconditional
 --- as you say, that would result in a link failure instead of a
 silently broken library.

 Right.

On further review, if the initial configure was done without
--with-libxml, xml2 is doomed anyway.  There's no value to changing
anything there one way or the other, because it relies not only on
symbols from the library, but also on symbols that are only defined
when PostgreSQL itself is configured with xml support.  In other
words, there's no chance of undetected failure here.

On the other hand, we've worked pretty hard to keep sepgsql at arm's
length from the core server, so as it stands, it's quite easy to build
a busted sepgsql module.

This probably explains why no one's complained about this before, and
I think the appropriate fix is to change just sepgsql.  I would like
to back-patch that (one-line) change into 9.1 as well, to eliminate
this as a foot-gun for anyone who may be packaging that module for the
first time.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Robert Haas robertmh...@gmail.com writes:
 On further review, if the initial configure was done without
 --with-libxml, xml2 is doomed anyway.

True, but it's still possible to build a shlib that will then not work.
I just did, after manually supplying the right -I switch:

make PROFILE=-I/usr/include/libxml2

Now admittedly a clueless person would be unlikely to know to do that,
but if his libxml installation were arranged so that no special -I
switch was needed (unlike the Fedora packaging), it'd be more likely
that this could happen.

 This probably explains why no one's complained about this before, and
 I think the appropriate fix is to change just sepgsql.  I would like
 to back-patch that (one-line) change into 9.1 as well, to eliminate
 this as a foot-gun for anyone who may be packaging that module for the
 first time.

No objection to fixing or backpatching this, but I'm not seeing the
argument for treating this module differently from contrib/xml2.  If you
believe that someone will try to manually build in contrib/sepgsql after
having failed to configure correctly, why do you not believe that for
xml2?

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-19 Thread Kohei Kaigai

 -Original Message-
 From: Robert Haas [mailto:robertmh...@gmail.com]
 Sent: 19. August 2011 15:55
 To: Tom Lane
 Cc: Kohei KaiGai; Kohei Kaigai; Yeb Havinga; PgHacker
 Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache

 On Fri, Aug 19, 2011 at 10:31 AM, Robert Haas robertmh...@gmail.com wrote:
  On Fri, Aug 19, 2011 at 10:20 AM, Tom Lane t...@sss.pgh.pa.us wrote:
  Why not just:

  SHLIB_LINK = -lselinux

  I wouldn't have any particular objection to that (although I think it's
  supposed to be += here).

  Oh, right.

  I don't see that any of the other changes
  Kaigai proposed are helpful, though.

  I was coming to the same conclusion.  I sort of liked his idea of
  sticking a conditional #error directive in the header files to make it
  more clear why it was failing.  But on closer examination there's
  really no benefit: it gets lost in a sea of other failures, and if you
  have to look through the failure messages anyway you may as well
  notice that the #include of selinux/selinux.h failed as anything
  else.  So I think changing that line to link with libselinux
  unconditionally is about as well as we can do.

  Similarly, in the case of xml2 we have:

  SHLIB_LINK += $(filter -lxslt, $(LIBS)) $(filter -lxml2, $(LIBS))

  For xslt, it probably makes sense to filter it out if it wasn't found,
  because the code has ifdefs for USE_XSLT that do something sensible if
  the library is not there.  But I fail to see what the point is of
  filtering out xml2, because surely we're doomed if that's not there...
  or am I confused?

  Hmm.  I think it's just that way to make the code look parallel for both
  libraries.  But I can see potential value in making -lxml2 unconditional
  --- as you say, that would result in a link failure instead of a
  silently broken library.

  Right.

 On further review, if the initial configure was done without
 --with-libxml, xml2 is doomed anyway.  There's no value to changing
 anything there one way or the other, because it relies not only on
 symbols from the library, but also on symbols that are only defined
 when PostgreSQL itself is configured with xml support.  In other
 words, there's no chance of undetected failure here.

 On the other hand, we've worked pretty hard to keep sepgsql at arm's
 length from the core server, so as it stands, it's quite easy to build
 a busted sepgsql module.

 This probably explains why no one's complained about this before, and
 I think the appropriate fix is to change just sepgsql.  I would like
 to back-patch that (one-line) change into 9.1 as well, to eliminate
 this as a foot-gun for anyone who may be packaging that module for the
 first time.

I almost agree with this change.

One point I'm worrying about is a case when contrib/sepgsql is compiled
with older libselinux than minimum requirement. In this case, we may not
notice the broken module unless user tries to load it actually.
Is there a good idea to ensure compile failure when we try to build sepgsql
module when libselinux-2.0.98 or older was installed?

Of course, using --with-selinux on ./configure time is the best way...

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 11:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On further review, if the initial configure was done without
 --with-libxml, xml2 is doomed anyway.

 True, but it's still possible to build a shlib that will then not work.
 I just did, after manually supplying the right -I switch:

 make PROFILE=-I/usr/include/libxml2

 Now admittedly a clueless person would be unlikely to know to do that,
 but if his libxml installation were arranged so that no special -I
 switch was needed (unlike the Fedora packaging), it'd be more likely
 that this could happen.

 This probably explains why no one's complained about this before, and
 I think the appropriate fix is to change just sepgsql.  I would like
 to back-patch that (one-line) change into 9.1 as well, to eliminate
 this as a foot-gun for anyone who may be packaging that module for the
 first time.

 No objection to fixing or backpatching this, but I'm not seeing the
 argument for treating this module differently from contrib/xml2.  If you
 believe that someone will try to manually build in contrib/sepgsql after
 having failed to configure correctly, why do you not believe that for
 xml2?

Because I screwed it up accidentally for sepgsql, and I can't screw it
up for xml2 on purpose even after working fairly hard.  Even after
shoving in the necessary -I switch (through a slightly different
mechanism than the one you just proposed), it still won't link,
whether -lxml2 is on the command-line or not.

That having been said, I don't mind changing them both symmetrically;
I'm just convinced that there's any benefit.  However, if you think
that way is more future-proof or that I might be overlooking some
scenario, fine!  It won't hurt anything.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Kohei Kaigai kohei.kai...@emea.nec.com writes:
 One point I'm worrying about is a case when contrib/sepgsql is compiled
 with older libselinux than minimum requirement. In this case, we may not
 notice the broken module unless user tries to load it actually.
 Is there a good idea to ensure compile failure when we try to build sepgsql
 module when libselinux-2.0.98 or older was installed?

Well, they should get at least a warning from referencing undefined
functions, no?

There's a limit to how friendly we can be here, since Linux's shlib
stuff is designed to not require all symbols to be resolvable at shlib
construction time.  This is one place where Darwin works better ...

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Robert Haas robertmh...@gmail.com writes:
 On Fri, Aug 19, 2011 at 11:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 No objection to fixing or backpatching this, but I'm not seeing the
 argument for treating this module differently from contrib/xml2.

 Because I screwed it up accidentally for sepgsql, and I can't screw it
 up for xml2 on purpose even after working fairly hard.  Even after
 shoving in the necessary -I switch (through a slightly different
 mechanism than the one you just proposed), it still won't link,
 whether -lxml2 is on the command-line or not.

Huh.  Links for me on Fedora 14 ...

[tgl@rh3 ~]$ cd ~/pgsql/contrib/xml2
[tgl@rh3 xml2]$ make clean
rm -f pgxml.so   libpgxml.a 
rm -f xpath.o xslt_proc.o
rm -rf results/ regression.diffs regression.out tmp_check/ log/
[tgl@rh3 xml2]$ make PROFILE=-I/usr/include/libxml2
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wformat-security 
-fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -I. -I. 
-I../../src/include -D_GNU_SOURCE   -c -o xpath.o xpath.c
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wformat-security 
-fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -I. -I. 
-I../../src/include -D_GNU_SOURCE   -c -o xslt_proc.o xslt_proc.c
gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -Wformat-security 
-fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -shared -o 
pgxml.so xpath.o xslt_proc.o -L../../src/port -Wl,--as-needed 
-Wl,-rpath,'/home/tgl/testversion/lib',--enable-new-dtags 
-I/usr/include/libxml2
[tgl@rh3 xml2]$ 

(and yes, this is in a build tree configured without --with-libxml).
As far as I can tell, this *must* work this way on Linux.  Maybe you
were testing the xml2 case on OS X?  That OS is pickier.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-19 Thread Kohei Kaigai

 -Original Message-
 From: Tom Lane [mailto:t...@sss.pgh.pa.us]
 Sent: 19. August 2011 16:34
 To: Kohei Kaigai
 Cc: Robert Haas; Kohei KaiGai; Yeb Havinga; PgHacker
 Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache

 Kohei Kaigai kohei.kai...@emea.nec.com writes:
  One point I'm worrying about is a case when contrib/sepgsql is compiled
  with older libselinux than minimum requirement. In this case, we may not
  notice the broken module unless user tries to load it actually.
  Is there a good idea to ensure compile failure when we try to build sepgsql
  module when libselinux-2.0.98 or older was installed?

 Well, they should get at least a warning from referencing undefined
 functions, no?

Yes. User should notice warning messages due to undefined symbols.
I'm not certain whether it makes sense to add -Werror here, or not.

 There's a limit to how friendly we can be here, since Linux's shlib
 stuff is designed to not require all symbols to be resolvable at shlib
 construction time.  This is one place where Darwin works better ...

Hmm...
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Kohei Kaigai kohei.kai...@emea.nec.com writes:
 From: Tom Lane [mailto:t...@sss.pgh.pa.us]
 Well, they should get at least a warning from referencing undefined
 functions, no?

 Yes. User should notice warning messages due to undefined symbols.
 I'm not certain whether it makes sense to add -Werror here, or not.

Hmm.  That would help catch the problem, but I'm a bit uncomfortable
with adding -Werror in relatively new code.  On the other hand, it's
not like we expect sepgsql to work on a wide variety of systems, so
maybe it'd be OK.

On the whole I don't think it's worth messing with the cflags for this.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 11:46 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Kohei Kaigai kohei.kai...@emea.nec.com writes:
 From: Tom Lane [mailto:t...@sss.pgh.pa.us]
 Well, they should get at least a warning from referencing undefined
 functions, no?

 Yes. User should notice warning messages due to undefined symbols.
 I'm not certain whether it makes sense to add -Werror here, or not.

 Hmm.  That would help catch the problem, but I'm a bit uncomfortable
 with adding -Werror in relatively new code.  On the other hand, it's
 not like we expect sepgsql to work on a wide variety of systems, so
 maybe it'd be OK.

 On the whole I don't think it's worth messing with the cflags for this.

Yeah, I agree.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Fri, Aug 19, 2011 at 11:40 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Fri, Aug 19, 2011 at 11:26 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 No objection to fixing or backpatching this, but I'm not seeing the
 argument for treating this module differently from contrib/xml2.

 Because I screwed it up accidentally for sepgsql, and I can't screw it
 up for xml2 on purpose even after working fairly hard.  Even after
 shoving in the necessary -I switch (through a slightly different
 mechanism than the one you just proposed), it still won't link,
 whether -lxml2 is on the command-line or not.

 Huh.  Links for me on Fedora 14 ...

 [tgl@rh3 ~]$ cd ~/pgsql/contrib/xml2
 [tgl@rh3 xml2]$ make clean
 rm -f pgxml.so   libpgxml.a
 rm -f xpath.o xslt_proc.o
 rm -rf results/ regression.diffs regression.out tmp_check/ log/
 [tgl@rh3 xml2]$ make PROFILE=-I/usr/include/libxml2
 gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wformat-security 
 -fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -I. -I. 
 -I../../src/include -D_GNU_SOURCE   -c -o xpath.o xpath.c
 gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wformat-security 
 -fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -I. -I. 
 -I../../src/include -D_GNU_SOURCE   -c -o xslt_proc.o xslt_proc.c
 gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
 -Wdeclaration-after-statement -Wendif-labels -Wformat-security 
 -fno-strict-aliasing -fwrapv -g -I/usr/include/libxml2 -fpic -shared -o 
 pgxml.so xpath.o xslt_proc.o -L../../src/port -Wl,--as-needed 
 -Wl,-rpath,'/home/tgl/testversion/lib',--enable-new-dtags 
 -I/usr/include/libxml2
 [tgl@rh3 xml2]$

 (and yes, this is in a build tree configured without --with-libxml).
 As far as I can tell, this *must* work this way on Linux.  Maybe you
 were testing the xml2 case on OS X?  That OS is pickier.

Hrm, I *thought* I had tested on Linux, but maybe I was on OS X that
time around.  Anyway, I can reproduce this now.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Thu, Jul 21, 2011 at 5:29 AM, Kohei Kaigai kohei.kai...@emea.nec.com wrote:
 The attached patch is revised userspace-avc patch.

 List of updates:
 - The GUC of sepgsql.avc_threshold was removed.
 - char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
 - Comments added onto static variables
 - Comments of sepgsql_avc_unlabeled() was revised.
 - Comments of sepgsql_avc_compute() was simplified.
 - Comments of sepgsql_avc_check_perms_label() also mention about
  permissive domain, that performs similar to system's permissive mode.
 - selinux_status_close() become invoked on on_proc_exit() hook.

I tried to give this a test drive today but got stuck.  I got sepgsql
compiled OK, but look what happens when I try to start the server:

[rhaas@f15selinux ~]$ postgres
FATAL:  could not load library
/home/rhaas/project/lib/postgresql/sepgsql.so:
/home/rhaas/project/lib/postgresql/sepgsql.so: undefined symbol:
getpeercon_raw

This is Fedora 15, with all available updates applied.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Thu, Aug 18, 2011 at 12:46 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jul 21, 2011 at 5:29 AM, Kohei Kaigai kohei.kai...@emea.nec.com 
 wrote:
 The attached patch is revised userspace-avc patch.

 List of updates:
 - The GUC of sepgsql.avc_threshold was removed.
 - char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
 - Comments added onto static variables
 - Comments of sepgsql_avc_unlabeled() was revised.
 - Comments of sepgsql_avc_compute() was simplified.
 - Comments of sepgsql_avc_check_perms_label() also mention about
  permissive domain, that performs similar to system's permissive mode.
 - selinux_status_close() become invoked on on_proc_exit() hook.

 I tried to give this a test drive today but got stuck.  I got sepgsql
 compiled OK, but look what happens when I try to start the server:

 [rhaas@f15selinux ~]$ postgres
 FATAL:  could not load library
 /home/rhaas/project/lib/postgresql/sepgsql.so:
 /home/rhaas/project/lib/postgresql/sepgsql.so: undefined symbol:
 getpeercon_raw

 This is Fedora 15, with all available updates applied.

Oh.  Apparently, this is what happens when you try to build sepgsql
without passing --with-selinux to configure.

That's lame.  I think we need to patch contrib/sepgsql so that it
fails to build in that case, rather than building and then not
working.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Thu, Aug 18, 2011 at 12:52 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Aug 18, 2011 at 12:46 PM, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jul 21, 2011 at 5:29 AM, Kohei Kaigai kohei.kai...@emea.nec.com 
 wrote:
 The attached patch is revised userspace-avc patch.

 List of updates:
 - The GUC of sepgsql.avc_threshold was removed.
 - char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
 - Comments added onto static variables
 - Comments of sepgsql_avc_unlabeled() was revised.
 - Comments of sepgsql_avc_compute() was simplified.
 - Comments of sepgsql_avc_check_perms_label() also mention about
  permissive domain, that performs similar to system's permissive mode.
 - selinux_status_close() become invoked on on_proc_exit() hook.

 I tried to give this a test drive today but got stuck.  I got sepgsql
 compiled OK, but look what happens when I try to start the server:

 [rhaas@f15selinux ~]$ postgres
 FATAL:  could not load library
 /home/rhaas/project/lib/postgresql/sepgsql.so:
 /home/rhaas/project/lib/postgresql/sepgsql.so: undefined symbol:
 getpeercon_raw

 This is Fedora 15, with all available updates applied.

 Oh.  Apparently, this is what happens when you try to build sepgsql
 without passing --with-selinux to configure.

 That's lame.  I think we need to patch contrib/sepgsql so that it
 fails to build in that case, rather than building and then not
 working.

Also, I get these warnings:

/etc/selinux/targeted/contexts/sepgsql_contexts:  line 33 has invalid
object type db_blobs
/etc/selinux/targeted/contexts/sepgsql_contexts:  line 36 has invalid
object type db_language
/etc/selinux/targeted/contexts/sepgsql_contexts:  line 37 has invalid
object type db_language
/etc/selinux/targeted/contexts/sepgsql_contexts:  line 38 has invalid
object type db_language
/etc/selinux/targeted/contexts/sepgsql_contexts:  line 39 has invalid
object type db_language
/etc/selinux/targeted/contexts/sepgsql_contexts:  line 40 has invalid
object type db_language
 1: sepgsql_restorecon = t(typeid = 16, len = 1, typmod = -1, 
byval = t)

The first is mentioned in the latest documentation, but the rest are not.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-18 Thread Kohei Kaigai

 That's lame.  I think we need to patch contrib/sepgsql so that it
 fails to build in that case, rather than building and then not
 working.
 
It might be the following fix, but I have no idea to generate an error when 
$(with_selinux) != yes on makefile.

diff --git a/contrib/sepgsql/Makefile b/contrib/sepgsql/Makefile
index 7f997ee..fec4f1a 100644
--- a/contrib/sepgsql/Makefile
+++ b/contrib/sepgsql/Makefile
@@ -19,6 +19,12 @@ include $(top_builddir)/src/Makefile.global
 include $(top_srcdir)/contrib/contrib-global.mk
 endif

+ifneq ($(with_selinux),yes)
+##
+## Error generation
+##
+endif
+
 SHLIB_LINK += $(filter -lselinux, $(LIBS))
 REGRESS_OPTS += --launcher $(top_builddir)/contrib/sepgsql/launcher

--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com


 -Original Message-
 From: Robert Haas [mailto:robertmh...@gmail.com]
 Sent: 18. August 2011 17:53
 To: Kohei Kaigai
 Cc: Yeb Havinga; PgHacker; Kohei KaiGai
 Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache
 
 On Thu, Aug 18, 2011 at 12:46 PM, Robert Haas robertmh...@gmail.com wrote:
  On Thu, Jul 21, 2011 at 5:29 AM, Kohei Kaigai kohei.kai...@emea.nec.com 
  wrote:
  The attached patch is revised userspace-avc patch.
 
  List of updates:
  - The GUC of sepgsql.avc_threshold was removed.
  - char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
  - Comments added onto static variables
  - Comments of sepgsql_avc_unlabeled() was revised.
  - Comments of sepgsql_avc_compute() was simplified.
  - Comments of sepgsql_avc_check_perms_label() also mention about
   permissive domain, that performs similar to system's permissive mode.
  - selinux_status_close() become invoked on on_proc_exit() hook.
 
  I tried to give this a test drive today but got stuck.  I got sepgsql
  compiled OK, but look what happens when I try to start the server:
 
  [rhaas@f15selinux ~]$ postgres
  FATAL:  could not load library
  /home/rhaas/project/lib/postgresql/sepgsql.so:
  /home/rhaas/project/lib/postgresql/sepgsql.so: undefined symbol:
  getpeercon_raw
 
  This is Fedora 15, with all available updates applied.
 
 Oh.  Apparently, this is what happens when you try to build sepgsql
 without passing --with-selinux to configure.
 
 That's lame.  I think we need to patch contrib/sepgsql so that it
 fails to build in that case, rather than building and then not
 working.
 
 --
 Robert Haas
 EnterpriseDB: http://www.enterprisedb.com
 The Enterprise PostgreSQL Company
 
 
  Click
 https://www.mailcontrol.com/sr/i+p!jARD6rnTndxI!oX7Uu+NqWBeKfvsxHen8ElqAIKK2vDQ5PIqETvu3D1VdIOLM1
 BV3YJKcc+1yubdBaCdqw==  to report this email as spam.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Thu, Aug 18, 2011 at 1:00 PM, Robert Haas robertmh...@gmail.com wrote:
 [more problems]

OK, I'm giving up for now.  I hit two more snags:

1. chkselinuxenv uses which, and a Fedora 15 minimal install doesn't
include that.  I fixed that by installing which, but maybe we ought
to be looking for a way to eliminate that dependency, like testing for
the commands you need by running them with --help, or something like
that.

2. restorecon doesn't correctly set the permissions for me on
~/project/bin/psql.  I get:

[rhaas@f15selinux sepgsql]$ ls -Z ~/project/bin/psql
-rwxr-xr-x. rhaas rhaas unconfined_u:object_r:user_home_t:s0
/home/rhaas/project/bin/psql

Now I can fix that by applying bin_t manually, as suggested in the
documentation.  However, that just moves the failure to library load
time.  regression.diffs has multiple copies of this error message:

/home/rhaas/project/bin/psql: error while loading shared libraries:
libpq.so.5: failed to map segment from shared object: Permission
denied

Help!

Thanks,

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-18 Thread Kohei Kaigai

OK, I'm giving up for now. I hit two more snags:

1. chkselinuxenv uses which, and a Fedora 15 minimal install doesn't
include that. I fixed that by installing which, but maybe we ought
to be looking for a way to eliminate that dependency, like testing for
the commands you need by running them with --help, or something like
that.

Oops, I thought which is a part of coreutils.

I'll try to update chkselinuxenv to print a help message when necessary
commands are not installed.

2. restorecon doesn't correctly set the permissions for me on
~/project/bin/psql. I get:

[rhaas@f15selinux sepgsql]$ ls -Z ~/project/bin/psql
-rwxr-xr-x. rhaas rhaas unconfined_u:object_r:user_home_t:s0
/home/rhaas/project/bin/psql

Now I can fix that by applying bin_t manually, as suggested in the
documentation. However, that just moves the failure to library load
time. regression.diffs has multiple copies of this error message:

/home/rhaas/project/bin/psql: error while loading shared libraries:
libpq.so.5: failed to map segment from shared object: Permission
denied

I guess it tries to mmap(2) libpq.so.5 (labeled as user_home_t) with executable
mode.
The regression test switches domain of psql command on its execution from
unconfined_t to sepgsql_regtest_user_t, however, I didn't allow this domain
to mmap(2) files in user's home directory with executable mode.
It may need to revise the security policy of regression test to support
installation onto home directory.

As a quick avoidance, how about --prefix=/usr/local/sepgsql instead?

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-Original Message-
From: Robert Haas [mailto:robertmh...@gmail.com]
Sent: 18. August 2011 18:22
To: Kohei Kaigai
Cc: Yeb Havinga; PgHacker; Kohei KaiGai
Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache

On Thu, Aug 18, 2011 at 1:00 PM, Robert Haas robertmh...@gmail.com wrote:
[more problems]

OK, I'm giving up for now. I hit two more snags:

2. restorecon doesn't correctly set the permissions for me on
~/project/bin/psql. I get:

[rhaas@f15selinux sepgsql]$ ls -Z ~/project/bin/psql
-rwxr-xr-x. rhaas rhaas unconfined_u:object_r:user_home_t:s0
/home/rhaas/project/bin/psql

/home/rhaas/project/bin/psql: error while loading shared libraries:
libpq.so.5: failed to map segment from shared object: Permission
denied

Help!

Thanks,

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Click
https://www.mailcontrol.com/sr/g7UEZIfD10rTndxI!oX7Unz1!gA0DCbilsfI53CIRke!PbNpuk4RnjmGfZ8cEe1DM1
BV3YJKcc9jEfBJ2k7YZA== to report this email as spam.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Thu, Aug 18, 2011 at 1:17 PM, Kohei Kaigai kohei.kai...@emea.nec.com wrote:
 That's lame.  I think we need to patch contrib/sepgsql so that it
 fails to build in that case, rather than building and then not
 working.

 It might be the following fix, but I have no idea to generate an error when 
 $(with_selinux) != yes on makefile.

Actually, as I look at this more, I think this build system is
completely mis-designed.  Given that you want to build sepgsql,
selinux is not an optional feature.  So the stuff in
contrib/sepgsql/Makefile that is intended to link against libselinux
only if --with-selinux was specified at configure time is nonsense.
We should just ALWAYS try to link against libselinux, and if it's not
there, then at least it'll fail right away at compile time instead of
appearing to compile OK but producing an so that then fails to load at
runtime.

The only actual legitimate purpose of --with-selinux is to allow
contrib/Makefile to decide whether, when someone tries to build all
the contrib modules, we should try to build sepgsql too.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-05 Thread Kohei KaiGai

BTW, what is the current status of this patch?
The status of contrib/sepgsql part is unclear for me, although we agreed that
syscache is suitable mechanism for security labels.

Thanks,

2011/7/22 Kohei KaiGai kai...@kaigai.gr.jp:
 2011/7/22 Yeb Havinga yebhavi...@gmail.com:
 On 2011-07-22 11:55, Kohei Kaigai wrote:

 2) Also I thought if it could work to not remember tcontext is valid, but
 instead remember the consequence,
 which is that it is replaced by unlabeled. It makes the avc_cache
 struct shorter and the code somewhat
 simpler.

 Here is a reason why we hold tcontext, even if it is not valid.
 The hash key of avc_cache is combination of scontext, tcontext and tclass.
 Thus, if we replaced an invalid
 tcontext by unlabeled context, it would always make cache mishit and
 performance loss.

 I see that now, thanks.

 I have no further comments, and I think that the patch in it's current
 status is ready for committer.

 Thanks for your reviewing.

 The attached patch is a revised one according to your suggestion to
 include fallback for 'unlabeled' label within sepgsql_avc_lookup().
 And I found a noise in regression test results, so eliminated it from v5.
 --
 KaiGai Kohei kai...@kaigai.gr.jp

-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-05 Thread Robert Haas

On Fri, Aug 5, 2011 at 2:36 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 BTW, what is the current status of this patch?

I think it's waiting for me to get around to reviewing it.  Sorry,
been busy :-(

 The status of contrib/sepgsql part is unclear for me, although we agreed that
 syscache is suitable mechanism for security labels.

I thought we agreed the opposite - viz, you need a custom cache.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-08-05 Thread Kohei KaiGai

2011/8/5 Robert Haas robertmh...@gmail.com:
 On Fri, Aug 5, 2011 at 2:36 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 BTW, what is the current status of this patch?

 I think it's waiting for me to get around to reviewing it.  Sorry,
 been busy :-(

Thanks for your efforts.

 The status of contrib/sepgsql part is unclear for me, although we agreed that
 syscache is suitable mechanism for security labels.

 I thought we agreed the opposite - viz, you need a custom cache.

Sorry, I missed to append not just after syscache is 
Your explanation is right.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-22 Thread Kohei Kaigai

 -Original Message-
 From: Yeb Havinga [mailto:yebhavi...@gmail.com]
 Sent: 22. Juli 2011 10:23
 To: Kohei Kaigai
 Cc: Robert Haas; PgHacker; Kohei KaiGai
 Subject: Re: [HACKERS] [v9.1] sepgsql - userspace access vector cache

 On 2011-07-21 11:29, Kohei Kaigai wrote:
  The attached patch is revised userspace-avc patch.

  List of updates:
  - The GUC of sepgsql.avc_threshold was removed.
  - char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
  - Comments added onto static variables
  - Comments of sepgsql_avc_unlabeled() was revised.
  - Comments of sepgsql_avc_compute() was simplified.
  - Comments of sepgsql_avc_check_perms_label() also mention about
 permissive domain, that performs similar to system's permissive mode.
  - selinux_status_close() become invoked on on_proc_exit() hook.
 Thank you for the update, I'm looking at it right now and with a new look 
 have some more questions.
 I took the liberty to supply a patch to be applied after your v5 uavc patch.

 1) At a few call sites of sepgsql_avc_lookup, a null tcontext is detected, 
 and then replaced by
 unlabeled. I moved this to sepgsql_avc_lookup itself.

Good improvement.

 2) Also I thought if it could work to not remember tcontext is valid, but 
 instead remember the consequence,
 which is that it is replaced by unlabeled. It makes the avc_cache struct 
 shorter and the code somewhat
 simpler.

Here is a reason why we hold tcontext, even if it is not valid.
The hash key of avc_cache is combination of scontext, tcontext and tclass. 
Thus, if we replaced an invalid
tcontext by unlabeled context, it would always make cache mishit and 
performance loss.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-22 Thread Yeb Havinga


On 2011-07-22 11:55, Kohei Kaigai wrote:



2) Also I thought if it could work to not remember tcontext is valid, but 
instead remember the consequence,
which is that it is replaced by unlabeled. It makes the avc_cache struct 
shorter and the code somewhat
simpler.


Here is a reason why we hold tcontext, even if it is not valid.
The hash key of avc_cache is combination of scontext, tcontext and tclass. 
Thus, if we replaced an invalid
tcontext by unlabeled context, it would always make cache mishit and 
performance loss.

I see that now, thanks.

I have no further comments, and I think that the patch in it's current 
status is ready for committer.


regards,
--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-22 Thread Kohei KaiGai

2011/7/22 Yeb Havinga yebhavi...@gmail.com:
 On 2011-07-22 11:55, Kohei Kaigai wrote:

 2) Also I thought if it could work to not remember tcontext is valid, but
 instead remember the consequence,
 which is that it is replaced by unlabeled. It makes the avc_cache
 struct shorter and the code somewhat
 simpler.

 Here is a reason why we hold tcontext, even if it is not valid.
 The hash key of avc_cache is combination of scontext, tcontext and tclass.
 Thus, if we replaced an invalid
 tcontext by unlabeled context, it would always make cache mishit and
 performance loss.

 I see that now, thanks.

 I have no further comments, and I think that the patch in it's current
 status is ready for committer.

Thanks for your reviewing.

The attached patch is a revised one according to your suggestion to
include fallback for 'unlabeled' label within sepgsql_avc_lookup().
And I found a noise in regression test results, so eliminated it from v5.
-- 
KaiGai Kohei kai...@kaigai.gr.jp
 configure.in   |4 +-
 contrib/sepgsql/Makefile   |2 +-
 contrib/sepgsql/dml.c  |   59 +++---
 contrib/sepgsql/hooks.c|   64 +++---
 contrib/sepgsql/label.c|   13 +-
 contrib/sepgsql/proc.c |   79 ++-
 contrib/sepgsql/relation.c |   93 -
 contrib/sepgsql/schema.c   |   39 ++--
 contrib/sepgsql/selinux.c  |2 +-
 contrib/sepgsql/sepgsql.h  |   20 ++-
 contrib/sepgsql/uavc.c |  510 
 doc/src/sgml/sepgsql.sgml  |   12 +-
 12 files changed, 673 insertions(+), 224 deletions(-)

diff --git a/configure.in b/configure.in
index b1f15f1..8e3bc62 100644
--- a/configure.in
+++ b/configure.in
@@ -964,8 +964,8 @@ fi
 
 # for contrib/sepgsql
 if test $with_selinux = yes; then
-  AC_CHECK_LIB(selinux, selinux_sepgsql_context_path, [],
-   [AC_MSG_ERROR([library 'libselinux', version 2.0.93 or newer, is required for SELinux support])])
+  AC_CHECK_LIB(selinux, selinux_status_open, [],
+   [AC_MSG_ERROR([library 'libselinux', version 2.0.99 or newer, is required for SELinux support])])
 fi
 
 # for contrib/uuid-ossp
diff --git a/contrib/sepgsql/Makefile b/contrib/sepgsql/Makefile
index bc995dd..5794921 100644
--- a/contrib/sepgsql/Makefile
+++ b/contrib/sepgsql/Makefile
@@ -1,7 +1,7 @@
 # contrib/sepgsql/Makefile
 
 MODULE_big = sepgsql
-OBJS = hooks.o selinux.o label.o dml.o \
+OBJS = hooks.o selinux.o uavc.o label.o dml.o \
 	schema.o relation.o proc.o
 DATA_built = sepgsql.sql
 REGRESS = label dml misc
diff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index 22666b7..3199337 100644
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -150,12 +150,11 @@ check_relation_privileges(Oid relOid,
 		  uint32 required,
 		  bool abort)
 {
-	char		relkind = get_rel_relkind(relOid);
-	char	   *scontext = sepgsql_get_client_label();
-	char	   *tcontext;
+	ObjectAddress	object;
 	char	   *audit_name;
 	Bitmapset  *columns;
 	int			index;
+	char		relkind = get_rel_relkind(relOid);
 	bool		result = true;
 
 	/*
@@ -184,45 +183,43 @@ check_relation_privileges(Oid relOid,
 	/*
 	 * Check permissions on the relation
 	 */
-	tcontext = sepgsql_get_label(RelationRelationId, relOid, 0);
-	audit_name = getObjectDescriptionOids(RelationRelationId, relOid);
+	object.classId = RelationRelationId;
+	object.objectId = relOid;
+	object.objectSubId = 0;
+	audit_name = getObjectDescription(object);
 	switch (relkind)
 	{
 		case RELKIND_RELATION:
-			result = sepgsql_check_perms(scontext,
-		 tcontext,
-		 SEPG_CLASS_DB_TABLE,
-		 required,
-		 audit_name,
-		 abort);
+			result = sepgsql_avc_check_perms(object,
+			 SEPG_CLASS_DB_TABLE,
+			 required,
+			 audit_name,
+			 abort);
 			break;
 
 		case RELKIND_SEQUENCE:
 			Assert((required  ~SEPG_DB_TABLE__SELECT) == 0);
 
 			if (required  SEPG_DB_TABLE__SELECT)
-result = sepgsql_check_perms(scontext,
-			 tcontext,
-			 SEPG_CLASS_DB_SEQUENCE,
-			 SEPG_DB_SEQUENCE__GET_VALUE,
-			 audit_name,
-			 abort);
+result = sepgsql_avc_check_perms(object,
+ SEPG_CLASS_DB_SEQUENCE,
+ SEPG_DB_SEQUENCE__GET_VALUE,
+ audit_name,
+ abort);
 			break;
 
 		case RELKIND_VIEW:
-			result = sepgsql_check_perms(scontext,
-		 tcontext,
-		 SEPG_CLASS_DB_VIEW,
-		 SEPG_DB_VIEW__EXPAND,
-		 audit_name,
-		 abort);
+			result = sepgsql_avc_check_perms(object,
+			 SEPG_CLASS_DB_VIEW,
+			 SEPG_DB_VIEW__EXPAND,
+			 audit_name,
+			 abort);
 			break;
 
 		default:
 			/* nothing to be checked */
 			break;
 	}
-	pfree(tcontext);
 	pfree(audit_name);
 
 	/*
@@ -242,7 +239,6 @@ check_relation_privileges(Oid relOid,
 	{
 		AttrNumber	attnum;
 		uint32		column_perms = 0;
-		ObjectAddress object;
 
 		if (bms_is_member(index, selected))
 			column_perms |= SEPG_DB_COLUMN__SELECT;

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

2011-07-22 Thread Yeb Havinga


On 2011-07-21 11:29, Kohei Kaigai wrote:

The attached patch is revised userspace-avc patch.

List of updates:
- The GUC of sepgsql.avc_threshold was removed.
- char *ucontext of avc_cache was replaced by bool tcontext_is_valid.
- Comments added onto static variables
- Comments of sepgsql_avc_unlabeled() was revised.
- Comments of sepgsql_avc_compute() was simplified.
- Comments of sepgsql_avc_check_perms_label() also mention about
   permissive domain, that performs similar to system's permissive mode.
- selinux_status_close() become invoked on on_proc_exit() hook.
Thank you for the update, I'm looking at it right now and with a new 
look have some more questions. I took the liberty to supply a patch to 
be applied after your v5 uavc patch.


1) At a few call sites of sepgsql_avc_lookup, a null tcontext is 
detected, and then replaced by unlabeled. I moved this to 
sepgsql_avc_lookup itself.
2) Also I thought if it could work to not remember tcontext is valid, 
but instead remember the consequence, which is that it is replaced by 
unlabeled. It makes the avc_cache struct shorter and the code somewhat 
simpler.


regards,
--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data

diff --git a/contrib/sepgsql/uavc.c b/contrib/sepgsql/uavc.c
new file mode 100644
index 57a5197..2bcfedf
*** a/contrib/sepgsql/uavc.c
--- b/contrib/sepgsql/uavc.c
*** typedef struct
*** 41,48 
  
  	bool		permissive;	/* true, if permissive rule */
  	bool		hot_cache;	/* true, if recently referenced */
- 	bool		tcontext_is_valid;
- 			/* true, if tcontext is valid */
  	char	   *ncontext;	/* temporary scontext on execution of trusted
  			 * procedure, or NULL elsewhere */
  } avc_cache;
--- 41,46 
*** sepgsql_avc_reset(void)
*** 88,94 
  
  /*
   * Reclaim caches recently unreferenced
!  */	
  static void
  sepgsql_avc_reclaim(void)
  {
--- 86,92 
  
  /*
   * Reclaim caches recently unreferenced
!  */
  static void
  sepgsql_avc_reclaim(void)
  {
*** sepgsql_avc_check_valid(void)
*** 153,159 
  /*
   * sepgsql_avc_unlabeled
   *
!  * It returns an alternative label to be applied when no label or invalid 
   * label would be assigned on objects.
   */
  static char *
--- 151,157 
  /*
   * sepgsql_avc_unlabeled
   *
!  * It returns an alternative label to be applied when no label or invalid
   * label would be assigned on objects.
   */
  static char *
*** sepgsql_avc_unlabeled(void)
*** 184,198 
  }
  
  /*
!  * sepgsql_avc_compute 
   *
   * A fallback path, when cache mishit. It asks SELinux its access control
   * decision for the supplied pair of security context and object class.
   */
  static avc_cache *
! sepgsql_avc_compute(const char *scontext, const char *tcontext, uint16 tclass)
  {
- 	char		   *ucontext = NULL;
  	char		   *ncontext = NULL;
  	MemoryContext	oldctx;
  	avc_cache	   *cache;
--- 182,195 
  }
  
  /*
!  * sepgsql_avc_compute
   *
   * A fallback path, when cache mishit. It asks SELinux its access control
   * decision for the supplied pair of security context and object class.
   */
  static avc_cache *
! sepgsql_avc_compute(const char *scontext, char *tcontext, uint16 tclass)
  {
  	char		   *ncontext = NULL;
  	MemoryContext	oldctx;
  	avc_cache	   *cache;
*** sepgsql_avc_compute(const char *scontext
*** 207,224 
  	 * Validation check of the supplied security context.
  	 * Because it always invoke system-call, frequent check should be avoided.
  	 * Unless security policy is reloaded, validation status shall be kept, so
! 	 * we also cache whether the supplied security context was valid, or not.
  	 */
  	if (security_check_context_raw((security_context_t)tcontext) != 0)
! 		ucontext = sepgsql_avc_unlabeled();
  
! 	/*
! 	 * Ask SELinux its access control decision
! 	 */
! 	if (!ucontext)
! 		sepgsql_compute_avd(scontext, tcontext, tclass, avd);
! 	else
! 		sepgsql_compute_avd(scontext, ucontext, tclass, avd);
  
  	/*
  	 * To boost up trusted procedure checks on db_procedure object
--- 204,216 
  	 * Validation check of the supplied security context.
  	 * Because it always invoke system-call, frequent check should be avoided.
  	 * Unless security policy is reloaded, validation status shall be kept, so
! 	 * we also cache the invalid tcontext replaced by the unlabeled context.
  	 */
  	if (security_check_context_raw((security_context_t)tcontext) != 0)
! 		tcontext = sepgsql_avc_unlabeled();
  
! 	/* Ask SELinux its access control decision */
! 	sepgsql_compute_avd(scontext, tcontext, tclass, avd);
  
  	/*
  	 * To boost up trusted procedure checks on db_procedure object
*** sepgsql_avc_compute(const char *scontext
*** 227,238 
  	 */
  	if (tclass == SEPG_CLASS_DB_PROCEDURE)
  	{
! 		if (!ucontext)
! 			ncontext = sepgsql_compute_create(scontext, tcontext,
! 			  SEPG_CLASS_PROCESS);
! 		else
! 			ncontext = sepgsql_compute_create(scontext, ucontext,
!

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

2011-07-21 Thread Yeb Havinga


On 2011-07-21 00:08, Robert Haas wrote:

On Wed, Jul 20, 2011 at 4:48 PM, Tom Lanet...@sss.pgh.pa.us  wrote:

Kohei Kaigaikohei.kai...@emea.nec.com  writes:

I'd like to have a discussion about syscache towards next commit-fest.
The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

There used to be support for limiting the number of entries in a
syscache.  It got removed (cf commit
8b9bc234ad43dfa788bde40ebf12e94f16556b7f) because (1) it was remarkably
expensive to do it (extra list manipulations, etc), and (2) performance
tended to fall off a cliff as soon as you had a few more tables or
whatever than the caches would hold.  I'm disinclined to reverse that
decision.  It appears to me that the security label stuff needs a
different set of performance tradeoffs than the rest of the catalogs,
which means it probably ought to do its own caching, rather than trying
to talk us into pessimizing the other catalogs for seclabel's benefit.

I agree that we don't want to limit the size of the catcaches.  We've
been careful to design them in such a way that they won't blow out
memory, and so far there's no evidence that they do.  If it ain't
broke, don't fix it.  Having catcaches that can grow in size as needed
sounds useful to me, though.
Is there a way to dump syscache statistics like there is for 
MemoryContext..Stats (something - gdb helped me there)?


Besides that I have to admit having problems understanding why the 5MB 
cache for pg_seclabel is a problem; it's memory consumption is lineair 
only to the size of the underlying database.  (in contrast with the 
other cache storing access vectors which would have O(n*m) space 
complexity if it wouldn't reclaim space). So it is proportional to the 
number of objects in a database and in size it seems to be in the same 
order as pg_proc, pg_class and pg_attribute.


regards,

--
Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Robert Haas

On Thu, Jul 21, 2011 at 4:00 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 Is there a way to dump syscache statistics like there is for
 MemoryContext..Stats (something - gdb helped me there)?

I don't know of one.

 Besides that I have to admit having problems understanding why the 5MB cache
 for pg_seclabel is a problem; it's memory consumption is lineair only to the
 size of the underlying database.  (in contrast with the other cache storing
 access vectors which would have O(n*m) space complexity if it wouldn't
 reclaim space). So it is proportional to the number of objects in a database
 and in size it seems to be in the same order as pg_proc, pg_class and
 pg_attribute.

Fair enough.  I'm not convinced that the sheer quantity of memory use
is a problem, although I would like to see a few more test results
before we decide that definitively.  I *am* unwilling to pay the
startup overhead of initializing an extra 2048 syscache that only
sepgsql users will actually need.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Yeb Havinga


On 2011-07-21 15:03, Robert Haas wrote:

On Thu, Jul 21, 2011 at 4:00 AM, Yeb Havingayebhavi...@gmail.com  wrote:

Besides that I have to admit having problems understanding why the 5MB cache
for pg_seclabel is a problem; it's memory consumption is lineair only to the
size of the underlying database.  (in contrast with the other cache storing
access vectors which would have O(n*m) space complexity if it wouldn't
reclaim space). So it is proportional to the number of objects in a database
and in size it seems to be in the same order as pg_proc, pg_class and
pg_attribute.

Fair enough.  I'm not convinced that the sheer quantity of memory use
is a problem, although I would like to see a few more test results
before we decide that definitively.  I *am* unwilling to pay the
startup overhead of initializing an extra 2048 syscache that only
sepgsql users will actually need.


Is it possible to only include the syscache on --enable-selinux 
configurations? It would imply physical data incompatibility with 
standard configurations, but that's also true for e.g. the block size.


Also, the tests I did with varying bucket sizes suggested that 
decreasing the syscache to 256 didn't show a significant performance 
decrease compared to the 2048 #buckets, for the restorecon test, which 
hits over 3000 objects with security labels. My guess is that that is a 
fair middle of the road database schema size. Are you unwilling to pay 
the startup overhead for a extra 256 syscache?


--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Robert Haas

On Thu, Jul 21, 2011 at 3:25 PM, Yeb Havinga yebhavi...@gmail.com wrote:
 Is it possible to only include the syscache on --enable-selinux
 configurations? It would imply physical data incompatibility with standard
 configurations, but that's also true for e.g. the block size.

Not really.  SECURITY LABEL is supposedly a generic facility that can
be used by a variety of providers, and the regression tests load a
dummy provider which works on any platform to test that it hasn't
gotten broken.

 Also, the tests I did with varying bucket sizes suggested that decreasing
 the syscache to 256 didn't show a significant performance decrease compared
 to the 2048 #buckets, for the restorecon test, which hits over 3000 objects
 with security labels. My guess is that that is a fair middle of the road
 database schema size. Are you unwilling to pay the startup overhead for a
 extra 256 syscache?

Not sure.  I'd rather not, if it's easy to rejigger things so we don't
have to.  I don't think this is necessarily a hard problem to solve -
it's just that no one has tried yet.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Yeb Havinga



Is it possible to only include the syscache on --enable-selinux 
configurations? It would imply physical data incompatibility with 
standard configurations, but that's also true for e.g. the block size.


Also, the tests I did with varying bucket sizes suggested that 
decreasing the syscache to 256 didn't show a significant performance 
decrease compared to the 2048 #buckets, for the restorecon test, which 
hits over 3000 objects with security labels. My guess is that that is 
a fair middle of the road database schema size. Are you unwilling to 
pay the startup overhead for a extra 256 syscache?




Hello KaiGai-san,

off-list,

I was wondering why the catalog pg_seclabel exists at all. Why not store 
the labels together with the objects (pg_class, pg_attribute etc) ? The 
syscache wouldn't be needed in that case.


regards,
Yeb



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Kohei KaiGai

2011/7/21 Yeb Havinga yebhavi...@gmail.com:

 Is it possible to only include the syscache on --enable-selinux
 configurations? It would imply physical data incompatibility with standard
 configurations, but that's also true for e.g. the block size.

 Also, the tests I did with varying bucket sizes suggested that decreasing
 the syscache to 256 didn't show a significant performance decrease compared
 to the 2048 #buckets, for the restorecon test, which hits over 3000 objects
 with security labels. My guess is that that is a fair middle of the road
 database schema size. Are you unwilling to pay the startup overhead for a
 extra 256 syscache?


 Hello KaiGai-san,

 off-list,

Unfortunatelly, not so...

 I was wondering why the catalog pg_seclabel exists at all. Why not store the
 labels together with the objects (pg_class, pg_attribute etc) ? The syscache
 wouldn't be needed in that case.

Although current sepgsql support to assign security label on limited number of
object classes (schema, relation, column and functions).
However, we have planed to control accesses on whole of objects managed by
PostgreSQL, not only these four.

If we needed to expand system catalog everytime when an object get newly
supported, the patch would be more invasive and make hard to upstream.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-21 Thread Kohei KaiGai

2011/7/21 Robert Haas robertmh...@gmail.com:
 On Thu, Jul 21, 2011 at 3:25 PM, Yeb Havinga yebhavi...@gmail.com wrote:
 Is it possible to only include the syscache on --enable-selinux
 configurations? It would imply physical data incompatibility with standard
 configurations, but that's also true for e.g. the block size.

 Not really.  SECURITY LABEL is supposedly a generic facility that can
 be used by a variety of providers, and the regression tests load a
 dummy provider which works on any platform to test that it hasn't
 gotten broken.

 Also, the tests I did with varying bucket sizes suggested that decreasing
 the syscache to 256 didn't show a significant performance decrease compared
 to the 2048 #buckets, for the restorecon test, which hits over 3000 objects
 with security labels. My guess is that that is a fair middle of the road
 database schema size. Are you unwilling to pay the startup overhead for a
 extra 256 syscache?

 Not sure.  I'd rather not, if it's easy to rejigger things so we don't
 have to.  I don't think this is necessarily a hard problem to solve -
 it's just that no one has tried yet.

Now, I tend to implement a cache mechanism to translate ObjectAddress
to security label by sepgsql module itself, rather than generic syscache,
although it requires a new hook on PrepareForTupleInvalidation() as Robert
suggested in this thread.
Indeed, it seems to me worthwhile not to allocate memory being unused for
90% of users; from perspective of startup performance and resource consumption.

In addition, we may be potentially able to have a cache stuff well optimized
to the access control of SELinux; such as cache reclaim for recently unused
entries. So, I'd like to focus on the stuff in sepgsql/uavc.c right now.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-20 Thread Kohei KaiGai

2011/7/19 Yeb Havinga yebhavi...@gmail.com:
 On 2011-07-19 22:39, Heikki Linnakangas wrote:

 On 19.07.2011 12:28, Yeb Havinga wrote:

 On 2011-07-18 22:21, Kohei KaiGai wrote:

 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.

 Installing right now, thanks for the heads up!

 Would it be reasonable to #ifdefs the parts that require version 2.0.99?
 That's very recent so might not be available on popular distributions for
 some time, so it would be nice to not have a hard dependency on it. You
 could have autoconf rules to check for the new functions, and only use them
 if they are available.

 In contrary to the subject I was under the impression the current patch is
 for the 9.2 release since it is in a commitfest for the 9.2 release cycle,
 which would make the libselinux-2.0.99 dependency less of a problem.

Sorry, the subject line was just my typo.

I'd like to agree with Yeb's opinion, because the reason why we determined
libselinux-2.0.93 as least requirement is the implementation in v9.1 used
a function that was supported in 2.0.93 due to omission of userspace avc
feature from the initial version to minimize patch size.
If we included userspace avc feature from the beginning, it would require
libselinux-2.0.99.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-20 Thread 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.
This is a review of the two userspace access vector cache patches for 
SE-PostgreSQL. Proofreading the source I've seen mostly cosmetic things. 
Though I have a few questions, I think the overal shape of the patch is 
good enough to mark it ready for comitter.


Remarks:

* The patches apply cleanly, compile cleanly on Fedora 15. It depends on 
libselinux-2.0.99, and that is checked on by the configure script: good.


* I run SELECT sepgsql_restorecon(NULL) and saw comparable numbers in 
speed gain and also backend process memory increase as indicated by 
KaiGai-san. The vmsize without the patch increases from running 
restorecon 3864kB, with the patch is increases 8160kB, a difference of 
~5MB. Where this change comes from is unclear to me. The avc_cache holds 
only 7 entries, and the avc memory context stats indicate it's only at 
8kB. Investigation into the SECLABELOID syscache revealed that even when 
that cache was set to a #buckets of 2, still after restorecon() the 
backend's vmsize increased about the ~5MB more.


* The size of SECLABELOID is currently set to 2048, which is equal to 
sizes of the pg_proc and pg_attribute caches and hence makes sense. The 
initial contents of pg_seclabel is 3346 entries. Just to get some idea 
what the cachesize means for restorecon performance I tested some 
different values (debugging was on). Until a size of 256, restorecon had 
comparable performance. Small drop from ~415 to ~425 from 128 to 64. 
Cache of 32 had ~435ms performance. Cache of 2 had 680ms. Without 
debugging symbols, I also got a slightly better performance on the 
restorecon call with a 1024 syscache size. This might be something to 
tweak in later versions, when there is more experience what this cache 
size means for performance on real databases.


* The cache is called userspace, presumably because it isn't cached in 
shared memory? If sepgsql is targeted at installations with many 
different kinds of clients (having different subject contexts), or 
access different objects, this is a good choice.


* Checked that the cache keeps working after errors, ok.

* uavc.c defines some static variables like lru_hint, threshold and 
unlabeled. It would be nice to have a small comment with each one that 
says what the variable represents (like is done for the avc_cache struct 
members right above it)


* I had to google SELINUX_AVD_FLAGS_PERMISSIVE to understand what was 
going on. Since the goal why it is recorded a domain is permissive, is 
to prevent flooding the log, maybe renaming avc_cache.permissive into 
avc_cache.log_violations_once would explain itself.


* selinux_status_open() is called and it's man page mentions 
selinux_status_close(). It currently unmaps memory and closes a file 
descriptor, which is done on process exit anyway. I don't have enough 
experience with these kinds of system calls to have a feeling whether it 
might change in the future (and in such cases I'd have added a call to 
selinux_status_close() on proc_exit, just to be on the safe side).


* sepgsel_avs_unlabeled(). I was confused a bit because objects can have 
a label 'unlabeled', and the comments use wording like 'when unlabeled'. 
Does this mean with no label, or with a label 'unlabeled'?


* sepgsql_avc_compute(): the large comment in the beginning is hard to 
follow since sentences seem to have been a bit mixed up.


* question: why is ucontext stored in avc_cache?

* there is a new guc parameter for the user: avc_threshold, which is 
also documented. My initial question after reading the description was: 
what are the 'items' and how can I see current usage / what are numbers 
to expect? Without that knowledge any educated change of that parameter 
would be impossible. Would it be possible to remove this guc?


* avc_init has magic numbers 384, 4096. Maybe these can be defines (with 
a small comment what it is?)


* Finally, IMO the call sites, e.g. check_relation_priviliges, have 
improved in readability with this patch.


--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Jul 20, 2011 at 9:47 AM, Yeb Havinga yebhavi...@gmail.com wrote:
 * I run SELECT sepgsql_restorecon(NULL) and saw comparable numbers in speed
 gain and also backend process memory increase as indicated by KaiGai-san.
 The vmsize without the patch increases from running restorecon 3864kB, with
 the patch is increases 8160kB, a difference of ~5MB. Where this change comes
 from is unclear to me. The avc_cache holds only 7 entries, and the avc
 memory context stats indicate it's only at 8kB. Investigation into the
 SECLABELOID syscache revealed that even when that cache was set to a
 #buckets of 2, still after restorecon() the backend's vmsize increased about
 the ~5MB more.

 * The size of SECLABELOID is currently set to 2048, which is equal to sizes
 of the pg_proc and pg_attribute caches and hence makes sense. The initial
 contents of pg_seclabel is 3346 entries. Just to get some idea what the
 cachesize means for restorecon performance I tested some different values
 (debugging was on). Until a size of 256, restorecon had comparable
 performance. Small drop from ~415 to ~425 from 128 to 64. Cache of 32 had
 ~435ms performance. Cache of 2 had 680ms. Without debugging symbols, I also
 got a slightly better performance on the restorecon call with a 1024
 syscache size. This might be something to tweak in later versions, when
 there is more experience what this cache size means for performance on real
 databases.

Both of the above points make me real nervous, especially the second
one.  We already know that the cost of initializing the system caches
is a significant chunk of backend startup overhead, and I believe that
sizeof(Dllist) = 16 on 64-bit machine, so that means that 2048-entry
catcache is going to require us to zero an additional 32kB of memory
on every backend startup for a feature that most people won't use.

http://archives.postgresql.org/pgsql-hackers/2010-11/msg01632.php
http://archives.postgresql.org/pgsql-hackers/2010-11/msg01640.php

As to the first point, the other big problem with adding a syscache
here is that it could get really big.  I've worried about that issue
previously, and Yev's perplexity about where the memory is going is
not too reassuring.

I think there are two realistic ways forward here:

1. Bite the bullet and implement a system allowing catalog caches to
be expanded on the fly at runtime.  This would be nice because, aside
from whatever value it may have for SE-PostgreSQL, it might allow us
to shrink some of the other caches down and thereby speed up backend
startup in general, with the confidence that if the session ends up
running for a while we can expand them as necessary.  In the
particular case of SE-PostgreSQL, it might be nice to be able to set
the initial cache size to 0, and only pay the cost of setting it up if
we discover that the security label feature is in use; but maybe
that's overly complex.  On the downside, this addresses only the
startup problem (zeroing previously unreferenced memory is fairly
expensive) and not the runtime problem (using too much memory is bad).
 But perhaps there is some other solution to the second problem.

2. Implement a custom cache tailored to the needs of SE-PostgreSQL
within contrib/sepgsql.  This is a bit tricky because you need some
mechanism for receiving invalidation events.
CacheRegisterSyscacheCallback() is the usual method, but that only
lets you notice catcache invalidations, and here the whole point would
be to avoid having a catcache in the first place.  Possibly we could
add a hook to PrepareForTupleInvalidation(); assuming that the hook
engages only after the IsSystemRelation and IsToastRelation checks, it
shouldn't cost that much.  Doing it this way would (1) allow the
sepgsql-specific cache to come into existence only when needed and (2)
allow the sepgsl-specific cache to apply sepgsql-specific policies for
controlling memory use.  For example, it could cap the number of
entries in the cache and age out any that are too old; or it could
arrange to maintain a single copy of each label rather than a copy for
each object.

I think that the uavc cache stuff may still be something we can think
about committing for this 'fest (I haven't reviewed it yet), but IMHO
the syscache parts need some more thought.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-20 Thread Yeb Havinga


On 2011-07-20 16:54, Robert Haas wrote:


As to the first point, the other big problem with adding a syscache
here is that it could get really big.  I've worried about that issue
previously, and Yev's perplexity about where the memory is going is
not too reassuring.
Yeah I just realized that #buckets  cache items stored, which makes 
some of the comments I made a bit strange. If the syscaches store 
everything that's looked up then the same 5MB memory usage under 
changing #buckets makes perfect sense.


regards,
Yeb


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

Yeb, Thanks for your volunteering.

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.
This is a review of the two userspace access vector cache patches for
SE-PostgreSQL. Proofreading the source I've seen mostly cosmetic things.
Though I have a few questions, I think the overal shape of the patch is
good enough to mark it ready for comitter.

Remarks:

* The patches apply cleanly, compile cleanly on Fedora 15. It depends on
libselinux-2.0.99, and that is checked on by the configure script: good.

* I run SELECT sepgsql_restorecon(NULL) and saw comparable numbers in
speed gain and also backend process memory increase as indicated by
KaiGai-san. The vmsize without the patch increases from running
restorecon 3864kB, with the patch is increases 8160kB, a difference of
~5MB. Where this change comes from is unclear to me. The avc_cache holds
only 7 entries, and the avc memory context stats indicate it's only at
8kB. Investigation into the SECLABELOID syscache revealed that even when
that cache was set to a #buckets of 2, still after restorecon() the
backend's vmsize increased about the ~5MB more.

The sepgsql_restorecon(NULL) assigns default security label on all the
database objects being controlled, thus, its workload caches security
label (including text data) of these objects.
So, ~5MB of difference is an upper limit of syscache usage because of
SECLABELOID.
The number of buckets is independent from the memory consumption.

* The size of SECLABELOID is currently set to 2048, which is equal to
sizes of the pg_proc and pg_attribute caches and hence makes sense. The
initial contents of pg_seclabel is 3346 entries. Just to get some idea
what the cachesize means for restorecon performance I tested some
different values (debugging was on). Until a size of 256, restorecon had
comparable performance. Small drop from ~415 to ~425 from 128 to 64.
Cache of 32 had ~435ms performance. Cache of 2 had 680ms. Without
debugging symbols, I also got a slightly better performance on the
restorecon call with a 1024 syscache size. This might be something to
tweak in later versions, when there is more experience what this cache
size means for performance on real databases.

Thanks for your research.
The reason why I set 2048 is just a copy from the catalog which may
hold biggest number of entries (pg_proc). It may be improved later.

* The cache is called userspace, presumably because it isn't cached in
shared memory? If sepgsql is targeted at installations with many
different kinds of clients (having different subject contexts), or
access different objects, this is a good choice.

SELinux's kernel implementation also has access vector cache:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=security/selinux/avc.c

The reason why I didn't choose shared memory is the length of security
label text is variable, so it is not feasible to acquire a fixed length
region on shared memory in startup time.

* Checked that the cache keeps working after errors, ok.

* uavc.c defines some static variables like lru_hint, threshold and
unlabeled. It would be nice to have a small comment with each one that
says what the variable represents (like is done for the avc_cache struct
members right above it)

OK, I'll add comments here.

* I had to google SELINUX_AVD_FLAGS_PERMISSIVE to understand what was
going on. Since the goal why it is recorded a domain is permissive, is
to prevent flooding the log, maybe renaming avc_cache.permissive into
avc_cache.log_violations_once would explain itself.

It is a bit concern for me, because the permissive domain means it shall
be performed like as system is configured as permissive mode.
The behavior of log-violation-at-once is a property of permissive mode.
So, how about an idea to add comments about permissive domain around
the code to modify cache-allowed?

* selinux_status_open() is called and it's man page mentions
selinux_status_close(). It currently unmaps memory and closes a file
descriptor, which is done on process exit anyway. I don't have enough
experience with these kinds of system calls to have a feeling whether it
might change in the future (and in such cases I'd have added a call to
selinux_status_close() on proc_exit, just to be on the safe side).

I omitted it because OS will handle these things correctly on process
Exit time, however, it is good idea to move on safe side.

* sepgsel_avs_unlabeled(). I was confused a bit because objects can have
a label 'unlabeled', and the comments use wording like 'when unlabeled'.
Does this mean with no label, or with a label 'unlabeled'?

It means object has no label.

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

On Wed, Jul 20, 2011 at 12:04 PM, Kohei Kaigai
kohei.kai...@emea.nec.com wrote:
 The sepgsql_restorecon(NULL) assigns default security label on all the
 database objects being controlled, thus, its workload caches security
 label (including text data) of these objects.
 So, ~5MB of difference is an upper limit of syscache usage because of
 SECLABELOID.

No, it's not.  It's just the upper limit of how large it can be on an
*empty* database.  A real database could have hundreds of tables and
views and thousands of columns.  To say nothing of large objects.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Jul 20, 2011 at 12:40 PM, Kohei Kaigai
kohei.kai...@emea.nec.com wrote:
 One question is why InitCatalogCache() should be invoked from InitPostgres()?
 If we initialize syscache on starting up postmaster process, I think
 all the syscache buckets will be ready on child process forks, and
 unused syscache does not consume physical memory, even if security
 label acquire 2048 of buckets.

Most of the overhead seems to be the cost of the page faults required
for the kernel to map the relevant pages into the process address
space.  After a fork(), all those pages become copy-on-write, so if
the syscaches are actually used this doesn't save much of anything.
In the specific case of sepgsql it would help, though, because what
you're proposing is to add a syscache that will in most cases never be
accessed at all.  We'd still have a problem in the EXEC_BACKEND (i.e.
Windows) case, however...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

 On Wed, Jul 20, 2011 at 12:40 PM, Kohei Kaigai
 kohei.kai...@emea.nec.com wrote:
  One question is why InitCatalogCache() should be invoked from 
  InitPostgres()?
  If we initialize syscache on starting up postmaster process, I think
  all the syscache buckets will be ready on child process forks, and
  unused syscache does not consume physical memory, even if security
  label acquire 2048 of buckets.
 
 Most of the overhead seems to be the cost of the page faults required
 for the kernel to map the relevant pages into the process address
 space.  After a fork(), all those pages become copy-on-write, so if
 the syscaches are actually used this doesn't save much of anything.
 In the specific case of sepgsql it would help, though, because what
 you're proposing is to add a syscache that will in most cases never be
 accessed at all.  We'd still have a problem in the EXEC_BACKEND (i.e.
 Windows) case, however...
 
Hmm. It might not work in windows case.

I'd like to have a discussion about syscache towards next commit-fest.
The issues may be:
 - Initial bucket allocation on most cases never be referenced.
 - Reclaim cache entries on growing up too large.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

 On Wed, Jul 20, 2011 at 12:04 PM, Kohei Kaigai
 kohei.kai...@emea.nec.com wrote:
  The sepgsql_restorecon(NULL) assigns default security label on all the
  database objects being controlled, thus, its workload caches security
  label (including text data) of these objects.
  So, ~5MB of difference is an upper limit of syscache usage because of
  SECLABELOID.
 
 No, it's not.  It's just the upper limit of how large it can be on an
 *empty* database.  A real database could have hundreds of tables and
 views and thousands of columns.  To say nothing of large objects.
 
Ah, sorry, you are correct.

Regarding to large objects, GetSecurityLabel() is modified not to use
SECLABELOID to flood of the syscache.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

 On Wed, Jul 20, 2011 at 9:47 AM, Yeb Havinga yebhavi...@gmail.com wrote:
  * I run SELECT sepgsql_restorecon(NULL) and saw comparable numbers in speed
  gain and also backend process memory increase as indicated by KaiGai-san.
  The vmsize without the patch increases from running restorecon 3864kB, with
  the patch is increases 8160kB, a difference of ~5MB. Where this change comes
  from is unclear to me. The avc_cache holds only 7 entries, and the avc
  memory context stats indicate it's only at 8kB. Investigation into the
  SECLABELOID syscache revealed that even when that cache was set to a
  #buckets of 2, still after restorecon() the backend's vmsize increased about
  the ~5MB more.
 
  * The size of SECLABELOID is currently set to 2048, which is equal to sizes
  of the pg_proc and pg_attribute caches and hence makes sense. The initial
  contents of pg_seclabel is 3346 entries. Just to get some idea what the
  cachesize means for restorecon performance I tested some different values
  (debugging was on). Until a size of 256, restorecon had comparable
  performance. Small drop from ~415 to ~425 from 128 to 64. Cache of 32 had
  ~435ms performance. Cache of 2 had 680ms. Without debugging symbols, I also
  got a slightly better performance on the restorecon call with a 1024
  syscache size. This might be something to tweak in later versions, when
  there is more experience what this cache size means for performance on real
  databases.
 
 Both of the above points make me real nervous, especially the second
 one.  We already know that the cost of initializing the system caches
 is a significant chunk of backend startup overhead, and I believe that
 sizeof(Dllist) = 16 on 64-bit machine, so that means that 2048-entry
 catcache is going to require us to zero an additional 32kB of memory
 on every backend startup for a feature that most people won't use.
 
 http://archives.postgresql.org/pgsql-hackers/2010-11/msg01632.php
 http://archives.postgresql.org/pgsql-hackers/2010-11/msg01640.php
 
 As to the first point, the other big problem with adding a syscache
 here is that it could get really big.  I've worried about that issue
 previously, and Yev's perplexity about where the memory is going is
 not too reassuring.
 
 I think there are two realistic ways forward here:
 
 1. Bite the bullet and implement a system allowing catalog caches to
 be expanded on the fly at runtime.  This would be nice because, aside
 from whatever value it may have for SE-PostgreSQL, it might allow us
 to shrink some of the other caches down and thereby speed up backend
 startup in general, with the confidence that if the session ends up
 running for a while we can expand them as necessary.  In the
 particular case of SE-PostgreSQL, it might be nice to be able to set
 the initial cache size to 0, and only pay the cost of setting it up if
 we discover that the security label feature is in use; but maybe
 that's overly complex.  On the downside, this addresses only the
 startup problem (zeroing previously unreferenced memory is fairly
 expensive) and not the runtime problem (using too much memory is bad).
  But perhaps there is some other solution to the second problem.
 
One question is why InitCatalogCache() should be invoked from InitPostgres()?
If we initialize syscache on starting up postmaster process, I think
all the syscache buckets will be ready on child process forks, and
unused syscache does not consume physical memory, even if security
label acquire 2048 of buckets.

 2. Implement a custom cache tailored to the needs of SE-PostgreSQL
 within contrib/sepgsql.  This is a bit tricky because you need some
 mechanism for receiving invalidation events.
 CacheRegisterSyscacheCallback() is the usual method, but that only
 lets you notice catcache invalidations, and here the whole point would
 be to avoid having a catcache in the first place.  Possibly we could
 add a hook to PrepareForTupleInvalidation(); assuming that the hook
 engages only after the IsSystemRelation and IsToastRelation checks, it
 shouldn't cost that much.  Doing it this way would (1) allow the
 sepgsql-specific cache to come into existence only when needed and (2)
 allow the sepgsl-specific cache to apply sepgsql-specific policies for
 controlling memory use.  For example, it could cap the number of
 entries in the cache and age out any that are too old; or it could
 arrange to maintain a single copy of each label rather than a copy for
 each object.
 
I'm interested in this idea, however, not a work in this commit-fest.

So, I'll detach syscache part from the existing uavc patch (and shared
object's security label patch).

 I think that the uavc cache stuff may still be something we can think
 about committing for this 'fest (I haven't reviewed it yet), but IMHO
 the syscache parts need some more thought.
 
Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com

-- 
Sent via pgsql-hackers mailing list

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

2011-07-20 Thread Josh Berkus

Kaigai,

 I'd like to have a discussion about syscache towards next commit-fest.
 The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

Should I move this patch to the next CF?

-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Jul 20, 2011 at 4:06 PM, Josh Berkus j...@agliodbs.com wrote:
 Kaigai,

 I'd like to have a discussion about syscache towards next commit-fest.
 The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

 Should I move this patch to the next CF?

Not yet.  I'm still going to review the other half of this patch.

The discussion above should be undertaken on a new thread.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Jul 20, 2011 at 4:19 PM, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Jul 20, 2011 at 4:06 PM, Josh Berkus j...@agliodbs.com wrote:
 Kaigai,

 I'd like to have a discussion about syscache towards next commit-fest.
 The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

 Should I move this patch to the next CF?

 Not yet.  I'm still going to review the other half of this patch.

On second thought, never mind: the second half still needs another
update from KaiGai, and I think we shouldn't hold the CF open for
that.  So I'll mark this Returned with Feedback.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-20 Thread Tom Lane

Kohei Kaigai kohei.kai...@emea.nec.com writes:
 I'd like to have a discussion about syscache towards next commit-fest.
 The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

There used to be support for limiting the number of entries in a
syscache.  It got removed (cf commit
8b9bc234ad43dfa788bde40ebf12e94f16556b7f) because (1) it was remarkably
expensive to do it (extra list manipulations, etc), and (2) performance
tended to fall off a cliff as soon as you had a few more tables or
whatever than the caches would hold.  I'm disinclined to reverse that
decision.  It appears to me that the security label stuff needs a
different set of performance tradeoffs than the rest of the catalogs,
which means it probably ought to do its own caching, rather than trying
to talk us into pessimizing the other catalogs for seclabel's benefit.

regards, tom lane

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

On Wed, Jul 20, 2011 at 4:48 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Kohei Kaigai kohei.kai...@emea.nec.com writes:
 I'd like to have a discussion about syscache towards next commit-fest.
 The issues may be:
  - Initial bucket allocation on most cases never be referenced.
  - Reclaim cache entries on growing up too large.

 There used to be support for limiting the number of entries in a
 syscache.  It got removed (cf commit
 8b9bc234ad43dfa788bde40ebf12e94f16556b7f) because (1) it was remarkably
 expensive to do it (extra list manipulations, etc), and (2) performance
 tended to fall off a cliff as soon as you had a few more tables or
 whatever than the caches would hold.  I'm disinclined to reverse that
 decision.  It appears to me that the security label stuff needs a
 different set of performance tradeoffs than the rest of the catalogs,
 which means it probably ought to do its own caching, rather than trying
 to talk us into pessimizing the other catalogs for seclabel's benefit.

I agree that we don't want to limit the size of the catcaches.  We've
been careful to design them in such a way that they won't blow out
memory, and so far there's no evidence that they do.  If it ain't
broke, don't fix it.  Having catcaches that can grow in size as needed
sounds useful to me, though.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-19 Thread Yeb Havinga


On 2011-07-18 22:21, Kohei KaiGai wrote:

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.

Installing right now, thanks for the heads up!


/etc/selinux/targeted/contexts/sepgsql_contexts:  line 33 has invalid object
type db_blobs

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.
This is in contradiction with the current phrase in the documentation 
that's right after the sepgsql.sql loading: If the installation process 
completes without error, you can now start the server normally. IMHO if 
there are warnings that can be ignored, it would limit confusion for 
sepgsql users if the documentation would say it at this point, e.g. If 
the installation process completes without error, you can now start the 
server normally. Warnings from errors in sepgsql_contexts, a file 
external to PostgreSQL, are harmless and can be ignored.



The point of this patch is replacement of existing mechanism...
So, it is not necessary to define a new policy for testing.

Thanks for elaborating on this.

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).
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.
You can define your own policy, however, I intend to run regression test
without any modification of the default security policy.


Thank you for this clarification. I have some ideas of things that if 
they were in the documentation they'd helped me. Instead of seeking 
agreement on each item, I propose that I gather documentation additions 
in a patch later after the review, and leave it up to you guys whether 
to include them or not.


regards,
Yeb
--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-19 Thread Kohei Kaigai

  /etc/selinux/targeted/contexts/sepgsql_contexts:  line 33 has invalid 
  object
  type db_blobs
  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.
 This is in contradiction with the current phrase in the documentation
 that's right after the sepgsql.sql loading: If the installation process
 completes without error, you can now start the server normally. IMHO if
 there are warnings that can be ignored, it would limit confusion for
 sepgsql users if the documentation would say it at this point, e.g. If
 the installation process completes without error, you can now start the
 server normally. Warnings from errors in sepgsql_contexts, a file
 external to PostgreSQL, are harmless and can be ignored.
 
Indeed, it might be confusable to understand whether the installation got
completed correctly, or not.
So, I appended more descriptions about this messages, as follows:

+  para
+   Please note that you may see the following notifications depending on
+   the combination of a particular version of productnamelibselinux/
+   and productnameselinux-policy/.
+screen
+/etc/selinux/targeted/contexts/sepgsql_contexts:  line 33 has invalid object ty
+/screen
+   It is harmless messages and already fixed. So, you can ignore these
+   messages or update related packages to the latest version.
+  /para

See the attached patch, that contains other 3 documentation updates.

 Thank you for this clarification. I have some ideas of things that if
 they were in the documentation they'd helped me. Instead of seeking
 agreement on each item, I propose that I gather documentation additions
 in a patch later after the review, and leave it up to you guys whether
 to include them or not.
 
OK, I like to check them. In addition, I'll also revise the wikipage in
parallel to inform correctly.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei kohei.kai...@emea.nec.com


pgsql-sepgsql-doc-revise.2.patch
Description: pgsql-sepgsql-doc-revise.2.patch

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-19 Thread Yeb Havinga


On 2011-07-19 12:10, Kohei Kaigai wrote:


See the attached patch, that contains other 3 documentation updates.
I looked at the patch and the additions look good, though I didn't 
actually apply it yet.


thanks
Yeb


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-19 Thread Heikki Linnakangas


On 19.07.2011 12:28, Yeb Havinga wrote:

On 2011-07-18 22:21, Kohei KaiGai wrote:

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.

Installing right now, thanks for the heads up!


Would it be reasonable to #ifdefs the parts that require version 2.0.99? 
That's very recent so might not be available on popular distributions 
for some time, so it would be nice to not have a hard dependency on it. 
You could have autoconf rules to check for the new functions, and only 
use them if they are available.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-19 Thread Yeb Havinga


On 2011-07-19 22:39, Heikki Linnakangas wrote:

On 19.07.2011 12:28, Yeb Havinga wrote:

On 2011-07-18 22:21, Kohei KaiGai wrote:

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.

Installing right now, thanks for the heads up!


Would it be reasonable to #ifdefs the parts that require version 
2.0.99? That's very recent so might not be available on popular 
distributions for some time, so it would be nice to not have a hard 
dependency on it. You could have autoconf rules to check for the new 
functions, and only use them if they are available.


In contrary to the subject I was under the impression the current patch 
is for the 9.2 release since it is in a commitfest for the 9.2 release 
cycle, which would make the libselinux-2.0.99 dependency less of a problem.


--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-18 Thread Yeb Havinga


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.


These things should probably be added to the 9.1beta3 documentation branch:

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.

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).


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.


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.


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'.


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?


3) In the examples so far I've seen unconfined_u and system_u? Can I 
define my own?


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 KaiGaikai...@kaigai.gr.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 KaiGaikai...@kaigai.gr.jp:

The attached patch re-defines

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

2011-07-18 Thread Kohei KaiGai

Yeb, Thanks for your volunteering.

2011/7/18 Yeb Havinga yebhavi...@gmail.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

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

2011-07-18 Thread Robert Haas

On Mon, Jul 18, 2011 at 4:21 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 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.

Actually, I think it's that way mostly because we committed the
SECURITY LABEL stuff first.  I'd be in favor of adding some kind of
cross-link.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-18 Thread Kohei KaiGai

2011/7/18 Robert Haas robertmh...@gmail.com:
 On Mon, Jul 18, 2011 at 4:21 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 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.

 Actually, I think it's that way mostly because we committed the
 SECURITY LABEL stuff first.  I'd be in favor of adding some kind of
 cross-link.

Hmm. OK, I'll submit a patch to update this documentation stuff tomorrow,
including an explicit -D option as Yeb mentioned.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-16 Thread 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.


I will be able to look at this patch next week on monday and tuesday, 
without wanting to raise any expectations about that time being enough 
for me to say anything useful. On a longer timescale, I believe that 
sepgsql is one of the most important new features of PostgreSQL and that 
I want to commit myself to spend more community work on this subject.


regards,
Yeb

--

Yeb Havinga
http://www.mgrid.net/
Mastering Medical Data


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-07-02 Thread Kohei KaiGai

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 kai...@kaigai.gr.jp:
 2011/6/13 Robert Haas robertmh...@gmail.com:
 On Mon, Jun 13, 2011 at 7:51 AM, Kohei KaiGai kai...@kaigai.gr.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 kai...@kaigai.gr.jp




-- 
KaiGai Kohei kai...@kaigai.gr.jp


pgsql-v9.2-uavc-syscache.v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-13 Thread Robert Haas

On Mon, Jun 13, 2011 at 7:51 AM, Kohei KaiGai kai...@kaigai.gr.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.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-13 Thread Kohei KaiGai

2011/6/13 Robert Haas robertmh...@gmail.com:
 On Mon, Jun 13, 2011 at 7:51 AM, Kohei KaiGai kai...@kaigai.gr.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 kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-11 Thread Robert Haas

On Thu, Jun 9, 2011 at 3:09 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 Here is two level lookups.
 The first is from object identifiers to security label; it can be boosted
 using syscache mechanism. The second is from security labels to
 access control decision; it can be boosted using userspace avc.

OK.  Let's have two separate patches, then.

Thinking a bit more about the issue of adding a syscache, I suspect
it's probably OK to use that mechanism rather than inventing something
more complicated that can kick out entries on an LRU basis.  Even if
you accessed a few tens of thousands of entries, the cache shouldn't
grow to more than a few megabytes.  And that's presumably going to be
rare, and could happen with other types of objects (such as tables)
using the existing system caches.  So I guess it's probably premature
optimization to worry about the issue now.

I would, however, like to see some performance results indicating how
much the cache helps, and how much memory it eats up in the process.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] [v9.1] sepgsql - userspace access vector cache

The attached patch adds contrib/sepgsql a cache mechanism for access
control decision of SELinux. It shall reduce the total number of
system call invocations to improve the performance on its access
controls.

In the current implementation, the sepgsql always raises a query to
SELinux in-kernel. However, same answer shall be returned for some
pair of security labels and object class, unless the security policy
got reloaded.
It is a situation caching mechanism works well. Of course, we don't
assume the security policy is reloaded so frequently.

I tried to measure the performance to run sepgsql_restorecon(NULL)
that is used to assign initial labels of schemas, relations, columns
and procedures. It also invokes massive number of relabelfrom and
relabelto permission checks.

$ time -p psql -c 'SELECT sepgsql_restorecon(NULL);' postgres

without patch
real 2.73
real 2.70
real 2.72
real 2.67
real 2.68

with patch
real 0.67
real 0.61
real 0.63
real 0.63
real 0.63

The improvement is obvious.

From the viewpoint of implementation, this patch replaces
sepgsql_check_perms() by sepgsql_avc_check_perms(), from non-cache
interface to cached interface.
Every cached items are hashed using a pair of security labels and
object class, so, even if different objects have same security label,
system call invocation shall happen only once for an identical
combination.

The only modification by this patch to the core routine is a new
syscache for pg_seclabel system catalog. The SECLABELOID enables to
reference security label of the object using syscache interface.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp


sepgsql-uavc.1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-09 Thread Robert Haas

On Thu, Jun 9, 2011 at 3:59 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

I believe we decided against that previously on the grounds that we
don't want to add syscaches that might get really really big.  In
particular, there could be a LOT of labelled large objects floating
around.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-09 Thread Stephen Frost

* Kohei KaiGai (kai...@kaigai.gr.jp) wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

Perhaps I'm missing it, but.. why is this necessary to implement such a
cache?  Also, I thought the SELinux userspace libraries provided a cache
solution?  This issue is hardly unique to SELinux in PostgreSQL...

THanks,

Stephen


signature.asc
Description: Digital signature

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

2011/6/9 Robert Haas robertmh...@gmail.com:
 On Thu, Jun 9, 2011 at 3:59 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

 I believe we decided against that previously on the grounds that we
 don't want to add syscaches that might get really really big.  In
 particular, there could be a LOT of labelled large objects floating
 around.

(Sorry, I missed to Cc: pgsql-hackers, so send again)

As long as we use syscache mechanism to hold security label of
relation or other cached objects, do you think it cause no troubles?

If so, it may be a good idea to distinct cases when we try to reference
the security label of blobs and others.
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011/6/9 Stephen Frost sfr...@snowman.net:
 * Kohei KaiGai (kai...@kaigai.gr.jp) wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

 Perhaps I'm missing it, but.. why is this necessary to implement such a
 cache?  Also, I thought the SELinux userspace libraries provided a cache
 solution?  This issue is hardly unique to SELinux in PostgreSQL...

I'm concerned about its interface, although it might be suitable for
X-Windows...

Its avc interface identifies security context using a pointer of
malloc()'ed cstring.
In our case, we need to look up this security context on the hash managed by
libselinux using the result of syscache lookup. It is quite nonsense.

In addition, avc of libselinux confirms whether the security policy is reloaded
for each avc lookup, unless we launch a system state monitoring thread.
But, it is not a suitable design to launch a worker thread for each
pgsql backend.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-09 Thread Robert Haas

On Thu, Jun 9, 2011 at 12:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2011/6/9 Robert Haas robertmh...@gmail.com:
 On Thu, Jun 9, 2011 at 3:59 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

 I believe we decided against that previously on the grounds that we
 don't want to add syscaches that might get really really big.  In
 particular, there could be a LOT of labelled large objects floating
 around.

 (Sorry, I missed to Cc: pgsql-hackers, so send again)

 As long as we use syscache mechanism to hold security label of
 relation or other cached objects, do you think it cause no troubles?

Maybe, but why do we need it?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011-06-09 Thread Robert Haas

On Thu, Jun 9, 2011 at 12:54 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2011/6/9 Stephen Frost sfr...@snowman.net:
 * Kohei KaiGai (kai...@kaigai.gr.jp) wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

 Perhaps I'm missing it, but.. why is this necessary to implement such a
 cache?  Also, I thought the SELinux userspace libraries provided a cache
 solution?  This issue is hardly unique to SELinux in PostgreSQL...

 I'm concerned about its interface, although it might be suitable for
 X-Windows...

 Its avc interface identifies security context using a pointer of
 malloc()'ed cstring.
 In our case, we need to look up this security context on the hash managed by
 libselinux using the result of syscache lookup. It is quite nonsense.

So you're going to depend on the syscache not to move the pointers
around?  Yikes.

 In addition, avc of libselinux confirms whether the security policy is 
 reloaded
 for each avc lookup, unless we launch a system state monitoring thread.
 But, it is not a suitable design to launch a worker thread for each
 pgsql backend.

I thought there was something you could mmap() into each backend...?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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

2011/6/9 Robert Haas robertmh...@gmail.com:
 On Thu, Jun 9, 2011 at 12:39 PM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 2011/6/9 Robert Haas robertmh...@gmail.com:
 On Thu, Jun 9, 2011 at 3:59 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:
 The only modification by this patch to the core routine is a new
 syscache for pg_seclabel system catalog. The SECLABELOID enables to
 reference security label of the object using syscache interface.

 I believe we decided against that previously on the grounds that we
 don't want to add syscaches that might get really really big.  In
 particular, there could be a LOT of labelled large objects floating
 around.

 (Sorry, I missed to Cc: pgsql-hackers, so send again)

 As long as we use syscache mechanism to hold security label of
 relation or other cached objects, do you think it cause no troubles?

 Maybe, but why do we need it?

Of course, I'd like to look up security label of the referenced object with
smallest cost as possible as we can.

Here is two level lookups.
The first is from object identifiers to security label; it can be boosted
using syscache mechanism. The second is from security labels to
access control decision; it can be boosted using userspace avc.

Thanks,
-- 
KaiGai Kohei kai...@kaigai.gr.jp

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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