Skip Menu |
 

History Show all quoted text — Show full headers
# Fri Apr 25 09:04:43 2008 guest (Public Submitter) - Ticket created
Subject: FFM.EXAMPLE.ORG -> M.EXAMPLE.ORG broken
Download (untitled) / with headers
text/plain 589B
Hi!

In an AD multi-domain/realm environment with FFM.EXAMPLE.ORG,
EXAMPLE.ORG and M.EXAMPLE.ORG winbind with a machine principal in
FFM.EXAMPLE.ORG wants to acquire a service ticket for
pdc$@M.EXAMPLE.ORG. This fails because krb5_walk_realm_tree() is wrong
in the case FFM and M share the "M". The loop that the attached patch
fixes goes one step too far, rendering slen==0, in which case it
believes that M.EXAMPLE.ORG is a parent realm of FFM.EXAMPLE.ORG.

EXAMPLE.ORG is obviously not the right domain I could provide a log for,
this is a customer of mine.

Volker Lendecke

Samba Team
Download krb5-1.6.3-bugfix.diff
text/x-patch 546B
--- src/lib/krb5/krb/walk_rtree.c~ 2007-06-19 21:22:50.000000000 +0200
+++ src/lib/krb5/krb/walk_rtree.c 2008-04-25 14:47:11.000000000 +0200
@@ -190,6 +190,14 @@
com_sdot = scp;
nocommon = 0;
}
+ if ((slen == 1) && (ccp == com_cdot - (com_sdot - scp))) {
+ /* 1st server component shares ending with 1st client comp. */
+ break;
+ }
+ if ((clen == 1) && (scp == com_sdot - (com_cdot - ccp))) {
+ /* 1st client component shares ending with 1st server comp. */
+ break;
+ }
}

