[sepgsql 2/3] Add db_schema:search permission checks

Lists: pgsql-hackers
From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-15 20:28:23
Message-ID: CADyhKSXatv4JHCVaPawXO7UXWRr-W-sn5r2a_GgLJDvt8j2jDA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

This patch adds sepgsql support for permission checks equivalent
to the existing SCHEMA USE privilege.

This feature is constructed on new OAT_SCHEMA_SEARCH event
type being invoked around pg_namespace_aclcheck().
So, its expected behavior also follows the behavior of existing
permissions; unprivileged schema is ignored from the search path,
or raise an error if object name is fully qualified.

This patch needs src/backend/catalog/objectaccess.c is existing,
so please apply this patch on top of this feature.
https://commitfest.postgresql.org/action/patch_view?id=1003

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

Attachment Content-Type Size
sepgsql-v9.3-schema-search-permission.v1.patch application/octet-stream 45.7 KB

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 11:22:15
Message-ID: CA+U5nM+VMUybz_fXMQ+fbcRZw4=a8FbDMw7=fWrOC+9BBPsVUg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 15 January 2013 20:28, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> This patch adds sepgsql support for permission checks equivalent
> to the existing SCHEMA USE privilege.
>
> This feature is constructed on new OAT_SCHEMA_SEARCH event
> type being invoked around pg_namespace_aclcheck().

Can you explain the exact detailed rationale behind this patch? Like
URLs or other info that explains *why* we are doing this, what
problems it causes if we don't, etc?

Otherwise there is no reference point for a review. Other patch types
like new features have syntax we can discuss and check, performance
patches have measurements we can check. With this, it is just "we add
some checks". No idea if that is all the places we need, or whether
there is a better way of doing this, or whether anyone cares if we do
this or not.

(Same comment for patch 3/3)

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 13:30:10
Message-ID: CADyhKSX5K14wiMa8mavbb8-NC9s-jQ=W_gUjBpEbOyAfqKdL4Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/1/29 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On 15 January 2013 20:28, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> This patch adds sepgsql support for permission checks equivalent
>> to the existing SCHEMA USE privilege.
>>
>> This feature is constructed on new OAT_SCHEMA_SEARCH event
>> type being invoked around pg_namespace_aclcheck().
>
> Can you explain the exact detailed rationale behind this patch? Like
> URLs or other info that explains *why* we are doing this, what
> problems it causes if we don't, etc?
>
Sorry for this inconvenient. The second and third sepgsql patch try to
add permission checks on the places being not covered by sepgsql,
even though we defined related permissions; search of db_schema
and execute of db_procedure.

It is unavailable to control user's access towards these objects from
perspective of selinux policy, without these patches, even though
existing DAC mechanism well controls.

Right now, sepgsql applies no checks when users tried to lookup
an object name underlying a particular schema. It is inconvenient
to set up an environment that enforces per-domain namespace
according to the selinux basis, because current sepgsql ignores
searching schema, thus, it means all the schema is allowed to
search from viewpoint of selinux.
What we want to do is almost same as existing permission checks
are doing when users tried to search a particular schema, except
for its criteria to make access control decision.

Also, sepgsql applies no checks when users tries to execute
functions, right now. It makes unavailable to control execution of
functions from viewpoint of selinux, and here is no way selinux
to prevent to execute functions defined by other domains, or
others being not permitted.
Also, what we want to do is almost same as existing permission
checks, except for its criteria to make access control decision.

SELinux model requires client domain has db_schema:{search}
permission when it tries to search its underlying objects, and
client domain has db_procedure:{execute} permission when
it tries to execute the function and db_procedure:{entrypoint}
additionally if this execution performs as entrypoint of trusted-
procedures.

These are the problems behind of these patches.
In short, I'd like to give sepgsql configuration more flexibility
as existing DAC doing. That helps use cases of typical SaaS
applications that shares a database with multiple clients but
to be separated each other.

It is the motivation why I'd like to add these permissions here.

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


From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 14:10:35
Message-ID: CA+U5nMKkDR=W0UusfP+wPhA-JHd2Sr+osr7DFc__kV+vyMZ3WQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 29 January 2013 13:30, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> It makes unavailable to control execution of
> functions from viewpoint of selinux, and here is no way selinux
> to prevent to execute functions defined by other domains, or
> others being not permitted.
> Also, what we want to do is almost same as existing permission
> checks, except for its criteria to make access control decision.

