date:20091213

(2009/12/13 12:18), Robert Haas wrote:

On Sat, Dec 12, 2009 at 7:41 PM, Josh Berkusj...@agliodbs.com wrote:

Stephen,

Please comment, update the wiki, let us know you're interested in this..

I blogged about this some time ago. One issue I can see is that I
believe that the RLS which many users want is different from the RLS
which SEPostgres implements.

Links:

http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732
http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757

--Josh Berkus

I read these blog entries a while ago but had forgotten about them.
They're very good, and summarize a lot of my thinking on this topic as
well. I think that we can design a framework for row-level security
which can encompass both constraint-based security and label-based
security. Both seem to me to be be based around doing essentially the
following things:

1. Adding columns to the table to store access control information.
2. Populating those columns with additional information (owner, ACL,
security label, etc.) which can be used to make access control
decisions.
3. Injecting logic into incoming queries which uses the information
inserted by (1) to filter out rows to which the access control policy
does not wish to allow access.

Waving my hands in the air, #1 and #2 seem pretty straightforward.
For constraint-based security, one can imagine just adding a column to
the table and then adding a BEFORE INSERT OR UPDATE FOR EACH ROW
trigger that populates that column. For label-based MAC, that's not
going to be quite sufficient, because the system needs to ensure that
the trigger that populates the security column must run last; if it
doesn't, some other trigger can come along afterwards and slip in a
value that isn't supposed to be there; plus, it might be inconvenient
to need to define this trigger for every table that needs RLS.

Right, label-based MAC need its hook being called after all the BR-Insert
triggers to assign a correct security label, not only access controls.
I'd like to point out one more thing. When we update tuples, invisible
tuples have to be filtered out before trigger functions.

However, those problems don't seem insurmountable. Suppose we provide
a hook function that essentially acts like a global BEFORE INSERT OR
UPDATE trigger but which fires after all of the regular triggers.

Basically, right. In my branch, SE-PgSQL put its hook after all the BR
trigger invocations.

http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.4.x/sepgsql/src/backend/executor/execMain.c#1883

But we have another approach. When RelationBuildTriggers() initializes
TriggerDesc of Relation, we can inject security hook as a special BR-trigger
at the last. If we initialize it here, we don't need to modify COPY FROM
implementation, not only INSERT.

The reason why I didn't apply this approach is it needs more modification
to the core routines, so it makes harder to manage out-of-tree code.

SE-PostgreSQL can gain control at that point and search through the
columns of the target relation for a column called, say,
sepg_security_label. If it finds such a column and that column is of
the appropriate type, then (1) if an explicit security label is
provided, it checks whether the specified label is permissible, (2)
otherwise, if the operation is insert, it determines the appropriate
default label for the current security context and inserts it, (3)
otherwise, it just leaves the current label alone. This might not be
quite the right behavior but the point is whatever behavior you want
to have in terms of assigning/disallowing values for that column
should be possible to implement here. The upshot is that if the
system administrator creates an sepg_security_label column of the
correct type, row-level security will be enabled for that table.
Otherwise, it will not.

Basically, right. SE-PgSQL (or others) assign a new tuple either an
explicitly given or a default security label, then it checks permission
whether the client can insert a tuple with this label, or not.

One point. MAC is mandatory, so the table owner should not be able to
control whether row-level checks are applied, or not.
So, I used a special purpose system column to represent security label.
It is generated for each tables, and no additional storage consumption
when MAC feature is disabled.

#3 seems a little bit trickier. I don't think the GRANT ... WHERE
syntax is going to be very easy to use. For constraint-based
row-security, I think we should have something more like:

ALTER TABLE table ADD ROW FILTER filtername USING othertable [, ...]
WHERE where-clause

(This suffers from the same problem as DELETE ... USING, namely that
sometimes you want an outer join between table and othertable.)

This gives the user a convenient way to insert a join against one or
more side tables if they are so inclined.

Is it reasonably possible to implement USING clause, even if

[HACKERS] [PATCH] ACE Framework - Database, Schema

Stephen,

The attached two patches are the first pieces of split out from
the previous large access control reworks patch.

The pgsql-ace-01-database-8.5devel-r2475.patch contains nigh
security hooks related to global initialization and databases.

The pgsql-ace-02-schema-8.5devel-r2475.patch contains the six
security hooks related to schema objects.

Note that these are not simple replacement for pg_xxx_aclcheck()
and pg_xxx_ownercheck(). For example, DefineRelation() calls
pg_namespace_aclcheck() with ACL_CREATE. This check shall be
abstracted in the pgsql-ace-0x-relation patch, so I don't touch
them yet.

Also note that these patches don't support any security label.
So, ace_xxx_create() is declared as void function, although it
has to return a security label to be assigned.
But these hooks are deployed on where we can easily support
security label management, so later patch will fix it.

The previous patch is too large to review.
Is this scale confortable to review?

$ diffstat pgsql-ace-01-database-8.5devel-r2475.patch
 backend/Makefile|2
 backend/catalog/aclchk.c|   68 +++!
 backend/commands/comment.c  |5
 backend/commands/dbcommands.c   |  154 +!
 backend/commands/indexcmds.c|6
 backend/security/Makefile   |   10 +
 backend/security/ace/Makefile   |   11 +
 backend/security/ace/ace_database.c |  285 
 backend/security/ace/ace_misc.c |   23 ++
 backend/utils/adt/dbsize.c  |9
 backend/utils/init/postinit.c   |   17 !!
 include/security/ace.h  |   39 
 12 files changed, 445 insertions(+), 63 deletions(-), 121 modifications(!)

$ diffstat pgsql-ace-02-schema-8.5devel-r2475.patch
 backend/catalog/aclchk.c  |   15 +!
 backend/catalog/namespace.c   |   42 ++---!!
 backend/commands/comment.c|4
 backend/commands/schemacmds.c |   57 -!
 backend/security/ace/Makefile |2
 backend/security/ace/ace_schema.c |  200 ++
 backend/tcop/fastpath.c   |6 !
 include/security/ace.h|   14 ++
 8 files changed, 234 insertions(+), 25 deletions(-), 81 modifications(!)

-- 
KaiGai Kohei kai...@kaigai.gr.jp
diff -Nrpc base/src/backend/Makefile ace_database/src/backend/Makefile
*** base/src/backend/Makefile	Fri Sep 11 15:31:36 2009
--- ace_database/src/backend/Makefile	Sun Dec 13 14:27:22 2009
*** include $(top_builddir)/src/Makefile.glo
*** 16,22 
  
  SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \
  	main nodes optimizer port postmaster regex rewrite \
! 	storage tcop tsearch utils $(top_builddir)/src/timezone
  
  include $(srcdir)/common.mk
  
--- 16,22 
  
  SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \
  	main nodes optimizer port postmaster regex rewrite \
! 	security storage tcop tsearch utils $(top_builddir)/src/timezone
  
  include $(srcdir)/common.mk
  
diff -Nrpc base/src/backend/catalog/aclchk.c ace_database/src/backend/catalog/aclchk.c
*** base/src/backend/catalog/aclchk.c	Fri Dec 11 14:58:31 2009
--- ace_database/src/backend/catalog/aclchk.c	Sun Dec 13 14:27:22 2009
***
*** 46,51 
--- 46,52 
  #include foreign/foreign.h
  #include miscadmin.h
  #include parser/parse_func.h
+ #include security/ace.h
  #include utils/acl.h
  #include utils/fmgroids.h
  #include utils/lsyscache.h
*** static void expand_all_col_privileges(Oi
*** 125,130 
--- 126,134 
  		  int num_col_privileges);
  static AclMode string_to_privilege(const char *privname);
  static const char *privilege_to_string(AclMode privilege);
