Re: contrib: auth_delay module

The attached patch is a contrib module to inject a few seconds
delay on authentication failed. It is also a proof of the concept
using the new ClientAuthentication_hook.

This module provides a similar feature to pam_faildelay on
operating systems. Injection of a few seconds delay on
authentication fails prevents (or makes hard at least) brute-force
attacks, because it limits number of candidates that attacker can
verify within a unit of time.

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

2010/11/4 KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>:
> The attached patch is a contrib module to inject a few seconds
> delay on authentication failed. It is also a proof of the concept
> using the new ClientAuthentication_hook.
>
> This module provides a similar feature to pam_faildelay on
> operating systems. Injection of a few seconds delay on
> authentication fails prevents (or makes hard at least) brute-force
> attacks, because it limits number of candidates that attacker can
> verify within a unit of time.

+1 for the feature. We have "post_auth_delay" parameter,
but it has different purpose; it's as DEVELOPER_OPTIONS
for delay to attach a debugger.

BTW, the module could save CPU usage of the server on attacks,
but do nothing about connection flood attacks, right?
If an attacker attacks the server with multiple connections,
the server still consumes max_connections even with the module.

--
Itagaki Takahiro

On Thu, Nov 4, 2010 at 6:05 AM, Itagaki Takahiro
<itagaki(dot)takahiro(at)gmail(dot)com> wrote:
> 2010/11/4 KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>:
>> The attached patch is a contrib module to inject a few seconds
>> delay on authentication failed. It is also a proof of the concept
>> using the new ClientAuthentication_hook.
>>
>> This module provides a similar feature to pam_faildelay on
>> operating systems. Injection of a few seconds delay on
>> authentication fails prevents (or makes hard at least) brute-force
>> attacks, because it limits number of candidates that attacker can
>> verify within a unit of time.
>
> +1 for the feature.  We have "post_auth_delay" parameter,
> but it has different purpose; it's as DEVELOPER_OPTIONS
> for delay to attach a debugger.
>
> BTW, the module could save CPU usage of the server on attacks,
> but do nothing about connection flood attacks, right?
> If an attacker attacks the server with multiple connections,
> the server still consumes max_connections even with the module.

Hmm, I wonder how useful this is given that restriction.

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

On 04/11/10 14:09, Robert Haas wrote:
> On Thu, Nov 4, 2010 at 6:05 AM, Itagaki Takahiro
> <itagaki(dot)takahiro(at)gmail(dot)com> wrote:
>> 2010/11/4 KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>:
>>> The attached patch is a contrib module to inject a few seconds
>>> delay on authentication failed. It is also a proof of the concept
>>> using the new ClientAuthentication_hook.
>>>
>>> This module provides a similar feature to pam_faildelay on
>>> operating systems. Injection of a few seconds delay on
>>> authentication fails prevents (or makes hard at least) brute-force
>>> attacks, because it limits number of candidates that attacker can
>>> verify within a unit of time.
>>
>> +1 for the feature. We have "post_auth_delay" parameter,
>> but it has different purpose; it's as DEVELOPER_OPTIONS
>> for delay to attach a debugger.
>>
>> BTW, the module could save CPU usage of the server on attacks,
>> but do nothing about connection flood attacks, right?
>> If an attacker attacks the server with multiple connections,
>> the server still consumes max_connections even with the module.
>
> Hmm, I wonder how useful this is given that restriction.

As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
consuming), right?

Jan

* Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
> On 04/11/10 14:09, Robert Haas wrote:
> > Hmm, I wonder how useful this is given that restriction.
>
> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
> consuming), right?

Which it would still do, since the attacker would be bumping up against
max_connections. max_connections would be a DOS point, but that's no
different from today. Other things could be put in place to address
that (max # of connections from a given IP or range could be implemented
using iptables, as an example).

