Thread: SE-PostgreSQL/Lite Review

It's funny; we started out this CommitFest with me scrambling to find 
someone, anyone, willing to review the latest SE-PostgreSQL patch, 
knowing it was a big job and few were likely to volunteer.  Then 
schedules lined up just right, and last night I managed to get a great 
group of people all together to do perhaps the biggest single patch 
review ever, to work on just that.    I gathered up a list of the 
biggest concerns about this feature and its associated implementation, 
we got a number of regular PostgreSQL hackers and two of the security 
guys you've seen on this list all in the same room, and we talked about 
little but SEPostgreSQL for hours.  Minutes are at 
http://wiki.postgresql.org/wiki/SEPostgreSQL_Review_at_the_BWPUG and I'd 
suggest anyone interested in this feature (or in rejecting this feature) 
to take a look at what we covered.

There's comments there, with references for you [citation needed] types, 
to help answer four of the most common complaints I've seen on this list 
about this feature:

-Is there really a user base for it?
-Has it been audited by security professionals?
-Is there a useful SELinux policty to support it?
-Will this work with other security frameworks?

I feel pretty good now that these are not really our community's 
problems--these have had at least modest, and in some cases extensive, 
due diligence applied to them.  And we've confirmed there's access to 
expertise from the security community to help out with remaining 
concerns there, in person even if we plan it out right.  I personally 
suspect they've been discouraged from getting involved more by the slow 
pace this has been proceeding within our community and the general 
disarray around it, which would be understandable.

The parts I do believe we have reason to be concerned are with the code 
integration into the PostgreSQL core, and no one has any easy answers to 
things like "isn't this going to increase CERT advisories?" and "who's 
going to maintain this besides KaiGai"?  I personally feel that Steven 
Frost's recent comments here about how the PostgreSQL code makes this 
harder than it should be really cuts to the core of a next step here.  
The problem facing us isn't "is SEPostgreSQL the right solution for 
providing external security checks?"; it's "how can the PostgreSQL code 
be improved so that integrating external security is easier?"  Looking 
at SEPostgreSQL is great because it really highlights where the existing 
set of places are.  This general idea matches where thinking on things 
like row-level security was already going too--implement this for the 
database in general, then link SEPostgres in as just one provider of a 
security restriction.

I hope the review from the BWPUG helps everyone out, and that the 
suggestions on the wiki for the "Follow-up plan" are helpful.  As CF 
Manager, I feel we've given this patch its fair chunk of time this last 
month.  I don't really see any options except to mark it "returned with 
feedback" yet again for now, as this CF is nearing its close and there's 
still plenty of open issues.  My hope is that we've made progress toward 
answering some of the fundamental concerns that keep popping up around 
this patch for good, and that a plan with community members who will act 
on it (in this country for once) is coming together.  As always, we owe 
KaiGai a debt for offering his code contributions, sticking through an 
immense amount of criticism, and suggesting ways the rest of the 
database might become better overall through that interaction.  I do 
hope a committer is able to give his "Largeobject access controls" patch 
proper attention and commit it if feasible to do so.  It would be nice 
to see confirmed progress toward the larger goal of both feature and 
buzzword/checkbox complete PostgreSQL security is being made through his 
contributions.

At this point, I think someone comfortable with hacking into the 
PostgreSQL core will need to work on this project from that angle before 
even SE/PostgreSQL Lite is likely to be something we can commit.  Maybe 
we can get KaiGai thinking in those terms instead of how he's been 
approaching the problem.  Maybe Bruce or Steven can champion that work.  
I have to be honest and say that I'm not optimistic that this is 
possible or even a good idea to accomplish in the time remaining during 
this release.  A patch that touches the security model in fairly 
fundamental ways seems like it would be much better as an alpha-1 
candidate, while there's still plenty of time to shake out issues, than 
as a beta-1 or even alpha-3 one.  And I'm skeptical that this feature 
really fits the true use-cases for SEPostgres without row-level 
filtering anyway.  After our talk last night, it's obvious we need to 
figure out how to get that back before including the code does what 
people really want here.  But based on looking at the market for this 
sort of feature, providing this new form of security integrated into the 
database does seem like a worthwhile goal.  I wouldn't have spent this 
much time talking about it if I didn't believe that to be true.  On my 
side, in addition to helping coordinate everyone pushing in the same 
direction, I'll also continue trying to shake out some sponsorship 
funding for additional work out of the people in this country it would 
benefit.  It seems I'm going to keep getting pulled into the middle of 
this area regularly anyway.

-- 
Greg Smith    2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com  www.2ndQuadrant.com

Greg Smith wrote:
> It's funny; we started out this CommitFest with me scrambling to find 
> someone, anyone, willing to review the latest SE-PostgreSQL patch, 
> knowing it was a big job and few were likely to volunteer.  Then 
> schedules lined up just right, and last night I managed to get a great 
> group of people all together to do perhaps the biggest single patch 
> review ever, to work on just that.    I gathered up a list of the 
> biggest concerns about this feature and its associated implementation, 
> we got a number of regular PostgreSQL hackers and two of the security 
> guys you've seen on this list all in the same room, and we talked about 
> little but SEPostgreSQL for hours.  Minutes are at 
> http://wiki.postgresql.org/wiki/SEPostgreSQL_Review_at_the_BWPUG and I'd 
> suggest anyone interested in this feature (or in rejecting this feature) 
> to take a look at what we covered.

I repent that I cannot attend here.
The wikipage is a good summarize. Thanks for your efforts.


> There's comments there, with references for you [citation needed] types, 
> to help answer four of the most common complaints I've seen on this list 
> about this feature:
> 
> -Is there really a user base for it?
> -Has it been audited by security professionals?
> -Is there a useful SELinux policty to support it?
> -Will this work with other security frameworks?
> 
> I feel pretty good now that these are not really our community's 
> problems--these have had at least modest, and in some cases extensive, 
> due diligence applied to them.  And we've confirmed there's access to 
> expertise from the security community to help out with remaining 
> concerns there, in person even if we plan it out right.  I personally 
> suspect they've been discouraged from getting involved more by the slow 
> pace this has been proceeding within our community and the general 
> disarray around it, which would be understandable.

IMO, the slow pace is not a primary reason. In fact, SELinux was released
at 2000 Dec, but it gets integtated into the mainline kernel at 2003 Aug
with various kind of improvements. It takes about 3 years from the first
relase. IIRC, now we take 2.5 years from the first announce of SE-PgSQL
in this list, and various kind of improvements and cleanups had been done.
It is a bit long term, but not too long term.

The reason of this gap is that people have individual consciousness about
their security. I often represent it as "security is a subjective term".
Needless to say, we don't have magic-bullets for any threats. Any technology
has its metirs and limitations. However, people tend to say "it is nonsense",
if the feature does not match with their recognizable demands or threats.

Security-folks know MAC is not magic-bullets, while it is a significant
piece of system security. But some of complaints might be pointless for
security experts, so had been dismotivated.
From the perspective of security folks, we have to introduce it doggedly.
And, I'd like to understand database folks there are various kind of
security models and viewpoints here. SELinux is one of them.

> The parts I do believe we have reason to be concerned are with the code 
> integration into the PostgreSQL core, and no one has any easy answers to 
> things like "isn't this going to increase CERT advisories?" and "who's 
> going to maintain this besides KaiGai"?  I personally feel that Steven 
> Frost's recent comments here about how the PostgreSQL code makes this 
> harder than it should be really cuts to the core of a next step here.  
> The problem facing us isn't "is SEPostgreSQL the right solution for 
> providing external security checks?"; it's "how can the PostgreSQL code 
> be improved so that integrating external security is easier?"  Looking 
> at SEPostgreSQL is great because it really highlights where the existing 
> set of places are.  This general idea matches where thinking on things 
> like row-level security was already going too--implement this for the 
> database in general, then link SEPostgres in as just one provider of a 
> security restriction.

Right, it seems to me the "security provider" is a good phrase to represent
this feature. It just provides additional access control decisions based on
the different perspective of security model.

Please note that the access control granularity is not an essential issue.
We can also assume table-level mandatory access controls for instance.

The latest patch provides table/column level controls without row-level,
because the current PgSQL has facilities to know what tables and columns
are referenced reasonably, so SE-PgSQL also can know what tables/columns
are referenced without special tricks.

Please remind the earlier SE-PgSQL in v8.2.x. It walked on the Query tree
to pick up what columns were accessed.

> I hope the review from the BWPUG helps everyone out, and that the 
> suggestions on the wiki for the "Follow-up plan" are helpful.  As CF 
> Manager, I feel we've given this patch its fair chunk of time this last 
> month.  I don't really see any options except to mark it "returned with 
> feedback" yet again for now, as this CF is nearing its close and there's 
> still plenty of open issues.  My hope is that we've made progress toward 
> answering some of the fundamental concerns that keep popping up around 
> this patch for good, and that a plan with community members who will act 
> on it (in this country for once) is coming together.

I don't believe all works will be closed within 5 days.
Thanks for your and BWPUG's offer. I'll provide my maximum efforts to
become it commitable in the last commit fest.

> As always, we owe 
> KaiGai a debt for offering his code contributions, sticking through an 
> immense amount of criticism, and suggesting ways the rest of the 
> database might become better overall through that interaction.  I do 
> hope a committer is able to give his "Largeobject access controls" patch 
> proper attention and commit it if feasible to do so.  It would be nice 
> to see confirmed progress toward the larger goal of both feature and 
> buzzword/checkbox complete PostgreSQL security is being made through his 
> contributions.

It is not a one-sided contribution.
When we implement SE-PgSQL with full functionality, access controls on
large objects are absolutely necessary. I consider it is a groundwork.

> At this point, I think someone comfortable with hacking into the 
> PostgreSQL core will need to work on this project from that angle before 
> even SE/PostgreSQL Lite is likely to be something we can commit.  Maybe 
> we can get KaiGai thinking in those terms instead of how he's been 
> approaching the problem.  Maybe Bruce or Steven can champion that work.  
> I have to be honest and say that I'm not optimistic that this is 
> possible or even a good idea to accomplish in the time remaining during 
> this release.  A patch that touches the security model in fairly 
> fundamental ways seems like it would be much better as an alpha-1 
> candidate, while there's still plenty of time to shake out issues, than 
> as a beta-1 or even alpha-3 one.  And I'm skeptical that this feature 
> really fits the true use-cases for SEPostgres without row-level 
> filtering anyway.  After our talk last night, it's obvious we need to 
> figure out how to get that back before including the code does what 
> people really want here.  But based on looking at the market for this 
> sort of feature, providing this new form of security integrated into the 
> database does seem like a worthwhile goal.  I wouldn't have spent this 
> much time talking about it if I didn't believe that to be true.  On my 
> side, in addition to helping coordinate everyone pushing in the same 
> direction, I'll also continue trying to shake out some sponsorship 
> funding for additional work out of the people in this country it would 
> benefit.  It seems I'm going to keep getting pulled into the middle of 
> this area regularly anyway.
> 
One point. I'd like to introduce a use case without row-level granularity.