+ static AclMode restrict_grant(bool is_grant, AclMode avail_goptions,
+ 			  bool all_privs, AclMode privileges,
+ 			  const char *objname);
  static AclMode restrict_and_check_grant(bool is_grant, AclMode avail_goptions,
  		 bool all_privs, AclMode privileges,
  		 Oid objectId, Oid grantorId,
*** merge_acl_with_grant(Acl *old_acl, bool 
*** 221,230 
--- 225,281 
  	return new_acl;
  }
  
+ 
  /*
   * Restrict the privileges to what we can actually grant, and emit
   * the standards-mandated warning and error messages.
   */
+ static AclMode restrict_grant(bool is_grant, AclMode avail_goptions,
+ 			  bool all_privs, AclMode privileges,
+ 			  const char *objname)
+ {
+ 	AclMode		this_privileges;
+ 
+ 	/*
+ 	 * Restrict the operation to what we can actually grant or revoke, and
+ 	 * issue a warning if appropriate.	(For REVOKE this isn't quite what the
+ 	 * spec says to do: the spec seems to want a warning only if no privilege
+ 	 * bits actually change in the ACL. In practice that behavior seems much
+ 	 * too noisy, as well as inconsistent with the GRANT case.)
+ 	 */
+ 	this_privileges = privileges  ACL_OPTION_TO_PRIVS(avail_goptions);
+ 	if (is_grant)
+ 	{
+ 		if

Re: [HACKERS] Winflex

2009-12-13 Thread Dave Page

On Sun, Dec 13, 2009 at 5:42 AM, Andrew Dunstan and...@dunslane.net wrote:

 Yes. I spent a few cents and a few hours wrestling with it. AFAICT your are
 hosed on 64bit Windows. I can't get flex built and Cygwin is behaving very
 oddly. There are indications that the problem could be fairly deep - see
 http://www.mail-archive.com/cyg...@cygwin.com/msg102463.html

What Linda describes there is all normal behaviour for a 32 bit app on
64 bit Windows. Windows is providing a virtual 32 bit environment,
where for the most part the 32 bit app doesn't realise it's running on
64 bit. Unfortunately there are always things that look a bit odd due
to this, but normally I've found that the 32bit code runs fine, it
just looks odd from Explorer or 64 bit apps because of the
folder/registry redirection that happens behind the scenes.

 I can try again with Cygwin 1.7. and see if that improves matters, but I bet
 it doesn't.

What about msys? Or is that not capable of building the newer versions of flex?

The other possible option that I hesitate to suggest is Windows
Services for Unix or SUA as I think it's now called.

-- 
Dave Page
EnterpriseDB UK: 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] Winflex

2009-12-13 Thread Magnus Hagander

On Sun, Dec 13, 2009 at 11:36, Dave Page dp...@pgadmin.org wrote:
 On Sun, Dec 13, 2009 at 5:42 AM, Andrew Dunstan and...@dunslane.net wrote:

 Yes. I spent a few cents and a few hours wrestling with it. AFAICT your are
 hosed on 64bit Windows. I can't get flex built and Cygwin is behaving very
 oddly. There are indications that the problem could be fairly deep - see
 http://www.mail-archive.com/cyg...@cygwin.com/msg102463.html

 What Linda describes there is all normal behaviour for a 32 bit app on
 64 bit Windows. Windows is providing a virtual 32 bit environment,
 where for the most part the 32 bit app doesn't realise it's running on
 64 bit. Unfortunately there are always things that look a bit odd due
 to this, but normally I've found that the 32bit code runs fine, it
 just looks odd from Explorer or 64 bit apps because of the
 folder/registry redirection that happens behind the scenes.

Yeah, none of that should have an effect on a tool like flex, though...


 I can try again with Cygwin 1.7. and see if that improves matters, but I bet
 it doesn't.

Sounds like it's worth a try - they seem to at least know the issues detect.


 What about msys? Or is that not capable of building the newer versions of 
 flex?

IIRC we looked at that before, and that one is also limited to the
version before they started doing fork() (that was the problem with
the newer ones and gnuwin32, iirc)



 The other possible option that I hesitate to suggest is Windows
 Services for Unix or SUA as I think it's now called.

You mean we should post flex to that? Or have you found someone who has already?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] Row-Level Security

On Sun, Dec 13, 2009 at 3:50 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
(2009/12/13 12:18), Robert Haas wrote:
On Sat, Dec 12, 2009 at 7:41 PM, Josh Berkusj...@agliodbs.com wrote:
I blogged about this some time ago. One issue I can see is that I
believe that the RLS which many users want is different from the RLS
which SEPostgres implements.

Links:

http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-1-30732
http://it.toolbox.com/blogs/database-soup/thinking-about-row-level-security-part-2-30757

Basically, right. In my branch, SE-PgSQL put its hook after all the BR
trigger invocations.

http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.4.x/sepgsql/src/backend/executor/execMain.c#1883

The reason why I didn't apply this approach is it needs more modification
to the core routines, so it makes harder to manage out-of-tree code.

That's definitely something to consider if it's true. Why did it
require more modification of the core routines?

My current feeling is that a special-purpose system column is not the
best approach. I don't see what we gain by doing it that way. Even
in an SE-PostgreSQL environment, row-level security might not be
desired on every table - after all, we've been told that SE-PostgreSQL
is useful without any row-level security AT ALL, so it's not hard to
think there could be environments where only some tables need to
protected. So I think we want to

Re: [HACKERS] Adding support for SE-Linux security

2009-12-13 Thread Stephen Frost

* Bruce Momjian (br...@momjian.us) wrote:
 I am not replying to many of these emails so I don't appear to be
 brow-beating (forcing) the community into accepting this features.  I
 might be brow-beating the community, but I don't want to _appear_ to be
 brow-beating.  ;-)

My apologies if I come across this way- I don't intend to...  But I'm
also very enthusiastic about this.  Also, it's become a much more
personal issue for me due to this:

http://csrc.nist.gov/news_events/documents/omb/draft-omb-fy2010-security-metrics.pdf

OMB is now looking to include label-based security in their metrics.
This directly impacts some of the PG-based systems I run.

Thanks,

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

2009/12/13 KaiGai Kohei kai...@kaigai.gr.jp:
 The previous patch is too large to review.
 Is this scale confortable to review?

The scale is fine.  But the content is not.  So I am faced with a bit
of a dilemma.  If I start enumerating specific reasons why it's not
OK, then it's going to take more time than I really want to put into
this project.  If I don't, then I may be accused of hiding the ball.
What I'm hoping is that there are other interested people on this
mailing list (or not on this mailing list, maybe in the security
community) who have the time and the ability to help you fix some of
the issues here so that we can then have a serious design discussion.

Just to name a few really obvious problems (I only looked at the
01-database patch):

1. We have been talking for several days about the need to make the
initial patch in this area strictly a code cleanup patch.  Is this
cleaner than the code that it is replacing?  Is it even making an
attempt to conform to that mandate?

2. What will happen when someone runs pgindent against this?

Perhaps you only intended this to be a starting point for discussion,
in which case that's fine, but I don't think I can really contribute
anything useful until it gets a little further along.

Thanks,

...Robert

-- 
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] Winflex

2009-12-13 Thread Andrew Dunstan




Magnus Hagander wrote:

Using
http://www.postgresql.org/ftp/misc/winflex/

on a 64-bit windows, but in 32-bit mode (this certainly used to work),
trying to build HEAD.

first of all, it returns error code 128 when running flex -V, which
means our script claims it doesn't work. Commenting out that check, it
crashes along the line of:
Running flex on src\backend\utils\misc\guc-file.l
 14 [main] flex 2372 _cygtls::handle_exceptions: Exception:
STATUS_ACCESS_VIOLATION

Sometimes it doesn't crash, but do this instead:
Running flex on src\backend\parser\scan.l
c:\gnuwin32\bin\m4.exe:stdin:16769: ERROR: end of file in string
Project : error PRJ0002 : Error result 128 returned from 'C:\WINDOWS\SysWow64\cm
d.exe'.


If I run it a couple of times, it eventually completes. But then bombs
out because the generated files aren't complete.

Anybody else seen this? Am I forgetting something typical?
  


Well, I have now reproduced the behaviour, so it's not just you. But I 
have not been able to set up a working Cygwin build environment, so I 
can't try to rebuild it right now. It sure looks like it's the exec 
that's failing.


cheers

andrew


--
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] Winflex

2009-12-13 Thread Petr Jelinek


Magnus Hagander napsal(a):

On Sun, Dec 13, 2009 at 11:36, Dave Page dp...@pgadmin.org wrote:
  

On Sun, Dec 13, 2009 at 5:42 AM, Andrew Dunstan and...@dunslane.net wrote:


Yes. I spent a few cents and a few hours wrestling with it. AFAICT your are
hosed on 64bit Windows. I can't get flex built and Cygwin is behaving very
oddly. There are indications that the problem could be fairly deep - see
http://www.mail-archive.com/cyg...@cygwin.com/msg102463.html
  

What Linda describes there is all normal behaviour for a 32 bit app on
64 bit Windows. Windows is providing a virtual 32 bit environment,
where for the most part the 32 bit app doesn't realise it's running on
64 bit. Unfortunately there are always things that look a bit odd due
to this, but normally I've found that the 32bit code runs fine, it
just looks odd from Explorer or 64 bit apps because of the
folder/registry redirection that happens behind the scenes.



Yeah, none of that should have an effect on a tool like flex, though...
  


I think the actual problem is the implementation of fork emulation which 
changed in 1.7.




