Thread: security hook on authorization

I also plan to add a security hook on authorization time.
It shall allow external security providers to set up credential of
the authenticated clients.

Please note that it is not intended to control authentication process.
It is typically checked based on a pair of username and password.
What I want to discuss is things after success of this authentication
steps.

>From viewpoint of SE-PostgreSQL, it uses getpeercon(3) which obtains
a security label of the peer process, so it does not need to consider
database username. But we can easily assume other security mechanism
which assigns a certain label based on the authenticated database user
such as Oracle Label Security.

So, I think this hook should be also invoked on the code path of
SET SESSION AUTHORIZATION, not only database login time, although
SE-PostgreSQL ignores this case.

So, I think SetSessionUserId() is a candidate to put this hook which is
entirely called from both of the code path.
This routine is to assign credential of the default database privilege
mechanism, so it seems to me it is a good point where external security
provider also assigns its credential of the authenticated database user.

Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>

2010/8/19 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> I also plan to add a security hook on authorization time.
> It shall allow external security providers to set up credential of
> the authenticated clients.
>
> Please note that it is not intended to control authentication process.
> It is typically checked based on a pair of username and password.
> What I want to discuss is things after success of this authentication
> steps.
>
> From viewpoint of SE-PostgreSQL, it uses getpeercon(3) which obtains
> a security label of the peer process, so it does not need to consider
> database username. But we can easily assume other security mechanism
> which assigns a certain label based on the authenticated database user
> such as Oracle Label Security.
>
> So, I think this hook should be also invoked on the code path of
> SET SESSION AUTHORIZATION, not only database login time, although
> SE-PostgreSQL ignores this case.
>
> So, I think SetSessionUserId() is a candidate to put this hook which is
> entirely called from both of the code path.
> This routine is to assign credential of the default database privilege
> mechanism, so it seems to me it is a good point where external security
> provider also assigns its credential of the authenticated database user.

How is this different from what we rejected before?

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

(2010/08/20 11:45), Robert Haas wrote:
> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> I also plan to add a security hook on authorization time.
>> It shall allow external security providers to set up credential of
>> the authenticated clients.
>>
>> Please note that it is not intended to control authentication process.
>> It is typically checked based on a pair of username and password.
>> What I want to discuss is things after success of this authentication
>> steps.
>>
>>  From viewpoint of SE-PostgreSQL, it uses getpeercon(3) which obtains
>> a security label of the peer process, so it does not need to consider
>> database username. But we can easily assume other security mechanism
>> which assigns a certain label based on the authenticated database user
>> such as Oracle Label Security.
>>
>> So, I think this hook should be also invoked on the code path of
>> SET SESSION AUTHORIZATION, not only database login time, although
>> SE-PostgreSQL ignores this case.
>>
>> So, I think SetSessionUserId() is a candidate to put this hook which is
>> entirely called from both of the code path.
>> This routine is to assign credential of the default database privilege
>> mechanism, so it seems to me it is a good point where external security
>> provider also assigns its credential of the authenticated database user.
> 
> How is this different from what we rejected before?
> 
It made clear the purpose of this hook.

I also intended to use the previous hook for authorization purpose,
but it was deployed just after initialize_acl() without no argument.
It might be suitable for SE-PostgreSQL, because it does not depend on
authenticated database user, but might be too specific.

The new hook shall be invoked on two code paths (database login and
SET SESSION AUTHORIZATION). It allows upcoming security module which
may assign client's credential based on the database user to utilize
this hook also.

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