The page.24 in this slide: http://sepgsql.googlecode.com/files/JLS2009-KaiGai-LAPP_SELinux.pdf

shows SELinux performs as a logical wall between virtual domains in
web-services. Unlike physical database separation, it also allows to
share a part of files/tables from multiple virtual hosts, because of
its flexibility.

The MCS (multi catagory security) policy is suitable for such a system,
and it has been provided for more than four years, easy configurable.
It will well match with ISP use case in the wiki.

I'm also an author of mod_selinux module. I don't have actual number of
users, but I often asked questions to set up it.

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

* Greg Smith (greg@2ndquadrant.com) wrote:
> I personally feel that Steven
> Frost's recent comments here about how the PostgreSQL code makes this
> harder than it should be really cuts to the core of a next step here.
> The problem facing us isn't "is SEPostgreSQL the right solution for
> providing external security checks?"; it's "how can the PostgreSQL code
> be improved so that integrating external security is easier?"  Looking

Thanks for that support, Greg.  This was what I was principally trying
to do with KaiGai and the commitfest patch I reviewed of his last round.
Unfortunately, the committer comments I received on that patch didn't
help us outline a path forward, just declared that the approach in the
current patch wasn't workable.  My, now much more optimistic thanks to
our meeting, view is that the concept of abstracting the access controls
is solid and a necessary first step; but we need to find a better way to
implement it.

Also thanks to our discussion, I've got a much better handle on how
SELinux and the general secuirty community views PostgreSQL (and the
Linux kernel for that matter)- it's an application which consists of a
set of object managers.  That then leads into an approach to address at
least some of Tom's comments:

Let's start by taking the patch I reviewed and splitting up
security/access_control.c along object lines.  Of course, the individual
security/<object>_ac.c files would only include the .h's that are
necessary.  This would be a set of much smaller and much more
managable files which only know about what they should know about- the
object type they're responsible for.

Clearly, code comments also need to be reviewed and issues with them
addressed.  I'm also not a fan of the "skip permissions check"
arguments, which are there specifically to address cascade deletions,
which is a requirment of our PG security model.  I'd love to see a
better solution and am open to suggestions or thoughts about what one
would be.  I know one of KaiGai's concerns in this area was performance,
butas we often do for PG, perhaps we should consider that second and
first consider what it would look like if we ignored performance
concerns.  This would change "skip permissions check" to "what object is
being deleted, and am I a sub-object of that?".  I don't feel like I've
explained that well enough, perhaps someone else could come up with
another solution, or maybe figure out a better way to describe what I'm
trying to get with this.

Regarding the special-purpose shims- I feel there should be a way for us
to handle them better.  This might be a good use-case for column-level
privileges in pg_catalog.  That's pure speculation at the moment tho, I
havn't re-looked at those shims lately to see if that even makes sense.
I don't like them either though, for what it's worth.

Regarding contrib modules- I view them as I do custom code which the
user writes and loads through dlopen() into the backend process- there
should be a way for it to do security "right", but it ultimately has to
be the responsibility of the contrib module.  Admins can review what
contrib modules have been made SELinux/etc aware and choose to install
only those which they trust.

> Maybe Bruce or Steven can champion that work.

See above? ;)  I've had enough of a break from this and our discussion
has revitalized me.  I certainly welcome Bruce's comments, as well as
anyone else.

> I have to be honest and say that I'm not optimistic that this is
> possible or even a good idea to accomplish in the time remaining during
> this release.

While I agree with you, I wish you hadn't brought it up. :)  Mostly
because I feel like it may discourage people from wanting to spend time
on it due to desire to focus on things which are likely to make it into
the next release.  That being said, I don't feel that'll be an issue for
KaiGai or myself, but getting someone beyond us working on this would
really be great, especially yourself and/or Bruce.

> On my side, in addition to helping coordinate everyone pushing in the
> same direction, I'll also continue trying to shake out some sponsorship
> funding for additional work out of the people in this country it would
> benefit.  It seems I'm going to keep getting pulled into the middle of
> this area regularly anyway.

Thank you for that.  I'm planning to do the same and will certainly let
people know, to the extent possible, of anything I'm able to dig up.
Thanks!
    Stephen

Stephen Frost wrote:
> * Greg Smith (greg@2ndquadrant.com) wrote:
>   
>> I have to be honest and say that I'm not optimistic that this is  
>> possible or even a good idea to accomplish in the time remaining during  
>> this release.  
>>     
> While I agree with you, I wish you hadn't brought it up. :)  Mostly
> because I feel like it may discourage people from wanting to spend time
> on it due to desire to focus on things which are likely to make it into
> the next release.
The reason I mentioned it is that I was getting a general feeling that 
the community at large was going to view not getting the patch into 8.5 
as a final straw for working on it.  I wanted to be clear that the scope 
of doing this right may extend beyond that, but that shouldn't be a 
reason to give up on it.  I think KaiGai's comments about how it took 3 
years to get SELinux similarly integrated into the Linux core rather 
then shipping as an add-on is encouraging, in that the way and scope of 
how we're struggling with this topic is not something we should consider 
extraordinary and therefore a failing.  This is not an easy thing to do 
for anyone who tries it.

-- 
Greg Smith    2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com  www.2ndQuadrant.com

On Thu, Dec 10, 2009 at 10:39 PM, Stephen Frost <sfrost@snowman.net> wrote:
> Let's start by taking the patch I reviewed and splitting up
> security/access_control.c along object lines.  Of course, the individual
> security/<object>_ac.c files would only include the .h's that are
> necessary.  This would be a set of much smaller and much more
> managable files which only know about what they should know about- the
> object type they're responsible for.

Good idea.

> Clearly, code comments also need to be reviewed and issues with them
> addressed.  I'm also not a fan of the "skip permissions check"
> arguments, which are there specifically to address cascade deletions,
> which is a requirment of our PG security model.  I'd love to see a
> better solution and am open to suggestions or thoughts about what one
> would be.  I know one of KaiGai's concerns in this area was performance,
> butas we often do for PG, perhaps we should consider that second and
> first consider what it would look like if we ignored performance
> concerns.  This would change "skip permissions check" to "what object is
> being deleted, and am I a sub-object of that?".  I don't feel like I've
> explained that well enough, perhaps someone else could come up with
> another solution, or maybe figure out a better way to describe what I'm
> trying to get with this.

I don't know the right solution to this either but I'm glad you're
thinking about it.

> Regarding the special-purpose shims- I feel there should be a way for us
> to handle them better.  This might be a good use-case for column-level
> privileges in pg_catalog.  That's pure speculation at the moment tho, I
> havn't re-looked at those shims lately to see if that even makes sense.
> I don't like them either though, for what it's worth.

I am not sure if I fully understand either the problem or your
proposed solution, but it seems to me that permissions checking needs
to be forcibly crammed into a fixed number of buckets.  In other
words, for, say, databases, there should be a finite list of objects
that can be performed on databases: create, drop, connect, etc.  Any
permissions question that comes up has to be asked in terms of one of
those operations.  So if somebody writes code, in core or contrib,
that displays the size of the database, it has to key off the same
permissions as one of those pre-existing categories.  It doesn't get
to invent rules specific to that case from scratch.

(Humerous interlude: I worked on a project written in C++ many years
ago where there were these security context objects, and all
user-visible interfaces had to check with the security context before
allowing any privileged operation, using the may_i() method.  By
convention, variables of the security context type were always named
"mother", thus mother->may_i(whatever)...)

[...snip...]