5 second delay w/ max connections at 100 would mean max of 20 attempts
per second, no? That's alot fewer than 100*(however many attempts can
be done in a second). Doing a stupid while true; psql -d blah; done
managed to get 50 successful ident auths+no-db-found errors done in a
second on one box here. 5000 >> 20, and I wasn't even trying.

Thanks,

Stephen

(2010/11/04 22:05), Itagaki Takahiro wrote:
> 2010/11/4 KaiGai Kohei<kaigai(at)kaigai(dot)gr(dot)jp>:
>> The attached patch is a contrib module to inject a few seconds
>> delay on authentication failed. It is also a proof of the concept
>> using the new ClientAuthentication_hook.
>>
>> This module provides a similar feature to pam_faildelay on
>> operating systems. Injection of a few seconds delay on
>> authentication fails prevents (or makes hard at least) brute-force
>> attacks, because it limits number of candidates that attacker can
>> verify within a unit of time.
>
> +1 for the feature. We have "post_auth_delay" parameter,
> but it has different purpose; it's as DEVELOPER_OPTIONS
> for delay to attach a debugger.
>
> BTW, the module could save CPU usage of the server on attacks,
> but do nothing about connection flood attacks, right?
> If an attacker attacks the server with multiple connections,
> the server still consumes max_connections even with the module.
>
Good point. The pam_faildelay being the model of this feature also
does nothing for flood of connections attack.
However, if it closes the connection immediately, the attacker can
try to verify next candidate very soon. Do you have any good idea?

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

On Thu, Nov 4, 2010 at 6:35 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
>> On 04/11/10 14:09, Robert Haas wrote:
>> > Hmm, I wonder how useful this is given that restriction.
>>
>> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
>> consuming), right?
>
> Which it would still do, since the attacker would be bumping up against
> max_connections.  max_connections would be a DOS point, but that's no
> different from today.  Other things could be put in place to address
> that (max # of connections from a given IP or range could be implemented
> using iptables, as an example).
>
> 5 second delay w/ max connections at 100 would mean max of 20 attempts
> per second, no?  That's alot fewer than 100*(however many attempts can
> be done in a second).  Doing a stupid while true; psql -d blah; done
> managed to get 50 successful ident auths+no-db-found errors done in a
> second on one box here.  5000 >> 20, and I wasn't even trying.

OK. I was just asking. I don't object to it if people think it's
useful, especially if they are looking at it as "I would actually use
this on my system" rather than "I can imagine a hypothetical person
using this".

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

On Thu, Nov 4, 2010 at 10:04 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Nov 4, 2010 at 6:35 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> * Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
>>> On 04/11/10 14:09, Robert Haas wrote:
>>> > Hmm, I wonder how useful this is given that restriction.
>>>
>>> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
>>> consuming), right?
>>
>> Which it would still do, since the attacker would be bumping up against
>> max_connections.  max_connections would be a DOS point, but that's no
>> different from today.  Other things could be put in place to address
>> that (max # of connections from a given IP or range could be implemented
>> using iptables, as an example).
>>
>> 5 second delay w/ max connections at 100 would mean max of 20 attempts
>> per second, no?  That's alot fewer than 100*(however many attempts can
>> be done in a second).  Doing a stupid while true; psql -d blah; done
>> managed to get 50 successful ident auths+no-db-found errors done in a
>> second on one box here.  5000 >> 20, and I wasn't even trying.
>
> OK.  I was just asking.  I don't object to it if people think it's
> useful, especially if they are looking at it as "I would actually use
> this on my system" rather than "I can imagine a hypothetical person
> using this".

I haven't heard anyone say "yes, I would actually use this on my
system"? Any takers?

If we're to commit this, then the patch needs to add a new file
authdelay.smgl, fill it in with appropriate contents, and update
contrib.sgml and filelist.sgml accordingly. I also note that the
patch offers the ability to log superuser logins. Since that seems a
bit off-topic for a contrib module called auth_delay, and since we
already have a GUC called log_connections, I'm inclined to propose
removing that part.

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