Do you have a roadmap of all the things this relates to?

If selinux has a viewpoint, I'd like to be able to see a list of
capabilities and then which ones are currently missing. I guess I'm
looking for external assurance that someone somewhere needs this and
that it fits into a complete overall plan of what we should do. Just
like we are able to use SQLStandard as a guide as to what we need to
implement, we would like something to refer back to. Does this have a
request id, specification document page number or whatever?

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 14:39:39
Message-ID: CADyhKSXBYwtcjF_njSLU2GN3dSw25fXR+Kqs5xT=GEb_pZQXtw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/1/29 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On 29 January 2013 13:30, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> It makes unavailable to control execution of
>> functions from viewpoint of selinux, and here is no way selinux
>> to prevent to execute functions defined by other domains, or
>> others being not permitted.
>> Also, what we want to do is almost same as existing permission
>> checks, except for its criteria to make access control decision.
>
> Do you have a roadmap of all the things this relates to?
>
> If selinux has a viewpoint, I'd like to be able to see a list of
> capabilities and then which ones are currently missing. I guess I'm
> looking for external assurance that someone somewhere needs this and
> that it fits into a complete overall plan of what we should do. Just
> like we are able to use SQLStandard as a guide as to what we need to
> implement, we would like something to refer back to. Does this have a
> request id, specification document page number or whatever?
>
I previously made several wiki pages for reference of permissions
to be checked, but it needs maintenance works towards the latest
state, such as newly added permissions.
http://wiki.postgresql.org/wiki/SEPostgreSQL_References

Even though selinuxproject.org hosts permission list, it is more
rough than what I described at wiki.postgresql.org.
http://www.selinuxproject.org/page/ObjectClassesPerms#Database_Object_Classes

Unlike SQL standard, we have less resource to document its spec
being validated by third persons. However, it is a reasonable solution
to write up which permission shall be checked on which timing.

Let me revise the above wikipage to show my overall plan.

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


From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 14:55:25
Message-ID: CA+U5nMLCii=Pq3J8RnU6A9JJiFZJwB6EziKkLq-CmWPMd1g0iQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 29 January 2013 14:39, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2013/1/29 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>> On 29 January 2013 13:30, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>
>>> It makes unavailable to control execution of
>>> functions from viewpoint of selinux, and here is no way selinux
>>> to prevent to execute functions defined by other domains, or
>>> others being not permitted.
>>> Also, what we want to do is almost same as existing permission
>>> checks, except for its criteria to make access control decision.
>>
>> Do you have a roadmap of all the things this relates to?
>>
>> If selinux has a viewpoint, I'd like to be able to see a list of
>> capabilities and then which ones are currently missing. I guess I'm
>> looking for external assurance that someone somewhere needs this and
>> that it fits into a complete overall plan of what we should do. Just
>> like we are able to use SQLStandard as a guide as to what we need to
>> implement, we would like something to refer back to. Does this have a
>> request id, specification document page number or whatever?
>>
> I previously made several wiki pages for reference of permissions
> to be checked, but it needs maintenance works towards the latest
> state, such as newly added permissions.
> http://wiki.postgresql.org/wiki/SEPostgreSQL_References
>
> Even though selinuxproject.org hosts permission list, it is more
> rough than what I described at wiki.postgresql.org.
> http://www.selinuxproject.org/page/ObjectClassesPerms#Database_Object_Classes
>
> Unlike SQL standard, we have less resource to document its spec
> being validated by third persons. However, it is a reasonable solution
> to write up which permission shall be checked on which timing.
>
> Let me revise the above wikipage to show my overall plan.

OK, that's looking like a good and useful set of info.

What we need to do is to give the SELinux API a spec/version number
(yes, the SELinux one) and then match what PostgreSQL implements
against that, so we can say we are moving towards spec compliance with
1.0 and we have a list of unimplemented features...

