Re: [v9.2] SECURITY LABEL on shared database object

[I noticed my new address was not registered to pgsql-hackers, so try again]

The attached patch enables to assign security labels of shared database object
types (databases, tablespaces, roles).
Like as pg_shdescription doing, it adds a new pg_shseclabel catalog corresponding
to the existing pg_seclabel catalog. Its internal APIs are kept; if and when
(Get|Set|Delete)SecurityLabel is invoked for shared catalogs, it references the
pg_shseclabel instead of the pg_seclabel.

This patch also contains pg_dump support, use cases of contrib/sepgsql, regression
test and updates of sgml documentation.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei <kohei(dot)kaigai(at)eu(dot)nec(dot)com>

The attached patch is an update revision of security label support
for shared database objects.

It contains bug fixes of cleanups when user drops databases,
tablespaces or roles.
And it also modified to use syscache to reference security label
of shared database objects according to the implementation
discussed in the sepgsql-uavc thread.

2011/5/9 Kohei Kaigai <Kohei(dot)Kaigai(at)emea(dot)nec(dot)com>:
> [I noticed my new address was not registered to pgsql-hackers, so try again]
>
> The attached patch enables to assign security labels of shared database object
> types (databases, tablespaces, roles).
> Like as pg_shdescription doing, it adds a new pg_shseclabel catalog corresponding
> to the existing pg_seclabel catalog. Its internal APIs are kept; if and when
> (Get|Set|Delete)SecurityLabel is invoked for shared catalogs, it references the
> pg_shseclabel instead of the pg_seclabel.
>
> This patch also contains pg_dump support, use cases of contrib/sepgsql, regression
> test and updates of sgml documentation.
>
> Thanks,
> --
> NEC Europe Ltd, SAP Global Competence Center
> KaiGai Kohei <kohei(dot)kaigai(at)eu(dot)nec(dot)com>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> The attached patch is an update revision of security label support
> for shared database objects.

I'm kind of unexcited about this whole idea. Adding a shared catalog
for a feature that's only of interest to a small percentage of our
user population seems unfortunate.

Are there any other possible approaches to this problem?

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

2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> The attached patch is an update revision of security label support
>> for shared database objects.
>
> I'm kind of unexcited about this whole idea.  Adding a shared catalog
> for a feature that's only of interest to a small percentage of our
> user population seems unfortunate.
>
> Are there any other possible approaches to this problem?
>
If unexcited about the new shared catalog, one possible idea
is to add a new field to pg_database, pg_tablespace and
pg_authid to store security labels?

The reason why we had pg_seclabel is to avoid massive amount
of modifications to system catalog. But only 3 catalogs to be
modified to support security label on shared object.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Mon, Jun 13, 2011 at 1:40 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> The attached patch is an update revision of security label support
>>> for shared database objects.
>>
>> I'm kind of unexcited about this whole idea.  Adding a shared catalog
>> for a feature that's only of interest to a small percentage of our
>> user population seems unfortunate.
>>
>> Are there any other possible approaches to this problem?
>>
> If unexcited about the new shared catalog, one possible idea
> is to add a new field to pg_database, pg_tablespace and
> pg_authid to store security labels?
>
> The reason why we had pg_seclabel is to avoid massive amount
> of modifications to system catalog. But only 3 catalogs to be
> modified to support security label on shared object.

I guess maybe my real question here is - what do you plan to do with
those security labels, from a security perspective? For example:
roles. The user's security contect AIUI is passed over from the
remote side; his DAC role doesn't even enter into it from a MAC
perspective. Or does it?

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

2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Mon, Jun 13, 2011 at 1:40 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> 2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>> On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>>> The attached patch is an update revision of security label support
>>>> for shared database objects.
>>>
>>> I'm kind of unexcited about this whole idea.  Adding a shared catalog
>>> for a feature that's only of interest to a small percentage of our
>>> user population seems unfortunate.
>>>
>>> Are there any other possible approaches to this problem?
>>>
>> If unexcited about the new shared catalog, one possible idea
>> is to add a new field to pg_database, pg_tablespace and
>> pg_authid to store security labels?
>>
>> The reason why we had pg_seclabel is to avoid massive amount
>> of modifications to system catalog. But only 3 catalogs to be
>> modified to support security label on shared object.
>
> I guess maybe my real question here is - what do you plan to do with
> those security labels, from a security perspective?  For example:
> roles.  The user's security contect AIUI is passed over from the
> remote side; his DAC role doesn't even enter into it from a MAC
> perspective.  Or does it?
>
The current primary target of security label of shared object is
to acquire control on databases.
It performs as a basis to compute default security label of
underlying objects, and I also plan to control when we open
the connection like ACL_CONNECT.
Right now, we assume any database has "system_u:object_r:sepgsql_db_t:s0",
then the default security label of schema objects are computed.
(See sepgsql_schema_post_create in contrib/sepgsql/schema.c)