One comment I have in general about this process is that I think it
would enormously reduce the level of pain associated with making these
kinds of changes if we could get patches that were not full of minor
issues that need to be cleaned up (like comments not properly
adjusted, spelling/grammar errors, formatting inconsistent with the
rest of the code base, poor English).  I can't speak for anyone else,
but it seems to me that asking a committer to fix all of that stuff on
top of substantive review of the patch is asking a lot.  I realize
that this is difficult for non-native speakers of English and a lot of
work even for those who are fluent in the language, but it is part of
our project culture and style to do these kinds of things right and I
certainly don't want to be the one who lowers our standards in this
area.  For a small patch, it's not so bad to have to fix these things
(though it's certainly nicer not to need to) but for a patch of five
or ten thousand lines, it's a LOT of work.

...Robert

Stephen Frost wrote:
> * Greg Smith (greg@2ndquadrant.com) wrote:
>> I personally feel that Steven  
>> Frost's recent comments here about how the PostgreSQL code makes this  
>> harder than it should be really cuts to the core of a next step here.   
>> The problem facing us isn't "is SEPostgreSQL the right solution for  
>> providing external security checks?"; it's "how can the PostgreSQL code  
>> be improved so that integrating external security is easier?"  Looking  
> 
> Thanks for that support, Greg.  This was what I was principally trying
> to do with KaiGai and the commitfest patch I reviewed of his last round.
> Unfortunately, the committer comments I received on that patch didn't
> help us outline a path forward, just declared that the approach in the
> current patch wasn't workable.  My, now much more optimistic thanks to
> our meeting, view is that the concept of abstracting the access controls
> is solid and a necessary first step; but we need to find a better way to
> implement it.

I agree. SELinux should be one of the user selectable options.
An abstraction layer for enhanced access controls are already adopted in
a few open source projects (Linux kernel, X-window), and gets successed.

> Also thanks to our discussion, I've got a much better handle on how
> SELinux and the general secuirty community views PostgreSQL (and the
> Linux kernel for that matter)- it's an application which consists of a
> set of object managers.  That then leads into an approach to address at
> least some of Tom's comments:
> 
> Let's start by taking the patch I reviewed and splitting up
> security/access_control.c along object lines.  Of course, the individual
> security/<object>_ac.c files would only include the .h's that are
> necessary.  This would be a set of much smaller and much more
> managable files which only know about what they should know about- the
> object type they're responsible for.

Basically, agreed. This file had more than 4KL in the last commit fest.

IMO, however, we should consider these issues before splitting up old patch.

(1) Whether the framework should host the default PG model, not only   enhanced security features, or not? This patch
triedto host both of the default PG model and SELinux. But, the default PG model does not have same origin with
label-basedmandatory access controls, such as SELinux, from a historical angle. So, this patch needed many of unobvious
changesto the core routines.
 

(2) Whether the framework should be comprehensive, or not? This patch tried to replace all the default PG checks in the
coreroutines from the begining. It required to review many of unobvious changes in the core routine. Because I've been
repeatedlypointed out to keep changeset smaller, I'm frightened for the direction. The coverage of the latest
SE-PgSQL/Litepatch is only databases, schemas, tables and columns. I think it is a good start to deploy security hooks
onthe routines related to these objects, rather than comprehensive support from the begining.
 

(3) In the future, whether we should allow multiple enhanced securities   at the same time, or not? It was not clear in
theprevious commit fest. But, in fact, Linux kernel does not support multiple security features in same time, because
itjust makes security label management complex. (It also have another reasons, but just now unlear.) I don't think we
haveany good reason to activate multiple enhanced securities at same time.
 

I think most of folks can agree with (2) and (3). But I expect opinion
is divided at (1).

For example, if we host the default PG checks to create a new database,
all the existing checks shall be moved as follows:
 http://code.google.com/p/sepgsql/source/browse/trunk/pgsec/src/backend/security/access_control.c?r=2273#1430

In addition, it has to return the security context to be assigned on
the new database, when it hosts SELinux.
Even if this rework is limited to four kind of object classes, I'm
worry about it overs an acceptable complexity.

If the framework hosts only "enhanced" securities, this hook shall be
implemented as:
 /*  * ac_database_create  * It checks permission to create a new database, and returns a security  * label to be
assignedon, or NULL if unnecessary.  *  * datName   : name of the new database  * srcDatOid : OID of the template
database * datLabel  : user given security label, or NULL  */ Value * ac_database_create(const char *datName, Oid
srcDatOid,Value *datLabel) { #ifdef HAVE_SELINUX     if (sepgsql_is_enabled())         return
sepgsql_database_create(datName,srcDatOid, datLabel); #endif     return NULL; }
 

In the caller (createdb()), this invocation shall be placed as follows:
     :
+  datLabel = ac_database_create(dbname, src_dboid, userDatLabel);     :
+  if (!datLabel)
+      new_record_nulls[Anum_pg_database_datselabel - 1] = true;
+  else
+      new_record[Anum_pg_database_datselabel - 1]
+          = CStringGetTextDatum(strVal(datLabel));     : simple_heap_insert(pg_database_rel, tuple);

It does not need unobvious logic changes in the default PG models, and
available for vaious kind of label-based security features. (Note that
the core routine handles the security label just as a text data without
special meanings.)


> Clearly, code comments also need to be reviewed and issues with them
> addressed.  I'm also not a fan of the "skip permissions check"
> arguments, which are there specifically to address cascade deletions,
> which is a requirment of our PG security model.  I'd love to see a
> better solution and am open to suggestions or thoughts about what one
> would be.  I know one of KaiGai's concerns in this area was performance,
> butas we often do for PG, perhaps we should consider that second and
> first consider what it would look like if we ignored performance
> concerns.  This would change "skip permissions check" to "what object is
> being deleted, and am I a sub-object of that?".  I don't feel like I've
> explained that well enough, perhaps someone else could come up with
> another solution, or maybe figure out a better way to describe what I'm
> trying to get with this.

Yep, IIRC, this argument is fixed into "cascade" to provide security
features a hint whether this deletion is due to the cascaded, or not.

If the caller really don't want permission checks, such as cleaning
up the temporary objects, the caller should skip invocation of the
framework, like "if (check_rights)" in DefineIndex().

> Regarding contrib modules- I view them as I do custom code which the
> user writes and loads through dlopen() into the backend process- there
> should be a way for it to do security "right", but it ultimately has to
> be the responsibility of the contrib module.  Admins can review what
> contrib modules have been made SELinux/etc aware and choose to install
> only those which they trust.

Yes, correct.
It is the job of administrator to decide what module should be loaded.

It is also a nature of the reference monitor model which widely includes
access control features, such as SELinux and the PG default checks.
Even if the module is not PG checks/SELinux aware, it is considered as
trusted.

>> I have to be honest and say that I'm not optimistic that this is  
>> possible or even a good idea to accomplish in the time remaining during  
>> this release.  
> 
> While I agree with you, I wish you hadn't brought it up. :)  Mostly
> because I feel like it may discourage people from wanting to spend time
> on it due to desire to focus on things which are likely to make it into
> the next release.  That being said, I don't feel that'll be an issue for
> KaiGai or myself, but getting someone beyond us working on this would
> really be great, especially yourself and/or Bruce.

Even if SE-PgSQL/Lite provides a limited feature in this release,
I believe it shall encourage security folks and enable to pull out their
contributions more than myself.
Hopefully, I'd like to merge a meaningful piece of SE-PgSQL. :-)

>> On my side, in addition to helping coordinate everyone pushing in the
>> same direction, I'll also continue trying to shake out some sponsorship  
>> funding for additional work out of the people in this country it would  
>> benefit.  It seems I'm going to keep getting pulled into the middle of  
>> this area regularly anyway.
> 
> Thank you for that.  I'm planning to do the same and will certainly let
> people know, to the extent possible, of anything I'm able to dig up.

Thanks so much!
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>

KaiGai,

* KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
> (1) Whether the framework should host the default PG model, not only
>     enhanced security features, or not?
>   This patch tried to host both of the default PG model and SELinux.
>   But, the default PG model does not have same origin with label-based
>   mandatory access controls, such as SELinux, from a historical angle.
>   So, this patch needed many of unobvious changes to the core routines.

I certainly understand what you're saying here.  However, I feel that
these changes to the core of PG are good, and are taking PG in the right
direction to have a much clearer and better implemented security
infrastructure.  The fact that you've found a number of odd cases
(duplicate checking, odd failure cases due to checks being done at
various parts of the process, etc) is a testement that this is bringing
the PG code forward even without the addition of SELinux support.

I realize that makes it more work for you and I don't envy you that.

> (2) Whether the framework should be comprehensive, or not?
>   This patch tried to replace all the default PG checks in the core
>   routines from the begining. It required to review many of unobvious
>   changes in the core routine. Because I've been repeatedly pointed out
>   to keep changeset smaller, I'm frightened for the direction.
>   The coverage of the latest SE-PgSQL/Lite patch is only databases,
>   schemas, tables and columns. I think it is a good start to deploy
>   security hooks on the routines related to these objects, rather than
>   comprehensive support from the begining.

I don't believe we will get support to commit a framework which is
intended to eventually support the PG model (following from my answer to
#1) in a partial form.  Perhaps it would be good to split the patch up
on a "per-object-type" basis, to make it simpler/easier to review (as
in, patch #1: database_ac.c + changes to core for it; patch #2:
table_ac.c + changes to core for it, etc; earlier patches assumed to be
already done in later patches), but the assumption will almost certainly
be that they will all be committed to wonderful CVS at the same time.

> (3) In the future, whether we should allow multiple enhanced securities
>     at the same time, or not?
>   It was not clear in the previous commit fest. But, in fact, Linux
>   kernel does not support multiple security features in same time,
>   because it just makes security label management complex. (It also
>   have another reasons, but just now unlear.)
>   I don't think we have any good reason to activate multiple enhanced
>   securities at same time.

To be honest, I feel that this will just fall out once we get through
doing #1.  I've just heard from Casey (author of the SMACK security
manager, an alternative to SELinux) that the PGACE framework (which is
what we suggested he look at) would be easily modified to support SMACK.
I feel the same would be true of what we're talking about in #1.  If you
disagree with that, please let me know.

> I think most of folks can agree with (2) and (3). But I expect opinion
> is divided at (1).
>
> For example, if we host the default PG checks to create a new database,
> all the existing checks shall be moved as follows:
>
>   http://code.google.com/p/sepgsql/source/browse/trunk/pgsec/src/backend/security/access_control.c?r=2273#1430
>
> In addition, it has to return the security context to be assigned on
> the new database, when it hosts SELinux.
> Even if this rework is limited to four kind of object classes, I'm
> worry about it overs an acceptable complexity.

I feel that what was at issue is that the complexity of
"access_control.c" was far too great- because *everything* was in that
one file.  Splitting access_control.c into smaller files would make them
more managable, and if we implement them all in a similar manner using a
similar policy for function names, arguments, etc, then once the first
object type is reviewed, the others will go more easily for the reviwer.

If the security framework does *not* support the SQL model, I think
we'll get push back from the community that it clearly couldn't be used
by any other security manager generically without alot of potential
changes and additional hooks that could end up bringing us all the way
to being capable of supporting the SQL model anyway.  I'm certainly
willing to hear core indicate otherwise though.

Thanks again, KaiGai.
Stephen

Robert Haas wrote:
> One comment I have in general about this process is that I think it
> would enormously reduce the level of pain associated with making these
> kinds of changes if we could get patches that were not full of minor
> issues that need to be cleaned up (like comments not properly
> adjusted, spelling/grammar errors, formatting inconsistent with the
> rest of the code base, poor English).
Sure, it's not fair to ask the committers to do that.  But it's not 
really something we can expect to get in all situations from our English 
as a second language contributors either--it's hard enough to get these 
details right even on stuff done in this country by native speakers.  At 
least we've got a big stack of comments that usually make sense if you 
read them to work with on the SE-PostgreSQL patches, which is much 
better than the alternative.

I think we need a two pronged attack on this issue.  Eventually I think 
someone who wants this feature in there will need to sponsor someone 
(and not even necessarily a coder) to do a sizable round of plain old 
wording cleanup on the comment text of the patch.  Robert is right to 
point out the committers are too expensive, in terms of what work for 
the community they could be doing besides that, to waste on that job.  
English editing help good enough to help a lot for this purpose isn't 
hard to find though.  As far as formatting, it drives me a little crazy 
that it's not more common practice for contributors to do their own 
targeted pgindent runs to at least nail some of the low-hanging fruit in 
that area, due the difficulty of working with the tool.  That's 
something else I think it would be nice to invest a little time in 
improving.  For example, there's no reason we should still have to kick 
back patches regularly just because of tab/space issues, and I know I 
run into one of those nearly every CommitFest.

-- 
Greg Smith    2ndQuadrant   Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com  www.2ndQuadrant.com

Stephen Frost wrote:
> KaiGai,
> 
> * KaiGai Kohei (kaigai@ak.jp.nec.com) wrote:
>> (1) Whether the framework should host the default PG model, not only
>>     enhanced security features, or not?
>>   This patch tried to host both of the default PG model and SELinux.
>>   But, the default PG model does not have same origin with label-based
>>   mandatory access controls, such as SELinux, from a historical angle.
>>   So, this patch needed many of unobvious changes to the core routines.
> 
> I certainly understand what you're saying here.  However, I feel that
> these changes to the core of PG are good, and are taking PG in the right
> direction to have a much clearer and better implemented security
> infrastructure.  The fact that you've found a number of odd cases
> (duplicate checking, odd failure cases due to checks being done at
> various parts of the process, etc) is a testement that this is bringing
> the PG code forward even without the addition of SELinux support.
> 
> I realize that makes it more work for you and I don't envy you that.

The reason why I prefer MAC only framework is not a technical reason.
As long as we can review it and acceptable, I have no reason to avoid it.

>> (2) Whether the framework should be comprehensive, or not?
>>   This patch tried to replace all the default PG checks in the core
>>   routines from the begining. It required to review many of unobvious
>>   changes in the core routine. Because I've been repeatedly pointed out
>>   to keep changeset smaller, I'm frightened for the direction.
>>   The coverage of the latest SE-PgSQL/Lite patch is only databases,
>>   schemas, tables and columns. I think it is a good start to deploy
>>   security hooks on the routines related to these objects, rather than
>>   comprehensive support from the begining.
> 
> I don't believe we will get support to commit a framework which is
> intended to eventually support the PG model (following from my answer to
> #1) in a partial form.  Perhaps it would be good to split the patch up
> on a "per-object-type" basis, to make it simpler/easier to review (as
> in, patch #1: database_ac.c + changes to core for it; patch #2:
> table_ac.c + changes to core for it, etc; earlier patches assumed to be
> already done in later patches), but the assumption will almost certainly
> be that they will all be committed to wonderful CVS at the same time.

As Rober Haas already suggested in another message, my patch in the last
commit fest is too large. It tried to rework anything in a single patch.
The "per-object-type" basis make sense for me.

In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
extendable, and "ace" feels like something cool. :-)
In X-window system, it calls its framework "XACE".

And I'd like to store these files in "backend/security/ace/*.c", because
"backend/security" will also store other security modules!

src/+ backend/   + security/      + ace/         ... access control framework      + sepgsql/     ... selinux specific
code     + smack/       ... (upcoming?) smack specific code      + solaristx/   ... (upcoming?) solaris-tx specific
code        :
 


>> (3) In the future, whether we should allow multiple enhanced securities
>>     at the same time, or not?
>>   It was not clear in the previous commit fest. But, in fact, Linux
>>   kernel does not support multiple security features in same time,
>>   because it just makes security label management complex. (It also
>>   have another reasons, but just now unlear.)
>>   I don't think we have any good reason to activate multiple enhanced
>>   securities at same time.
> 
> To be honest, I feel that this will just fall out once we get through
> doing #1.  I've just heard from Casey (author of the SMACK security
> manager, an alternative to SELinux) that the PGACE framework (which is
> what we suggested he look at) would be easily modified to support SMACK.
> I feel the same would be true of what we're talking about in #1.  If you
> disagree with that, please let me know.

Ahh, what I want to say is, whether we should allow "enhanced" securities.
If we host the default PG model on the framework, it is an exception of
the count.

The reason why I prefer the default PG check + one enhanced security at
most is simplification of the security label management.
If two label-based enhanced securities are enabled at same time,
we need two field on pg_class and others to store security context.
In addition, OS allows to choose one enhanced security at most eventually.

In my image, the hook should be as:
 Value * ac_database_create([arguments ...]) {     /*      * The default PG checks here.      * It is never disabled
 */
 
     /* "enhanced" security as follows */ #if defined(HAVE_SELINUX)     if (sepgsql_is_enabled())         return
sepgsql_database_create(...);#elif defined(HAVE_SMACK)     if (smack_is_enabled())         return
smack_database_create(...);#elif defined(HAVE_SOLARISTX)     if (soltx_is_enabled())         return
soltx_database_create(...);#endif     return NULL; }
 

We can share a common image image?

>> I think most of folks can agree with (2) and (3). But I expect opinion
>> is divided at (1).
>>
>> For example, if we host the default PG checks to create a new database,
>> all the existing checks shall be moved as follows:
>>
>>   http://code.google.com/p/sepgsql/source/browse/trunk/pgsec/src/backend/security/access_control.c?r=2273#1430
>>
>> In addition, it has to return the security context to be assigned on
>> the new database, when it hosts SELinux.
>> Even if this rework is limited to four kind of object classes, I'm
>> worry about it overs an acceptable complexity.
> 
> I feel that what was at issue is that the complexity of
> "access_control.c" was far too great- because *everything* was in that
> one file.  Splitting access_control.c into smaller files would make them
> more managable, and if we implement them all in a similar manner using a
> similar policy for function names, arguments, etc, then once the first
> object type is reviewed, the others will go more easily for the reviwer.

Yes, agreed.

> If the security framework does *not* support the SQL model, I think
> we'll get push back from the community that it clearly couldn't be used
> by any other security manager generically without alot of potential
> changes and additional hooks that could end up bringing us all the way
> to being capable of supporting the SQL model anyway.  I'm certainly
> willing to hear core indicate otherwise though.

OK, I seems to me it is reasonable reason to integrate DAC and MAC into
a common framework.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Greg Smith wrote:
> It's funny; we started out this CommitFest with me scrambling to find
> someone, anyone, willing to review the latest SE-PostgreSQL patch,
> knowing it was a big job and few were likely to volunteer. Then
> schedules lined up just right, and last night I managed to get a great
> group of people all together to do perhaps the biggest single patch
> review ever, to work on just that. I gathered up a list of the biggest
> concerns about this feature and its associated implementation, we got a
> number of regular PostgreSQL hackers and two of the security guys you've
> seen on this list all in the same room, and we talked about little but
> SEPostgreSQL for hours. Minutes are at
> http://wiki.postgresql.org/wiki/SEPostgreSQL_Review_at_the_BWPUG and I'd
> suggest anyone interested in this feature (or in rejecting this feature)
> to take a look at what we covered.
>

I just wanted to add some talking notes here.

User base for the feature:

While my goals for this feature line up with military/government users 
this is in no way the extent of the potential user base. The fact is 
most people won't know they want this feature until it is available. Why 
is that? Well, how many of you have written webapps and implemented 
policy logic in your application rather than the database level? Why do 
people currently feel the need to do this? Is it even possible to 
implement some complex policies (eg., PCI compliance) at the database 
level? If PostgreSQL version whatever suddenly had the ability to 
implement the policy logic in the database, would you move it there? I 
know I would..

Audit:

In past conversations it sounded like some of the Postgres community was 
skeptical even about the design of the security model. For an even 
earlier look (September 2006) of KaiGai and the SELinux community 
talking about the object model and even high level design of the 
solution see <http://marc.info/?l=selinux&m=115762285013528&w=2>

I've read through some of the prior patches, but haven't done an 
extensive audit, not only because of the size but because it became 
apparent relatively quickly that it was a waste of time and the 
community here wasn't going to accept it anyway. If this situation 
changes I think you'll find a few of us willing to donate time to the cause.


Policy:

The policy is easy once you have an object model that covers your use 
cases. You can see in the above discussion how we came to the object 
model we have now and why I've been comfortable with it since then.

An interesting aside, we must have hit the object model pretty well 
since another RDBMS (Trusted Rubix) uses the same one as SEPostgreSQL. 
See <http://rubix.com/downloads/documentation/RX_SELinux_Guide_6_0_R4.pdf>


Works outside of SELinux:

As Stephen already pointed out, Casey Schaufler (CC'd) who is the author 
of SMACK <http://schaufler-ca.com/> believes that the abstractions 
provided by PGACE will allow integration of his security system as well. 
SMACK is already in the Linux kernel as an alternative LSM to SELinux.

Further, not that I've seen the code or know how they did it, Trusted 
Rubix has support for multiple access control systems 
<http://rubix.com/cms/features> including Trusted Solaris, Solaris 
Trusted Extensions, SELinux and their own RBAC system.

--

With all that said, I've very interested in seeing this work move along. 
In its current shape it has limited utility in my world (although I know 
of at least 2 solutions I've seen that run 20 Postgres servers on a 
single system just to have database separation). The main thing that 
prevents it from being used in that situation today is superuser access 
(eg., we can't have superusers that can go around and muck in data he's 
not cleared for). But I recognize that this is a first step to a 
potentially great system and I definitely want to see it moving forward.

Joshua Brindle wrote:
> Greg Smith wrote:
>> It's funny; we started out this CommitFest with me scrambling to find
>> someone, anyone, willing to review the latest SE-PostgreSQL patch,
>> knowing it was a big job and few were likely to volunteer. Then
>> schedules lined up just right, and last night I managed to get a great
>> group of people all together to do perhaps the biggest single patch
>> review ever, to work on just that. I gathered up a list of the biggest
>> concerns about this feature and its associated implementation, we got a
>> number of regular PostgreSQL hackers and two of the security guys you've
>> seen on this list all in the same room, and we talked about little but
>> SEPostgreSQL for hours. Minutes are at
>> http://wiki.postgresql.org/wiki/SEPostgreSQL_Review_at_the_BWPUG and I'd
>> suggest anyone interested in this feature (or in rejecting this feature)
>> to take a look at what we covered.
>>
>
> I just wanted to add some talking notes here.
>
> User base for the feature:
>
> While my goals for this feature line up with military/government users
> this is in no way the extent of the potential user base. The fact is
> most people won't know they want this feature until it is available. Why
> is that? Well, how many of you have written webapps and implemented
> policy logic in your application rather than the database level? Why do
> people currently feel the need to do this? Is it even possible to
> implement some complex policies (eg., PCI compliance) at the database
> level? If PostgreSQL version whatever suddenly had the ability to
> implement the policy logic in the database, would you move it there? I
> know I would..
>
> Audit:
>
> In past conversations it sounded like some of the Postgres community was
> skeptical even about the design of the security model. For an even
> earlier look (September 2006) of KaiGai and the SELinux community
> talking about the object model and even high level design of the
> solution see <http://marc.info/?l=selinux&m=115762285013528&w=2>
>

I highly suggest a quick read of the above thread, it shows how we 
established an object model in fairly short order. The conversation also 
continues here: <http://marc.info/?l=selinux&m=115786457722767&w=2>

and also here:
<http://marc.info/?l=selinux&m=117160445604805&w=2>
<http://marc.info/?l=selinux&m=117160445611588&w=2>
<http://marc.info/?l=selinux&m=117160445608517&w=2>

KaiGai,

* KaiGai Kohei (kaigai@kaigai.gr.jp) wrote:
> As Rober Haas already suggested in another message, my patch in the last
> commit fest is too large. It tried to rework anything in a single patch.
> The "per-object-type" basis make sense for me.

Agreed.

> In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
> extendable, and "ace" feels like something cool. :-)

No complaints here..  I just hope this doesn't end up being *exactly*
the same as your original PGACE patches..  I'd feel terrible if we
weren't able to at least improve something with this extremely long and
drawn our process!

> And I'd like to store these files in "backend/security/ace/*.c", because
> "backend/security" will also store other security modules!

This is perfectly reasonable in my view.  No complaints here.

> src/
>  + backend/
>     + security/
>        + ace/         ... access control framework
>        + sepgsql/     ... selinux specific code
>        + smack/       ... (upcoming?) smack specific code
>        + solaristx/   ... (upcoming?) solaris-tx specific code

Looks good to me.  Perhaps we'll have a smack/ patch showing up very
shortly as well..

> The reason why I prefer the default PG check + one enhanced security at
> most is simplification of the security label management.
> If two label-based enhanced securities are enabled at same time,
> we need two field on pg_class and others to store security context.

Ah, yes, I see your point must more clearly now.  This sounds reasonable
to me.

> In addition, OS allows to choose one enhanced security at most eventually.
>
> In my image, the hook should be as:
>
>   Value *
>   ac_database_create([arguments ...])
>   {
>       /*
>        * The default PG checks here.
>        * It is never disabled
>        */
>
>       /* "enhanced" security as follows */
>   #if defined(HAVE_SELINUX)
>       if (sepgsql_is_enabled())
>           return sepgsql_database_create(...);
>   #elif defined(HAVE_SMACK)
>       if (smack_is_enabled())
>           return smack_database_create(...);
>   #elif defined(HAVE_SOLARISTX)
>       if (soltx_is_enabled())
>           return soltx_database_create(...);
>   #endif
>       return NULL;
>   }
>
> We can share a common image image?

If all of these security modules make sense as "only-more-restrictive",
then I have no problem with this approach.  Honestly, I'm fine with the
initial hooks looking as above in any case, since it provides a clear
way to deal with switching out the 'default PG checks' if someone
desires it- you could #if around that block as well.  As it's the
principle/primary, and I could see "only-more-restrictive" being more
popular, I don't mind having that code in-line in the hook.  The check
itself is still being brought out and into the security/ framework.

I do think that, technically, there's no reason we couldn't allow for
multiple "only-more-restrictive" models to be enabled and built in a
single binary for systems which support it.  As such, I would make those
just "#if defined()" rather than "#elif".  Let it be decided at runtime
which are actually used, otherwise it becomes a much bigger problem for
packagers too.
Thanks!
    Stephen

Greg,

* Greg Smith (greg@2ndquadrant.com) wrote:
> I think we need a two pronged attack on this issue.  Eventually I think
> someone who wants this feature in there will need to sponsor someone
> (and not even necessarily a coder) to do a sizable round of plain old
> wording cleanup on the comment text of the patch.

For the benefit of this list (this was already discussed some at the
BWPUG meeting), I agree with Greg on this and am actively looking to
try and help (either directly in my spare time or indirectly as a
sponsor).  If anyone else is interested or can do so as well, that would
be great!  Please speak up!
Thanks!
    Stephen

On Fri, 2009-12-11 at 11:36 -0500, Stephen Frost wrote:
[Snip...]
> 
> > In addition, OS allows to choose one enhanced security at most eventually.
> > 
> > In my image, the hook should be as:
> > 
> >   Value *
> >   ac_database_create([arguments ...])
> >   {
> >       /*
> >        * The default PG checks here.
> >        * It is never disabled
> >        */
> > 
> >       /* "enhanced" security as follows */
> >   #if defined(HAVE_SELINUX)
> >       if (sepgsql_is_enabled())
> >           return sepgsql_database_create(...);
> >   #elif defined(HAVE_SMACK)
> >       if (smack_is_enabled())
> >           return smack_database_create(...);
> >   #elif defined(HAVE_SOLARISTX)
> >       if (soltx_is_enabled())
> >           return soltx_database_create(...);
> >   #endif
> >       return NULL;
> >   }
> > 
> > We can share a common image image?
> 
> If all of these security modules make sense as "only-more-restrictive",
> then I have no problem with this approach.  Honestly, I'm fine with the
> initial hooks looking as above in any case, since it provides a clear
> way to deal with switching out the 'default PG checks' if someone
> desires it- you could #if around that block as well.  As it's the
> principle/primary, and I could see "only-more-restrictive" being more
> popular, I don't mind having that code in-line in the hook.  The check
> itself is still being brought out and into the security/ framework.
> 
> I do think that, technically, there's no reason we couldn't allow for
> multiple "only-more-restrictive" models to be enabled and built in a
> single binary for systems which support it.  As such, I would make those
> just "#if defined()" rather than "#elif".  Let it be decided at runtime
> which are actually used, otherwise it becomes a much bigger problem for
> packagers too.
> 

So when Eamon did the XACE work he allowed for stackable security
modules. We do not allow for this in the Linux kernel but we can still
figure out whether we want to support it for now. I'm not convinced that
the ifdef model is the right way to go. There should be a security
structure where pointers to the appropriate functions reside (like we
have in linux) so this way you have a register_module call which takes
the mac model and puts all the necessary function pointers into the
structure and the hook is just a call to security->create_db. If you
want to make it stackable you make security->create_db a list of
function pointers that get executed in turn. I'll ask Eamon when I see
him next how he handled deregistration of a particular model.

Dave

Stephen Frost wrote:
> KaiGai,
>
<snip>
> I do think that, technically, there's no reason we couldn't allow for
> multiple "only-more-restrictive" models to be enabled and built in a
> single binary for systems which support it.  As such, I would make those
> just "#if defined()" rather than "#elif".  Let it be decided at runtime
> which are actually used, otherwise it becomes a much bigger problem for
> packagers too.
>

It isn't just a case of using #if and it magically working. You'd need a 
system to manage multiple labels on each object that can be addressed by 
different systems. So instead of having an object mapped only to 
"system_u:object_r:mydb_t:s15" you'd also have to have it mapped to, 
eg., "^" for Smack.

Josh,

* Joshua Brindle (method@manicmethod.com) wrote:
> Stephen Frost wrote:
>> I do think that, technically, there's no reason we couldn't allow for
>> multiple "only-more-restrictive" models to be enabled and built in a
>> single binary for systems which support it.  As such, I would make those
>> just "#if defined()" rather than "#elif".  Let it be decided at runtime
>> which are actually used, otherwise it becomes a much bigger problem for
>> packagers too.
>
> It isn't just a case of using #if and it magically working. You'd need a
> system to manage multiple labels on each object that can be addressed by
> different systems. So instead of having an object mapped only to
> "system_u:object_r:mydb_t:s15" you'd also have to have it mapped to,
> eg., "^" for Smack.

I'm not sure I see that being a problem..  We're going to have
references in our object managers which make sense to us (eg: table
OIDs) and then a way of mapping those to some label (or possibly a set
of labels, as you describe).  We might want to reconsider the catalog
structure a bit if we want to support more than one at a time, but I
don't see it as a huge problem to support more than one label existing
for a given object.
    Thanks,
        Stephen

On Thursday 10 December 2009 21:47:18 KaiGai Kohei wrote:
> Greg Smith wrote:
> > It's funny; we started out this CommitFest with me scrambling to find
> > someone, anyone, willing to review the latest SE-PostgreSQL patch,
> > knowing it was a big job and few were likely to volunteer.  Then
> > schedules lined up just right, and last night I managed to get a great
> > group of people all together to do perhaps the biggest single patch
> > review ever, to work on just that.    I gathered up a list of the
> > biggest concerns about this feature and its associated implementation,
> > we got a number of regular PostgreSQL hackers and two of the security
> > guys you've seen on this list all in the same room, and we talked about
> > little but SEPostgreSQL for hours.  Minutes are at
> > http://wiki.postgresql.org/wiki/SEPostgreSQL_Review_at_the_BWPUG and I'd
> > suggest anyone interested in this feature (or in rejecting this feature)
> > to take a look at what we covered.
>
> I repent that I cannot attend here.
> The wikipage is a good summarize. Thanks for your efforts.
>
> > There's comments there, with references for you [citation needed] types,
> > to help answer four of the most common complaints I've seen on this list
> > about this feature:
> >
> > -Is there really a user base for it?
> > -Has it been audited by security professionals?
> > -Is there a useful SELinux policty to support it?
> > -Will this work with other security frameworks?
> >
> > I feel pretty good now that these are not really our community's
> > problems--these have had at least modest, and in some cases extensive,
> > due diligence applied to them.  And we've confirmed there's access to
> > expertise from the security community to help out with remaining
> > concerns there, in person even if we plan it out right.  I personally
> > suspect they've been discouraged from getting involved more by the slow
> > pace this has been proceeding within our community and the general
> > disarray around it, which would be understandable.
>
> IMO, the slow pace is not a primary reason. In fact, SELinux was released
> at 2000 Dec, but it gets integtated into the mainline kernel at 2003 Aug
> with various kind of improvements. It takes about 3 years from the first
> relase. IIRC, now we take 2.5 years from the first announce of SE-PgSQL
> in this list, and various kind of improvements and cleanups had been done.
> It is a bit long term, but not too long term.
>
> The reason of this gap is that people have individual consciousness about
> their security. I often represent it as "security is a subjective term".
> Needless to say, we don't have magic-bullets for any threats. Any
> technology has its metirs and limitations. However, people tend to say "it
> is nonsense", if the feature does not match with their recognizable demands
> or threats.
>
> Security-folks know MAC is not magic-bullets, while it is a significant
> piece of system security. But some of complaints might be pointless for
> security experts, so had been dismotivated.
> From the perspective of security folks, we have to introduce it doggedly.
> And, I'd like to understand database folks there are various kind of
> security models and viewpoints here. SELinux is one of them.
>
> > The parts I do believe we have reason to be concerned are with the code
> > integration into the PostgreSQL core, and no one has any easy answers to
> > things like "isn't this going to increase CERT advisories?" and "who's
> > going to maintain this besides KaiGai"?  I personally feel that Steven
> > Frost's recent comments here about how the PostgreSQL code makes this
> > harder than it should be really cuts to the core of a next step here.
> > The problem facing us isn't "is SEPostgreSQL the right solution for
> > providing external security checks?"; it's "how can the PostgreSQL code
> > be improved so that integrating external security is easier?"  Looking
> > at SEPostgreSQL is great because it really highlights where the existing
> > set of places are.  This general idea matches where thinking on things
> > like row-level security was already going too--implement this for the
> > database in general, then link SEPostgres in as just one provider of a
> > security restriction.
>
> Right, it seems to me the "security provider" is a good phrase to represent
> this feature. It just provides additional access control decisions based on
> the different perspective of security model.
>
> Please note that the access control granularity is not an essential issue.
> We can also assume table-level mandatory access controls for instance.
>
> The latest patch provides table/column level controls without row-level,
> because the current PgSQL has facilities to know what tables and columns
> are referenced reasonably, so SE-PgSQL also can know what tables/columns
> are referenced without special tricks.
>
> Please remind the earlier SE-PgSQL in v8.2.x. It walked on the Query tree
> to pick up what columns were accessed.
>
> > I hope the review from the BWPUG helps everyone out, and that the
> > suggestions on the wiki for the "Follow-up plan" are helpful.  As CF
> > Manager, I feel we've given this patch its fair chunk of time this last
> > month.  I don't really see any options except to mark it "returned with
> > feedback" yet again for now, as this CF is nearing its close and there's
> > still plenty of open issues.  My hope is that we've made progress toward
> > answering some of the fundamental concerns that keep popping up around
> > this patch for good, and that a plan with community members who will act
> > on it (in this country for once) is coming together.
>
> I don't believe all works will be closed within 5 days.
> Thanks for your and BWPUG's offer. I'll provide my maximum efforts to
> become it commitable in the last commit fest.
>
> > As always, we owe
> > KaiGai a debt for offering his code contributions, sticking through an
> > immense amount of criticism, and suggesting ways the rest of the
> > database might become better overall through that interaction.  I do
> > hope a committer is able to give his "Largeobject access controls" patch
> > proper attention and commit it if feasible to do so.  It would be nice
> > to see confirmed progress toward the larger goal of both feature and
> > buzzword/checkbox complete PostgreSQL security is being made through his
> > contributions.
>
> It is not a one-sided contribution.
> When we implement SE-PgSQL with full functionality, access controls on
> large objects are absolutely necessary. I consider it is a groundwork.
>
> > At this point, I think someone comfortable with hacking into the
> > PostgreSQL core will need to work on this project from that angle before
> > even SE/PostgreSQL Lite is likely to be something we can commit.  Maybe
> > we can get KaiGai thinking in those terms instead of how he's been
> > approaching the problem.  Maybe Bruce or Steven can champion that work.
> > I have to be honest and say that I'm not optimistic that this is
> > possible or even a good idea to accomplish in the time remaining during
> > this release.  A patch that touches the security model in fairly
> > fundamental ways seems like it would be much better as an alpha-1
> > candidate, while there's still plenty of time to shake out issues, than
> > as a beta-1 or even alpha-3 one.  And I'm skeptical that this feature
> > really fits the true use-cases for SEPostgres without row-level
> > filtering anyway.  After our talk last night, it's obvious we need to
> > figure out how to get that back before including the code does what
> > people really want here.  But based on looking at the market for this
> > sort of feature, providing this new form of security integrated into the
> > database does seem like a worthwhile goal.  I wouldn't have spent this
> > much time talking about it if I didn't believe that to be true.  On my
> > side, in addition to helping coordinate everyone pushing in the same
> > direction, I'll also continue trying to shake out some sponsorship
> > funding for additional work out of the people in this country it would
> > benefit.  It seems I'm going to keep getting pulled into the middle of
> > this area regularly anyway.
>
> One point. I'd like to introduce a use case without row-level granularity.
>
> The page.24 in this slide:
>   http://sepgsql.googlecode.com/files/JLS2009-KaiGai-LAPP_SELinux.pdf
>
> shows SELinux performs as a logical wall between virtual domains in
> web-services. Unlike physical database separation, it also allows to
> share a part of files/tables from multiple virtual hosts, because of
> its flexibility.
>

I got the impression that this is doable with current SEPostgres stuff, would 
be nice to see a little more detailed writeup on how to do it. Especially if 
it could be linked to the hosting providors page in the wiki. 

-- 
Robert Treat
Conjecture: http://www.xzilla.net
Consulting: http://www.omniti.com

Stephen Frost wrote:
>> In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
>> extendable, and "ace" feels like something cool. :-)
> 
> No complaints here..  I just hope this doesn't end up being *exactly*
> the same as your original PGACE patches..  I'd feel terrible if we
> weren't able to at least improve something with this extremely long and
> drawn our process!

Please forget old PGACE. We're talking about our future. :-)

>> And I'd like to store these files in "backend/security/ace/*.c", because
>> "backend/security" will also store other security modules!
> 
> This is perfectly reasonable in my view.  No complaints here.
> 
>> src/
>>  + backend/
>>     + security/
>>        + ace/         ... access control framework
>>        + sepgsql/     ... selinux specific code
>>        + smack/       ... (upcoming?) smack specific code
>>        + solaristx/   ... (upcoming?) solaris-tx specific code
> 
> Looks good to me.  Perhaps we'll have a smack/ patch showing up very
> shortly as well..

It's a welcome feature.

>> The reason why I prefer the default PG check + one enhanced security at
>> most is simplification of the security label management.
>> If two label-based enhanced securities are enabled at same time,
>> we need two field on pg_class and others to store security context.
> 
> Ah, yes, I see your point must more clearly now.  This sounds reasonable
> to me.
> 
>> In addition, OS allows to choose one enhanced security at most eventually.
>>
>> In my image, the hook should be as:
>>
>>   Value *
>>   ac_database_create([arguments ...])
>>   {
>>       /*
>>        * The default PG checks here.
>>        * It is never disabled
>>        */
>>
>>       /* "enhanced" security as follows */
>>   #if defined(HAVE_SELINUX)
>>       if (sepgsql_is_enabled())
>>           return sepgsql_database_create(...);
>>   #elif defined(HAVE_SMACK)
>>       if (smack_is_enabled())
>>           return smack_database_create(...);
>>   #elif defined(HAVE_SOLARISTX)
>>       if (soltx_is_enabled())
>>           return soltx_database_create(...);
>>   #endif
>>       return NULL;
>>   }
>>
>> We can share a common image image?
> 
> If all of these security modules make sense as "only-more-restrictive",
> then I have no problem with this approach.  Honestly, I'm fine with the
> initial hooks looking as above in any case, since it provides a clear
> way to deal with switching out the 'default PG checks' if someone
> desires it- you could #if around that block as well.  As it's the
> principle/primary, and I could see "only-more-restrictive" being more
> popular, I don't mind having that code in-line in the hook.  The check
> itself is still being brought out and into the security/ framework.

Ahh, indeed, #elif does not allow a single binary to provide multiple
options. You are right.

I'd like to rewrite it as follows:
 Value * ac_database_create([arguments ...]) {     Value *retval = NULL;
     /*      * The default PG checks here.      * It is never disabled      */
     switch (ace_feature)     { #ifdef HAVE_SELINUX     case ACE_SELINUX_SUPPORT:         if (sepgsql_is_enabled())
       retval = sepgsql_database_create(...);         break; #endif #ifdef HAVE_SMACK     case ACE_SMACK_SUPPORT:
 if (smack_is_enabled())             retval = smack_database_create(...);         break; #endif #ifdef HAVE_SOLARISTX
 case ACE_SOLARISTX_SUPPORT:         if (soltx_is_enabled())             retval = soltx_database_create(...);
break;#endif     default:         break;     }
 
     return retval; }

> I do think that, technically, there's no reason we couldn't allow for
> multiple "only-more-restrictive" models to be enabled and built in a
> single binary for systems which support it.  As such, I would make those
> just "#if defined()" rather than "#elif".  Let it be decided at runtime
> which are actually used, otherwise it becomes a much bigger problem for
> packagers too.

The reason why I don't want to support multiple enhanced securities at
same time is complexity of security label management, not access control
model.

Unlike X-server, RDBMS have to manage the security label of database
object persistently on the disk. It naturally needs enhancement of data
structure, such as pg_class.relsecon field in the latest SE-PgSQL/Lite.

If the third enhanced security does not use security label, basically,
I think it is feasible. In fact, the default PG check is a security
provider without security label, and it can coexist with SELinux.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

Stephen Frost wrote:
> Josh,
> 
> * Joshua Brindle (method@manicmethod.com) wrote:
>> Stephen Frost wrote:
>>> I do think that, technically, there's no reason we couldn't allow for
>>> multiple "only-more-restrictive" models to be enabled and built in a
>>> single binary for systems which support it.  As such, I would make those
>>> just "#if defined()" rather than "#elif".  Let it be decided at runtime
>>> which are actually used, otherwise it becomes a much bigger problem for
>>> packagers too.
>> It isn't just a case of using #if and it magically working. You'd need a  
>> system to manage multiple labels on each object that can be addressed by  
>> different systems. So instead of having an object mapped only to  
>> "system_u:object_r:mydb_t:s15" you'd also have to have it mapped to,  
>> eg., "^" for Smack.
> 
> I'm not sure I see that being a problem..  We're going to have
> references in our object managers which make sense to us (eg: table
> OIDs) and then a way of mapping those to some label (or possibly a set
> of labels, as you describe).  We might want to reconsider the catalog
> structure a bit if we want to support more than one at a time, but I
> don't see it as a huge problem to support more than one label existing
> for a given object.

If we allow multiple security labels on a database object, we have to
expand the structure of system catalog whenever a new security feature
will come in. I think it against to the purpose of the framework.

Even if we store them external relations to reference the object by OID,
we have to provide multiple interface to import/export a security label
for each enhanced securities. For example, it requires much complex patch
to the pg_dump.

My preference is all the enhanced securities shares a common facility
to manage security label, a common statement support and a common
backup/restore support.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

I'd like to summary about the framework.

* Functionalities

The ACE framework hosts both of the default PG checks and upcoming
enhanced securities. We can build a binary with multiple enhanced
security features, but user can choose one from them at most due
to the security label management.

So, it basically has the following structure:
 Value * ac_database_create([arguments ...]) {     Value *retval = NULL;
     /*      * The default PG checks here. It is never disabled.      */
     switch (ace_feature)     { #ifdef HAVE_SELINUX     case ACE_SELINUX_SUPPORT:         if (sepgsql_is_enabled())
       retval = sepgsql_database_create(...);         break; #endif #ifdef HAVE_SMACK     case ACE_SMACK_SUPPORT:
 if (smack_is_enabled())             retval = smack_database_create(...);         break; #endif     default:
break;    }     return retval; }
 


* Namespace

The framework is stored in the src/backend/security/ace.
As a general rule, Each source file has this naming convension: src/backend/security/ace/<object_class>_ace.c

Uncategolized hooks (such as cascaded-deletion called from dependency
system) are stored in ace/sysobj_ace.c. It is an exception.

All the framework functions (except for static) have "ace_" prefix
to distinguish from any other internal routines.

The security providers (SELinux, upcoming SMACK, ...) specific files
are stored in a certain directory in src/backend/security/ace/.
SE-PgSQL shall use "sepgsql" directory.

* Documentation

I don't plan to provide SGML documentation about ACE itself, except for
internal section, because it is an internal infrastructure.
For developers, src/backend/security/ace/README will provide a brief
overview of the ACE framework, and every security hooks have source
code comments to introduce the specification of itself.

* Patches

As a general rule, a patch is organized for each object classes, except
for very tiny object classes.
e.g) pgsql-ace-02-database.patch pgsql-ace-03-schema.patch    :

I'll try to submit a set of patches related to the following object classes
and functionalities by the 12/16.
* pgsql-ace-01-common.patch  It contains common part of ACE and uncategolized ones, such as cascaded  deletion from
dependency,and so on.* pgsql-ace-02-database.patch  It contains security hooks related to pg_database.*
pgsql-ace-03-schema.patch It contains security hooks related to pg_namespace.* pgsql-ace-04-relation.patch  It contains
securityhooks related to pg_class and pg_attribtue* pgace-ace-05-label.patch  It contains security label support in SQL
statement.

I'd like to conclude arguable things earlier than whole of reworks.

Then, we shall be able to see the rest of patches within this year.

Thanks,

KaiGai Kohei wrote:
> Stephen Frost wrote:
>>> In my cosmetic preference, "ace_" is better than "ac_". The 'e' means
>>> extendable, and "ace" feels like something cool. :-)
>> No complaints here..  I just hope this doesn't end up being *exactly*
>> the same as your original PGACE patches..  I'd feel terrible if we
>> weren't able to at least improve something with this extremely long and
>> drawn our process!
> 
> Please forget old PGACE. We're talking about our future. :-)
> 
>>> And I'd like to store these files in "backend/security/ace/*.c", because
>>> "backend/security" will also store other security modules!
>> This is perfectly reasonable in my view.  No complaints here.
>>
>>> src/
>>>  + backend/
>>>     + security/
>>>        + ace/         ... access control framework
>>>        + sepgsql/     ... selinux specific code
>>>        + smack/       ... (upcoming?) smack specific code
>>>        + solaristx/   ... (upcoming?) solaris-tx specific code
>> Looks good to me.  Perhaps we'll have a smack/ patch showing up very
>> shortly as well..
> 
> It's a welcome feature.
> 
>>> The reason why I prefer the default PG check + one enhanced security at
>>> most is simplification of the security label management.
>>> If two label-based enhanced securities are enabled at same time,
>>> we need two field on pg_class and others to store security context.
>> Ah, yes, I see your point must more clearly now.  This sounds reasonable
>> to me.
>>
>>> In addition, OS allows to choose one enhanced security at most eventually.
>>>
>>> In my image, the hook should be as:
>>>
>>>   Value *
>>>   ac_database_create([arguments ...])
>>>   {
>>>       /*
>>>        * The default PG checks here.
>>>        * It is never disabled
>>>        */
>>>
>>>       /* "enhanced" security as follows */
>>>   #if defined(HAVE_SELINUX)
>>>       if (sepgsql_is_enabled())
>>>           return sepgsql_database_create(...);
>>>   #elif defined(HAVE_SMACK)
>>>       if (smack_is_enabled())
>>>           return smack_database_create(...);
>>>   #elif defined(HAVE_SOLARISTX)
>>>       if (soltx_is_enabled())
>>>           return soltx_database_create(...);
>>>   #endif
>>>       return NULL;
>>>   }
>>>
>>> We can share a common image image?
>> If all of these security modules make sense as "only-more-restrictive",
>> then I have no problem with this approach.  Honestly, I'm fine with the
>> initial hooks looking as above in any case, since it provides a clear
>> way to deal with switching out the 'default PG checks' if someone
>> desires it- you could #if around that block as well.  As it's the
>> principle/primary, and I could see "only-more-restrictive" being more
>> popular, I don't mind having that code in-line in the hook.  The check
>> itself is still being brought out and into the security/ framework.
> 
> Ahh, indeed, #elif does not allow a single binary to provide multiple
> options. You are right.
> 
> I'd like to rewrite it as follows:
> 
>   Value *
>   ac_database_create([arguments ...])
>   {
>       Value *retval = NULL;
> 
>       /*
>        * The default PG checks here.
>        * It is never disabled
>        */
> 
>       switch (ace_feature)
>       {
>   #ifdef HAVE_SELINUX
>       case ACE_SELINUX_SUPPORT:
>           if (sepgsql_is_enabled())
>               retval = sepgsql_database_create(...);
>           break;
>   #endif
>   #ifdef HAVE_SMACK
>       case ACE_SMACK_SUPPORT:
>           if (smack_is_enabled())
>               retval = smack_database_create(...);
>           break;
>   #endif
>   #ifdef HAVE_SOLARISTX
>       case ACE_SOLARISTX_SUPPORT:
>           if (soltx_is_enabled())
>               retval = soltx_database_create(...);
>           break;
>   #endif
>       default:
>           break;
>       }
> 
>       return retval;
>   }
> 
>> I do think that, technically, there's no reason we couldn't allow for
>> multiple "only-more-restrictive" models to be enabled and built in a
>> single binary for systems which support it.  As such, I would make those
>> just "#if defined()" rather than "#elif".  Let it be decided at runtime
>> which are actually used, otherwise it becomes a much bigger problem for
>> packagers too.
> 
> The reason why I don't want to support multiple enhanced securities at
> same time is complexity of security label management, not access control
> model.
> 
> Unlike X-server, RDBMS have to manage the security label of database
> object persistently on the disk. It naturally needs enhancement of data
> structure, such as pg_class.relsecon field in the latest SE-PgSQL/Lite.
> 
> If the third enhanced security does not use security label, basically,
> I think it is feasible. In fact, the default PG check is a security
> provider without security label, and it can coexist with SELinux.
> 
> Thanks,