2010/8/19 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> (2010/08/20 11:45), Robert Haas wrote:
>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>> I also plan to add a security hook on authorization time.
>>> It shall allow external security providers to set up credential of
>>> the authenticated clients.
>>>
>>> Please note that it is not intended to control authentication process.
>>> It is typically checked based on a pair of username and password.
>>> What I want to discuss is things after success of this authentication
>>> steps.
>>>
>>>  From viewpoint of SE-PostgreSQL, it uses getpeercon(3) which obtains
>>> a security label of the peer process, so it does not need to consider
>>> database username. But we can easily assume other security mechanism
>>> which assigns a certain label based on the authenticated database user
>>> such as Oracle Label Security.
>>>
>>> So, I think this hook should be also invoked on the code path of
>>> SET SESSION AUTHORIZATION, not only database login time, although
>>> SE-PostgreSQL ignores this case.
>>>
>>> So, I think SetSessionUserId() is a candidate to put this hook which is
>>> entirely called from both of the code path.
>>> This routine is to assign credential of the default database privilege
>>> mechanism, so it seems to me it is a good point where external security
>>> provider also assigns its credential of the authenticated database user.
>>
>> How is this different from what we rejected before?
>>
> It made clear the purpose of this hook.
>
> I also intended to use the previous hook for authorization purpose,
> but it was deployed just after initialize_acl() without no argument.
> It might be suitable for SE-PostgreSQL, because it does not depend on
> authenticated database user, but might be too specific.
>
> The new hook shall be invoked on two code paths (database login and
> SET SESSION AUTHORIZATION). It allows upcoming security module which
> may assign client's credential based on the database user to utilize
> this hook also.

I think our standard criteria for the inclusion of hooks is that you
must demonstrate that the hook can be used to do something interesting
that couldn't be done without the hook.  So far I'm unconvinced.

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

(2010/08/20 23:34), Robert Haas wrote:
> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> (2010/08/20 11:45), Robert Haas wrote:
>>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>>> I also plan to add a security hook on authorization time.
>>>> It shall allow external security providers to set up credential of
>>>> the authenticated clients.
>>>>
>>>> Please note that it is not intended to control authentication process.
>>>> It is typically checked based on a pair of username and password.
>>>> What I want to discuss is things after success of this authentication
>>>> steps.
>>>>
>>>>   From viewpoint of SE-PostgreSQL, it uses getpeercon(3) which obtains
>>>> a security label of the peer process, so it does not need to consider
>>>> database username. But we can easily assume other security mechanism
>>>> which assigns a certain label based on the authenticated database user
>>>> such as Oracle Label Security.
>>>>
>>>> So, I think this hook should be also invoked on the code path of
>>>> SET SESSION AUTHORIZATION, not only database login time, although
>>>> SE-PostgreSQL ignores this case.
>>>>
>>>> So, I think SetSessionUserId() is a candidate to put this hook which is
>>>> entirely called from both of the code path.
>>>> This routine is to assign credential of the default database privilege
>>>> mechanism, so it seems to me it is a good point where external security
>>>> provider also assigns its credential of the authenticated database user.
>>>
>>> How is this different from what we rejected before?
>>>
>> It made clear the purpose of this hook.
>>
>> I also intended to use the previous hook for authorization purpose,
>> but it was deployed just after initialize_acl() without no argument.
>> It might be suitable for SE-PostgreSQL, because it does not depend on
>> authenticated database user, but might be too specific.
>>
>> The new hook shall be invoked on two code paths (database login and
>> SET SESSION AUTHORIZATION). It allows upcoming security module which
>> may assign client's credential based on the database user to utilize
>> this hook also.
>
> I think our standard criteria for the inclusion of hooks is that you
> must demonstrate that the hook can be used to do something interesting
> that couldn't be done without the hook.  So far I'm unconvinced.
>
We cannot handle an error of labeled networking (getpeercon(3)),
if we don't have any hook during client authorization stage.

If and when a connection came from a host but we don't accept the
delivered security label, or labeled networking is misconfigured,
getpeercon(3) returns NULL. In this case, server cannot identify
what label should be applied on the client, then, we should
disconnect this connection due to the error on database login,
not any access control decision.

In similar case, psm_selinux.so disconnect the connection when
it cannot identify what security label shall be assigned on the
session, due to some reasons such as misconfigurations.

Without any hooks at authorization stage (but it might be different
place from this patch, of course), we need to delay the error
handling by the time when SE-PostgreSQL module is invoked at first.
But it is already connection established and user sends a query.
It seems to me quite strange behavior.

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