I can try again with Cygwin 1.7. and see if that improves matters, but I bet
it doesn't.
  


Cygwin 1.7.0-52 (iirc) with flex works for me on x64 Vista.



What about msys? Or is that not capable of building the newer versions of flex?



IIRC we looked at that before, and that one is also limited to the
version before they started doing fork() (that was the problem with
the newer ones and gnuwin32, iirc)
  


No they have newest flex, but the whole thing is even more broken on 
64bit then (old) cygwin version - it just exits without doing anything 
and does not even report any errors.


--
Regards
Petr Jelinek (PJMODOS)

[HACKERS] compiling with Visual Studio

2009-12-13 Thread Joe Conway

I'm trying to get PL/R binaries built on Windows, which I have not been
able to do successfully since the switch to Visual Studio from MinGW in
PostgreSQL 8.3. For some reason the MinGW PL/R binary does not load in
the VS compiled Postgres. Everything works if I also build PostgreSQL
with MinGW, but most people want to grab the standard binary release of
Postgres and combine it with a precompiled binary PL/R.

So in any case, my questions are:

1) Can Postgres be compiled with VS 2008 yet?
2) If not, does anyone have a link for VS 2005? I haven't been able to
locate it on the MS site.
3) Once I get Postgres compiling under VS, is the compilation of contrib
libraries automated as part of the build, or is there anything special I
would need to do?

Thanks,

Joe



signature.asc
Description: OpenPGP digital signature

Re: [HACKERS] compiling with Visual Studio

2009-12-13 Thread Magnus Hagander

On Sun, Dec 13, 2009 at 19:45, Joe Conway m...@joeconway.com wrote:
 I'm trying to get PL/R binaries built on Windows, which I have not been
 able to do successfully since the switch to Visual Studio from MinGW in
 PostgreSQL 8.3. For some reason the MinGW PL/R binary does not load in
 the VS compiled Postgres. Everything works if I also build PostgreSQL
 with MinGW, but most people want to grab the standard binary release of
 Postgres and combine it with a precompiled binary PL/R.

 So in any case, my questions are:

 1) Can Postgres be compiled with VS 2008 yet?

No.

 2) If not, does anyone have a link for VS 2005? I haven't been able to
 locate it on the MS site.

Yes. I'm unsure if they want that public or anything, so I'll send it
to you offlist :-)

 3) Once I get Postgres compiling under VS, is the compilation of contrib
 libraries automated as part of the build, or is there anything special I
 would need to do?

The buildscript that's there can generate buildfiles for contrib. I
haven't used it to build external contrib modules, but it's probably
a pretty good start. If not, just steal the project file from another
contrib module and modify the files it references (yeah, I know,
ugly..)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.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] compiling with Visual Studio

2009-12-13 Thread Joe Conway

On 12/13/2009 10:48 AM, Magnus Hagander wrote:
 2) If not, does anyone have a link for VS 2005? I haven't been able to
 locate it on the MS site.
 
 Yes. I'm unsure if they want that public or anything, so I'll send it
 to you offlist :-)

Thanks!

 3) Once I get Postgres compiling under VS, is the compilation of contrib
 libraries automated as part of the build, or is there anything special I
 would need to do?
 
 The buildscript that's there can generate buildfiles for contrib. I
 haven't used it to build external contrib modules, but it's probably
 a pretty good start. If not, just steal the project file from another
 contrib module and modify the files it references (yeah, I know,
 ugly..)

Thanks, that helps. I don't care if it's ugly, as long as it works ;-)

Joe




signature.asc
Description: OpenPGP digital signature

[HACKERS] WAL Info messages

2009-12-13 Thread Simon Riggs


One of the reasons for last years Rmgr plugin patch was the idea of a
signalling interface between Primary and Standby nodes. Such a feature
would help in producing further programmatic tests for Hot Standby.

Proposal is to use up the last rmgr id, slot 7: RM_INFO_ID. 
Functions would be provided on primary side to send a user-defined
binary message into WAL stream, with a single message type
XLOG_INFO_MSG. (Superuser only, though can be wrapped and given to
trusted users).

On Standby side, we would provide a plugin interface to allow the
message to be read and handled by user-written code. This would then
allow a simple value or notification to be passed to a waiting (polling)
connection on standby. If new LISTEN/NOTIFY gets into 8.5 then this
would be an obvious fit, but I have no need for message passing, just
info passing, so no need to wait for that; something much simpler.

We could do this by adding stuff into Streaming Rep patch, but seems
best to provide a generic interface that lives in the WAL stream itself,
rather than being at a meta level and divorced from the WAL.

This would allow the writing of a test suite that can create certain
conditions on the primary, then send notification of the event to the
standby. Standby would then halt recovery in a precise state while a
user runs an appropriate test.

-- 
 Simon Riggs   www.2ndQuadrant.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] Winflex

2009-12-13 Thread Andrew Dunstan




Petr Jelinek wrote:


Cygwin 1.7.0-52 (iirc) with flex works for me on x64 Vista.



Can you let me have the flex.exe and cygwin1.dll that I can try on a 
virgin WinS2k3 64-bit box?


It will probably need to be configured with --disable-nls.

cheers

andrew


--
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] Winflex

2009-12-13 Thread Petr Jelinek


Andrew Dunstan napsal(a):



Petr Jelinek wrote:


Cygwin 1.7.0-52 (iirc) with flex works for me on x64 Vista.



Can you let me have the flex.exe and cygwin1.dll that I can try on a 
virgin WinS2k3 64-bit box?


It will probably need to be configured with --disable-nls.


This is the version I am using (probably not newest one or anything, but 
I used it for testing all of my patches for 8.5 and I tried it 
successfully today on git head).

http://pjmodos.parba.cz/temp/cygflex.zip

--
Regards
Petr Jelinek (PJMODOS)


--
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] [PATCH] ACE Framework - Database, Schema

I read through the database patch a little more.  Here are some
further thoughts.

ace_database_create() calls have_createdb_privilege(), but of course
what ace_database_create() is supposed to be checking is whether you
have privileges to create the DB.  This is extremely confusing.  The
calling sequence for ace_database_create() seems to indicate a failure
of abstraction.  The srcIsTemplate argument seems quite problematic.
It is one of many values that are returned by get_db_info(), and it's
passed to ace_database_create() because the current PG model happens
to care.  It seems like a near-certainty that future security models
will care about something else.  (Incidentally, the interface to the
existing get_db_info function seems to me to be rather poor.  By the
time we get up to 10 out parameters, I think we ought to be using a
structure.  This might be material for a standalone patch.)

I don't understand the modifications to restrict_and_check_grant().
It seems to me that this is going in the wrong direction, although
since I don't understand the motivation for the change, I might be
wrong.  If we have a piece of code that centralizes permissions
checking now, splitting that up into a bunch of ace_*_grant()
functions and duplicating it doesn't seem like an improvement.

ace_database_alter() appears to never be called with more than one
argument that is non-NULL/InvalidOid, which is usually a sign that the
interface is wrong.  There are two calls to this function where,
apparently, we're checking alter database permissions without
specifying any details whatsoever about exactly what's going to get
altered.  That sounds like we don't really know what things we want to
pass across the interface layer, so we're just passing the ones that
we happen to care about at the moment.  It certainly looks like we
need separate functions for alter, rename, and alter-set.  In some
cases it might make sense to pass the parsenode as an argument rather
than trying to decide which bits of data contained within it are
sufficiently interesting to be worth sending over.

Tom objected to ace_database_calculate_size() before.  It seems to me
that in order to keep this managable we have to settle on a finite set
of permissions and stick with it.  For example, in this case, we might
decide that in EVERY security model, calculating the database size
requires the same permissions as connect.  The individual security
models might want to make different decisions about how to check that
permission, but they all have to treat it the same way they would
treat an actual connection attempt.  It seems to me that if every
security model is allowed to introduce not only new logic for making
checks but also new kinds of checks, we might as well give up on
designing an interface layer.

(Similarly, skipping back to ace_database_create() for a minute ...
since I asked for a patch that only deals with one object type, I
won't now complain about having gotten one.  But I will note that I
think where ace_database_create() calls pg_tablespace_aclchk() it
probably eventually needs to call some ace_tablespace_...() function.
Although frankly I'm not sure which one.  Would
ace_tablespace_create() mean permission to create a tablespace or
permission to create tables within that tablespace?)

security/ace.h, like a good number of other things in this patch, does
not conform to project indentation style.  All of the parameter blocks
(parameter : description) don't either; unless I'm mistaken, pgindent
will do something awful to those.  The comments generally need some
editing help from a native English speaker, and I spotted at least two
typos (defatult for default, provides for providers).  They also make
reference to multiple security providers, which is not within the
purview of this patch.

