On Tue, Oct 5, 2010 at 11:01, Magnus Hagander <mag...@hagander.net> wrote:
> On Sun, Oct 3, 2010 at 18:30, Alan T DeKok <al...@freeradius.org> wrote:
>> Tom Lane wrote:
>>> Hm ... seems to me that is a network security problem, not our problem.
>>> Who's to say one of the spoofed packets won't pass verification?
>>
>> The packets are signed with a shared key. Passing verification means
>> either the attacker knows the key, or the attacker has broken MD5 in
>> ways that are currently unknown.
>>
>>> If you want to change it, I won't stand in the way, but I have real
>>> doubts about both the credibility of this threat and the usefulness
>>> of the proposed fix.
>>
>> The credibility of the threat is high. Anyone can trivially send a
>> packet which will cause authentication to fail. This is a DoS attack.
>
> I don't agree about how high it is - unless I misunderstand the
> wording. You still need to have unfiltered access to the network that
> the database server is on (unlikely) and you need to guess/bruteforce
> the port (using bruteforce not really hard, but likely to be detected
> by an IDS pretty quickly)
>
> It is definitely an opportunity for a DoS attack though, so it should be
> fixed.
>
> I find your suggested patches kind of hard to read posted inline that
> way - any chance you can repost as attachment or publish it as a git
> repository I can fetch from?
Actually, nevermind that one. Here's a patch I worked up from your
description, and that turns out to be fairly similar to yours in what
it does I think - except I'm not rearranging the code into a separate
function. We already have a while-loop.
See attached context diff, and I've also included a diff without
whitespace changes since the majority of the diff is otherwise coming
from indenting the code one tab...
(so far untested, I seem to have deleted my test-instance of the
radius server, but I figured I should post my attempt anyway)
Also, my patch does not change from log to warning - note that warning
is actually *below* log when it comes to the logfile (see
log_min_messages comments in postgresql.conf). I keep making that
mistake myself...
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 2619,2625 **** CheckRADIUSAuth(Port *port)
char portstr[128];
ACCEPT_TYPE_ARG3 addrsize;
fd_set fdset;
! struct timeval timeout;
int i,
r;
--- 2619,2625 ----
char portstr[128];
ACCEPT_TYPE_ARG3 addrsize;
fd_set fdset;
! struct timeval endtime;
int i,
r;
***************
*** 2777,2790 **** CheckRADIUSAuth(Port *port)
/* Don't need the server address anymore */
pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
! /* Wait for a response */
! timeout.tv_sec = RADIUS_TIMEOUT;
! timeout.tv_usec = 0;
! FD_ZERO(&fdset);
! FD_SET(sock, &fdset);
while (true)
{
r = select(sock + 1, &fdset, NULL, NULL, &timeout);
if (r < 0)
{
--- 2777,2812 ----
/* Don't need the server address anymore */
pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
! /*
! * Figure out at what time we should time out. We can't just use
! * a single call to select() with a timeout, since somebody can
! * be sending invalid packets to our port thus causing us to
! * retry in a loop and never time out.
! */
! gettimeofday(&endtime, NULL);
! endtime.tv_sec += RADIUS_TIMEOUT;
while (true)
{
+ struct timeval timeout;
+ struct timeval now;
+ int64 timeoutval;
+
+ gettimeofday(&now, NULL);
+ timeoutval = (endtime.tv_sec * 1000000 + endtime.tv_usec) - (now.tv_sec * 1000000 + now.tv_usec);
+ if (timeoutval <= 0)
+ {
+ ereport(LOG,
+ (errmsg("timeout waiting for RADIUS response")));
+ closesocket(sock);
+ return STATUS_ERROR;
+ }
+ timeout.tv_sec = timeoutval / 1000000;
+ timeout.tv_usec = timeoutval % 1000000;
+
+ FD_ZERO(&fdset);
+ FD_SET(sock, &fdset);
+
r = select(sock + 1, &fdset, NULL, NULL, &timeout);
if (r < 0)
{
***************
*** 2805,2911 **** CheckRADIUSAuth(Port *port)
return STATUS_ERROR;
}
! /* else we actually have a packet ready to read */
! break;
! }
!
! /* Read the response packet */
! addrsize = sizeof(remoteaddr);
! packetlength = recvfrom(sock, receive_buffer, RADIUS_BUFFER_SIZE, 0,
! (struct sockaddr *) & remoteaddr, &addrsize);
! if (packetlength < 0)
! {
! ereport(LOG,
! (errmsg("could not read RADIUS response: %m")));
! closesocket(sock);
! return STATUS_ERROR;
! }
! closesocket(sock);
#ifdef HAVE_IPV6
! if (remoteaddr.sin6_port != htons(port->hba->radiusport))
#else
! if (remoteaddr.sin_port != htons(port->hba->radiusport))
#endif
! {
#ifdef HAVE_IPV6
! ereport(LOG,
! (errmsg("RADIUS response was sent from incorrect port: %i",
! ntohs(remoteaddr.sin6_port))));
#else
! ereport(LOG,
! (errmsg("RADIUS response was sent from incorrect port: %i",
! ntohs(remoteaddr.sin_port))));
#endif
! return STATUS_ERROR;
! }
!
! if (packetlength < RADIUS_HEADER_LENGTH)
! {
! ereport(LOG,
! (errmsg("RADIUS response too short: %i", packetlength)));
! return STATUS_ERROR;
! }
!
! if (packetlength != ntohs(receivepacket->length))
! {
! ereport(LOG,
! (errmsg("RADIUS response has corrupt length: %i (actual length %i)",
! ntohs(receivepacket->length), packetlength)));
! return STATUS_ERROR;
! }
! if (packet->id != receivepacket->id)
! {
! ereport(LOG,
! (errmsg("RADIUS response is to a different request: %i (should be %i)",
! receivepacket->id, packet->id)));
! return STATUS_ERROR;
! }
! /*
! * Verify the response authenticator, which is calculated as
! * MD5(Code+ID+Length+RequestAuthenticator+Attributes+Secret)
! */
! cryptvector = palloc(packetlength + strlen(port->hba->radiussecret));
! memcpy(cryptvector, receivepacket, 4); /* code+id+length */
! memcpy(cryptvector + 4, packet->vector, RADIUS_VECTOR_LENGTH); /* request
! * authenticator, from
! * original packet */
! if (packetlength > RADIUS_HEADER_LENGTH) /* there may be no attributes
! * at all */
! memcpy(cryptvector + RADIUS_HEADER_LENGTH, receive_buffer + RADIUS_HEADER_LENGTH, packetlength - RADIUS_HEADER_LENGTH);
! memcpy(cryptvector + packetlength, port->hba->radiussecret, strlen(port->hba->radiussecret));
! if (!pg_md5_binary(cryptvector,
! packetlength + strlen(port->hba->radiussecret),
! encryptedpassword))
! {
! ereport(LOG,
! (errmsg("could not perform MD5 encryption of received packet")));
pfree(cryptvector);
- return STATUS_ERROR;
- }
- pfree(cryptvector);
! if (memcmp(receivepacket->vector, encryptedpassword, RADIUS_VECTOR_LENGTH) != 0)
! {
! ereport(LOG,
! (errmsg("RADIUS response has incorrect MD5 signature")));
! return STATUS_ERROR;
! }
! if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
! return STATUS_OK;
! else if (receivepacket->code == RADIUS_ACCESS_REJECT)
! return STATUS_ERROR;
! else
! {
! ereport(LOG,
! (errmsg("RADIUS response has invalid code (%i) for user \"%s\"",
! receivepacket->code, port->user_name)));
! return STATUS_ERROR;
! }
}
--- 2827,2943 ----
return STATUS_ERROR;
}
! /*
! * Attempt to read the response packet, and verify the contents.
! *
! * Any packet that's not actually a RADIUS packet, or otherwise
! * does not validate as an explicit reject, is just ignored and
! * we retry for another packet (until we reach the timeout). This
! * is to avoid the possibility to denial-of-service the login by
! * flooding the server with invalid packets on the port that
! * we're expecting the RADIUS response on.
! */
! addrsize = sizeof(remoteaddr);
! packetlength = recvfrom(sock, receive_buffer, RADIUS_BUFFER_SIZE, 0,
! (struct sockaddr *) & remoteaddr, &addrsize);
! if (packetlength < 0)
! {
! ereport(LOG,
! (errmsg("could not read RADIUS response: %m")));
! return STATUS_ERROR;
! }
#ifdef HAVE_IPV6
! if (remoteaddr.sin6_port != htons(port->hba->radiusport))
#else
! if (remoteaddr.sin_port != htons(port->hba->radiusport))
#endif
! {
#ifdef HAVE_IPV6
! ereport(LOG,
! (errmsg("RADIUS response was sent from incorrect port: %i",
! ntohs(remoteaddr.sin6_port))));
#else
! ereport(LOG,
! (errmsg("RADIUS response was sent from incorrect port: %i",
! ntohs(remoteaddr.sin_port))));
#endif
! continue;
! }
! if (packetlength < RADIUS_HEADER_LENGTH)
! {
! ereport(LOG,
! (errmsg("RADIUS response too short: %i", packetlength)));
! continue;
! }
! if (packetlength != ntohs(receivepacket->length))
! {
! ereport(LOG,
! (errmsg("RADIUS response has corrupt length: %i (actual length %i)",
! ntohs(receivepacket->length), packetlength)));
! continue;
! }
! if (packet->id != receivepacket->id)
! {
! ereport(LOG,
! (errmsg("RADIUS response is to a different request: %i (should be %i)",
! receivepacket->id, packet->id)));
! continue;
! }
! /*
! * Verify the response authenticator, which is calculated as
! * MD5(Code+ID+Length+RequestAuthenticator+Attributes+Secret)
! */
! cryptvector = palloc(packetlength + strlen(port->hba->radiussecret));
!
! memcpy(cryptvector, receivepacket, 4); /* code+id+length */
! memcpy(cryptvector + 4, packet->vector, RADIUS_VECTOR_LENGTH); /* request
! * authenticator, from
! * original packet */
! if (packetlength > RADIUS_HEADER_LENGTH) /* there may be no attributes
! * at all */
! memcpy(cryptvector + RADIUS_HEADER_LENGTH, receive_buffer + RADIUS_HEADER_LENGTH, packetlength - RADIUS_HEADER_LENGTH);
! memcpy(cryptvector + packetlength, port->hba->radiussecret, strlen(port->hba->radiussecret));
!
! if (!pg_md5_binary(cryptvector,
! packetlength + strlen(port->hba->radiussecret),
! encryptedpassword))
! {
! ereport(LOG,
! (errmsg("could not perform MD5 encryption of received packet")));
! pfree(cryptvector);
! continue;
! }
pfree(cryptvector);
! if (memcmp(receivepacket->vector, encryptedpassword, RADIUS_VECTOR_LENGTH) != 0)
! {
! ereport(LOG,
! (errmsg("RADIUS response has incorrect MD5 signature")));
! continue;
! }
! if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
! {
! closesocket(sock);
! return STATUS_OK;
! }
! else if (receivepacket->code == RADIUS_ACCESS_REJECT)
! {
! closesocket(sock);
! return STATUS_ERROR;
! }
! else
! {
! ereport(LOG,
! (errmsg("RADIUS response has invalid code (%i) for user \"%s\"",
! receivepacket->code, port->user_name)));
! continue;
! }
! } /* while (true) */
}
*** a/src/backend/libpq/auth.c
--- b/src/backend/libpq/auth.c
***************
*** 2619,2625 **** CheckRADIUSAuth(Port *port)
char portstr[128];
ACCEPT_TYPE_ARG3 addrsize;
fd_set fdset;
! struct timeval timeout;
int i,
r;
--- 2619,2625 ----
char portstr[128];
ACCEPT_TYPE_ARG3 addrsize;
fd_set fdset;
! struct timeval endtime;
int i,
r;
***************
*** 2777,2790 **** CheckRADIUSAuth(Port *port)
/* Don't need the server address anymore */
pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
! /* Wait for a response */
! timeout.tv_sec = RADIUS_TIMEOUT;
! timeout.tv_usec = 0;
! FD_ZERO(&fdset);
! FD_SET(sock, &fdset);
while (true)
{
r = select(sock + 1, &fdset, NULL, NULL, &timeout);
if (r < 0)
{
--- 2777,2812 ----
/* Don't need the server address anymore */
pg_freeaddrinfo_all(hint.ai_family, serveraddrs);
! /*
! * Figure out at what time we should time out. We can't just use
! * a single call to select() with a timeout, since somebody can
! * be sending invalid packets to our port thus causing us to
! * retry in a loop and never time out.
! */
! gettimeofday(&endtime, NULL);
! endtime.tv_sec += RADIUS_TIMEOUT;
while (true)
{
+ struct timeval timeout;
+ struct timeval now;
+ int64 timeoutval;
+
+ gettimeofday(&now, NULL);
+ timeoutval = (endtime.tv_sec * 1000000 + endtime.tv_usec) - (now.tv_sec * 1000000 + now.tv_usec);
+ if (timeoutval <= 0)
+ {
+ ereport(LOG,
+ (errmsg("timeout waiting for RADIUS response")));
+ closesocket(sock);
+ return STATUS_ERROR;
+ }
+ timeout.tv_sec = timeoutval / 1000000;
+ timeout.tv_usec = timeoutval % 1000000;
+
+ FD_ZERO(&fdset);
+ FD_SET(sock, &fdset);
+
r = select(sock + 1, &fdset, NULL, NULL, &timeout);
if (r < 0)
{
***************
*** 2805,2815 **** CheckRADIUSAuth(Port *port)
return STATUS_ERROR;
}
! /* else we actually have a packet ready to read */
! break;
! }
- /* Read the response packet */
addrsize = sizeof(remoteaddr);
packetlength = recvfrom(sock, receive_buffer, RADIUS_BUFFER_SIZE, 0,
(struct sockaddr *) & remoteaddr, &addrsize);
--- 2827,2843 ----
return STATUS_ERROR;
}
! /*
! * Attempt to read the response packet, and verify the contents.
! *
! * Any packet that's not actually a RADIUS packet, or otherwise
! * does not validate as an explicit reject, is just ignored and
! * we retry for another packet (until we reach the timeout). This
! * is to avoid the possibility to denial-of-service the login by
! * flooding the server with invalid packets on the port that
! * we're expecting the RADIUS response on.
! */
addrsize = sizeof(remoteaddr);
packetlength = recvfrom(sock, receive_buffer, RADIUS_BUFFER_SIZE, 0,
(struct sockaddr *) & remoteaddr, &addrsize);
***************
*** 2817,2828 **** CheckRADIUSAuth(Port *port)
{
ereport(LOG,
(errmsg("could not read RADIUS response: %m")));
- closesocket(sock);
return STATUS_ERROR;
}
- closesocket(sock);
-
#ifdef HAVE_IPV6
if (remoteaddr.sin6_port != htons(port->hba->radiusport))
#else
--- 2845,2853 ----
***************
*** 2838,2851 **** CheckRADIUSAuth(Port *port)
(errmsg("RADIUS response was sent from incorrect port: %i",
ntohs(remoteaddr.sin_port))));
#endif
! return STATUS_ERROR;
}
if (packetlength < RADIUS_HEADER_LENGTH)
{
ereport(LOG,
(errmsg("RADIUS response too short: %i", packetlength)));
! return STATUS_ERROR;
}
if (packetlength != ntohs(receivepacket->length))
--- 2863,2876 ----
(errmsg("RADIUS response was sent from incorrect port: %i",
ntohs(remoteaddr.sin_port))));
#endif
! continue;
}
if (packetlength < RADIUS_HEADER_LENGTH)
{
ereport(LOG,
(errmsg("RADIUS response too short: %i", packetlength)));
! continue;
}
if (packetlength != ntohs(receivepacket->length))
***************
*** 2853,2859 **** CheckRADIUSAuth(Port *port)
ereport(LOG,
(errmsg("RADIUS response has corrupt length: %i (actual length %i)",
ntohs(receivepacket->length), packetlength)));
! return STATUS_ERROR;
}
if (packet->id != receivepacket->id)
--- 2878,2884 ----
ereport(LOG,
(errmsg("RADIUS response has corrupt length: %i (actual length %i)",
ntohs(receivepacket->length), packetlength)));
! continue;
}
if (packet->id != receivepacket->id)
***************
*** 2861,2867 **** CheckRADIUSAuth(Port *port)
ereport(LOG,
(errmsg("RADIUS response is to a different request: %i (should be %i)",
receivepacket->id, packet->id)));
! return STATUS_ERROR;
}
/*
--- 2886,2892 ----
ereport(LOG,
(errmsg("RADIUS response is to a different request: %i (should be %i)",
receivepacket->id, packet->id)));
! continue;
}
/*
***************
*** 2886,2892 **** CheckRADIUSAuth(Port *port)
ereport(LOG,
(errmsg("could not perform MD5 encryption of received packet")));
pfree(cryptvector);
! return STATUS_ERROR;
}
pfree(cryptvector);
--- 2911,2917 ----
ereport(LOG,
(errmsg("could not perform MD5 encryption of received packet")));
pfree(cryptvector);
! continue;
}
pfree(cryptvector);
***************
*** 2894,2911 **** CheckRADIUSAuth(Port *port)
{
ereport(LOG,
(errmsg("RADIUS response has incorrect MD5 signature")));
! return STATUS_ERROR;
}
if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
return STATUS_OK;
else if (receivepacket->code == RADIUS_ACCESS_REJECT)
return STATUS_ERROR;
else
{
ereport(LOG,
(errmsg("RADIUS response has invalid code (%i) for user \"%s\"",
receivepacket->code, port->user_name)));
! return STATUS_ERROR;
}
}
--- 2919,2943 ----
{
ereport(LOG,
(errmsg("RADIUS response has incorrect MD5 signature")));
! continue;
}
if (receivepacket->code == RADIUS_ACCESS_ACCEPT)
+ {
+ closesocket(sock);
return STATUS_OK;
+ }
else if (receivepacket->code == RADIUS_ACCESS_REJECT)
+ {
+ closesocket(sock);
return STATUS_ERROR;
+ }
else
{
ereport(LOG,
(errmsg("RADIUS response has invalid code (%i) for user \"%s\"",
receivepacket->code, port->user_name)));
! continue;
}
+ } /* while (true) */
}
--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