-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

KaiGai Kohei escribio':
> Stephen Frost wrote:
>   
>> Josh,
>>
>> * Joshua Brindle (method@manicmethod.com) wrote:
>>     
>>> Stephen Frost wrote:
>>>       
>>>> I do think that, technically, there's no reason we couldn't allow for
>>>> multiple "only-more-restrictive" models to be enabled and built in a
>>>> single binary for systems which support it.  As such, I would make those
>>>> just "#if defined()" rather than "#elif".  Let it be decided at runtime
>>>> which are actually used, otherwise it becomes a much bigger problem for
>>>> packagers too.
>>>>         
>>> It isn't just a case of using #if and it magically working. You'd need a  
>>> system to manage multiple labels on each object that can be addressed by  
>>> different systems. So instead of having an object mapped only to  
>>> "system_u:object_r:mydb_t:s15" you'd also have to have it mapped to,  
>>> eg., "^" for Smack.
>>>       
>> I'm not sure I see that being a problem..  We're going to have
>> references in our object managers which make sense to us (eg: table
>> OIDs) and then a way of mapping those to some label (or possibly a set
>> of labels, as you describe).  We might want to reconsider the catalog
>> structure a bit if we want to support more than one at a time, but I
>> don't see it as a huge problem to support more than one label existing
>> for a given object.
>>     
>
> If we allow multiple security labels on a database object, we have to
> expand the structure of system catalog whenever a new security feature
> will come in. I think it against to the purpose of the framework.
>
> Even if we store them external relations to reference the object by OID,
> we have to provide multiple interface to import/export a security label
> for each enhanced securities. For example, it requires much complex patch
> to the pg_dump.
>
> My preference is all the enhanced securities shares a common facility
> to manage security label, a common statement support and a common
> backup/restore support.
>
> Thanks,
>   
I'm worried with the performance implications that have this patch on
the core of the system.
If you are aggregating more security layers, There is not a database
degradation? I don't know how you attack these issues.
Regards.