Regarding to the pg_tablespace and pg_authid, I have a plan to
control DDL statements on these objects, However, its priority
is not higher than databases or other non-shared objects such
as tables or procedures.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Mon, Jun 13, 2011 at 2:33 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
>> On Mon, Jun 13, 2011 at 1:40 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>> 2011/6/13 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>>> On Mon, Jun 13, 2011 at 12:24 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>>>> The attached patch is an update revision of security label support
>>>>> for shared database objects.
>>>>
>>>> I'm kind of unexcited about this whole idea.  Adding a shared catalog
>>>> for a feature that's only of interest to a small percentage of our
>>>> user population seems unfortunate.
>>>>
>>>> Are there any other possible approaches to this problem?
>>>>
>>> If unexcited about the new shared catalog, one possible idea
>>> is to add a new field to pg_database, pg_tablespace and
>>> pg_authid to store security labels?
>>>
>>> The reason why we had pg_seclabel is to avoid massive amount
>>> of modifications to system catalog. But only 3 catalogs to be
>>> modified to support security label on shared object.
>>
>> I guess maybe my real question here is - what do you plan to do with
>> those security labels, from a security perspective?  For example:
>> roles.  The user's security contect AIUI is passed over from the
>> remote side; his DAC role doesn't even enter into it from a MAC
>> perspective.  Or does it?
>>
> The current primary target of security label of shared object is
> to acquire control on databases.
> It performs as a basis to compute default security label of
> underlying objects, and I also plan to control when we open
> the connection like ACL_CONNECT.
> Right now, we assume any database has "system_u:object_r:sepgsql_db_t:s0",
> then the default security label of schema objects are computed.
> (See sepgsql_schema_post_create in contrib/sepgsql/schema.c)
>
> Regarding to the pg_tablespace and pg_authid, I have a plan to
> control DDL statements on these objects, However, its priority
> is not higher than databases or other non-shared objects such
> as tables or procedures.

Hmm, OK. I guess what I'm not sure about is - how much should we
worry about the fact that this creates several more shared (and
therefore nailed?) system catalogs? Anyone have an opinion on that?

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

Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:

> Hmm, OK. I guess what I'm not sure about is - how much should we
> worry about the fact that this creates several more shared (and
> therefore nailed?) system catalogs? Anyone have an opinion on that?

"Several"? That would worry me, given that we currently have a small
number (eight currently). If it's just one more, I don't think it's
such a big deal. I'm not sure what you mean by nailed though -- I mean,
for example pg_shdescription is shared but not nailed in the rd_isnailed
sense of the word, AFAICS.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:
>
>> Hmm, OK.  I guess what I'm not sure about is - how much should we
>> worry about the fact that this creates several more shared (and
>> therefore nailed?) system catalogs?  Anyone have an opinion on that?
>
> "Several"?  That would worry me, given that we currently have a small
> number (eight currently).  If it's just one more, I don't think it's
> such a big deal.  I'm not sure what you mean by nailed though -- I mean,
> for example pg_shdescription is shared but not nailed in the rd_isnailed
> sense of the word, AFAICS.

Well, right now the patch has pg_shseclabel, and its index, plus a
toast table and a toast index. Not sure why we want/need the toast
table & index there, but the patch has 'em as of now.

As for whether it needs to be nailed, I'm not sure I understand what
the rules are there. I *think* the rule is that anything that might
need to be consulted before choosing a database must be nailed. If
that's right, we might be able to get by without nailing it, as long
as the label isn't needed during authentication (or its use can be
postponed until after we've connected to a database).

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