On Aug 20, 2010, at 8:27 PM, KaiGai Kohei <kaigai@kaigai.gr.jp> wrote:
> (2010/08/20 23:34), Robert Haas wrote:
>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>> I think our standard criteria for the inclusion of hooks is that you
>> must demonstrate that the hook can be used to do something interesting
>> that couldn't be done without the hook.  So far I'm unconvinced.
>>
> We cannot handle an error of labeled networking (getpeercon(3)),
> if we don't have any hook during client authorization stage.
>
> If and when a connection came from a host but we don't accept the
> delivered security label, or labeled networking is misconfigured,
> getpeercon(3) returns NULL. In this case, server cannot identify
> what label should be applied on the client, then, we should
> disconnect this connection due to the error on database login,
> not any access control decision.
>
> In similar case, psm_selinux.so disconnect the connection when
> it cannot identify what security label shall be assigned on the
> session, due to some reasons such as misconfigurations.
>
> Without any hooks at authorization stage (but it might be different
> place from this patch, of course), we need to delay the error
> handling by the time when SE-PostgreSQL module is invoked at first.
> But it is already connection established and user sends a query.
> It seems to me quite strange behavior.

You mentioned that before.  I'm not totally sure I buy it, and I think there are other applications that might benefit
froma hook in this area.  We need to think about trying to do this in a way that is as general as possible.  So I'd
liketo see some analysis of other possible applications. 

...Robert

(2010/08/22 0:20), Robert Haas wrote:
> On Aug 20, 2010, at 8:27 PM, KaiGai Kohei<kaigai@kaigai.gr.jp>  wrote:
>> (2010/08/20 23:34), Robert Haas wrote:
>>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>> I think our standard criteria for the inclusion of hooks is that you
>>> must demonstrate that the hook can be used to do something interesting
>>> that couldn't be done without the hook.  So far I'm unconvinced.
>>>
>> We cannot handle an error of labeled networking (getpeercon(3)),
>> if we don't have any hook during client authorization stage.
>>
>> If and when a connection came from a host but we don't accept the
>> delivered security label, or labeled networking is misconfigured,
>> getpeercon(3) returns NULL. In this case, server cannot identify
>> what label should be applied on the client, then, we should
>> disconnect this connection due to the error on database login,
>> not any access control decision.
>>
>> In similar case, psm_selinux.so disconnect the connection when
>> it cannot identify what security label shall be assigned on the
>> session, due to some reasons such as misconfigurations.
>>
>> Without any hooks at authorization stage (but it might be different
>> place from this patch, of course), we need to delay the error
>> handling by the time when SE-PostgreSQL module is invoked at first.
>> But it is already connection established and user sends a query.
>> It seems to me quite strange behavior.
>
> You mentioned that before.  I'm not totally sure I buy it, and I think
>  there are other applications that might benefit from a hook in this area.
>  We need to think about trying to do this in a way that is as general as
>  possible.  So I'd like to see some analysis of other possible applications.
>
Yes, I also think this kind of authorization hook should benefit other
applications, not only label based mac features.

For example, something like 'last' command in operations system which
records username and login-time. Stephen mentioned pam_tally that locks
down certain accounts who failed authentication too much.
Perhaps, PAM modules in operating system give us some hints about other
possible applications.

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