-- 
-------------------------------------
"TIP 4: No hagas 'kill -9' a postmaster"
Ing. Marcos Lui's Orti'z Valmaseda
PostgreSQL System DBA 
Centro de Tecnologi'as de Almacenamiento y Ana'lis de Datos (CENTALAD)
Universidad de las Ciencias Informa'ticas(http://www.uci.cu)

Linux User # 418229
http://www.postgresql-es.org
http://www.postgresql.org
http://www.planetpostgresql.org

2009/12/12 KaiGai Kohei <kaigai@kaigai.gr.jp>:
> I'd like to summary about the framework.

I am not necessarily in agreement with many of the points listed in
this email.

> * Functionalities
>
> The ACE framework hosts both of the default PG checks and upcoming
> enhanced securities. We can build a binary with multiple enhanced
> security features, but user can choose one from them at most due
> to the security label management.

Just yesterday, Stephen Frost and I were discussing whether to stick a
layer of interdirection into the proposed interface layer using
function pointers.  We agreed to defer that decision to a later date.
If, however, we eventually decide to do that, it will clearly mean
that multiple security frameworks are possible simultaneously and that
they can be implemented as loadable modules.  On the other hand, we
might decide that that design does NOT make sense, in which case we
might end up with something more like the code snippet you proposed
below, although I'm not sure you have the details right.