I am not very happy with the ace_object-type_operation naming
convention.  Most PG functions are named a little more descriptively
than that.  Like I can pretty much guess that AlterDatabase() is going
to alter a database, and get_db_info() is going to get info about a
DB, and ExecHashJoin() is going to execute a hash join.  It's not
possible to look at ace_database_create() and have any idea what the
function does, unless you have the preexisting knowledge that ACE
stands for access control extensions, and then you still don't know
what the function does because access control extensions database
create is a sentence with no verb.  Granted, we have some poorly
named functions in the system already, but I'd like to not increase
the number.  Apart from all of the above, I still believe that for a
first phase of this we should just be looking to centralize access
control decisions without promising to ever do anything more, and
access control extensions doesn't send that message.  I don't know
exactly what the best way to structure this is but I think putting the
word check somewhere in there would be a good start (or maybe the
already-used-elsewhere

Re: [HACKERS] WAL Info messages

On Sun, Dec 13, 2009 at 2:20 PM, Simon Riggs si...@2ndquadrant.com wrote:
 This would allow the writing of a test suite that can create certain
 conditions on the primary, then send notification of the event to the
 standby. Standby would then halt recovery in a precise state while a
 user runs an appropriate test.

That sounds neat.  What kinds of conditions did you have in mind?

...Robert

-- 
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] WAL Info messages

Simon Riggs si...@2ndquadrant.com writes:
 Proposal is to use up the last rmgr id, slot 7: RM_INFO_ID. 
 Functions would be provided on primary side to send a user-defined
 binary message into WAL stream, with a single message type
 XLOG_INFO_MSG. (Superuser only, though can be wrapped and given to
 trusted users).

Seems like a frammish that we could maybe worry about a few months from
now, after the core patch has settled down.

I also dislike reserving a whole RM_ID just for this.  If we are only
going to send one type of message it seems like it could be one infomask
type within an existing RM_ID.

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] Hot Standby, release candidate?

Simon Riggs si...@2ndquadrant.com writes:
 * NonTransactionalInvalidation logging has been removed following
 review, but AFAICS that means VACUUM FULL doesn't work correctly on
 catalog tables, which regrettably will be the only ones still standing
 even after we apply VFI patch. Did I misunderstand the original intent?
 Was it just buggy somehow? Or is this hoping VF goes completely, which
 seems unlikely in this release.

For my money, the only reason VF is still around is there hasn't been
an urgent reason to get rid of it.  If it doesn't play with HS, I think
we'd be better served to put work into getting rid of it than to put
work into fixing it.

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] compiling with Visual Studio

2009-12-13 Thread Dimitri Fontaine

Le 13 déc. 2009 à 19:48, Magnus Hagander a écrit :
 The buildscript that's there can generate buildfiles for contrib. I
 haven't used it to build external contrib modules, but it's probably
 a pretty good start. If not, just steal the project file from another
 contrib module and modify the files it references (yeah, I know,
 ugly..)

Or, well, you have loads of time in your hand so you can port PGXS support to 
windows building, and this way when extensions are another PostgreSQL facility 
this problem is already solved. So easy to say it I couldn't resist :)

Regards,
-- 
dim
-- 
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] Winflex

2009-12-13 Thread Andrew Dunstan




Petr Jelinek wrote:

Andrew Dunstan napsal(a):



Petr Jelinek wrote:


Cygwin 1.7.0-52 (iirc) with flex works for me on x64 Vista.



Can you let me have the flex.exe and cygwin1.dll that I can try on a 
virgin WinS2k3 64-bit box?


It will probably need to be configured with --disable-nls.


This is the version I am using (probably not newest one or anything, 
but I used it for testing all of my patches for 8.5 and I tried it 
successfully today on git head).

http://pjmodos.parba.cz/temp/cygflex.zip


Sadly this still gives me:

   flex: fatal internal error, exec failed

on WinS2K3 64bit on EC2.

cheers

andrew

--
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] Hot Standby, release candidate?

2009-12-13 Thread Simon Riggs

On Sun, 2009-12-13 at 15:45 -0500, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  * NonTransactionalInvalidation logging has been removed following
  review, but AFAICS that means VACUUM FULL doesn't work correctly on
  catalog tables, which regrettably will be the only ones still standing
  even after we apply VFI patch. Did I misunderstand the original intent?
  Was it just buggy somehow? Or is this hoping VF goes completely, which
  seems unlikely in this release.
 
 For my money, the only reason VF is still around is there hasn't been
 an urgent reason to get rid of it.  If it doesn't play with HS, I think
 we'd be better served to put work into getting rid of it than to put
 work into fixing it.

I see the logic, though it has many implications. I'll step up, if I can
get some help from you and Itagaki on the VF side.

You have a rough design here
http://archives.postgresql.org/message-id/19750.1252094...@sss.pgh.pa.us

Some thoughts and some further work on a detailed design

* Which exact tables are we talking about: just pg_class and the shared
catalogs? Everything else is in pg_class, so if we can find it we're OK?
formrdesc() tells me the list of nailed relations is: pg_database,
pg_class, pg_attribute, pg_proc, and pg_type. Are the nailed relations
the ones we care about, or are they just a subset? 

* Restrict set of operations to *only* VACUUM FULL. Is there a need for
anything else to do this, at least in this release?

* Each backend needs to access two map files: shared and local

* Get relcache to read map files at startup in formrdesc(). Rather than
use RelationInitPhysicalAddr() set relation-rd_node.relNode directly

* Get VF to write a new type of invalidation message that means re-read
the two map files to overwrite the relation-rd_node.relNode in the
nailed relations

* Map files would have a very structured format, so each table listed
has its exact place. Sounds like best place for shared catalogs is
pg_control. We only need a few additional bytes for that and everything
else to manipulate it already exists.

* Map files for specific databases would be called pg_database_control,
with roughly same concepts as pg_control. It's then an obvious place to
add any further db specific things in future, if we need them.

* Protect all map files reading/writing using ControlFileLock. Sequence
of update is acquire lock, send invalidation, rewrite file, release lock
all inside a critical section. Readers would take shared, writers
exclusive.

* Work would be in two tranches: add new way of working then later
remove code we don't need; I would actually rather do the second part at
start of next dev cycle.

-- 
 Simon Riggs   www.2ndQuadrant.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] Largeobject Access Controls and pg_migrator


KaiGai Kohei kai...@kaigai.gr.jp wrote:

 Can SELECT lo_create(16385); help this situation?

SELECT lo_create(loid) FROM (SELECT DISTINCT loid FROM pg_largeobject) AS t

would work for pg_migrator.

 I'm not clear whether we also check pg_largeobejct has chunks with same
 LOID on lo_create(). In the regular operation, it shall never happen.

I think the omission is a reasonable optimization.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] Largeobject Access Controls (r2460)

KaiGai Kohei kai...@kaigai.gr.jp wrote:

 We don't have any reason why still CASE ... WHEN and subquery for the given
 LOID. Right?

Ah, I see. I used your suggestion.

I applied the bug fixes. Our tools and contrib modules will always use
pg_largeobject_metadata instead of pg_largeobject to enumerate large objects.

I removed GRANT SELECT (loid) ON pg_largeobject TO PUBLIC from initdb
because users must use pg_largeobject_metadata.oid when they want to check
OIDs of large objects; If not, they could misjudge the existence of objects.
This is an unavoidable incompatibility unless we always have corresponding
tuples in pg_largeobject even for zero-length large objects.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] EXPLAIN BUFFERS


Robert Haas robertmh...@gmail.com wrote:

 OK, done, see attached.  I also noticed when looking through this that
 the documentation says that auto_explain.log_buffers is ignored unless
 auto_explain.log_analyze is set.  That is true and seems right to me,
 but for some reason explain_ExecutorEnd() had been changed to set
 es.analyze if either log_analyze or log_buffers was set.

Thanks. It was my bug.

Could you apply the patch? Or, may I do by myself?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] EXPLAIN BUFFERS

On Sun, Dec 13, 2009 at 7:55 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:

 Robert Haas robertmh...@gmail.com wrote:

 OK, done, see attached.  I also noticed when looking through this that
 the documentation says that auto_explain.log_buffers is ignored unless
 auto_explain.log_analyze is set.  That is true and seems right to me,
 but for some reason explain_ExecutorEnd() had been changed to set
 es.analyze if either log_analyze or log_buffers was set.

 Thanks. It was my bug.

 Could you apply the patch? Or, may I do by myself?

Sorry, I've been meaning to look at this a little more and have gotten
distracted.