/* ccp, scp point to common root.
# Tue Jul 15 19:58:52 2008 raeburn (Ken Raeburn) - Status changed from 'new' to 'open'
# Tue Jul 15 19:58:52 2008 raeburn (Ken Raeburn) - Correspondence added
Download (untitled) / with headers
text/plain 869B
The supplied patch tests conditions which I think will always be true -- that the ccp-com_cdot
and scp-com_sdot offsets are the same. So I think it's really only breaking out of the loop when
slen and/or clen are 1 and therefore about to be decremented to 0, which would break out of
the loop, but only after the decrements of clen, slen, ccp, and scp. The patch quits the loop
without those decrements, which changes the code paths following that check for slen==0 or
clen==0.

This causes a different result if the client and server realms supplied are the same (current
code: return KRB5_NO_TKT_IN_RLM; with patch: walk up and down the realm tree).

It also causes different results if one realm is above or below the other in the hierarchy, e.g.,
A.EXAMPLE.COM and EXAMPLE.COM.

I've added a test script on the trunk that should exercise this code a bit...
# Wed Jul 16 03:47:52 2008 <Volker.Lendecke@SerNet.DE> - Correspondence added
Date: Wed, 16 Jul 2008 09:49:53 +0200
From: Volker Lendecke <Volker.Lendecke@SerNet.DE>
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #5947] FFM.EXAMPLE.ORG -> M.EXAMPLE.ORG broken
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.2KiB
On Tue, Jul 15, 2008 at 07:58:54PM -0400, Ken Raeburn via RT wrote:
Show quoted text
> The supplied patch tests conditions which I think will always be true -- that the ccp-com_cdot
> and scp-com_sdot offsets are the same. So I think it's really only breaking out of the loop when
> slen and/or clen are 1 and therefore about to be decremented to 0, which would break out of
> the loop, but only after the decrements of clen, slen, ccp, and scp. The patch quits the loop
> without those decrements, which changes the code paths following that check for slen==0 or
> clen==0.
>
> This causes a different result if the client and server realms supplied are the same (current
> code: return KRB5_NO_TKT_IN_RLM; with patch: walk up and down the realm tree).
>
> It also causes different results if one realm is above or below the other in the hierarchy, e.g.,
> A.EXAMPLE.COM and EXAMPLE.COM.
>
> I've added a test script on the trunk that should exercise this code a bit...

Sorry to reply by EMail, I don't see a way to comment on the
bug inside the trouble ticket system.

Thanks for looking at the bug. Are you saying that my patch
breaks other setups?

At my customer's site it does work, also for subrealms which
have different lengths in the subrealm part of EXAMPLE.COM.

Volker
Download (untitled)
application/pgp-signature 189B

Message body not shown because it is not plain text.

# Wed Jul 16 12:10:45 2008 raeburn (Ken Raeburn) - Correspondence added
From: Ken Raeburn <raeburn@MIT.EDU>
To: rt@krbdev.mit.edu
Subject: Re: [krbdev.mit.edu #5947] FFM.EXAMPLE.ORG -> M.EXAMPLE.ORG broken
Date: Wed, 16 Jul 2008 12:10:33 -0400
RT-Send-Cc:
Download (untitled) / with headers
text/plain 1.1KiB
On Jul 16, 2008, at 03:47, Volker.Lendecke@SerNet.DE via RT wrote:
Show quoted text
> Sorry to reply by EMail, I don't see a way to comment on the
> bug inside the trouble ticket system.

It used to be allowed, but we got too much spam through the web
forms. Unless/until we find a way around that, this is how it's going
to stay. :(

Show quoted text
> Thanks for looking at the bug. Are you saying that my patch
> breaks other setups?

Yes, it appears that way.

Show quoted text
> At my customer's site it does work, also for subrealms which
> have different lengths in the subrealm part of EXAMPLE.COM.

My tests that got wrong results were (and you can check them with the
t_walk_rtree program in lib/krb5/krb, which you'll have to build
explicitly if you haven't used "make check"):

from A.EXAMPLE.COM to EXAMPLE.COM or vice versa (with patch, went up
to COM and back down)

from EXAMPLE.COM to EXAMPLE.COM (should've returned a specific error,
instead went up to COM and back down)

Tom Yu has put together some code to replace part of the walk_rtree
string processing, perhaps his version will be clearer, as well as
correct. :-) I haven't had a chance to try plugging it in yet though.

Ken
# Tue Sep 02 18:25:51 2008 tlyu (Taylor Yu) - Taken
# Tue Sep 02 18:27:27 2008 tlyu (Taylor Yu) - Subject changed from 'FFM.EXAMPLE.ORG -> M.EXAMPLE.ORG broken' to 'krb5_walk_realm_tree broken substring logic'
# Tue Sep 02 18:27:28 2008 tlyu (Taylor Yu) - Component krb5-libs added
# Tue Sep 02 18:27:28 2008 tlyu (Taylor Yu) - Target_Version 1.7 added
# Fri Oct 24 15:33:33 2008 tlyu (Taylor Yu) - FinalPriority changed from (no value) to '75'
# Fri Oct 24 15:33:33 2008 tlyu (Taylor Yu) - Priority changed from (no value) to '75'
# Thu Jan 01 20:40:48 2009 tlyu (Taylor Yu) - Status changed from 'open' to 'review'
# Thu Jan 01 20:40:48 2009 tlyu (Taylor Yu) - Correspondence added
From: tlyu@mit.edu
Subject: SVN Commit
Download (untitled) / with headers
text/plain 355B

Rewrite walk_rtree.c to handle hierarchical traversal better and to be
less convoluted. Update test cases.

https://github.com/krb5/krb5/commit/1a56ea6a655b191ea2ecde21c57efe746d64c378
Commit By: tlyu
Revision: 21659
Changed Files:
U trunk/src/lib/krb5/krb/Makefile.in
U trunk/src/lib/krb5/krb/walk_rtree.c
U trunk/src/lib/krb5/krb/walktree-tests
# Fri Jan 02 21:05:29 2009 tlyu (Taylor Yu) - Given to raeburn (Ken Raeburn)
# Fri Jan 30 23:08:18 2009 tlyu (Taylor Yu) - Version_Fixed 1.7 added
# Sun Sep 24 20:39:51 2017 ghudson@mit.edu (Greg Hudson) - Status changed from 'review' to 'resolved'