We have also not decided whether to support a single security label or
multiple security labels per object.  I tend to think that the case
for multiple labels is pretty thin, but on the other hand it's not
unmangeably difficult to do so.  It could be coded up using an array
representation, just as we now do for reloptions.  Again, I want to
defer this decision until later.  Right now, I want to focus on seeing
whether it is all possible for us to get community buy-in on just
centralizing the existing checks, without making any commitment to
what may come after that.

> So, it basically has the following structure:
>
>  Value *
>  ac_database_create([arguments ...])
>  {
>      Value *retval = NULL;
>
>      /*
>       * The default PG checks here. It is never disabled.
>       */
>
>      switch (ace_feature)
>      {
>  #ifdef HAVE_SELINUX
>      case ACE_SELINUX_SUPPORT:
>          if (sepgsql_is_enabled())
>              retval = sepgsql_database_create(...);
>          break;
>  #endif
>  #ifdef HAVE_SMACK
>      case ACE_SMACK_SUPPORT:
>          if (smack_is_enabled())
>              retval = smack_database_create(...);
>          break;
>  #endif
>      default:
>           break;
>      }
>      return retval;
>  }

Again, if we decided on a function-pointer interface, it would be up
to the loaded security module whether or not to still apply the
default PG checks.  We have made no decision on that one way or the
other.

