pgsql: sepgsql: Support for new post-ALTER access hook.

Started by Robert Haasalmost 13 years ago9 messages
#1Robert Haas
rhaas@postgresql.org

sepgsql: Support for new post-ALTER access hook.

KaiGai Kohei

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/1cea9bbb21e9e90dc7085ce605d9160e7161fa58

Modified Files
--------------
contrib/sepgsql/database.c | 27 +++++
contrib/sepgsql/expected/alter.out | 192 ++++++++++++++++++++++++++++++++++++
contrib/sepgsql/expected/ddl.out | 9 ++
contrib/sepgsql/hooks.c | 48 +++++++++
contrib/sepgsql/proc.c | 90 ++++++++++++++++-
contrib/sepgsql/relation.c | 91 ++++++++++++++++--
contrib/sepgsql/schema.c | 51 ++++++++++
contrib/sepgsql/sepgsql.h | 7 ++
contrib/sepgsql/sql/alter.sql | 136 +++++++++++++++++++++++++
contrib/sepgsql/sql/ddl.sql | 6 +
contrib/sepgsql/test_sepgsql | 27 +++++-
doc/src/sgml/sepgsql.sgml | 22 ++++-
12 files changed, 693 insertions(+), 13 deletions(-)

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

#2Thom Brown
thom@linux.com
In reply to: Robert Haas (#1)
Re: pgsql: sepgsql: Support for new post-ALTER access hook.

On 27 March 2013 12:33, Robert Haas <rhaas@postgresql.org> wrote:

sepgsql: Support for new post-ALTER access hook.

I notice that due to commit bc5334d8 I can't actually build the docs
at the moment.

But I think the language here definitely needs improving:

"On CREATE FUNCTION, install permission will be checked if leakproof
attribute was given, not only create on the new function. This
permission will be also checked when user tries to turn on leakproof
attribute using ALTER FUNCTION command, with setattr permission on the
function being altered."

And are the literals there capitalised when rendered? If not, could I
suggest they be capitalised in the SGML?

also:

s/for each object types/for each object type/

--
Thom

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

#3Robert Haas
robertmhaas@gmail.com
In reply to: Thom Brown (#2)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On Wed, Mar 27, 2013 at 8:44 AM, Thom Brown <thom@linux.com> wrote:

On 27 March 2013 12:33, Robert Haas <rhaas@postgresql.org> wrote:

sepgsql: Support for new post-ALTER access hook.

I notice that due to commit bc5334d8 I can't actually build the docs
at the moment.

But I think the language here definitely needs improving:

"On CREATE FUNCTION, install permission will be checked if leakproof
attribute was given, not only create on the new function. This
permission will be also checked when user tries to turn on leakproof
attribute using ALTER FUNCTION command, with setattr permission on the
function being altered."

What do you suggest? I thought about changing the wording but the new
wording is parallel to what's already in that paragraph, so likely the
whole thing needs to be rewritten if we change any of it. That seemed
beyond the scope of this commit, but I'm happy to have us do it.

And are the literals there capitalised when rendered? If not, could I
suggest they be capitalised in the SGML?

AFAIK, there's nothing that would change capitalization automatically
in our doc toolchain. Possibly LEAKPROOF should be capitalized but
the rest look right. setattr, etc. should not be capitalized, at
least according to my limited understanding of how SELinux
capitalization conventions work.

s/for each object types/for each object type/

In the interest of avoiding a proliferation of tiny commits, I'll hold
off on fixing this until we figure out what to do about the rest of
it.

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

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

#4Thom Brown
thom@linux.com
In reply to: Robert Haas (#3)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On 27 March 2013 12:58, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 27, 2013 at 8:44 AM, Thom Brown <thom@linux.com> wrote:

On 27 March 2013 12:33, Robert Haas <rhaas@postgresql.org> wrote:

sepgsql: Support for new post-ALTER access hook.

I notice that due to commit bc5334d8 I can't actually build the docs
at the moment.

But I think the language here definitely needs improving:

"On CREATE FUNCTION, install permission will be checked if leakproof
attribute was given, not only create on the new function. This
permission will be also checked when user tries to turn on leakproof
attribute using ALTER FUNCTION command, with setattr permission on the
function being altered."

What do you suggest? I thought about changing the wording but the new
wording is parallel to what's already in that paragraph, so likely the
whole thing needs to be rewritten if we change any of it. That seemed
beyond the scope of this commit, but I'm happy to have us do it.

Perhaps something along the lines of:

"When a CREATE FUNCTION command is executed, the install permission
will be checked to determine whether the LEAKPROOF attribute was
present. This permission will also be checked when the user tries to
apply the LEAKPROOF attribute using the ALTER FUNCTION command."

I'm not sure what the last part is actually describing ("with setattr
permission on the function being altered."), so I'm not sure how that
should be read. It doesn't help that I'm not familiar with SELinux
terms.

And are the literals there capitalised when rendered? If not, could I
suggest they be capitalised in the SGML?

AFAIK, there's nothing that would change capitalization automatically
in our doc toolchain. Possibly LEAKPROOF should be capitalized but
the rest look right. setattr, etc. should not be capitalized, at
least according to my limited understanding of how SELinux
capitalization conventions work.

I was really just thinking of CREATE and LEAKPROOF, but I'm not sure
"CREATE" should be in there anyway.

--
Thom

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

#5Robert Haas
robertmhaas@gmail.com
In reply to: Thom Brown (#4)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On Wed, Mar 27, 2013 at 9:09 AM, Thom Brown <thom@linux.com> wrote:

Perhaps something along the lines of:

"When a CREATE FUNCTION command is executed, the install permission
will be checked to determine whether the LEAKPROOF attribute was
present. This permission will also be checked when the user tries to
apply the LEAKPROOF attribute using the ALTER FUNCTION command."

I'm not sure what the last part is actually describing ("with setattr
permission on the function being altered."), so I'm not sure how that
should be read. It doesn't help that I'm not familiar with SELinux
terms.

Right, so what it's trying to say is: whenever you modify an object,
we check whether you've got {setattr} permission for that object and
disallow the operation if not. However, for some operations on some
object types, {setattr} is necessary but not sufficient. The
paragraph is recapping, for various cases, which operations require
additional permissions, and what those additional things are.

I was really just thinking of CREATE and LEAKPROOF, but I'm not sure
"CREATE" should be in there anyway.

create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.

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

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

#6Thom Brown
thom@linux.com
In reply to: Robert Haas (#5)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On 27 March 2013 14:50, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 27, 2013 at 9:09 AM, Thom Brown <thom@linux.com> wrote:

Perhaps something along the lines of:

"When a CREATE FUNCTION command is executed, the install permission
will be checked to determine whether the LEAKPROOF attribute was
present. This permission will also be checked when the user tries to
apply the LEAKPROOF attribute using the ALTER FUNCTION command."

I'm not sure what the last part is actually describing ("with setattr
permission on the function being altered."), so I'm not sure how that
should be read. It doesn't help that I'm not familiar with SELinux
terms.

Right, so what it's trying to say is: whenever you modify an object,
we check whether you've got {setattr} permission for that object and
disallow the operation if not. However, for some operations on some
object types, {setattr} is necessary but not sufficient. The
paragraph is recapping, for various cases, which operations require
additional permissions, and what those additional things are.

I was really just thinking of CREATE and LEAKPROOF, but I'm not sure
"CREATE" should be in there anyway.

create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.

My bad.

--
Thom

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

#7Robert Haas
robertmhaas@gmail.com
In reply to: Thom Brown (#6)
1 attachment(s)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On Wed, Mar 27, 2013 at 10:51 AM, Thom Brown <thom@linux.com> wrote:

create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.

My bad.

Here's an attempt at reworking the whole section to be more
understandable. I took the liberty of rearranging the text into
bulleted lists, which I hope is more clear.

Comments welcome.

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

Attachments:

sepgsql-ddl-doc-fix.patchapplication/octet-stream; name=sepgsql-ddl-doc-fix.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6488399..0995bc5 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -330,7 +330,6 @@ include 'filename'
       </listitem>
      </varlistentry>
 
-     <variablelist>
      <varlistentry id="guc-recovery-config-directory" xreflabel="recovery_config_directory">
       <term><varname>recovery_config_directory</varname> (<type>string</type>)</term>
       <indexterm>
diff --git a/doc/src/sgml/sepgsql.sgml b/doc/src/sgml/sepgsql.sgml
index 5ee08e1..e3c7d0a 100644
--- a/doc/src/sgml/sepgsql.sgml
+++ b/doc/src/sgml/sepgsql.sgml
@@ -421,69 +421,85 @@ UPDATE t1 SET x = 2, y = md5sum(y) WHERE z = 100;
     addition or deletion of name entries within a particular schema.
    </para>
    <para>
-    When a <literal>CREATE</> command is executed, <literal>create</> will
-    be checked on the object being constructed for each object types.
-    A default security label will be assigned to the new database object,
-    and the <literal>create</> permission will be checked on the pair
-    of security label of the client and the new object itself.
-    We consider <xref linkend="sql-createtable"> to construct a table and
-    underlying columns at the same time, so it requires the users to have
-    permission to create both the table and its columns.
-   </para>
-   <para>
-    A few additional checks are applied depending on object types.
-    On <xref linkend="sql-createdatabase">, <literal>getattr</> permission
-    will be checked on the source or template database of the new database,
-    not only <literal>create</> on the new database.
-    On creation of objects within a particular schema (tables, views,
-    sequences and procedures), <literal>add_name</> will be also checked
-    on the schema, not only <literal>create</> on the new object itself.
-    On <xref linkend="sql-createfunction">, <literal>install</> permission
-    will be checked if <literal>leakproof</> attribute was given, not only
-    <literal>create</> on the new function. This permission will be also
-    checked when user tries to turn on <literal>leakproof</> attribute
-    using <xref linkend="sql-alterfunction"> command, with
-    <literal>setattr</> permission on the function being altered.
+    Creating a new database object requires <literal>create</> permission.
+    <productname>SELinux</> will grant or deny this permission based on the
+    client's security label and the proposed security label for the new
+    object.  In some cases, additional privileges are required:
    </para>
 
+   <itemizedlist>
+    <listitem>
+     <para>
+      <xref linkend="sql-createdatabase"> additionally requires
+      <literal>getattr</> permission for the source or template database.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Creating a schema object additionally requires <literal>add_name</>
+      permission on the parent schema.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Creating a table additionally requires permission to create each
+      individual table column, just as if each table column were a
+      separate top-level object.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Creating a function marked as <literal>LEAKPROOF</> additionally
+      requires <literal>install</> permission.  (This permission is also
+      checked when <literal>LEAKPROOF</> is set for an existing function.)
+     </para>
+    </listitem>
+   </itemizedlist>
+
    <para>
     When <literal>DROP</> command is executed, <literal>drop</> will be
-    checked on the object being removed for each object types.  Permissions
-    will be also checked for objects dropped indirectly via <literal>CASCADE</>.
-    Deletion of objects contained within a particular schema (tables, views,
-    sequences and procedures) additionally requires
-    <literal>remove_name</> on the schema.
+    checked on the object being removed.  Permissions will be also checked for
+    objects dropped indirectly via <literal>CASCADE</>.  Deletion of objects
+    contained within a particular schema (tables, views, sequences and
+    procedures) additionally requires <literal>remove_name</> on the schema.
    </para>
 
    <para>
     When <literal>ALTER</> command is executed, <literal>setattr</> will be
-    checked on the object being modified for each object types. 
-    In addition, <literal>remove_name</> and <literal>add_name</>
-    will be checked on the old and new schemas, respectively, when an
-    object is moved to a new schema.
-    For certain object types, additional checks are performed.
+    checked on the object being modified for each object types, except for
+    subsidiary objects such as the indexes or triggers of a table, where
+    permissions are instead checked on the parent object.  In some cases,
+    additional permssions are required:
    </para>
 
-   <para>
-    When objects that are subsidiary of other objects (such as a table's
-    indexes or triggers) are created, dropped or altered,
-    <literal>setattr</> permission will be checked on the main object,
-    instead of the subsidiary object itself.
-   </para>
-
-   <para>
-    When <xref linkend="sql-security-label"> is executed, <literal>setattr</>
-    and <literal>relabelfrom</> will be checked on the object being relabeled
-    with its old security label, then <literal>relabelto</> with the supplied
-    new security label.
-   </para>
+   <itemizedlist>
+    <listitem>
+     <para>
+      Moving an object to a new schema additionally requires
+      <literal>remove_name</> permission on the old schema and
+      <literal>add_name</> permission on the new one.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Setting the <literal>LEAKPROOF</> attribute on a function requires
+      <literal>install</> permission.
+     </para>
+    </listitem>
+    <listitem>
+     <para>
+      Using <xref linkend="sql-security-label"> on an object additionally
+      requires <literal>relabelfrom</> permission for the object in
+      conjunction with its old security label and <literal>relabelto</>
+      permission for the object in conjunction with its new security label.
+      (In cases where multiple label providers are installed and the user
+      tries to set a security label, but it is not managed by
+      <productname>SELinux</>, only <literal>setattr</> should be checked here.
+      This is currently not done due to implementation restrictions.)
+     </para>
+    </listitem>
+   </itemizedlist>
 
-   <para>
-    In the case where multiple label providers are installed and the user tries
-    to set a security label, but it is not managed by <productname>SELinux</>,
-    only <literal>setattr</> should be checked here.
-    This is currently not done due to implementation restrictions.
-   </para>
   </sect3>
 
   <sect3>
#8Thom Brown
thom@linux.com
In reply to: Robert Haas (#7)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On 27 March 2013 15:19, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 27, 2013 at 10:51 AM, Thom Brown <thom@linux.com> wrote:

create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.

My bad.

Here's an attempt at reworking the whole section to be more
understandable. I took the liberty of rearranging the text into
bulleted lists, which I hope is more clear.

Comments welcome.

This looks a lot better, apart from:

s/permssions/permissions/

+1 from me.

--
Thom

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

#9Robert Haas
robertmhaas@gmail.com
In reply to: Thom Brown (#8)
Re: [COMMITTERS] pgsql: sepgsql: Support for new post-ALTER access hook.

On Wed, Mar 27, 2013 at 11:27 AM, Thom Brown <thom@linux.com> wrote:

On 27 March 2013 15:19, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 27, 2013 at 10:51 AM, Thom Brown <thom@linux.com> wrote:

create here is referring to the sepgsql permission, not the SQL
command, so it's correct as-is.

My bad.

Here's an attempt at reworking the whole section to be more
understandable. I took the liberty of rearranging the text into
bulleted lists, which I hope is more clear.

Comments welcome.

This looks a lot better, apart from:

s/permssions/permissions/

+1 from me.

OK, committed.

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

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