(2010/08/22 21:38), KaiGai Kohei wrote:
> (2010/08/22 0:20), Robert Haas wrote:
>> On Aug 20, 2010, at 8:27 PM, KaiGai Kohei<kaigai@kaigai.gr.jp> wrote:
>>> (2010/08/20 23:34), Robert Haas wrote:
>>>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>>> I think our standard criteria for the inclusion of hooks is that you
>>>> must demonstrate that the hook can be used to do something interesting
>>>> that couldn't be done without the hook. So far I'm unconvinced.
>>>>
>>> We cannot handle an error of labeled networking (getpeercon(3)),
>>> if we don't have any hook during client authorization stage.
>>>
>>> If and when a connection came from a host but we don't accept the
>>> delivered security label, or labeled networking is misconfigured,
>>> getpeercon(3) returns NULL. In this case, server cannot identify
>>> what label should be applied on the client, then, we should
>>> disconnect this connection due to the error on database login,
>>> not any access control decision.
>>>
>>> In similar case, psm_selinux.so disconnect the connection when
>>> it cannot identify what security label shall be assigned on the
>>> session, due to some reasons such as misconfigurations.
>>>
>>> Without any hooks at authorization stage (but it might be different
>>> place from this patch, of course), we need to delay the error
>>> handling by the time when SE-PostgreSQL module is invoked at first.
>>> But it is already connection established and user sends a query.
>>> It seems to me quite strange behavior.
>>
>> You mentioned that before. I'm not totally sure I buy it, and I think
>> there are other applications that might benefit from a hook in this area.
>> We need to think about trying to do this in a way that is as general as
>> possible. So I'd like to see some analysis of other possible 
>> applications.
>>
> Yes, I also think this kind of authorization hook should benefit other
> applications, not only label based mac features.
> 
> For example, something like 'last' command in operations system which
> records username and login-time. Stephen mentioned pam_tally that locks
> down certain accounts who failed authentication too much.
> Perhaps, PAM modules in operating system give us some hints about other
> possible applications.
> 

I've checked some documentation files of pam modules in operating system
to think about other possible applications.

* pam_env.so
It allows to set/unset environment variables, perhaps, per users.
In PG, we may be able to assume a module which set/unset guc variables
depending on authenticated user?

* pam_faildelay.so
It enables to delay to disconnect when authentication was failed.
It prevents brute-force attack on passwords.

* pam_lastlog.so
It enables to display a line of information about the last login of
the user. In addition, the module maintains the /var/log/lastlog file.

* pam_selinux.so
It sets up the default security context for the next execed shell.
It is equivalent to set up a set of privileges of the authenticated
user.

* pam_tally.so
It maintains a count of attempted accesses, can reset count on success,
can deny access if too many attempts fail.


If and when we try to provide something similar features of them,
the pam_env.so, pam_lastlog.so and pam_selinux.so need to be called
on the code path of authentication succeeded only.
But the pam_faildelay.so needs to be called on authentication failed
path, and the pam_tally.so needs to be called on both paths because
it maintain a count of authentication failed and locks down certain
user accounts which failed too many.

In the current patch, I put the authorization hook on SetSessionUserId()
but it is only called when authentication succeeded path.

Here is only one place where we can put the authorization hook where
is called on both of authentication succeeded and failed.

The ClientAuthentication() has a big switch statement which branches
to each authentication methods, then status will be updated to
STATUS_OK or others.
How about the security hook just after the big switch statement but
before sending a response to the client, as follows?
---------------- void ClientAuthentication(Port *port) {     int     status = STATUS_ERROR;           :     /*      *
Nowproceed to do the actual authentication check      */     switch (port->hba->auth_method)     {         case
uaReject:          :     }
 

+     if (ClientAuthenticationHook)
+         status = (*ClientAuthenticationHook)(port, status);
     if (status == STATUS_OK)         sendAuthRequest(port, AUTH_REQ_OK);     else         auth_failed(port, status);
     /* Done with authentication, so we should turn off immediate interrupts */     ImmediateInterruptOK = false; }
----------------

Thanks,
-- 
KaiGai Kohei <kaigai@ak.jp.nec.com>

I tried to revise the patch. It allows plugins to get control next to
client authentication, but before returning the status to users.

This change enables plugins which should be invoked on authentication
failed to utilize this hook, not only assignment of session security
label.
At the same time, it disables to hook on SET SESSION AUTHORIZATION.
But it is a bit unclear whether we should hook here, or not.