That puts this in a proper context, so we know what we are doing, why
we are doing it and also when we've finished it. And also, how to know
what future external changes will cause additional work.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Craig Ringer <craig(at)2ndQuadrant(dot)com>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-01-29 23:15:20
Message-ID: 51085808.1010107@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/29/2013 10:10 PM, Simon Riggs wrote:
> On 29 January 2013 13:30, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> It makes unavailable to control execution of
>> functions from viewpoint of selinux, and here is no way selinux
>> to prevent to execute functions defined by other domains, or
>> others being not permitted.
>> Also, what we want to do is almost same as existing permission
>> checks, except for its criteria to make access control decision.
> Do you have a roadmap of all the things this relates to?
>
> If selinux has a viewpoint, I'd like to be able to see a list of
> capabilities and then which ones are currently missing. I guess I'm
> looking for external assurance that someone somewhere needs this and
> that it fits into a complete overall plan of what we should do. Just
> like we are able to use SQLStandard as a guide as to what we need to
> implement, we would like something to refer back to. Does this have a
> request id, specification document page number or whatever?

I think that would greatly assist people in understanding why these
patches are neccessary, what real-world functionality they lead to, and
what problems they solve.

Some info on the wiki may be a good option.

For example, if you were to say "these changes will help with
multi-tenant PostgreSQL installations by <x>" then that will catch some
people's eye.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Simon Riggs <simon(at)2ndquadrant(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-02-13 15:27:35
Message-ID: CADyhKSVMOiGLvBwYe43qw+patcSasJGNPaU8G_FFF4MU50wDFQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Sorry for my late updates.

I tried to update list of permissions that sepgsql expects, even though
the description might be still a bit rough...
https://wiki.postgresql.org/wiki/SEPostgreSQL_Permissions

Set of permissions are defined for each object class that represents
a particular database object. This list summarize all the defined
permissions and introduction of the case when it shall be checked.

Right now, the list of permissions are based on the latest selinux
policy release at 20120725, but db_materialized_view class will
be (probably) added in the future release somewhere in 2013.
So, I added a short mention of this.

My 2/3 and 3.3 patch try to add support "search" permission of
db_schema class and "execute" permission of db_procedure class.
It tries to implement relevant checks, but not supported yet.

Does the permission list help to understand what does these
patch try to tackle?

Thanks,

2013/1/29 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On 29 January 2013 14:39, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> 2013/1/29 Simon Riggs <simon(at)2ndquadrant(dot)com>:
>>> On 29 January 2013 13:30, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>>
>>>> It makes unavailable to control execution of
>>>> functions from viewpoint of selinux, and here is no way selinux
>>>> to prevent to execute functions defined by other domains, or
>>>> others being not permitted.
>>>> Also, what we want to do is almost same as existing permission
>>>> checks, except for its criteria to make access control decision.
>>>
>>> Do you have a roadmap of all the things this relates to?
>>>
>>> If selinux has a viewpoint, I'd like to be able to see a list of
>>> capabilities and then which ones are currently missing. I guess I'm
>>> looking for external assurance that someone somewhere needs this and
>>> that it fits into a complete overall plan of what we should do. Just
>>> like we are able to use SQLStandard as a guide as to what we need to
>>> implement, we would like something to refer back to. Does this have a
>>> request id, specification document page number or whatever?
>>>
>> I previously made several wiki pages for reference of permissions
>> to be checked, but it needs maintenance works towards the latest
>> state, such as newly added permissions.
>> http://wiki.postgresql.org/wiki/SEPostgreSQL_References
>>
>> Even though selinuxproject.org hosts permission list, it is more
>> rough than what I described at wiki.postgresql.org.
>> http://www.selinuxproject.org/page/ObjectClassesPerms#Database_Object_Classes
>>
>> Unlike SQL standard, we have less resource to document its spec
>> being validated by third persons. However, it is a reasonable solution
>> to write up which permission shall be checked on which timing.
>>
>> Let me revise the above wikipage to show my overall plan.
>
> OK, that's looking like a good and useful set of info.
>
> What we need to do is to give the SELinux API a spec/version number
> (yes, the SELinux one) and then match what PostgreSQL implements
> against that, so we can say we are moving towards spec compliance with
> 1.0 and we have a list of unimplemented features...
>
> That puts this in a proper context, so we know what we are doing, why
> we are doing it and also when we've finished it. And also, how to know
> what future external changes will cause additional work.
>
> --
> Simon Riggs http://www.2ndQuadrant.com/
> PostgreSQL Development, 24x7 Support, Training & Services

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


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-01 19:41:14
Message-ID: CA+TgmoY0-TG2xp7FBAQBLATdkJWd+B995YtLV_2aFEJ1oa=mbw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Jan 15, 2013 at 3:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> This patch adds sepgsql support for permission checks equivalent
> to the existing SCHEMA USE privilege.
>
> This feature is constructed on new OAT_SCHEMA_SEARCH event
> type being invoked around pg_namespace_aclcheck().
> So, its expected behavior also follows the behavior of existing
> permissions; unprivileged schema is ignored from the search path,
> or raise an error if object name is fully qualified.
>
> This patch needs src/backend/catalog/objectaccess.c is existing,
> so please apply this patch on top of this feature.
> https://commitfest.postgresql.org/action/patch_view?id=1003