I have a question about the comment in InstrStopNode(), which reads:
Adds delta of buffer usage to node's count and resets counter to
start so that the counters are not double counted by parent nodes.
It then calls BufferUsageAccumDiff(), but that function doesn't
actually reset anything, so it seems like the comment is wrong.

Two other thoughts:

1. It doesn't appear that there is any provision to ever zero
pgBufferUsage.  Shouldn't we do this, say, once per explain, just to
avoid the possibility of overflowing the counters?

2. We seem to do all the work associated with pgBufferUsage even when
the buffers option is not passed to explain.  The overhead of
incrementing the counters is probably negligible (and we were paying
the equivalent overhead before anyway) but I'm not sure whether saving
the starting counters and accumulating the deltas might be enough to
slow down EXPLAIN ANALYZE.  That's sorta slow already so I'd hate to
whack it any more - have you benchmarked this at all?

...Robert

-- 
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] Streaming replication and non-blocking I/O

2009-12-13 Thread Fujii Masao

On Sun, Dec 13, 2009 at 5:42 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 Walreceiver wants to wait for data to arrive from the master or a
 signal. PQgetXLogData(), which is the libpq function to read a piece of
 WAL, takes a timeout argument to support that. Walreceiver calls
 PQgetXLogData() in an endless loop, checking for a received sighup or
 death of postmaster at every iteration.

 In the synchronous replication mode, I presume it's also going to listen
 for a signal from the startup process, so that it can send a
 acknowledgment to the master as soon as a COMMIT record has been
 replayed that a backend on the master is waiting for.

Right.

 To implement the timeout in PQgetXLogData(), pqWaitTimed() was changed
 to take a timeout instead of finishing_time argument. Which is a mistake
 because it breaks PQconnectdb, and as I said I don't think
 PQgetXLogData(9 should have a timeout argument to begin with. Instead,
 it should have a boolean 'async' argument to return immediately if
 there's no data, and walreceiver main loop should call poll()/select()
 to wait. Ie. just like PQgetCopyData() works.

Seems good. I'll revise the code.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] Streaming replication and non-blocking I/O

Fujii Masao masao.fu...@gmail.com writes:
 On Sun, Dec 13, 2009 at 5:42 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 To implement the timeout in PQgetXLogData(), pqWaitTimed() was changed
 to take a timeout instead of finishing_time argument. Which is a mistake
 because it breaks PQconnectdb, and as I said I don't think
 PQgetXLogData(9 should have a timeout argument to begin with. Instead,
 it should have a boolean 'async' argument to return immediately if
 there's no data, and walreceiver main loop should call poll()/select()
 to wait. Ie. just like PQgetCopyData() works.

 Seems good. I'll revise the code.

Do we need a new PQgetXLogData function at all?  Seems like you could
shove the data through the COPY protocol and not have to touch libpq
at all, rather than duplicating a nontrivial amount of code there.

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] EXPLAIN BUFFERS


Robert Haas robertmh...@gmail.com wrote:

 I have a question about the comment in InstrStopNode(), which reads:
 Adds delta of buffer usage to node's count and resets counter to
 start so that the counters are not double counted by parent nodes.
 It then calls BufferUsageAccumDiff(), but that function doesn't
 actually reset anything, so it seems like the comment is wrong.

Oops, it's wrong. It just does Adds delta of buffer usage to node's count.

 Two other thoughts:
 
 1. It doesn't appear that there is any provision to ever zero
 pgBufferUsage.  Shouldn't we do this, say, once per explain, just to
 avoid the possibility of overflowing the counters?

I think the overflowing will not be a problem because we only use
the differences of values. The delta is always corrent unless we use
2^32 buffer accesses during one execution of a node.

 2. We seem to do all the work associated with pgBufferUsage even when
 the buffers option is not passed to explain.  The overhead of
 incrementing the counters is probably negligible (and we were paying
 the equivalent overhead before anyway) but I'm not sure whether saving
 the starting counters and accumulating the deltas might be enough to
 slow down EXPLAIN ANALYZE.  That's sorta slow already so I'd hate to
 whack it any more - have you benchmarked this at all?

There are 5% of overheads in the worst cases. The difference will be
little if we have more complex operations or some disk I/Os.

Adding Instrumentation-count_bufusage flag could reduce the overheads.
if (instr-count_bufusage)
BufferUsageAccumDiff(...)

Should I add countBufferUsage boolean arguments to all places
doInstrument booleans are currently used? This requires several
minor modifications of codes in many places.

[without patch]
=# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts;
   QUERY PLAN

 Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
(actual time=0.003..571.794 rows=1000 loops=1)
 Total runtime: 899.427 ms

[with patch]
=# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts;
   QUERY PLAN

 Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
(actual time=0.003..585.885 rows=1000 loops=1)
 Total runtime: 955.280 ms

- shared_buffers = 1500MB
- pgbench -i -s100
- Read all pages in the pgbench_accounts into shared buffers before runs.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] EXPLAIN BUFFERS

Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
 Should I add countBufferUsage boolean arguments to all places
 doInstrument booleans are currently used? This requires several
 minor modifications of codes in many places.

Pushing extra arguments around would create overhead of its own ...
overhead that would be paid even when not using EXPLAIN at all.

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] pg_dump enhancement proposal

2009-12-13 Thread daveg

On Thu, Nov 12, 2009 at 04:31:37PM -0500, Tom Lane wrote:
 Mark Hammonds mhammo...@omniti.com writes:
  2.  Custom Query Exports
 
  In my use of mysqldump, I found one feature very useful: the ability  
  to execute a custom SELECT. . .WHERE statement and then dump only the  
  results. This feature currently provides MySQL users with the ability  
  to quickly and easily export very granular data subsets, and I see no  
  reason why PostgreSQL users wouldn't benefit from the same capability.  
  While it is true that this functionality can already be achieved in  
  PostgreSQL using Copy, it seems to me that it would logically fit well  
  as an extension to pg_dump, especially since many beginning and even  
  some intermediate PostgreSQL users aren't aware of the alternatives.
 
 As you say, we already have this using COPY, and I don't agree that
 it would be a good idea to plaster it into pg_dump as well.  pg_dump
 is intended for dumping and restoring data, not for ETL-type tasks.
 Furthermore, pg_dump is a overly complex beast already --- much more
 so than one could wish, for a tool that is absolutely fundamental to
 database reliability.  Putting requirements on it that are well outside
 its charter seems like a short route to maintenance disaster.
 
 There has been some occasional chatter about developing one or more
 tools focused on ETL rather than dump/restore, and my thought is that
 this idea would fit better there.  An ETL tool would not have the
 kind of requirements pg_dump has for coping with multiple server
 versions and knowing everything there is to know about database
 contents, so it seems like it could address new areas of functionality
 without a complexity explosion.
 
 You might want to check the archives for previous discussions ---
 I think the last go-round started with someone wanting to add an
 arbitrary WHERE filter to pg_dump dumps.

Sorry I missed this thread.

Not only has there been previous discussion, there have been at least two,
and I seem to recall three, patches implementing this. None of the patches
was very large, and none of them impacted the basic make a backup paths
in pg_dump.

-dg

-- 
David Gould   da...@sonic.net  510 536 1443510 282 0869
If simplicity worked, the world would be overrun with insects.

-- 
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] EXPLAIN BUFFERS


Tom Lane t...@sss.pgh.pa.us wrote:

 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
  Should I add countBufferUsage boolean arguments to all places
  doInstrument booleans are currently used? This requires several
  minor modifications of codes in many places.
 
 Pushing extra arguments around would create overhead of its own ...
 overhead that would be paid even when not using EXPLAIN at all.

I cannot understand what you mean... The additional argument should
not be a performance overhead because the code path is run only once
per execution. Instrumentation structures are still not allocated
in normal or EXPLAIN queries; allocated only in EXPLAIN ANALYZE.

Or, are you suggesting to separate buffer counters with Instrumentation
structure? It still requires extra arguments, but it could minimize the
overhead when we use EXPLAIN ANALYZE without BUFFERS. However, we need
additional codes around InstrStartNode/InstrStopNode calls.

Or, are you complaining about non-performance overheads,
something like overheads of code maintenance?

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] Streaming replication and non-blocking I/O

2009-12-13 Thread Fujii Masao

On Mon, Dec 14, 2009 at 11:38 AM, Tom Lane t...@sss.pgh.pa.us wrote:
 Do we need a new PQgetXLogData function at all?  Seems like you could
 shove the data through the COPY protocol and not have to touch libpq
 at all, rather than duplicating a nontrivial amount of code there.