(2010/08/24 14:19), KaiGai Kohei wrote:
> (2010/08/22 21:38), KaiGai Kohei wrote:
>> (2010/08/22 0:20), Robert Haas wrote:
>>> On Aug 20, 2010, at 8:27 PM, KaiGai Kohei<kaigai@kaigai.gr.jp>  wrote:
>>>> (2010/08/20 23:34), Robert Haas wrote:
>>>>> 2010/8/19 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>>>> I think our standard criteria for the inclusion of hooks is that you
>>>>> must demonstrate that the hook can be used to do something interesting
>>>>> that couldn't be done without the hook. So far I'm unconvinced.
>>>>>
>>>> We cannot handle an error of labeled networking (getpeercon(3)),
>>>> if we don't have any hook during client authorization stage.
>>>>
>>>> If and when a connection came from a host but we don't accept the
>>>> delivered security label, or labeled networking is misconfigured,
>>>> getpeercon(3) returns NULL. In this case, server cannot identify
>>>> what label should be applied on the client, then, we should
>>>> disconnect this connection due to the error on database login,
>>>> not any access control decision.
>>>>
>>>> In similar case, psm_selinux.so disconnect the connection when
>>>> it cannot identify what security label shall be assigned on the
>>>> session, due to some reasons such as misconfigurations.
>>>>
>>>> Without any hooks at authorization stage (but it might be different
>>>> place from this patch, of course), we need to delay the error
>>>> handling by the time when SE-PostgreSQL module is invoked at first.
>>>> But it is already connection established and user sends a query.
>>>> It seems to me quite strange behavior.
>>>
>>> You mentioned that before. I'm not totally sure I buy it, and I think
>>> there are other applications that might benefit from a hook in this area.
>>> We need to think about trying to do this in a way that is as general as
>>> possible. So I'd like to see some analysis of other possible
>>> applications.
>>>
>> Yes, I also think this kind of authorization hook should benefit other
>> applications, not only label based mac features.
>>
>> For example, something like 'last' command in operations system which
>> records username and login-time. Stephen mentioned pam_tally that locks
>> down certain accounts who failed authentication too much.
>> Perhaps, PAM modules in operating system give us some hints about other
>> possible applications.
>>
>
> I've checked some documentation files of pam modules in operating system
> to think about other possible applications.
>
> * pam_env.so
> It allows to set/unset environment variables, perhaps, per users.
> In PG, we may be able to assume a module which set/unset guc variables
> depending on authenticated user?
>
> * pam_faildelay.so
> It enables to delay to disconnect when authentication was failed.
> It prevents brute-force attack on passwords.
>
> * pam_lastlog.so
> It enables to display a line of information about the last login of
> the user. In addition, the module maintains the /var/log/lastlog file.
>
> * pam_selinux.so
> It sets up the default security context for the next execed shell.
> It is equivalent to set up a set of privileges of the authenticated
> user.
>
> * pam_tally.so
> It maintains a count of attempted accesses, can reset count on success,
> can deny access if too many attempts fail.
>
>
> If and when we try to provide something similar features of them,
> the pam_env.so, pam_lastlog.so and pam_selinux.so need to be called
> on the code path of authentication succeeded only.
> But the pam_faildelay.so needs to be called on authentication failed
> path, and the pam_tally.so needs to be called on both paths because
> it maintain a count of authentication failed and locks down certain
> user accounts which failed too many.
>
> In the current patch, I put the authorization hook on SetSessionUserId()
> but it is only called when authentication succeeded path.
>
> Here is only one place where we can put the authorization hook where
> is called on both of authentication succeeded and failed.
>
> The ClientAuthentication() has a big switch statement which branches
> to each authentication methods, then status will be updated to
> STATUS_OK or others.
> How about the security hook just after the big switch statement but
> before sending a response to the client, as follows?
> ----------------
>    void
>    ClientAuthentication(Port *port)
>    {
>        int     status = STATUS_ERROR;
>              :
>        /*
>         * Now proceed to do the actual authentication check
>         */
>        switch (port->hba->auth_method)
>        {
>            case uaReject:
>              :
>        }
>
> +     if (ClientAuthenticationHook)
> +         status = (*ClientAuthenticationHook)(port, status);
>
>        if (status == STATUS_OK)
>            sendAuthRequest(port, AUTH_REQ_OK);
>        else
>            auth_failed(port, status);
>
>        /* Done with authentication, so we should turn off immediate interrupts */
>        ImmediateInterruptOK = false;
>    }
> ----------------
>
> Thanks,