KaiGai,

Could you please rebase this patch?

Thanks,

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


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-02 18:22:56
Message-ID: CADyhKSVULwuQEe9Z1YH0U-SeTMye8K_WWi1NRV0xLOA_R7ATqA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/4/1 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Jan 15, 2013 at 3:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> This patch adds sepgsql support for permission checks equivalent
>> to the existing SCHEMA USE privilege.
>>
>> This feature is constructed on new OAT_SCHEMA_SEARCH event
>> type being invoked around pg_namespace_aclcheck().
>> So, its expected behavior also follows the behavior of existing
>> permissions; unprivileged schema is ignored from the search path,
>> or raise an error if object name is fully qualified.
>>
>> This patch needs src/backend/catalog/objectaccess.c is existing,
>> so please apply this patch on top of this feature.
>> https://commitfest.postgresql.org/action/patch_view?id=1003
>
> KaiGai,
>
> Could you please rebase this patch?
>
OK, please check the attached ones.

Both patches were rebased to the latest master branch, thus, once either
of them got committed, another one has to be rebased later.
Please also pay attention security policy module for regression test was
also adjusted for these features.

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

Attachment Content-Type Size
sepgsql-v9.3-schema-search-permission.v3.patch application/octet-stream 45.9 KB
sepgsql-v9.3-function-execute-permission.v3.patch application/octet-stream 23.3 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-03 16:43:14
Message-ID: CA+Tgmob2WttDsUY5TFC_aTmkYzERP099J91ihFwhoKhtHQx+jQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Apr 2, 2013 at 2:22 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> OK, please check the attached ones.

Thanks. I reviewed the schema-search patch and I think it looks
reasonable, but shouldn't we be calling the event OAT_NAMESPACE_SEARCH
rather than OAT_SCHEMA_SEARCH? And, similarly,
ObjectAccessNamespaceSearch and InvokeNamespaceSearchHook? I think
this terminology could be confusing to backend hackers who are used to
seeing the term "namespace" internally when referring to schemas.

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


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-04 12:26:13
Message-ID: CADyhKSVo5dyTSdPDRC8k5doh2rwSYBUPMU6wek017BS9J4ryGw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/4/3 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Tue, Apr 2, 2013 at 2:22 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> OK, please check the attached ones.
>
> Thanks. I reviewed the schema-search patch and I think it looks
> reasonable, but shouldn't we be calling the event OAT_NAMESPACE_SEARCH
> rather than OAT_SCHEMA_SEARCH? And, similarly,
> ObjectAccessNamespaceSearch and InvokeNamespaceSearchHook? I think
> this terminology could be confusing to backend hackers who are used to
> seeing the term "namespace" internally when referring to schemas.
>
OK, I follow the manner of the terminology as we usually call it.
The attached patch just replaced things you suggested.

I'll also rebase the function-exec permission stuff later.

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

Attachment Content-Type Size
sepgsql-v9.3-schema-search-permission.v4.patch application/octet-stream 45.2 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-05 13:20:12
Message-ID: CA+Tgmoay0Yd7oxFfJuNEg4DQu_NStLh0fL+Sx99HZ3mu60+ajQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Apr 4, 2013 at 8:26 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> OK, I follow the manner of the terminology as we usually call it.
> The attached patch just replaced things you suggested.

Thanks, I have committed this, after making some changes to the
comments and documentation. Please review the changes and let me know
if you see any mistakes.

BTW, if it were possible to set things up so that the test_sepgsql
script could validate the version of the sepgsql-regtest policy
installed, that would eliminate a certain category of errors. I
notice also that the regression tests themselves seem to fail if you
invoke the script as contrib/sepgsql/test_sepgsql rather than
./test_sepgsql, which suggests another possible pre-validation step.