> On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
> > Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:
> >
> >> Hmm, OK.  I guess what I'm not sure about is - how much should we
> >> worry about the fact that this creates several more shared (and
> >> therefore nailed?) system catalogs?  Anyone have an opinion on that?
> >
> > "Several"?  That would worry me, given that we currently have a small
> > number (eight currently).  If it's just one more, I don't think it's
> > such a big deal.  I'm not sure what you mean by nailed though -- I mean,
> > for example pg_shdescription is shared but not nailed in the rd_isnailed
> > sense of the word, AFAICS.
>
> Well, right now the patch has pg_shseclabel, and its index, plus a
> toast table and a toast index. Not sure why we want/need the toast
> table & index there, but the patch has 'em as of now.
>
As a common belief, TEXT is a variable length data type, so pg_shseclabel
need to have its toast table. However, I don't expect the label field get
represented as a reference to external pointer, because average length of
security context is about 40-60 bytes much less than the threshold to
launch toast_save_datum().
Do I need to remove these toast table & index?

> As for whether it needs to be nailed, I'm not sure I understand what
> the rules are there. I *think* the rule is that anything that might
> need to be consulted before choosing a database must be nailed. If
> that's right, we might be able to get by without nailing it, as long
> as the label isn't needed during authentication (or its use can be
> postponed until after we've connected to a database).
>
In SELinux, all we are doing in the authentication hook is to acquire
security label of the client, without referencing any catalogs.

I also plan to support permission checks on the selected database
in the future, however, I believe its hook should be placed in
CheckMyDatabase() according to the existing checks.

Thanks,
--
NEC Europe Ltd, SAP Global Competence Center
KaiGai Kohei <kohei(dot)kaigai(at)emea(dot)nec(dot)com>

Excerpts from Kohei Kaigai's message of mar jul 05 11:46:06 -0400 2011:
> > On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera
> > <alvherre(at)commandprompt(dot)com> wrote:
> > > Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:
> > >
> > >> Hmm, OK.  I guess what I'm not sure about is - how much should we
> > >> worry about the fact that this creates several more shared (and
> > >> therefore nailed?) system catalogs?  Anyone have an opinion on that?
> > >
> > > "Several"?  That would worry me, given that we currently have a small
> > > number (eight currently).  If it's just one more, I don't think it's
> > > such a big deal.  I'm not sure what you mean by nailed though -- I mean,
> > > for example pg_shdescription is shared but not nailed in the rd_isnailed
> > > sense of the word, AFAICS.
> >
> > Well, right now the patch has pg_shseclabel, and its index, plus a
> > toast table and a toast index. Not sure why we want/need the toast
> > table & index there, but the patch has 'em as of now.
> >
> As a common belief, TEXT is a variable length data type, so pg_shseclabel
> need to have its toast table. However, I don't expect the label field get
> represented as a reference to external pointer, because average length of
> security context is about 40-60 bytes much less than the threshold to
> launch toast_save_datum().
> Do I need to remove these toast table & index?

We don't have toast tables for pg_database and so on, for example, which
means that datacl cannot go over a few hundred bytes long. I think it
makes sense to not have toast tables for pg_shseclabel. Keep in mind
that the label might be compressed before it's stored out of line, which
gives you quite a bit of actual space. If a security context is over
5000 bytes in length I think you're in trouble :-)

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

2011/7/5 Alvaro Herrera <alvherre(at)commandprompt(dot)com>:
> Excerpts from Kohei Kaigai's message of mar jul 05 11:46:06 -0400 2011:
>> > On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera
>> > <alvherre(at)commandprompt(dot)com> wrote:
>> > > Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:
>> > >
>> > >> Hmm, OK.  I guess what I'm not sure about is - how much should we
>> > >> worry about the fact that this creates several more shared (and
>> > >> therefore nailed?) system catalogs?  Anyone have an opinion on that?
>> > >
>> > > "Several"?  That would worry me, given that we currently have a small
>> > > number (eight currently).  If it's just one more, I don't think it's
>> > > such a big deal.  I'm not sure what you mean by nailed though -- I mean,
>> > > for example pg_shdescription is shared but not nailed in the rd_isnailed
>> > > sense of the word, AFAICS.
>> >
>> > Well, right now the patch has pg_shseclabel, and its index, plus a
>> > toast table and a toast index.  Not sure why we want/need the toast
>> > table & index there, but the patch has 'em as of now.
>> >
>> As a common belief, TEXT is a variable length data type, so pg_shseclabel
>> need to have its toast table. However, I don't expect the label field get
>> represented as a reference to external pointer, because average length of
>> security context is about 40-60 bytes much less than the threshold to
>> launch toast_save_datum().
>> Do I need to remove these toast table & index?
>
> We don't have toast tables for pg_database and so on, for example, which
> means that datacl cannot go over a few hundred bytes long.  I think it
> makes sense to not have toast tables for pg_shseclabel.  Keep in mind
> that the label might be compressed before it's stored out of line, which
> gives you quite a bit of actual space.  If a security context is over
> 5000 bytes in length I think you're in trouble :-)
>
The attached patch removes toast table & index for pg_shseclabel.