--
KaiGai Kohei <kaigai@ak.jp.nec.com>

2010/8/24 KaiGai Kohei <kaigai@ak.jp.nec.com>:
> I tried to revise the patch. It allows plugins to get control next to
> client authentication, but before returning the status to users.
>
> This change enables plugins which should be invoked on authentication
> failed to utilize this hook, not only assignment of session security
> label.
> At the same time, it disables to hook on SET SESSION AUTHORIZATION.
> But it is a bit unclear whether we should hook here, or not.

Stephen -

You've been listed as a reviewer for this in the CF app since 9/17 -
are you planning to review it?

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

On Wed, Oct 13, 2010 at 2:13 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 2010/8/24 KaiGai Kohei <kaigai@ak.jp.nec.com>:
>> I tried to revise the patch. It allows plugins to get control next to
>> client authentication, but before returning the status to users.
>>
>> This change enables plugins which should be invoked on authentication
>> failed to utilize this hook, not only assignment of session security
>> label.
>> At the same time, it disables to hook on SET SESSION AUTHORIZATION.
>> But it is a bit unclear whether we should hook here, or not.
>
> Stephen -
>
> You've been listed as a reviewer for this in the CF app since 9/17 -
> are you planning to review it?

I guess not.

I took a brief look at this tonight, and it seems to me that it still
fails the test I proposed nearly two months ago:

http://archives.postgresql.org/pgsql-hackers/2010-08/msg01458.php

KaiGai responded with:

> If and when a connection came from a host but we don't accept the
> delivered security label, or labeled networking is misconfigured,
> getpeercon(3) returns NULL. In this case, server cannot identify
> what label should be applied on the client, then, we should
> disconnect this connection due to the error on database login,
> not any access control decision.
>
> In similar case, psm_selinux.so disconnect the connection when
> it cannot identify what security label shall be assigned on the
> session, due to some reasons such as misconfigurations.
>
> Without any hooks at authorization stage (but it might be different
> place from this patch, of course), we need to delay the error
> handling by the time when SE-PostgreSQL module is invoked at first.
> But it is already connection established and user sends a query.
> It seems to me quite strange behavior.

I don't find this very convincing.  We are still several patches from
having a working SE-PostgreSQL module, and I think we should be
worried about getting off the ground before we worry about this sort
of fine-tuning.  I don't see reporting an SE-PostgreSQL error slightly
sooner is worth a separate hook, especially given that we're still
several patches from having even a toy SE-PostgreSQL implementation.
For example, we may find that some other hook that is more certainly
necessary can also serve the purpose intended for this one.

And later with:

> Yes, I also think this kind of authorization hook should benefit other
> applications, not only label based mac features.
>
> For example, something like 'last' command in operations system which
> records username and login-time. Stephen mentioned pam_tally that locks
> down certain accounts who failed authentication too much.
> Perhaps, PAM modules in operating system give us some hints about other
> possible applications.

This is closer to the mark, but mostly speculative, and not detailed
enough to determine whether the proposed hook is properly located.  It
seems rather early to me: this is before we've sent the authentication
packet to the client, so we couldn't, for example, log the success or
failure of the authentication; we don't know whether it will succeed
or fail.

I am going to mark this Returned with Feedback.  I would like to
request that any future submissions to add a hook in this area be
accompanied by a working sample contrib module that is not SE-Linux
specific.

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

Sorry for this late responding. I got a cold later half of the last week.