Yeah, I also think that all data (the WAL data itself, its LSN and
the flag bits) which the PQgetXLogData handles could be shoved
through the COPY protocol. But, outside libpq, it's somewhat messy
to extract the LSN and the flag bits from the data buffer which
PQgetCopyData returns, by using ntohs(). So I provided the new
libpq function only for replication. That is, I didn't want to expose
the low layer of network which libpq should handle.

I think that the friendly function would be useful to implement
the standby program (e.g., a stand-alone walreceiver tool) outside
the core.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

-- 
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] [PATCH] ACE Framework - Database, Schema

Robert, Thanks for your detailed comments.

 Just to name a few really obvious problems (I only looked at the
 01-database patch):

 1. We have been talking for several days about the need to make the
 initial patch in this area strictly a code cleanup patch.  Is this
 cleaner than the code that it is replacing?  Is it even making an
 attempt to conform to that mandate?

Even if it is unclear whether the current form is more clear than the
current inlined pg_xxx_aclcheck() form, or not, it will obviously
provide a set of common entry points for upcoming enhanced security
providers.
Eventually, it is more clear than enumeration of #ifdef ... #endif
blocks for SELinux, Smack, Solaris-TX and others.

 2. What will happen when someone runs pgindent against this?

 Perhaps you only intended this to be a starting point for discussion,
 in which case that's fine, but I don't think I can really contribute
 anything useful until it gets a little further along.

If someone runs pgindent, I'll have to fix up a series of patches
mechanically to resolve conflictions, if necessary.
Indeed, this framework does not provide any additional user visible
features by itself, but necessary to accept upcoming anything useful.


Robert Haas wrote:
 I read through the database patch a little more.  Here are some
 further thoughts.
 
 ace_database_create() calls have_createdb_privilege(), but of course
 what ace_database_create() is supposed to be checking is whether you
 have privileges to create the DB.  This is extremely confusing.  The
 calling sequence for ace_database_create() seems to indicate a failure
 of abstraction.  The srcIsTemplate argument seems quite problematic.
 It is one of many values that are returned by get_db_info(), and it's
 passed to ace_database_create() because the current PG model happens
 to care.  It seems like a near-certainty that future security models
 will care about something else.  (Incidentally, the interface to the
 existing get_db_info function seems to me to be rather poor.  By the
 time we get up to 10 out parameters, I think we ought to be using a
 structure.  This might be material for a standalone patch.)

The have_createdb_privilege() is a copy  paste from the dbcommand.c.
It lookups AUTHOID syscache and returns pg_authid.rolcreatedb, but
indeed, its name is confusable. I'll consider another name.

The srcIsTemplate might be pulled out from DATABASEOID syscache
using the given srcDatOid. I guess the reason why get_db_info()
is it tries to resolve dbname - db_oid and fetch its properties
in same time. But OID of the source database is already given,
so security provider (including the default PG) can lookup this
as neccessity.

 I don't understand the modifications to restrict_and_check_grant().
 It seems to me that this is going in the wrong direction, although
 since I don't understand the motivation for the change, I might be
 wrong.  If we have a piece of code that centralizes permissions
 checking now, splitting that up into a bunch of ace_*_grant()
 functions and duplicating it doesn't seem like an improvement.

From perspective of the enhanced security providers, GRANT/REVOKE is
a kind of modification of properties on a certain database object.
So, it has a motivation to check GRANT/REVOKE using its access control
rules.
If we don't modify restrict_and_check_grant(), the caller (aclchk.c)
correctly handles the default PG checks, so rest of enhanced security
providers will apply additional checks in the ace_*_grant().
In this case, we keep ace_*_grant() empty at the moment, and it shall
be a special purpose hook for enhanced security providers.
I don't have any preference on either of them.

 ace_database_alter() appears to never be called with more than one
 argument that is non-NULL/InvalidOid, which is usually a sign that the
 interface is wrong.  There are two calls to this function where,
 apparently, we're checking alter database permissions without
 specifying any details whatsoever about exactly what's going to get
 altered.  That sounds like we don't really know what things we want to
 pass across the interface layer, so we're just passing the ones that
 we happen to care about at the moment.  It certainly looks like we
 need separate functions for alter, rename, and alter-set.  In some
 cases it might make sense to pass the parsenode as an argument rather
 than trying to decide which bits of data contained within it are
 sufficiently interesting to be worth sending over.

It makes sense for me. I'll separate it as follows:

- ace_database_alter_rename()   ... ALTER DATABASE xxx RENAME TO
- ace_database_alter_owner()... ALTER DATABASE xxx OWNER TO
- ace_database_alter()  ... ALTER DATABASE with other options

When we support security label on database, it will also necessary:

- ace_database_alter_relabel()

 Tom objected to ace_database_calculate_size() before.  It seems to me
 that in order to keep this managable we have to settle on a finite set
 of

Re: [HACKERS] EXPLAIN BUFFERS

On Sun, Dec 13, 2009 at 10:15 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Takahiro Itagaki itagaki.takah...@oss.ntt.co.jp writes:
 Should I add countBufferUsage boolean arguments to all places
 doInstrument booleans are currently used? This requires several
 minor modifications of codes in many places.

 Pushing extra arguments around would create overhead of its own ...
 overhead that would be paid even when not using EXPLAIN at all.

Well, I think we need to do something.  I don't really want to tack
another 5-6% overhead onto EXPLAIN ANALYZE.  Maybe we could recast the
doInstrument argument as a set of OR'd flags?

...Robert

-- 
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] EXPLAIN BUFFERS

On Sun, Dec 13, 2009 at 10:00 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:
 Two other thoughts:

 1. It doesn't appear that there is any provision to ever zero
 pgBufferUsage.  Shouldn't we do this, say, once per explain, just to
 avoid the possibility of overflowing the counters?

 I think the overflowing will not be a problem because we only use
 the differences of values. The delta is always corrent unless we use
 2^32 buffer accesses during one execution of a node.

Hmm... you might be right.  I'm not savvy enough to know whether there
are any portability concerns here.

Anyone else know?

...Robert

-- 
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] thread safety on clients

2009-12-13 Thread Greg Stark

On Fri, Dec 11, 2009 at 6:08 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 I'll commit a fix for that shortly, but this reminds me once again that
 the EvalPlanQual logic is desperately under-tested in our normal
 regression testing.  We really need some kind of testing infrastructure
 that would let us exercise concurrent-update cases a bit more than not
 at all.

If i went back and cleaned up the parallel psql patch we would be able
to write tests which tested basic concurrent functionality such as
transactions blocking when they're supposed to. But that wouldn't
catch something like this I don't think, not unless it got triggered
pretty reliably by a simple evalplanqual recheck.

 I'm inclined to think this is bad, and we should fix pgbench to
 re-randomize after forking.  If we don't, we'll have synchronized
 behavior on some platforms and not others, which doesn't seem like a
 good idea.  On the other hand, if you did want that type of behavior,
 it's hard to see how you'd get it.  Is it worth trying to provide that
 as a (probably non-default) mode in pgbench?  If so, how would you
 do it on threaded platforms?

Well it's pretty useless for benchmarking unless someone comes up with
a different plan for resolving these kinds of conflicts and we wanted
to compare the costs. But it's clearly something we should do for
testing for precisely this kind of bug. EvalPlanQual could trigger all
kinds of bugs throughout the executor and it would be worth having
something like this to check for them.

I believe it's possible to give at least one of the random number
generating apis a preallocated buffer for it to use to store the
generator state, that could easily be per-thread state along with the
connection and other state. I'm not sure which api that was and
whether it's as portable or as good a generator as what we're using
now though.


-- 
greg

-- 
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] EXPLAIN BUFFERS


Robert Haas robertmh...@gmail.com wrote:

 Well, I think we need to do something.  I don't really want to tack
 another 5-6% overhead onto EXPLAIN ANALYZE.  Maybe we could recast the
 doInstrument argument as a set of OR'd flags?

I'm thinking the same thing (OR'd flags) right now.

The attached patch adds INSTRUMENT_TIMER and INSTRUMENT_BUFFERS flags.
The types of QueryDesc.doInstrument (renamed to instrument_options) and
EState.es_instrument are changed from bool to int, and they store
OR of InstrumentOption flags. INSTRUMENT_TIMER is always enabled when
instrumetations are initialized, but INSTRUMENT_BUFFERS is enabled only if
we use EXPLAIN BUFFERS. I think the flag options are not so bad idea because
of extensibility. For example, we could support EXPLAIN CPU_USAGE someday.

One issue is in the top-level instrumentation (queryDesc-totaltime).
Since the field might be used by multiple plugins, the first initializer
need to initialize the counter with all options. I used INSTRUMENT_ALL
for it in the patch.

=# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts;
   QUERY PLAN

 Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
