Skip Menu |
 

History Show all quoted text — Show full headers
# Wed Feb 14 18:53:00 2007 guest (Public Submitter) - Ticket created
Subject: sendto_kdc() not signal safe, doesn't respond well to staggered TCP responses.
Download (untitled) / with headers
text/plain 956B
From code inspection sendto_kdc() is fundamentally flawed insofar as
it assumes that when select(2) returns, it has consumed all of the
time that it was allotted. This is incorrect in two cases:

1. if the process catches signals then select(2) will return
EINTR, and

2. while processing a TCP connexion, select(2) can return true
but the subsequent read will not contain enough data to
complete the request.

In both cases, it appears from code inspection that sendto_kdc() will
proceed to either send a new UDP packet or start a new TCP connexion
assuming that the specified delay was entirely consumed. In a worst
case scenario, e.g. a process that spawns and catches lots of kids,
the loop might exit prematurely with an error.

Actually, it looks like if select(2) returns EINTR then the entire
loop punts returning an error. In short, this means that processes
cannot both process signals and reliably use Kerberos...
# Wed Feb 14 20:15:26 2007 raeburn (Ken Raeburn) - Correspondence added
From: Ken Raeburn <raeburn@MIT.EDU>
Subject: Re: [krbdev.mit.edu #5440] sendto_kdc() not signal safe, doesn't respond well to staggered TCP responses.
Date: Wed, 14 Feb 2007 20:14:53 -0500
To: rt@krbdev.mit.edu
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.9KiB
On Feb 14, 2007, at 18:53, Public Submitter via RT wrote:
Show quoted text
>> From code inspection sendto_kdc() is fundamentally flawed insofar as
> it assumes that when select(2) returns, it has consumed all of the
> time that it was allotted. This is incorrect in two cases:

We should be going back into the select call unless
krb5int_cm_call_select returns zero *and* sets *sret to 0.

Show quoted text
> 1. if the process catches signals then select(2) will return
> EINTR, and

It looks like that won't be handled properly, true... probably the
call_select routine should check for that, and just try calling
select again.

Show quoted text
> 2. while processing a TCP connexion, select(2) can return true
> but the subsequent read will not contain enough data to
> complete the request.

Show quoted text
> In both cases, it appears from code inspection that sendto_kdc() will
> proceed to either send a new UDP packet or start a new TCP connexion
> assuming that the specified delay was entirely consumed.

The while loop in service_fds should continue until:

1) all open file descriptors have been closed (selstate->nfds)
2) krb5int_cm_call_select returns nonzero, which indicates an error
return from gettimeofday or select (including, yes, EINTR)
3) krb5int_cm_call_select returns zero but sets *sret (number of
ready fds) to 0, which should happen only when the specified end time
is reached
4) the fd service routine returns nonzero, which for the TCP sockets
should only be if the full message is received

If an incomplete message is received, and there's still time left, we
should wind up back in the select call.

Show quoted text
> In a worst
> case scenario, e.g. a process that spawns and catches lots of kids,
> the loop might exit prematurely with an error.
>
> Actually, it looks like if select(2) returns EINTR then the entire
> loop punts returning an error. In short, this means that processes
> cannot both process signals and reliably use Kerberos...

Yeah, that's a bug...

Ken
# Fri Apr 13 00:22:19 2007 raeburn (Ken Raeburn) - Given to raeburn (Ken Raeburn)
# Fri Apr 13 00:22:19 2007 raeburn (Ken Raeburn) - Status changed from 'new' to 'open'
# Fri Apr 13 00:22:20 2007 raeburn (Ken Raeburn) - Correspondence added
From: raeburn@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 166B
If select returns EINTR, recompute the expiration time and call it again.
Commit By: raeburn



Revision: 19452
Changed Files:
U trunk/src/lib/krb5/os/sendto_kdc.c
# Fri Apr 13 03:04:44 2007 raeburn (Ken Raeburn) - Status changed from 'open' to 'resolved'
# Fri Apr 13 03:04:44 2007 raeburn (Ken Raeburn) - Correspondence added
From: raeburn@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 287B
Revert previous change; krb5int_cm_call_select is used by the KDC, and should
return when interrupted by a signal. Instead, check for EINTR in service_fds
and call krb5int_cm_call_select again.
Commit By: raeburn



Revision: 19457
Changed Files:
U trunk/src/lib/krb5/os/sendto_kdc.c
# Fri Apr 13 03:20:20 2007 raeburn (Ken Raeburn) - Correspondence added
Download (untitled) / with headers
text/plain 296B
After a little testing with a hacked KDC that sends back TCP responses in pieces with delays, I
don't think that there is a problem with that part of the client code.

The library changes I've just checked in appear to fix the select/EINTR problem, so I'm going to
mark this ticket as resolved.
# Fri Apr 13 11:28:00 2007 Roland C. Dowdeswell <elric@imrryr.org> - Comments added
To: rt-comment@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #5440] sendto_kdc() not signal safe, doesn't respond well to staggered TCP responses.
Date: Fri, 13 Apr 2007 11:25:13 -0400
From: Roland Dowdeswell <elric@imrryr.org>
RT-Send-Cc:
Download (untitled) / with headers
text/plain 670B
On 1176448821 seconds since the Beginning of the UNIX epoch
"Ken Raeburn via RT" wrote:
Show quoted text
>

Show quoted text
>After a little testing with a hacked KDC that sends back TCP responses in piec
>es with delays, I
>don't think that there is a problem with that part of the client code.
>
>The library changes I've just checked in appear to fix the select/EINTR proble
>m, so I'm going to
>mark this ticket as resolved.

Yeah, I think that I was a little aggressive in the scope of the
problem. It wasn't signal safe but on the TCP staggering, I didn't
notice a different bit of code that dealt with that.

Thanks,

--
Roland Dowdeswell http://www.Imrryr.ORG/~elric/
# Fri Jan 30 23:08:17 2009 tlyu (Taylor Yu) - Version_Fixed 1.7 added