(2010/10/20 12:10), Robert Haas wrote:
> On Wed, Oct 13, 2010 at 2:13 AM, Robert Haas<robertmhaas@gmail.com>  wrote:
>> 2010/8/24 KaiGai Kohei<kaigai@ak.jp.nec.com>:
>>> I tried to revise the patch. It allows plugins to get control next to
>>> client authentication, but before returning the status to users.
>>>
>>> This change enables plugins which should be invoked on authentication
>>> failed to utilize this hook, not only assignment of session security
>>> label.
>>> At the same time, it disables to hook on SET SESSION AUTHORIZATION.
>>> But it is a bit unclear whether we should hook here, or not.
>>
>> Stephen -
>>
>> You've been listed as a reviewer for this in the CF app since 9/17 -
>> are you planning to review it?
>
> I guess not.
>
> I took a brief look at this tonight, and it seems to me that it still
> fails the test I proposed nearly two months ago:
>
> http://archives.postgresql.org/pgsql-hackers/2010-08/msg01458.php
>
> KaiGai responded with:
>
>> If and when a connection came from a host but we don't accept the
>> delivered security label, or labeled networking is misconfigured,
>> getpeercon(3) returns NULL. In this case, server cannot identify
>> what label should be applied on the client, then, we should
>> disconnect this connection due to the error on database login,
>> not any access control decision.
>>
>> In similar case, psm_selinux.so disconnect the connection when
>> it cannot identify what security label shall be assigned on the
>> session, due to some reasons such as misconfigurations.
>>
>> Without any hooks at authorization stage (but it might be different
>> place from this patch, of course), we need to delay the error
>> handling by the time when SE-PostgreSQL module is invoked at first.
>> But it is already connection established and user sends a query.
>> It seems to me quite strange behavior.
>
> I don't find this very convincing.  We are still several patches from
> having a working SE-PostgreSQL module, and I think we should be
> worried about getting off the ground before we worry about this sort
> of fine-tuning.  I don't see reporting an SE-PostgreSQL error slightly
> sooner is worth a separate hook, especially given that we're still
> several patches from having even a toy SE-PostgreSQL implementation.
> For example, we may find that some other hook that is more certainly
> necessary can also serve the purpose intended for this one.
>
At least, we need a feature to raise an error when the SE-PgSQL module
cannot retrieve security context of the peer process, because it is
similar to a connection string without username/password.

However, the post-authentication hook is not the only option for us.
What I want here is the core PG gives the SE-PgSQL module a chance to
call getpeercon(3) before accepting user's queries.

One possible candidate is CheckMyDatabase() that checks ACL_CONNECT
permission for the required database prior to execution of all the
queries.
Currently, we don't have any security hook around here.
But, if we have "InvokeSecurityHook()" here, it will be able to
kill two birds with one stone. The 1st bird is getpeercon(3), and
the 2nd bird is permission check on the selected database.

> And later with:
>
>> Yes, I also think this kind of authorization hook should benefit other
>> applications, not only label based mac features.
>>
>> For example, something like 'last' command in operations system which
>> records username and login-time. Stephen mentioned pam_tally that locks
>> down certain accounts who failed authentication too much.
>> Perhaps, PAM modules in operating system give us some hints about other
>> possible applications.
>
> This is closer to the mark, but mostly speculative, and not detailed
> enough to determine whether the proposed hook is properly located.  It
> seems rather early to me: this is before we've sent the authentication
> packet to the client, so we couldn't, for example, log the success or
> failure of the authentication; we don't know whether it will succeed
> or fail.
>
Hmm. But the auth_failed() raises a fatal error, so we need to put
a hook before the invocation to log a case of authentication failed.

| +   if (ClientAuthentication_hook)
| +       (*ClientAuthentication_hook)(port, status);
| +
|     if (status == STATUS_OK)
|         sendAuthRequest(port, AUTH_REQ_OK);
|     else
|         auth_failed(port, status);

Or, perhaps, we should modify this if-block to ensure the hook being
called after sendAuthRequest() but before auth_failed().

> I am going to mark this Returned with Feedback.  I would like to
> request that any future submissions to add a hook in this area be
> accompanied by a working sample contrib module that is not SE-Linux
> specific.
>
For example, if a contrib module provides a feature to sleep a few
seconds when authentication failed, it prevents brute-force attack.
Do you think it is a good example as an evidence of this module?

Anyway, I have any preference on these two ideas right now.
It seems to me the contrib module will be small enough, but well
works as proof of concept. On the other hand, eventually we will
put a hook on CheckMyDatabase(), if so, it is not a bad idea to
kill two birds in a stone (hook) now.

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