(2010/11/15 11:50), Robert Haas wrote:
> On Thu, Nov 4, 2010 at 10:04 AM, Robert Haas<robertmhaas(at)gmail(dot)com> wrote:
>> On Thu, Nov 4, 2010 at 6:35 AM, Stephen Frost<sfrost(at)snowman(dot)net> wrote:
>>> * Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
>>>> On 04/11/10 14:09, Robert Haas wrote:
>>>>> Hmm, I wonder how useful this is given that restriction.
>>>>
>>>> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
>>>> consuming), right?
>>>
>>> Which it would still do, since the attacker would be bumping up against
>>> max_connections. max_connections would be a DOS point, but that's no
>>> different from today. Other things could be put in place to address
>>> that (max # of connections from a given IP or range could be implemented
>>> using iptables, as an example).
>>>
>>> 5 second delay w/ max connections at 100 would mean max of 20 attempts
>>> per second, no? That's alot fewer than 100*(however many attempts can
>>> be done in a second). Doing a stupid while true; psql -d blah; done
>>> managed to get 50 successful ident auths+no-db-found errors done in a
>>> second on one box here. 5000>> 20, and I wasn't even trying.
>>
>> OK. I was just asking. I don't object to it if people think it's
>> useful, especially if they are looking at it as "I would actually use
>> this on my system" rather than "I can imagine a hypothetical person
>> using this".
>
> I haven't heard anyone say "yes, I would actually use this on my
> system"? Any takers?
>
My original motivation is to provide a proof of concept module for
the ClientAuthentication_hook, to show this hook is useful to other
purposes not only getpeercon() in SE-PgSQL.

If we don't need a PoC module for each new hooks, I'm not strongly
motivated to push it into contrib tree.
How about your opinion?

> If we're to commit this, then the patch needs to add a new file
> authdelay.smgl, fill it in with appropriate contents, and update
> contrib.sgml and filelist.sgml accordingly.

Sorry, I missed the manner of contrib modules.
It seems to me the dummy_seclabel module also lacks these documentation
updates, although its purpose is regression testing.

> I also note that the
> patch offers the ability to log superuser logins. Since that seems a
> bit off-topic for a contrib module called auth_delay, and since we
> already have a GUC called log_connections, I'm inclined to propose
> removing that part.
>
I agree with the suggestion.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Tue, Nov 16, 2010 at 8:15 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> If we don't need a PoC module for each new hooks, I'm not strongly
> motivated to push it into contrib tree.
> How about your opinion?

I'd say let it go, unless someone else feels strongly about it.

> Sorry, I missed the manner of contrib modules.
> It seems to me the dummy_seclabel module also lacks these documentation
> updates, although its purpose is regression testing.

Ah, we should probably fix that.

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

On Tue, Nov 16, 2010 at 09:41:37PM -0500, Robert Haas wrote:
> On Tue, Nov 16, 2010 at 8:15 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> > If we don't need a PoC module for each new hooks, I'm not strongly
> > motivated to push it into contrib tree.
> > How about your opinion?
>
> I'd say let it go, unless someone else feels strongly about it.

I would use this module (rate limit new connection attempts) as soon as
I could. Putting a cap on potential CPU usage on a production DB by either
a blackhat or mistake by a developer caused by a mistake in
configuration (leaving the port accessible) is definitely useful, even
in the face of max_connections. My production apps already have
their connections and seldom need new ones. They all use CPU though.

Ross
--
Ross Reedstrom, Ph.D. reedstrm(at)rice(dot)edu
Systems Engineer & Admin, Research Scientist phone: 713-348-6166
Connexions http://cnx.org fax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE

On Wed, Nov 17, 2010 at 10:32 AM, Ross J. Reedstrom <reedstrm(at)rice(dot)edu> wrote:
> On Tue, Nov 16, 2010 at 09:41:37PM -0500, Robert Haas wrote:
>> On Tue, Nov 16, 2010 at 8:15 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>> > If we don't need a PoC module for each new hooks, I'm not strongly
>> > motivated to push it into contrib tree.
>> > How about your opinion?
>>
>> I'd say let it go, unless someone else feels strongly about it.
>
> I would use this module (rate limit new connection attempts) as soon as
> I could. Putting a cap on potential CPU usage on a production DB by either
> a blackhat or mistake by a developer caused by a mistake in
> configuration (leaving the port accessible) is definitely useful, even
> in the face of max_connections. My production apps already have
> their connections and seldom need new ones. They all use CPU though.

If KaiGai updates the code per previous discussion, would you be
willing to take a crack at adding documentation?

P.S. Your email client seems to be setting the Reply-To address to a
ridiculous value.

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

(2010/11/18 2:17), Robert Haas wrote:
> On Wed, Nov 17, 2010 at 10:32 AM, Ross J. Reedstrom<reedstrm(at)rice(dot)edu> wrote:
>> On Tue, Nov 16, 2010 at 09:41:37PM -0500, Robert Haas wrote:
>>> On Tue, Nov 16, 2010 at 8:15 PM, KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>>>> If we don't need a PoC module for each new hooks, I'm not strongly
>>>> motivated to push it into contrib tree.
>>>> How about your opinion?
>>>
>>> I'd say let it go, unless someone else feels strongly about it.
>>
>> I would use this module (rate limit new connection attempts) as soon as
>> I could. Putting a cap on potential CPU usage on a production DB by either
>> a blackhat or mistake by a developer caused by a mistake in
>> configuration (leaving the port accessible) is definitely useful, even
>> in the face of max_connections. My production apps already have
>> their connections and seldom need new ones. They all use CPU though.
>
> If KaiGai updates the code per previous discussion, would you be
> willing to take a crack at adding documentation?
>
> P.S. Your email client seems to be setting the Reply-To address to a
> ridiculous value.
>
OK, I'll revise my patch according to the previous discussion.
Please wait for about one week. I have a big event in this weekend.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Fri, Nov 19, 2010 at 04:57:03PM +0900, KaiGai Kohei wrote:
> (2010/11/18 2:17), Robert Haas wrote:
> >
> >If KaiGai updates the code per previous discussion, would you be
> >willing to take a crack at adding documentation?
> >
> >P.S. Your email client seems to be setting the Reply-To address to a
> >ridiculous value.
> >
> OK, I'll revise my patch according to the previous discussion.
> Please wait for about one week. I have a big event in this weekend.
>