Thanks,

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


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-08 16:28:29
Message-ID: CADyhKSXHNeyvGrNgo1BfmxA=soWxD=SC1nTTvpk8Bppc3Y_2Xw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/4/5 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Thu, Apr 4, 2013 at 8:26 AM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> OK, I follow the manner of the terminology as we usually call it.
>> The attached patch just replaced things you suggested.
>
> Thanks, I have committed this, after making some changes to the
> comments and documentation. Please review the changes and let me know
> if you see any mistakes.
>
Thanks. I could find two obvious wording stuffs here, please see smaller
one of the attached patches. I didn't fixup manner to use "XXX" in source
code comments.

Also, the attached function-execute-permission patch is a rebased
version. I rethought its event name should be OAT_FUNCTION_EXECUTE,
rather than OAT_FUNCTION_EXEC according to the manner without
abbreviation. Other portion is same as previous ones.

> BTW, if it were possible to set things up so that the test_sepgsql
> script could validate the version of the sepgsql-regtest policy
> installed, that would eliminate a certain category of errors. I
> notice also that the regression tests themselves seem to fail if you
> invoke the script as contrib/sepgsql/test_sepgsql rather than
> ./test_sepgsql, which suggests another possible pre-validation step.
>
Please see the test-script-fixup patch.
I added "cd `dirname $0`" on top of the script. It makes pg_regress to
avoid this troubles. Probably, pg_regress was unavailable to read
sql commands to run.

A problem regarding to validation of sepgsql-regtest policy module
is originated by semodule commands that takes root privilege to
list up installed policy modules. So, I avoided to use this command
in the test_sepgsql script.
However, I have an idea that does not raise script fail even if "sudo
semodule -l" returned an error, except for a case when it can run
correctly and the policy version is not expected one.
How about your opinion for this check?

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

Attachment Content-Type Size
pgsql-v9.3-obvious-wording-fixes.patch application/octet-stream 1.0 KB
sepgsql-v9.3-test-script-fixup.v1.patch application/octet-stream 2.4 KB
sepgsql-v9.3-function-execute-permission.v4.patch application/octet-stream 23.7 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-12 13:00:51
Message-ID: CA+Tgmobep=DSM904H+qKienWz5t+-grEz1ZQ1S+kDwBUx-K2vw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Apr 8, 2013 at 12:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> Thanks. I could find two obvious wording stuffs here, please see smaller
> one of the attached patches. I didn't fixup manner to use "XXX" in source
> code comments.

Committed.

> Also, the attached function-execute-permission patch is a rebased
> version. I rethought its event name should be OAT_FUNCTION_EXECUTE,
> rather than OAT_FUNCTION_EXEC according to the manner without
> abbreviation. Other portion is same as previous ones.

Great. This looks fine to me, committed. I also committed your
getObjectIdentity patch, which caused some regression test output
changes. I applied the necessary correction before committing, I
think, but please check.

>> BTW, if it were possible to set things up so that the test_sepgsql
>> script could validate the version of the sepgsql-regtest policy
>> installed, that would eliminate a certain category of errors. I
>> notice also that the regression tests themselves seem to fail if you
>> invoke the script as contrib/sepgsql/test_sepgsql rather than
>> ./test_sepgsql, which suggests another possible pre-validation step.
>>
> Please see the test-script-fixup patch.
> I added "cd `dirname $0`" on top of the script. It makes pg_regress to
> avoid this troubles. Probably, pg_regress was unavailable to read
> sql commands to run.
>
> A problem regarding to validation of sepgsql-regtest policy module
> is originated by semodule commands that takes root privilege to
> list up installed policy modules. So, I avoided to use this command
> in the test_sepgsql script.
> However, I have an idea that does not raise script fail even if "sudo
> semodule -l" returned an error, except for a case when it can run
> correctly and the policy version is not expected one.
> How about your opinion for this check?

Not sure that's too useful. And I don't like the idea of putting sudo
commands in a test harness script. That seems too much like the sort
of thing bad people do.

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


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-12 14:42:33
Message-ID: 20130412144232.GF30671@eldon.alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas escribió:
> On Mon, Apr 8, 2013 at 12:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:

> > Also, the attached function-execute-permission patch is a rebased
> > version. I rethought its event name should be OAT_FUNCTION_EXECUTE,
> > rather than OAT_FUNCTION_EXEC according to the manner without
> > abbreviation. Other portion is same as previous ones.
>
> Great. This looks fine to me, committed. I also committed your
> getObjectIdentity patch, which caused some regression test output
> changes. I applied the necessary correction before committing, I
> think, but please check.

