Обсуждение: [PATCH] SE-PgSQL/lite (r2429)
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>
Вложения
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
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>
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
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>