(actual time=0.003..572.126 rows=1000 loops=1)
 Total runtime: 897.729 ms

=# EXPLAIN (ANALYZE, BUFFERS) SELECT * FROM pgbench_accounts;
   QUERY PLAN

 Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
(actual time=0.002..580.642 rows=1000 loops=1)
   Buffers: shared hit=163935
 Total runtime: 955.744 ms

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



explain_buffers_20091214.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] [PATCH] ACE Framework - Database, Schema

2009/12/13 KaiGai Kohei kai...@ak.jp.nec.com:
 Just to name a few really obvious problems (I only looked at the
 01-database patch):

 1. We have been talking for several days about the need to make the
 initial patch in this area strictly a code cleanup patch.  Is this
 cleaner than the code that it is replacing?  Is it even making an
 attempt to conform to that mandate?

 Even if it is unclear whether the current form is more clear than the
 current inlined pg_xxx_aclcheck() form, or not, it will obviously
 provide a set of common entry points for upcoming enhanced security
 providers.
 Eventually, it is more clear than enumeration of #ifdef ... #endif
 blocks for SELinux, Smack, Solaris-TX and others.

Right, but it will also not get committed.  :-(

You seem to still be failing to understand the approach that Stephen
and I have been discussing for the last several days...  but at any
rate, I think that it is possible to do this in a way that is more
clear (or at least, AS clear) as the current method.  Stephen is also
working on a concept patch that I'm eager to see, and I would like to
give him a little time to get that together before we spend too much
more time working on this.

 I don't understand the modifications to restrict_and_check_grant().
 It seems to me that this is going in the wrong direction, although
 since I don't understand the motivation for the change, I might be
 wrong.  If we have a piece of code that centralizes permissions
 checking now, splitting that up into a bunch of ace_*_grant()
 functions and duplicating it doesn't seem like an improvement.

 From perspective of the enhanced security providers, GRANT/REVOKE is
 a kind of modification of properties on a certain database object.
 So, it has a motivation to check GRANT/REVOKE using its access control
 rules.
 If we don't modify restrict_and_check_grant(), the caller (aclchk.c)
 correctly handles the default PG checks, so rest of enhanced security
 providers will apply additional checks in the ace_*_grant().
 In this case, we keep ace_*_grant() empty at the moment, and it shall
 be a special purpose hook for enhanced security providers.
 I don't have any preference on either of them.
[...]
 When we support security label on database, it will also necessary:

 - ace_database_alter_relabel()

This seems like bad design to me.  I don't see why SE-Linux would get
a vote on whether a user is allowed to change the DAC permissions, any
more than DAC gets a vote on whether an SE-Linux relabel operation can
go through.  If it does, then this attempt to create a framework is
not going to succeed, because every new provider will require hooks to
let every exist provider muck with what it does, and they'll all have
to have complete knowledge of each other's internals.

 It seems to me ace_database_calculate_size() might be a bad name
 because of too specific naming for a certin functionality.

That's exactly the problem.  I'm not sure if I believe in the
particular solution you've offered here, but it's certainly better
than the way it is now.  I think this needs more thought from more
people.

 What about acechk_* to mean access control extension checks something
 to be action'ed? For example, acechk_database_create() means access
 control extension checks database to be created.

That fails to address all of my comments, so, -1 from me.

...Robert

-- 
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] EXPLAIN BUFFERS

On Sun, Dec 13, 2009 at 11:49 PM, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:

 Robert Haas robertmh...@gmail.com wrote:

 Well, I think we need to do something.  I don't really want to tack
 another 5-6% overhead onto EXPLAIN ANALYZE.  Maybe we could recast the
 doInstrument argument as a set of OR'd flags?

 I'm thinking the same thing (OR'd flags) right now.

 The attached patch adds INSTRUMENT_TIMER and INSTRUMENT_BUFFERS flags.
 The types of QueryDesc.doInstrument (renamed to instrument_options) and
 EState.es_instrument are changed from bool to int, and they store
 OR of InstrumentOption flags. INSTRUMENT_TIMER is always enabled when
 instrumetations are initialized, but INSTRUMENT_BUFFERS is enabled only if
 we use EXPLAIN BUFFERS. I think the flag options are not so bad idea because
 of extensibility. For example, we could support EXPLAIN CPU_USAGE someday.

 One issue is in the top-level instrumentation (queryDesc-totaltime).
 Since the field might be used by multiple plugins, the first initializer
 need to initialize the counter with all options. I used INSTRUMENT_ALL
 for it in the patch.

 =# EXPLAIN (ANALYZE) SELECT * FROM pgbench_accounts;
                                                           QUERY PLAN
 
  Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
 (actual time=0.003..572.126 rows=1000 loops=1)
  Total runtime: 897.729 ms

 =# EXPLAIN (ANALYZE, BUFFERS) SELECT * FROM pgbench_accounts;
                                                           QUERY PLAN
 
  Seq Scan on pgbench_accounts  (cost=0.00..263935.00 rows=1000 width=97) 
 (actual time=0.002..580.642 rows=1000 loops=1)
   Buffers: shared hit=163935
  Total runtime: 955.744 ms

That seems very promising, but it's almost midnight here so I have to
turn in for now.  I'll take another look at this tomorrow.

...Robert

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

[HACKERS] Re: Hot Standby, deferred conflict resolution for cleanup records (v2)

2009-12-13 Thread Greg Stark

On Sat, Dec 12, 2009 at 3:06 PM, Simon Riggs si...@2ndquadrant.com wrote:
 Anyone acquiring a lock on a table should check the latestRemovedXid for
 the table and abort if their xmin is too old. This prevents new lockers
 from accessing a cleaned relation immediately after we decide to abort
 anyone looking at that table. (Anyone queuing for the existing locks
 would be caught by this).

I fear given HOT pruning that this could mean no query can even get
started against a busy table. It seems like you would have to start
your transaction several times until you manage to get a lock on the
busy table soon enough after taking the snapshot to not have missed
any cleanups in the table. Or have I missed something that protects
against that?

The bigger problem with this is that I don't see any way to tune this
to have a safe replica. In the current system you can set
standby_max_delay to 0 or -1 or whatever to completely disable killing
off valid queries on the replica. In this setup you're going ahead
with cleanup records which may or may not be safe and then have no
recourse if they turn out to conflict.

-- 
greg

-- 
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] Row-Level Security

Robert Haas wrote:
On Sun, Dec 13, 2009 at 3:50 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
Basically, right. In my branch, SE-PgSQL put its hook after all the BR
trigger invocations.

http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.4.x/sepgsql/src/backend/executor/execMain.c#1883

The reason why I didn't apply this approach is it needs more modification
to the core routines, so it makes harder to manage out-of-tree code.

That's definitely something to consider if it's true. Why did it
require more modification of the core routines?

In my local branch, it just adds two lines as follows:
+ /* SELinux labeling and permission checks */
+ sepgsql_heap_insert(resultRelationDesc, tuple);

It is obviously less than modify RelationBuildTriggers() to allocate an
additional slot for the TrigDesc array and put an entry.
The reason was just from the perspective to maintain out-of-tree code,
but different perspective will be necessary to propose a featuer to upstream.

Of course, as you point out, we have to make sure that anyone who
tries to turn RLS on or off for a particular table is authorized to
perform that operation. But that's a separate problem which is I
don't think has much to do with row-level security.

Yes, it is a separate problem not to be concluded at the moment.
(Perhaps, it depends on security model. In DAC, per-table basis is preferable.)