The current toasting.h defines toast table & index on pg_database,
pg_shdescription and pg_db_role_setting only.
The pg_authid and pg_tablespace don't have toast table & index
in spite of variable-length field.
So, it might not be a necessary stuff for all the shared relations.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Wed, Jul 6, 2011 at 1:25 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2011/7/5 Alvaro Herrera <alvherre(at)commandprompt(dot)com>:
>> Excerpts from Kohei Kaigai's message of mar jul 05 11:46:06 -0400 2011:
>>> > On Tue, Jul 5, 2011 at 10:49 AM, Alvaro Herrera
>>> > <alvherre(at)commandprompt(dot)com> wrote:
>>> > > Excerpts from Robert Haas's message of mar jul 05 10:19:18 -0400 2011:
>>> > >
>>> > >> Hmm, OK.  I guess what I'm not sure about is - how much should we
>>> > >> worry about the fact that this creates several more shared (and
>>> > >> therefore nailed?) system catalogs?  Anyone have an opinion on that?
>>> > >
>>> > > "Several"?  That would worry me, given that we currently have a small
>>> > > number (eight currently).  If it's just one more, I don't think it's
>>> > > such a big deal.  I'm not sure what you mean by nailed though -- I mean,
>>> > > for example pg_shdescription is shared but not nailed in the rd_isnailed
>>> > > sense of the word, AFAICS.
>>> >
>>> > Well, right now the patch has pg_shseclabel, and its index, plus a
>>> > toast table and a toast index.  Not sure why we want/need the toast
>>> > table & index there, but the patch has 'em as of now.
>>> >
>>> As a common belief, TEXT is a variable length data type, so pg_shseclabel
>>> need to have its toast table. However, I don't expect the label field get
>>> represented as a reference to external pointer, because average length of
>>> security context is about 40-60 bytes much less than the threshold to
>>> launch toast_save_datum().
>>> Do I need to remove these toast table & index?
>>
>> We don't have toast tables for pg_database and so on, for example, which
>> means that datacl cannot go over a few hundred bytes long.  I think it
>> makes sense to not have toast tables for pg_shseclabel.  Keep in mind
>> that the label might be compressed before it's stored out of line, which
>> gives you quite a bit of actual space.  If a security context is over
>> 5000 bytes in length I think you're in trouble :-)
>>
> The attached patch removes toast table & index for pg_shseclabel.
>
> The current toasting.h defines toast table & index on pg_database,
> pg_shdescription and pg_db_role_setting only.
> The pg_authid and pg_tablespace don't have toast table & index
> in spite of variable-length field.
> So, it might not be a necessary stuff for all the shared relations.

I have committed this with fairly extensive revisions. The main
things I changed were:

- The pg_dump/pg_dumpall support was broken for databases and
needlessly inefficient for roles and tablespaces. (Parenthetically,
it is hard to blame anyone for screwing up the code here when it is
spaghetti code to begin with.)

- I did not commit the contrib/sepgsql parts of the patch, as I
haven't reviewed them. I think it would be helpful for you to
resubmit those separately.

- I didn't think it was a good idea to make pg_shseclabel.provider a
name while leaving pg_seclabel.provider as a text field. Surely these
two catalogs need to stay parallel. For the same reason, I didn't
like the idea of installing a syscache for pg_shseclabel while
pg_seclabel soldiers on without one. So for now I ripped out that
logic and instead made it work the old, slow way. I know we need a
better solution here, but I want to come up with a plan that handles
both catalogs symmetrically and then do it all at once, rather than
piecemeal.

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