On Mon, Oct 25, 2010 at 8:51 AM, KaiGai Kohei <kaigai@kaigai.gr.jp> wrote:
> One possible candidate is CheckMyDatabase() that checks ACL_CONNECT
> permission for the required database prior to execution of all the
> queries.
> Currently, we don't have any security hook around here.
> But, if we have "InvokeSecurityHook()" here, it will be able to
> kill two birds with one stone. The 1st bird is getpeercon(3), and
> the 2nd bird is permission check on the selected database.

Yeah, that might be a possibility.

>> This is closer to the mark, but mostly speculative, and not detailed
>> enough to determine whether the proposed hook is properly located.  It
>> seems rather early to me: this is before we've sent the authentication
>> packet to the client, so we couldn't, for example, log the success or
>> failure of the authentication; we don't know whether it will succeed
>> or fail.
>>
> Hmm. But the auth_failed() raises a fatal error, so we need to put
> a hook before the invocation to log a case of authentication failed.
>
> | +   if (ClientAuthentication_hook)
> | +       (*ClientAuthentication_hook)(port, status);
> | +
> |     if (status == STATUS_OK)
> |         sendAuthRequest(port, AUTH_REQ_OK);
> |     else
> |         auth_failed(port, status);
>
> Or, perhaps, we should modify this if-block to ensure the hook being
> called after sendAuthRequest() but before auth_failed().

Oh.  You know, I am realizing that I misread this patch.  This hook is
actually after authentication has been done; it's merely before we've
told the client what happened.  So maybe this is committable as-is,
modulo some work on the comments.

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

On Mon, 25 Oct 2010 21:45:23 -0400
Robert Haas <robertmhaas@gmail.com> wrote:

> On Mon, Oct 25, 2010 at 8:51 AM, KaiGai Kohei <kaigai@kaigai.gr.jp> wrote:
> > One possible candidate is CheckMyDatabase() that checks ACL_CONNECT
> > permission for the required database prior to execution of all the
> > queries.
> > Currently, we don't have any security hook around here.
> > But, if we have "InvokeSecurityHook()" here, it will be able to
> > kill two birds with one stone. The 1st bird is getpeercon(3), and
> > the 2nd bird is permission check on the selected database.
> 
> Yeah, that might be a possibility.
> 
> >> This is closer to the mark, but mostly speculative, and not detailed
> >> enough to determine whether the proposed hook is properly located. ?It
> >> seems rather early to me: this is before we've sent the authentication
> >> packet to the client, so we couldn't, for example, log the success or
> >> failure of the authentication; we don't know whether it will succeed
> >> or fail.
> >>
> > Hmm. But the auth_failed() raises a fatal error, so we need to put
> > a hook before the invocation to log a case of authentication failed.
> >
> > | + ? if (ClientAuthentication_hook)
> > | + ? ? ? (*ClientAuthentication_hook)(port, status);
> > | +
> > | ? ? if (status == STATUS_OK)
> > | ? ? ? ? sendAuthRequest(port, AUTH_REQ_OK);
> > | ? ? else
> > | ? ? ? ? auth_failed(port, status);
> >
> > Or, perhaps, we should modify this if-block to ensure the hook being
> > called after sendAuthRequest() but before auth_failed().
> 
> Oh.  You know, I am realizing that I misread this patch.  This hook is
> actually after authentication has been done; it's merely before we've
> told the client what happened.  So maybe this is committable as-is,
> modulo some work on the comments.
> 

OK, I'll re-submit my patch with a contrib module that utilizes
the authentication hook.
(Maybe, it just sleep a few seconds just after authentication failed.)

A hook on CheckMyDatabase() might be a candidate of this feature
and upcoming permission check.
But it will be a future work, is't it?

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

On Mon, Oct 25, 2010 at 9:45 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> Oh.  You know, I am realizing that I misread this patch.  This hook is
> actually after authentication has been done; it's merely before we've
> told the client what happened.  So maybe this is committable as-is,
> modulo some work on the comments.

Committed, with some work on the comments.

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