I'll take a crack at the docs, though I might need hand-holding for the
new git stuff (I'll hit the wiki)

Ross
--
Ross Reedstrom, Ph.D. reedstrm(at)rice(dot)edu
Systems Engineer & Admin, Research Scientist phone: 713-348-6166
Connexions http://cnx.org fax: 713-348-3665
Rice University MS-375, Houston, TX 77005
GPG Key fingerprint = F023 82C8 9B0E 2CC6 0D8E F888 D3AE 810E 88F0 BEDE

(2010/11/19 16:57), KaiGai Kohei wrote:
> (2010/11/18 2:17), Robert Haas wrote:
>> On Wed, Nov 17, 2010 at 10:32 AM, Ross J. Reedstrom<reedstrm(at)rice(dot)edu> wrote:
>>> On Tue, Nov 16, 2010 at 09:41:37PM -0500, Robert Haas wrote:
>>>> On Tue, Nov 16, 2010 at 8:15 PM, KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>>>>> If we don't need a PoC module for each new hooks, I'm not strongly
>>>>> motivated to push it into contrib tree.
>>>>> How about your opinion?
>>>>
>>>> I'd say let it go, unless someone else feels strongly about it.
>>>
>>> I would use this module (rate limit new connection attempts) as soon as
>>> I could. Putting a cap on potential CPU usage on a production DB by either
>>> a blackhat or mistake by a developer caused by a mistake in
>>> configuration (leaving the port accessible) is definitely useful, even
>>> in the face of max_connections. My production apps already have
>>> their connections and seldom need new ones. They all use CPU though.
>>
>> If KaiGai updates the code per previous discussion, would you be
>> willing to take a crack at adding documentation?
>>
>> P.S. Your email client seems to be setting the Reply-To address to a
>> ridiculous value.
>>
> OK, I'll revise my patch according to the previous discussion.

The attached patch is revised version.

- Logging part within auth_delay was removed. This module now focuses on
injection of a few seconds delay on authentication failed.
- Documentation parts were added like any other contrib modules.

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Thu, Nov 25, 2010 at 3:18 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> The attached patch is revised version.
>
> - Logging part within auth_delay was removed. This module now focuses on
>  injection of a few seconds delay on authentication failed.
> - Documentation parts were added like any other contrib modules.

Something like the following is not required? Though I'm not sure
if there is the case where auth_delay is unload.

----------------------------
void
_PG_fini(void)
{
/* Uninstall hooks. */
ClientAuthentication_hook = original_client_auth_hook;
}
----------------------------

+ if (status != STATUS_OK)
+ {
+ sleep(auth_delay_seconds);
+ }

We should use pg_usleep rather than sleep?

+ DefineCustomIntVariable("auth_delay.seconds",
+ "Seconds to be delayed on authentication failed",
+ NULL,
+ &auth_delay_seconds,
+ 2,
+ 0, INT_MAX,
+ PGC_POSTMASTER,
+ GUC_UNIT_S,
+ NULL,
+ NULL);

Can we relax the context from PGC_POSTMASTER to PGC_SIGHUP?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

(2010/11/26 11:35), Fujii Masao wrote:
> On Thu, Nov 25, 2010 at 3:18 PM, KaiGai Kohei<kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>> The attached patch is revised version.
>>
>> - Logging part within auth_delay was removed. This module now focuses on
>> injection of a few seconds delay on authentication failed.
>> - Documentation parts were added like any other contrib modules.
>
> Something like the following is not required? Though I'm not sure
> if there is the case where auth_delay is unload.
>
> ----------------------------
> void
> _PG_fini(void)
> {
> /* Uninstall hooks. */
> ClientAuthentication_hook = original_client_auth_hook;
> }
> ----------------------------
>
I'm not also sure whether we have situation libraries are unloaded.
Right now, internal_unload_library() is just a placeholder, so
it seems to me _PG_fini() is never invoked.

> + if (status != STATUS_OK)
> + {
> + sleep(auth_delay_seconds);
> + }
>
> We should use pg_usleep rather than sleep?
>
Indeed, pg_usleep() is mainly used rather than sleep().

> + DefineCustomIntVariable("auth_delay.seconds",
> + "Seconds to be delayed on authentication failed",
> + NULL,
> + &auth_delay_seconds,
> + 2,
> + 0, INT_MAX,
> + PGC_POSTMASTER,
> + GUC_UNIT_S,
> + NULL,
> + NULL);
>
> Can we relax the context from PGC_POSTMASTER to PGC_SIGHUP?
>
It seems to me reasonable change.

I'll revise my patch. How about _PG_fini()?

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

On Thu, Nov 25, 2010 at 9:54 PM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> I'll revise my patch. How about _PG_fini()?

In modules like auto_explain and pg_stat_statements, we have some
token code in there to handle unload, but I don't believe there's any
way to invoke it, nor would it work if there were multiple users of
the hook. passwordcheck, on the other hand, has no _PG_fini, which
seems like the more sensible approach since it isn't going to work
anyway.

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

On Thu, Nov 25, 2010 at 1:18 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
> The attached patch is revised version.
>
> - Logging part within auth_delay was removed. This module now focuses on
>  injection of a few seconds delay on authentication failed.
> - Documentation parts were added like any other contrib modules.

Committed, with a few adjustments. Per Fujii Masao's suggestion, I
changed sleep() to pg_usleep(); I also changed the GUC to be reckoned
in milliseconds rather than seconds.

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

On Thu, Nov 4, 2010 at 6:35 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
>> On 04/11/10 14:09, Robert Haas wrote:
>> > Hmm, I wonder how useful this is given that restriction.
>>
>> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
>> consuming), right?
>
> Which it would still do, since the attacker would be bumping up against
> max_connections.  max_connections would be a DOS point, but that's no
> different from today.

