Thread

1. [PATCH] SE-PgSQL/lite (r2429)

   Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2009-11-13T02:12:53Z

   The attached patch is a "lite" version of SE-PostgreSQL,
   for the CommitFest#3.
   
   Its characteristic changes from the previous patches are
   developer documentations and slimed-up code.
   
   The developer documentation is a suggestion on the last
   commit fest. In the CF#2, we worked to implement a common
   abstraction layer for both of DAC and MAC, but its change
   set got larger than we expected at the begining.
   Then, we decided to implement them with deployment of hooks
   on the strategic points with clear documentation.
   
   The src/backend/security/sepgsql/README is a documentation
   from the viewpoint of developers. I belive it can help to
   understand intention of the SE-PgSQL design.
   In addition, I paid much efforts to write source code comments
   to introduce the purpose and its requirement.
   
   See the:
     http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/backend/security/sepgsql/README
   
   
   The scale of change set has been a headache for us.
   Some quantity of change set is unavoidable to implement access
   controls on comprehensive database objects.
   For example, the catalog/aclchk.c and utils/adt/acl.c have more
   than 9,000 lines in total.
   
   In the v8.4 development cycle, I got a suggestion to reduce
   a burden of reviewer to split off a few functionalities, such
   as "security_context" system column and row-level access controls.
   
   In this patch, I splited off a few functionalities furthermore.
   It does not apply any access controls except for four types of
   database objects classes; databases, schemas, tables and columns.
   We can incrementally implement access controls on the rest of
   database objects, such as procedures, in the future.
   
   In addition, I also splited off a few features.
   Now it has no special feature to manage security context. It is
   stored within a certain text field of system catalogs related to
   the four object types.
   Now it has no access control decision cache which contributes
   abour 800 lines of code reduction.
   
   
   In total, this patch contains about 9.5KL of change set.
   - About 2.1KL for documentaions (doc/src/sgml/* and the README).
   - About 1.7KL for test cases (src/test/*).
   - Rest of 5.5KL are as follows:
     - About 3.3KL for SE-PgSQL specific code with comments.
       (include/security/* and backend/security/*)
     - About 0.6KL for headers.
     - About 1.2KL for core routines.
     - About 0.3KL for pg_dump and initdb (bin/*)
   
   You might feel the 3.3KL is still a bit large. But its reason is
   obvious in the source code. I paid much efforts to describe source
   code comments to help understand the purpose and intention of the
   functions.
   
   About half of the 1.2KL is due to the statement support to give
   an explicit security context on CREATE or ALTER statement.
   But these are commonly-used code, implemented such as:
   
     rel = heap_open(RelationRelationId, RowExclusiveLock);
             :
     value[Anum_pg_class_relsecon - 1] = CStringGetTextDatum(relsecon);
             :
     simple_heap_update(rel, &newtuple->t_self, newtuple);
             :
     heap_close(rel);
   
   I don't think it is a major hurdle for reviewing.
   So, all we need to pay great attention is the rest of 0.6KL.
   I think it is enough small change set as long as SE-PgSQL does not
   lose its soul.
   
   Thanks,
   -- 
   OSS Platform Development Division, NEC
   KaiGai Kohei <kaigai@ak.jp.nec.com>
   

2. Re: [PATCH] SE-PgSQL/lite (r2429)

   Greg Smith <greg@2ndquadrant.com> — 2009-11-13T05:06:51Z

   KaiGai Kohei wrote:
   > In the v8.4 development cycle, I got a suggestion to reduce
   > a burden of reviewer to split off a few functionalities, such
   > as "security_context" system column and row-level access controls.
   >   
   I lost track of this patch and related bits somewhere along the way, had
   to triage my unread mail a few times. Could someone summarize how it now
   fits into plans for more general row-level access controls in the
   database? I know incompatibilities between the SEPosgreSQL model for row
   filtering and thoughts for a more general permissions feature that did
   something similar were a major design issue in the early 8.4 versions of
   SEPostgreSQL, and that as you say you've been working on that. I'm not
   sure what relationship there is between those two today though, or
   exactly where the general non-SELinux row filtering is at on the roadmap.
   
   -- 
   Greg Smith    2ndQuadrant   Baltimore, MD
   PostgreSQL Training, Services and Support
   greg@2ndQuadrant.com  www.2ndQuadrant.com
   
   
   

3. Re: [PATCH] SE-PgSQL/lite (r2429)

   Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2009-11-13T05:34:07Z

   Greg Smith wrote:
   > KaiGai Kohei wrote:
   >> In the v8.4 development cycle, I got a suggestion to reduce
   >> a burden of reviewer to split off a few functionalities, such
   >> as "security_context" system column and row-level access controls.
   >>   
   > I lost track of this patch and related bits somewhere along the way, had
   > to triage my unread mail a few times. Could someone summarize how it now
   > fits into plans for more general row-level access controls in the
   > database? I know incompatibilities between the SEPosgreSQL model for row
   > filtering and thoughts for a more general permissions feature that did
   > something similar were a major design issue in the early 8.4 versions of
   > SEPostgreSQL, and that as you say you've been working on that. I'm not
   > sure what relationship there is between those two today though, or
   > exactly where the general non-SELinux row filtering is at on the roadmap.
   
   At least, I don't have a plan to submit a patch for row-level access
   controls in the v8.5 development cycle. We should focus on the "lite"
   version here.
   On that basis, I shall propose the row-level access controls after
   the current efforts getting closed.
   
   I found a uncertain term in your comment.
   It seems to me the "model" has two meanings in this context.
   - The way to make access control decision (allowed? or denied?).
   - The granularity of access controls (tables? columns? or tuples?).
   
   I think you are saying about the latet point.
   In my plan, I'll propose a feature something like Oracle Virtual
   Private Database which filters violated rows based on the decision
   making function. (e.g tbl.username = getpgusername() )
   Needless to say, it is a general non-SELinux feature. But, if we
   have such a PG-VPD, it is not difficult to implement a decision
   making function based on SELinux.
   
   Is it correct for the answer?
   
   Thanks,
   -- 
   OSS Platform Development Division, NEC
   KaiGai Kohei <kaigai@ak.jp.nec.com>
   
   

4. Re: [PATCH] SE-PgSQL/lite (r2429)

   Greg Smith <greg@2ndquadrant.com> — 2009-11-13T06:19:42Z

   KaiGai Kohei wrote:
   > I found a uncertain term in your comment.
   > It seems to me the "model" has two meanings in this context.
   > - The way to make access control decision (allowed? or denied?).
   > - The granularity of access controls (tables? columns? or tuples?).
   >   
   What I meant by "the SEPosgreSQL model for row filtering" was the
   original implementation you had, where row filtering was handled by code
   specific to SEPostgreSQL, not something generic enough to be used for
   other purposes. I wasn't sure what if anything from there was still in
   the patch, and you answered that clearly enough. Thanks for clarifying
   where things are at.
   
   -- 
   Greg Smith    2ndQuadrant   Baltimore, MD
   PostgreSQL Training, Services and Support
   greg@2ndQuadrant.com  www.2ndQuadrant.com
   
   
   

5. [PATCH] SE-PgSQL/lite (r2451)

   Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2009-11-30T08:42:23Z

   The attached patch is a revised version based on the comments.
   
   - rebased to the latest CVS HEAD.
   
   - revise: statement supports are changed as follows:
       old style:
           CREATE TABLE tbl (
               <col_name> <type> AS SECURITY_CONTEXT = 'label of column',
                   :
           ) SECURITY_CONTEXT = 'label of table';
   
       new style:
           CREATE TABLE tbl (...)
               SECURITY CONTEXT ([<col_name> = ]'<label of table/column>' [, ...]);
   
   - revise: replace a term of 'SE-PgSQL' from user visible error messages
             and SGML documentations. 'SELinux support' is used instead.
   
   - revise: GUC option was renamed.
               'sepostgresql'          -> 'selinux_support'
               'sepostgresql_mcstrans' -> 'selinux_mcstrans'
   
   - revise: SE-PgSQL specific error codes were integrated to other class.
               ERRCODE_SELINUX_ERROR -> moved to system error ('58xxx')
               ERRCODE_INVALID_SECURITY_CONTEXT -> moved to syntax error
                 or access rule violation ('42xxx')
               ERRCODE_SELINUX_AUDIT_LOG -> removed, because access violation
                 log shall be generated on ERRCODE_INSUFFICIENT_PRIVILEGE.
   
   - bugfix: copyfunc.c didn't support IntoClause->seconList.
   - bugfix: SGML documentation was not patched for the new error codes.
   
   Thanks,
   
   
   KaiGai Kohei wrote:
   > The attached patch is a "lite" version of SE-PostgreSQL,
   > for the CommitFest#3.
   > 
   > Its characteristic changes from the previous patches are
   > developer documentations and slimed-up code.
   > 
   > The developer documentation is a suggestion on the last
   > commit fest. In the CF#2, we worked to implement a common
   > abstraction layer for both of DAC and MAC, but its change
   > set got larger than we expected at the begining.
   > Then, we decided to implement them with deployment of hooks
   > on the strategic points with clear documentation.
   > 
   > The src/backend/security/sepgsql/README is a documentation
   > from the viewpoint of developers. I belive it can help to
   > understand intention of the SE-PgSQL design.
   > In addition, I paid much efforts to write source code comments
   > to introduce the purpose and its requirement.
   > 
   > See the:
   >   http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/src/backend/security/sepgsql/README
   > 
   > 
   > The scale of change set has been a headache for us.
   > Some quantity of change set is unavoidable to implement access
   > controls on comprehensive database objects.
   > For example, the catalog/aclchk.c and utils/adt/acl.c have more
   > than 9,000 lines in total.
   > 
   > In the v8.4 development cycle, I got a suggestion to reduce
   > a burden of reviewer to split off a few functionalities, such
   > as "security_context" system column and row-level access controls.
   > 
   > In this patch, I splited off a few functionalities furthermore.
   > It does not apply any access controls except for four types of
   > database objects classes; databases, schemas, tables and columns.
   > We can incrementally implement access controls on the rest of
   > database objects, such as procedures, in the future.
   > 
   > In addition, I also splited off a few features.
   > Now it has no special feature to manage security context. It is
   > stored within a certain text field of system catalogs related to
   > the four object types.
   > Now it has no access control decision cache which contributes
   > abour 800 lines of code reduction.
   > 
   > 
   > In total, this patch contains about 9.5KL of change set.
   > - About 2.1KL for documentaions (doc/src/sgml/* and the README).
   > - About 1.7KL for test cases (src/test/*).
   > - Rest of 5.5KL are as follows:
   >   - About 3.3KL for SE-PgSQL specific code with comments.
   >     (include/security/* and backend/security/*)
   >   - About 0.6KL for headers.
   >   - About 1.2KL for core routines.
   >   - About 0.3KL for pg_dump and initdb (bin/*)
   > 
   > You might feel the 3.3KL is still a bit large. But its reason is
   > obvious in the source code. I paid much efforts to describe source
   > code comments to help understand the purpose and intention of the
   > functions.
   > 
   > About half of the 1.2KL is due to the statement support to give
   > an explicit security context on CREATE or ALTER statement.
   > But these are commonly-used code, implemented such as:
   > 
   >   rel = heap_open(RelationRelationId, RowExclusiveLock);
   >           :
   >   value[Anum_pg_class_relsecon - 1] = CStringGetTextDatum(relsecon);
   >           :
   >   simple_heap_update(rel, &newtuple->t_self, newtuple);
   >           :
   >   heap_close(rel);
   > 
   > I don't think it is a major hurdle for reviewing.
   > So, all we need to pay great attention is the rest of 0.6KL.
   > I think it is enough small change set as long as SE-PgSQL does not
   > lose its soul.
   > 
   > Thanks,
   > 
   > 
   > ------------------------------------------------------------------------
   > 
   > 
   
   
   -- 
   OSS Platform Development Division, NEC
   KaiGai Kohei <kaigai@ak.jp.nec.com>