> * Namespace
>
> The framework is stored in the src/backend/security/ace.
> As a general rule, Each source file has this naming convension:
>  src/backend/security/ace/<object_class>_ace.c

Maybe.  Generally I think if there's a common prefix it should go at
the beginning of the filename, not the end, but I don't want to get
into source code organization too much at this point.  It's too early
to make those decisions.

> Uncategolized hooks (such as cascaded-deletion called from dependency
> system) are stored in ace/sysobj_ace.c. It is an exception.

The uncategorized hooks are one of the big design decisions we need to
deal with.  Let's put our energy into thinking about how to make a
tight, clean interface there, rather than worrying about where we put
the code.

> All the framework functions (except for static) have "ace_" prefix
> to distinguish from any other internal routines.

I'm not convinced.  Probably there will be a prefix, but since I'm
envisioning the first phase of this as strictly code cleanup, it
doesn't seem right to give it a prefix that means access control
*extensions*.

> The security providers (SELinux, upcoming SMACK, ...) specific files
> are stored in a certain directory in src/backend/security/ace/.
> SE-PgSQL shall use "sepgsql" directory.

It's way too early to decide this, IMO.

> * Documentation
>
> I don't plan to provide SGML documentation about ACE itself, except for
> internal section, because it is an internal infrastructure.
> For developers, src/backend/security/ace/README will provide a brief
> overview of the ACE framework, and every security hooks have source
> code comments to introduce the specification of itself.