I haven' t thought of a way to test this, so I guess I'll just ask.
If the attacking client just waits a few milliseconds for a response
and then drops the socket, opening a new one, will the server-side
walking-dead process continue to be charged against max_connections
until it's sleep expires?

Cheers,

Jeff

On Sat, Nov 27, 2010 at 2:44 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> On Thu, Nov 4, 2010 at 6:35 AM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
>> * Jan Urbański (wulczer(at)wulczer(dot)org) wrote:
>>> On 04/11/10 14:09, Robert Haas wrote:
>>> > Hmm, I wonder how useful this is given that restriction.
>>>
>>> As KaiGai mentined, it's more to make bruteforcing difficult (read: tmie
>>> consuming), right?
>>
>> Which it would still do, since the attacker would be bumping up against
>> max_connections.  max_connections would be a DOS point, but that's no
>> different from today.
>
> I haven' t thought of a way to test this, so I guess I'll just ask.
> If the attacking client just waits a few milliseconds for a response
> and then drops the socket, opening a new one, will the server-side
> walking-dead process continue to be charged against max_connections
> until it's sleep expires?

I'm not sure, either. I suspect the answer is yes. I guess you could
test this by writing a loop like this:

while true; do psql <connection parameters that will fail authentication>; done

...and then hitting ^C every few seconds during execution. After
doing that for a bit, run select * from pg_stat_activity or ps auxww |
grep postgres in another window.

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

On Sun, Nov 28, 2010 at 5:38 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Nov 27, 2010 at 2:44 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:

>> I haven' t thought of a way to test this, so I guess I'll just ask.
>> If the attacking client just waits a few milliseconds for a response
>> and then drops the socket, opening a new one, will the server-side
>> walking-dead process continue to be charged against max_connections
>> until it's sleep expires?
>
> I'm not sure, either.  I suspect the answer is yes.  I guess you could
> test this by writing a loop like this:
>
> while true; do psql <connection parameters that will fail authentication>; done
>
> ...and then hitting ^C every few seconds during execution.  After
> doing that for a bit, run select * from pg_stat_activity or ps auxww |
> grep postgres in another window.

Right, I didn't think of using psql, I thought I'd have to wrangle my
own socket code.

I wrote up a perl script that spawns psql and immediately kills it. I
quickly start getting "psql: FATAL: sorry, too many clients already"
errors. And that condition doesn't clear until the sleep expires on
the earliest ones spawned.

So it looks like the max_connections is charged until the auth_delay expires.

Cheers,

Jeff

On Sun, Nov 28, 2010 at 5:41 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> On Sun, Nov 28, 2010 at 5:38 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Sat, Nov 27, 2010 at 2:44 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>
>>> I haven' t thought of a way to test this, so I guess I'll just ask.
>>> If the attacking client just waits a few milliseconds for a response
>>> and then drops the socket, opening a new one, will the server-side
>>> walking-dead process continue to be charged against max_connections
>>> until it's sleep expires?
>>
>> I'm not sure, either.  I suspect the answer is yes.  I guess you could
>> test this by writing a loop like this:
>>
>> while true; do psql <connection parameters that will fail authentication>; done
>>
>> ...and then hitting ^C every few seconds during execution.  After
>> doing that for a bit, run select * from pg_stat_activity or ps auxww |
>> grep postgres in another window.
>
> Right, I didn't think of using psql, I thought I'd have to wrangle my
> own socket code.
>
> I wrote up a perl script that spawns psql and immediately kills it.  I
> quickly start getting "psql: FATAL:  sorry, too many clients already"
> errors.  And that condition doesn't clear until the sleep expires on
> the earliest ones spawned.
>
> So it looks like the max_connections is charged until the auth_delay expires.