So, I'd like to bring up just an issue to be discussed later.
When we build a binary with a label-based MAC, such as SE-PgSQL, it shall
be turned on/off in the startup time.
(I don't assume it should be configurable in runtime.)

If we set up database cluster without any label-based MAC, all the tuple
shall not have any security label. If the security label is stored within
regular column, we have to modify schema for any tables at first.
If system column provides a security label of tuple, we can dynamically
generate an appropriate security label. In SELinux case, it assumes any
unlabeled objects performs as if it has a pseudo security label:
system_u:object_r:unlabeled_t:s0

Needless to say, we need to assign appropriate security labels for
meaningful access controls later, but it does not require any schema
changes, even if we repeat to turn on/off the label-based MAC feature.

When label-based MAC feature is disabled, this system column can return
a pseudo value such as NULL or empty string.

#3 seems a little bit trickier. I don't think the GRANT ... WHERE
syntax is going to be very easy to use. For constraint-based
row-security, I think we should have something more like:

ALTER TABLE table ADD ROW FILTER filtername USING othertable [, ...]
WHERE where-clause

(This suffers from the same problem as DELETE ... USING, namely that
sometimes you want an outer join between table and othertable.)

This gives the user a convenient way to insert a join against one or
more side tables if they are so inclined.
Is it reasonably possible to implement USING clause, even if row-level
security is applied on COPY FROM/TO statement?
And, isn't it necessary to specify condition to apply the filter?
(such as select, update and delete)

The filter is the WHERE clause. I would think that the operation
being performed (select, update, delete) wouldn't enter into it. This
part is just to decide which tuples will actually be accessible AT
ALL. If you want to further prevent certain tuples that are being
accessed from being update or deleted, you can use a trigger for that
(possibly one of the global, always-applied-last triggers discussed
above).

For INSERT and COPY, I don't think that the ALTER TABLE ... ADD ROW
FILTER stuff would apply. If you want to restrict what gets inserted,
that's another job for triggers.

Are you talking about COPY TO, not only COPY FROM?
For INSERT and COPY FROM, I agree with the direction. Access controls
(and labeling) should be

Re: [HACKERS] WAL Info messages

2009-12-13 Thread Greg Stark

On Sun, Dec 13, 2009 at 7:20 PM, Simon Riggs si...@2ndquadrant.com wrote:
 On Standby side, we would provide a plugin interface to allow the
 message to be read and handled by user-written code. This would then
 allow a simple value or notification to be passed to a waiting (polling)
 connection on standby. If new LISTEN/NOTIFY gets into 8.5 then this
 would be an obvious fit, but I have no need for message passing, just
 info passing, so no need to wait for that; something much simpler.

What happens on the slave when normal NOTIFYs are generated on the
master? IIRC NOTIFYs are wal-logged so I imagine LISTEN on the slave
would just work and a plain old NOTIFY/LISTEN would suffice for this
use case?



-- 
greg

-- 
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] Row-Level Security

2009/12/13 KaiGai Kohei kai...@ak.jp.nec.com:
Robert Haas wrote:
On Sun, Dec 13, 2009 at 3:50 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote:
Basically, right. In my branch, SE-PgSQL put its hook after all the BR
trigger invocations.

http://code.google.com/p/sepgsql/source/browse/branches/pgsql-8.4.x/sepgsql/src/backend/executor/execMain.c#1883

The reason why I didn't apply this approach is it needs more modification
to the core routines, so it makes harder to manage out-of-tree code.

That's definitely something to consider if it's true. Why did it
require more modification of the core routines?

In my local branch, it just adds two lines as follows:
+ /* SELinux labeling and permission checks */
+ sepgsql_heap_insert(resultRelationDesc, tuple);

Yes, the difference in code impact between those two will not be
relevant for upstream, I think.

Yes, it is a separate problem not to be concluded at the moment.
(Perhaps, it depends on security model. In DAC, per-table basis is
preferable.)

Even for MAC, it might be desirable to turn it off on codes tables or
the like, to minimize the performance hit. But we can defer this
question to another day.

I don't see any real reason why it couldn't be configurable at
runtime, but I don't have a terribly strong opinion at this point. I
might have an opinion later when I'm more informed.

When label-based MAC feature is disabled, this system column can return
a pseudo value such as NULL or empty string.

I think you are wrong about all of this. To add security labels to
existing tuples, you're going to need to rewrite the table, period.
Whether you're adding a column in the process or just populating the
contents of a previous-omitted column doesn't seem particularly
relevant. Similarly you can insert a pseudo security label when the
column is missing just as well as you can when it's present but
unpopulated.

#3 seems a little bit trickier. I don't think the GRANT ... WHERE
syntax is going to be very easy to use. For constraint-based
row-security, I think we should have something more like:

ALTER TABLE table ADD ROW FILTER filtername USING othertable [, ...]
WHERE where-clause

(This suffers from the same problem as DELETE ... USING, namely that
sometimes you want an outer join between table and othertable.)

This gives the user a convenient way to insert a join against one or
more side tables if they are so inclined.
Is it reasonably possible to implement USING

Re: [HACKERS] [PATCH] ACE Framework - Database, Schema

Robert Haas wrote:
 2009/12/13 KaiGai Kohei kai...@ak.jp.nec.com:
 Just to name a few really obvious problems (I only looked at the
 01-database patch):

 1. We have been talking for several days about the need to make the
 initial patch in this area strictly a code cleanup patch.  Is this
 cleaner than the code that it is replacing?  Is it even making an
 attempt to conform to that mandate?
 Even if it is unclear whether the current form is more clear than the
 current inlined pg_xxx_aclcheck() form, or not, it will obviously
 provide a set of common entry points for upcoming enhanced security
 providers.
 Eventually, it is more clear than enumeration of #ifdef ... #endif
 blocks for SELinux, Smack, Solaris-TX and others.
 
 Right, but it will also not get committed.  :-(

The framework will be necessary to get them committed.
Which is an egg, and which is a chicken? :-(

 You seem to still be failing to understand the approach that Stephen
 and I have been discussing for the last several days...  but at any
 rate, I think that it is possible to do this in a way that is more
 clear (or at least, AS clear) as the current method.  Stephen is also
 working on a concept patch that I'm eager to see, and I would like to
 give him a little time to get that together before we spend too much
 more time working on this.

Hmm... In my hope, I'd like to work on these patches during the Christmas
vacation terms for US people. So, it seems to me we need to decide the
direction in this week.

Stephen, sorry for the prod.

 I don't understand the modifications to restrict_and_check_grant().
 It seems to me that this is going in the wrong direction, although
 since I don't understand the motivation for the change, I might be
 wrong.  If we have a piece of code that centralizes permissions
 checking now, splitting that up into a bunch of ace_*_grant()
 functions and duplicating it doesn't seem like an improvement.
 From perspective of the enhanced security providers, GRANT/REVOKE is
 a kind of modification of properties on a certain database object.
 So, it has a motivation to check GRANT/REVOKE using its access control
 rules.
 If we don't modify restrict_and_check_grant(), the caller (aclchk.c)
 correctly handles the default PG checks, so rest of enhanced security
 providers will apply additional checks in the ace_*_grant().
 In this case, we keep ace_*_grant() empty at the moment, and it shall
 be a special purpose hook for enhanced security providers.
 I don't have any preference on either of them.
 [...]
 When we support security label on database, it will also necessary:

 - ace_database_alter_relabel()
 
 This seems like bad design to me.  I don't see why SE-Linux would get
 a vote on whether a user is allowed to change the DAC permissions, any
 more than DAC gets a vote on whether an SE-Linux relabel operation can
 go through.  If it does, then this attempt to create a framework is
 not going to succeed, because every new provider will require hooks to
 let every exist provider muck with what it does, and they'll all have
 to have complete knowledge of each other's internals.

From the DAC perspective, a relabel operation is setting an attribute
of a certain database, such as ALTER DATABASE ... SET xxx. So, it is
quite natural to check ownership of the target database like other
ALTER DATABASE stuff, not only SELinux relabeling checks.

I intend to use the *_alter_relabel() hook for any other label-based
security providers, not only SELinux. The caller, syntax support and
the default PG checks don't need to know details in any other security
providers. (It is also reason why I prefer one enhanced provider at
the same time, because they can share syntax support such as SECURITY
LABEL ('...') option.)

In my image, it shall be implemented as follows:

  ALTER DATABASE datname SECURITY LABEL ( 'new label' );
 |
 V
  Value *
  ace_database_alter_relabel(Oid datOid, Value *newLabel)
  {
  if (pg_database_ownercheck(datOid, GetUserId())
  aclcheck_error(...);

  switch (ace_provider)
  {
  #if HAVE_SELINUX
  case ACE_PROVIDER_SELINUX:
  if (sepgsql_is_enabled())
  return sepgsql_database_alter_relabel(...);
  break;
  #endif
  #if HAVE_SMACK
  case ACE_PROVIDER_SMACK:
  if (smack_is_enabled())
  return smack_database_alter_relabel(...);
  break;
  #endif
  default:
  break;
  }

  ereport(ERROR, No label-based MAC is now enabled);
  return NULL;
  }
  |
  V
  The caller update pg_database system catalog with the returned
  security label.

However, it is not necessary to conclude at the moment.
We can add it later.

 It seems to me ace_database_calculate_size() might be a bad name
 because of too specific naming for a certin functionality.
 
 That's exactly the problem.  I'm not sure if I believe in the
 particular solution you've offered here, but it's certainly better
 than the way it is now.  I think

Re: [HACKERS] WAL Info messages

2009-12-13 Thread Heikki Linnakangas

Simon Riggs wrote:
 Proposal is to use up the last rmgr id, slot 7: RM_INFO_ID. 

Rmgr id is a 8-bit integer, there's plenty of free rmgr ids left.

-- 
  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] Row-Level Security