That's probably about right, but I don't want to prejudge that without
further discussion in the community.

> * Patches
>
> As a general rule, a patch is organized for each object classes, except
> for very tiny object classes.
> e.g)
>  pgsql-ace-02-database.patch
>  pgsql-ace-03-schema.patch
>     :
>
> I'll try to submit a set of patches related to the following object classes
> and functionalities by the 12/16.

Stephen Frost is working on a patch for just one object type.  I think
that is a better approach than writing code for everything and then
trying to review it all at once.  We need to try to get consensus on
the basic design before we write a lot of code.  I do agree however
that **if** we are able to get consensus on one object type, we should
probably design the whole thing as a stack of patches that apply on
top of each other, one per object type, for easier review.  That's not
an approach I usually favor, but in this case I think it makes sense.

...Robert

(2009/12/12 15:42), "Ing . Marcos Lui's Orti'z Valmaseda" wrote:
> KaiGai Kohei escribio':
>> Stephen Frost wrote:
>>
>>> Josh,
>>>
>>> * Joshua Brindle (method@manicmethod.com) wrote:
>>>
>>>> Stephen Frost wrote:
>>>>
>>>>> I do think that, technically, there's no reason we couldn't allow for
>>>>> multiple "only-more-restrictive" models to be enabled and built in a
>>>>> single binary for systems which support it.  As such, I would make those
>>>>> just "#if defined()" rather than "#elif".  Let it be decided at runtime
>>>>> which are actually used, otherwise it becomes a much bigger problem for
>>>>> packagers too.
>>>>>
>>>> It isn't just a case of using #if and it magically working. You'd need a
>>>> system to manage multiple labels on each object that can be addressed by
>>>> different systems. So instead of having an object mapped only to
>>>> "system_u:object_r:mydb_t:s15" you'd also have to have it mapped to,
>>>> eg., "^" for Smack.
>>>>
>>> I'm not sure I see that being a problem..  We're going to have
>>> references in our object managers which make sense to us (eg: table
>>> OIDs) and then a way of mapping those to some label (or possibly a set
>>> of labels, as you describe).  We might want to reconsider the catalog
>>> structure a bit if we want to support more than one at a time, but I
>>> don't see it as a huge problem to support more than one label existing
>>> for a given object.
>>>
>>
>> If we allow multiple security labels on a database object, we have to
>> expand the structure of system catalog whenever a new security feature
>> will come in. I think it against to the purpose of the framework.
>>
>> Even if we store them external relations to reference the object by OID,
>> we have to provide multiple interface to import/export a security label
>> for each enhanced securities. For example, it requires much complex patch
>> to the pg_dump.
>>
>> My preference is all the enhanced securities shares a common facility
>> to manage security label, a common statement support and a common
>> backup/restore support.
>>
>> Thanks,
>>
> I'm worried with the performance implications that have this patch on
> the core of the system.
> If you are aggregating more security layers, There is not a database
> degradation? I don't know how you attack these issues.
> Regards.

If you don't enable any enhanced security providers, the cost to make
access control decision is much the same with existing permission
checks, because it just applies same checks.

If you enable an enhanced security providers (such as SELinux), here
is a trade-off between additional restriction and maximum performance.
In my past measurement, SE-PostgreSQL with full functionalities had
2-4% of performance trade-off in pgbench test. (Note that it also
enables row-level checks in my local branch.)

See the page.16 of this slides: http://sepgsql.googlecode.com/files/JLS2009-KaiGai-LAPP_SELinux.pdf

We assume the user of SE-PgSQL gives security the first priority, so
it will be enough acceptable penalty.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

(2009/12/12 21:51), Robert Haas wrote:
> 2009/12/12 KaiGai Kohei<kaigai@kaigai.gr.jp>:
>> I'd like to summary about the framework.
>
> I am not necessarily in agreement with many of the points listed in
> this email.
>
>> * Functionalities
>>
>> The ACE framework hosts both of the default PG checks and upcoming
>> enhanced securities. We can build a binary with multiple enhanced
>> security features, but user can choose one from them at most due
>> to the security label management.
>
> Just yesterday, Stephen Frost and I were discussing whether to stick a
> layer of interdirection into the proposed interface layer using
> function pointers.  We agreed to defer that decision to a later date.
> If, however, we eventually decide to do that, it will clearly mean
> that multiple security frameworks are possible simultaneously and that
> they can be implemented as loadable modules.  On the other hand, we
> might decide that that design does NOT make sense, in which case we
> might end up with something more like the code snippet you proposed
> below, although I'm not sure you have the details right.

I don't intend to include #ifdef ... #endif block in the patches which
I'll submit in the next week. OK, it can be decided later.

My opinion is enhanced security provider should be inlined.

> We have also not decided whether to support a single security label or
> multiple security labels per object.  I tend to think that the case
> for multiple labels is pretty thin, but on the other hand it's not
> unmangeably difficult to do so.  It could be coded up using an array
> representation, just as we now do for reloptions.  Again, I want to
> defer this decision until later.  Right now, I want to focus on seeing
> whether it is all possible for us to get community buy-in on just
> centralizing the existing checks, without making any commitment to
> what may come after that.

OK, it to be later.

>> * Namespace
>>
>> The framework is stored in the src/backend/security/ace.
>> As a general rule, Each source file has this naming convension:
>>   src/backend/security/ace/<object_class>_ace.c
>
> Maybe.  Generally I think if there's a common prefix it should go at
> the beginning of the filename, not the end, but I don't want to get
> into source code organization too much at this point.  It's too early
> to make those decisions.

But we have to deploy source code somewhere. :(
At the moment, "e" of "ace" might not necessary, but it is not productive
to review a patch to replace all the "ac_" by "ace_" later.
So, I'll use this name just as a name at the moment.

>> * Documentation
>>
>> I don't plan to provide SGML documentation about ACE itself, except for
>> internal section, because it is an internal infrastructure.
>> For developers, src/backend/security/ace/README will provide a brief
>> overview of the ACE framework, and every security hooks have source
>> code comments to introduce the specification of itself.
>
> That's probably about right, but I don't want to prejudge that without
> further discussion in the community.

If something are necessary, I'll also describe SGML documentation later.
At this moment, I'll include README and source code comments in the
first patches in the next week.

>> * Patches
>>
>> As a general rule, a patch is organized for each object classes, except
>> for very tiny object classes.
>> e.g)
>>   pgsql-ace-02-database.patch
>>   pgsql-ace-03-schema.patch
>>      :
>>
>> I'll try to submit a set of patches related to the following object classes
>> and functionalities by the 12/16.
>
> Stephen Frost is working on a patch for just one object type.  I think
> that is a better approach than writing code for everything and then
> trying to review it all at once.  We need to try to get consensus on
> the basic design before we write a lot of code.  I do agree however
> that **if** we are able to get consensus on one object type, we should
> probably design the whole thing as a stack of patches that apply on
> top of each other, one per object type, for easier review.  That's not
> an approach I usually favor, but in this case I think it makes sense.

What I wanted to say is that about a dozen of patches at once are not
happy for us, so I'd like to submit a limited number of patches earlier.
Are you saying that I should submit one-by-one and more rapidly?
I don't have any reason to oppose it.

Thanks,
-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>

(2009/12/12 6:27), Robert Treat wrote:
>> One point. I'd like to introduce a use case without row-level granularity.
>>
>> The page.24 in this slide:
>>    http://sepgsql.googlecode.com/files/JLS2009-KaiGai-LAPP_SELinux.pdf
>>
>> shows SELinux performs as a logical wall between virtual domains in
>> web-services. Unlike physical database separation, it also allows to
>> share a part of files/tables from multiple virtual hosts, because of
>> its flexibility.
>>
> 
> I got the impression that this is doable with current SEPostgres stuff, would
> be nice to see a little more detailed writeup on how to do it. Especially if
> it could be linked to the hosting providors page in the wiki.

Sorry, I missed to reply your message.

It needs to set up apache and selinux support module (mod_selinux.so)
correctly. This wiki article introduce the way to set up per virtualhost
separation using SELinux.

http://code.google.com/p/sepgsql/wiki/Apache_SELinux_plus?wl=en#Per_virtual-host_separation

Thanks,
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>