I think the function-execute code path is still using
getObjectDescription rather than identity.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-12 14:45:19
Message-ID: CA+TgmoYP9Z26Ht7M6nkiuHUfZXaDVmFJsV3Vnsm1UR9Cedu=ug@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Apr 12, 2013 at 10:42 AM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Robert Haas escribió:
>> On Mon, Apr 8, 2013 at 12:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>
>> > Also, the attached function-execute-permission patch is a rebased
>> > version. I rethought its event name should be OAT_FUNCTION_EXECUTE,
>> > rather than OAT_FUNCTION_EXEC according to the manner without
>> > abbreviation. Other portion is same as previous ones.
>>
>> Great. This looks fine to me, committed. I also committed your
>> getObjectIdentity patch, which caused some regression test output
>> changes. I applied the necessary correction before committing, I
>> think, but please check.
>
> I think the function-execute code path is still using
> getObjectDescription rather than identity.

Yeah, I think so. KaiGai, can you provide a follow-on patch to fix that?

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


From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-12 18:44:47
Message-ID: CADyhKSWUXgGJhUXwixCz-u9COb6ig1Usud3R_THVrtVk8N8jOQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

2013/4/12 Robert Haas <robertmhaas(at)gmail(dot)com>:
> On Fri, Apr 12, 2013 at 10:42 AM, Alvaro Herrera
> <alvherre(at)2ndquadrant(dot)com> wrote:
>> Robert Haas escribió:
>>> On Mon, Apr 8, 2013 at 12:28 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>>
>>> > Also, the attached function-execute-permission patch is a rebased
>>> > version. I rethought its event name should be OAT_FUNCTION_EXECUTE,
>>> > rather than OAT_FUNCTION_EXEC according to the manner without
>>> > abbreviation. Other portion is same as previous ones.
>>>
>>> Great. This looks fine to me, committed. I also committed your
>>> getObjectIdentity patch, which caused some regression test output
>>> changes. I applied the necessary correction before committing, I
>>> think, but please check.
>>
>> I think the function-execute code path is still using
>> getObjectDescription rather than identity.
>
> Yeah, I think so. KaiGai, can you provide a follow-on patch to fix that?
>
Yes, of course. The attached one replaces the getObjectDescription in
sepgsql/proc.c, and relative changes in regression test.

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

Attachment Content-Type Size
sepgsql-v9.3-replace-get-object-description.v2.patch application/octet-stream 11.7 KB

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-12 18:48:43
Message-ID: CADyhKSUsvc6orap1SxmHphTPN00Hz-zNH4toqh6dkySgqL6mHw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

>> A problem regarding to validation of sepgsql-regtest policy module
>> is originated by semodule commands that takes root privilege to
>> list up installed policy modules. So, I avoided to use this command
>> in the test_sepgsql script.
>> However, I have an idea that does not raise script fail even if "sudo
>> semodule -l" returned an error, except for a case when it can run
>> correctly and the policy version is not expected one.
>> How about your opinion for this check?
>
> Not sure that's too useful. And I don't like the idea of putting sudo
> commands in a test harness script. That seems too much like the sort
> of thing bad people do.
>
OK, I also doubt whether my idea make sense.

The attached patch omitted the portion to check the version of
sepgsql-regtest, and add some notice in the document instead.
Also, it moves current directory to the contrib/sepgsql on top of
the script, to avoid the problem when we run test_sepgsql
on the directory except for contring/sepgsql.

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

Attachment Content-Type Size
sepgsql-v9.3-test-script-fixup.v2.patch application/octet-stream 1.7 KB

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [sepgsql 2/3] Add db_schema:search permission checks
Date: 2013-04-17 13:57:02
Message-ID: CA+Tgmob_dDihyK9pybbz72tDbC8Rj1TGA_dqxcjdYi9EyH+HEA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Apr 12, 2013 at 2:44 PM, Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> Yes, of course. The attached one replaces the getObjectDescription in
> sepgsql/proc.c, and relative changes in regression test.

Thanks. Committed. I also committed the first two hunks of your
cleanup patch but omitted the third one, which is not well-worded and
seems like overkill anyway.

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