Yeah. Avoiding that would be hard, and it's not clear that there's
any demand. The demand for doing this much seems a bit marginal too,
but there were several people who seemed to think it worth committing,
so I did.

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

On Sun, Nov 28, 2010 at 3:57 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sun, Nov 28, 2010 at 5:41 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> On Sun, Nov 28, 2010 at 5:38 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Sat, Nov 27, 2010 at 2:44 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>>
>>>> I haven' t thought of a way to test this, so I guess I'll just ask.
>>>> If the attacking client just waits a few milliseconds for a response
>>>> and then drops the socket, opening a new one, will the server-side
>>>> walking-dead process continue to be charged against max_connections
>>>> until it's sleep expires?
>>>
>>> I'm not sure, either.  I suspect the answer is yes.  I guess you could
>>> test this by writing a loop like this:
>>>
>>> while true; do psql <connection parameters that will fail authentication>; done
>>>
>>> ...and then hitting ^C every few seconds during execution.  After
>>> doing that for a bit, run select * from pg_stat_activity or ps auxww |
>>> grep postgres in another window.
>>
>> Right, I didn't think of using psql, I thought I'd have to wrangle my
>> own socket code.
>>
>> I wrote up a perl script that spawns psql and immediately kills it.  I
>> quickly start getting "psql: FATAL:  sorry, too many clients already"
>> errors.  And that condition doesn't clear until the sleep expires on
>> the earliest ones spawned.
>>
>> So it looks like the max_connections is charged until the auth_delay expires.
>
> Yeah.  Avoiding that would be hard, and it's not clear that there's
> any demand.  The demand for doing this much seems a bit marginal too,
> but there were several people who seemed to think it worth committing,
> so I did.
>

Oh, I wasn't complaining. I think that having max_connections be
charged for the duration even if the socket is dropped is the only
reasonable thing to do, and wanted to verify that it did happen.
Otherwise the module wouldn't do a very good job at its purpose, the
attacker would simply wait a few milliseconds and then assume it got
the wrong password and kill the connection and start new one.
Preventing the brute force password attack by shunting it into a DOS
attack instead seems reasonable.

Cheers,

Jeff

On Sun, Nov 28, 2010 at 7:10 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> Oh, I wasn't complaining.  I think that having max_connections be
> charged for the duration even if the socket is dropped is the only
> reasonable thing to do, and wanted to verify that it did happen.
> Otherwise the module wouldn't do a very good job at its purpose, the
> attacker would simply wait a few milliseconds and then assume it got
> the wrong password and kill the connection and start new one.

Good point.

> Preventing the brute force password attack by shunting it into a DOS
> attack instead seems reasonable.

OK.

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

On Sat, Nov 27, 2010 at 9:25 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Thu, Nov 25, 2010 at 1:18 AM, KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com> wrote:
>> The attached patch is revised version.
>>
>> - Logging part within auth_delay was removed. This module now focuses on
>>  injection of a few seconds delay on authentication failed.
>> - Documentation parts were added like any other contrib modules.
>
> Committed, with a few adjustments.  Per Fujii Masao's suggestion, I
> changed sleep() to pg_usleep(); I also changed the GUC to be reckoned
> in milliseconds rather than seconds.

Thanks. I found the typo:

-----------------------------
diff --git a/contrib/README b/contrib/README
index 9e223ef..8a12cc1 100644
--- a/contrib/README
+++ b/contrib/README
@@ -30,7 +30,7 @@ adminpack -

auth_delay
Add a short delay after a failed authentication attempt, to make
- make brute-force attacks on database passwords a bit harder.
+ brute-force attacks on database passwords a bit harder.
by KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>

auto_explain -
-----------------------------

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Sun, Nov 28, 2010 at 7:45 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> Thanks. I found the typo:

I only have one? :-)

Thanks, fixed.

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