> I do so enjoy working with users who I can ask to 'put some code in' and who
> can handle this so well :-).
Why thank you, kind Sir :-)
I do so enjoy working with people who quite obviously really, REALLY, know
their subject :-)
In my case, evidence only of far too many years stuck in front of a keyboard,
I'm afraid ... Anyway, the code wasn't that good - for some reason it's not
actually replacing the '\' in any principal names - never mind, it'll do for
this purpose ...
>> That was very slightly more complicated as I wanted to use the
>> Kerberos principal name (if known) in creating the dump file name, so
>> it would be easy to work out which dump file was which.
>>
>> In leaving that code running for a while, it was perhaps of interest
>> to note that although several user accounts caused dump files to be
>> created named with their Kerberos principal name, this did not happen
>> at all for the problematic user. I'm not sure whether or not that's
>> significant, but I thought it at least worth mentioning - I'm not sure
>> what the potential code paths are into this function which may, or may
>> not, result in the principal name being known on entry ...
>
> I think it is quite significant. We should dig into this some more, once we
> sort out the PAC.
Excellent - I'm certainly up for that. Thanks.
>> ===============================================================
>> INTERNAL ERROR: Signal 11 in pid 8122 (4.1.0pre1-GIT-3e5acc1) Please
>> read the Trouble-Shooting section of the Samba HOWTO
>> ===============================================================
>> PANIC: internal error
>
> I don't get the panic, so getting a 'bt full' on that under gdb would be very
> helpful.
>
> gdb --args 'ndrdump krb5pac decode_pac' in /var/tmp/PAC-NDR-1819
As requested:
--------
Program received signal SIGSEGV, Segmentation fault.
0xfe264ccb in strlen () from /usr/lib/libc.so.1
(gdb) bt full
#0 0xfe264ccb in strlen () from /usr/lib/libc.so.1
No symbol table info available.
#1 0xfe2b1a69 in _ndoprnt () from /usr/lib/libc.so.1
No symbol table info available.
#2 0xfe2b43c6 in vprintf () from /usr/lib/libc.so.1
No symbol table info available.
#3 0xfea5c2fa in ndr_print_printf_helper () <------- Remember
this address (take 1) ...
from /var/tmp/samba/samba-master/bin/shared/libndr.so.0
No symbol table info available.
#4 0xfea55e22 in ndr_print_string ()
from /var/tmp/samba/samba-master/bin/shared/libndr.so.0
No symbol table info available.
#5 0xfeac540d in ndr_print_PAC_UPN_DNS_INFO () <------- ...
and this one (take 2) ...
from /var/tmp/samba/samba-master/bin/shared/libndr-krb5pac.so.0
No symbol table info available.
#6 0xfeac65ec in ndr_print_PAC_INFO ()
from /var/tmp/samba/samba-master/bin/shared/libndr-krb5pac.so.0
No symbol table info available.
#7 0xfeac3895 in ndr_print_PAC_BUFFER ()
from /var/tmp/samba/samba-master/bin/shared/libndr-krb5pac.so.0
No symbol table info available.
#8 0xfeac6bf9 in ndr_print_PAC_DATA ()
from /var/tmp/samba/samba-master/bin/shared/libndr-krb5pac.so.0
No symbol table info available.
#9 0xfeac8240 in ndr_print_decode_pac ()
from /var/tmp/samba/samba-master/bin/shared/libndr-krb5pac.so.0
No symbol table info available.
#10 0x080533d2 in main ()
No symbol table info available.
(gdb)
--------
That's not particularly helpful is it ... Not sure where the symbol table has
gone:
--------
% file ndrdump
ndrdump: ELF 32-bit LSB executable 80386 Version 1 [FPU], dynamically
linked, not stripped
%
--------
(also not sure why it's a 32-bit image on a 64-bit system, so something
somewhere seems not to have figured out the appropriate architecture - probably
not significant, but I might try and look into that sometime; anyway, a 32-bit
image should be fine for this executable).
So, next best thing is a dump of that address:
--------
(gdb) disas 0xfea5c2fa
Dump of assembler code for function ndr_print_printf_helper:
0xfea5c29f <ndr_print_printf_helper+0>: push %ebp
0xfea5c2a0 <ndr_print_printf_helper+1>: mov %esp,%ebp
0xfea5c2a2 <ndr_print_printf_helper+3>: push %ebx
0xfea5c2a3 <ndr_print_printf_helper+4>: sub $0x14,%esp
0xfea5c2a6 <ndr_print_printf_helper+7>: call 0xfea5c2ab
<ndr_print_printf_helper+12>
0xfea5c2ab <ndr_print_printf_helper+12>: pop %ebx
0xfea5c2ac <ndr_print_printf_helper+13>: add $0x161c1,%ebx
0xfea5c2b2 <ndr_print_printf_helper+19>: mov 0x8(%ebp),%eax
0xfea5c2b5 <ndr_print_printf_helper+22>: cmpb $0x0,0x14(%eax)
0xfea5c2b9 <ndr_print_printf_helper+26>: jne 0xfea5c2e6
<ndr_print_printf_helper+71>
0xfea5c2bb <ndr_print_printf_helper+28>: movl $0x0,-0xc(%ebp)
0xfea5c2c2 <ndr_print_printf_helper+35>: mov 0x8(%ebp),%eax
0xfea5c2c5 <ndr_print_printf_helper+38>: mov 0x4(%eax),%eax
0xfea5c2c8 <ndr_print_printf_helper+41>: cmp -0xc(%ebp),%eax
0xfea5c2cb <ndr_print_printf_helper+44>: jbe 0xfea5c2e6
<ndr_print_printf_helper+71>
0xfea5c2cd <ndr_print_printf_helper+46>: sub $0xc,%esp
0xfea5c2d0 <ndr_print_printf_helper+49>: lea -0x10fe3(%ebx),%eax
0xfea5c2d6 <ndr_print_printf_helper+55>: push %eax
0xfea5c2d7 <ndr_print_printf_helper+56>: call 0xfea54d38 <printf@plt>
0xfea5c2dc <ndr_print_printf_helper+61>: add $0x10,%esp
0xfea5c2df <ndr_print_printf_helper+64>: lea -0xc(%ebp),%eax
0xfea5c2e2 <ndr_print_printf_helper+67>: incl (%eax)
0xfea5c2e4 <ndr_print_printf_helper+69>: jmp 0xfea5c2c2
<ndr_print_printf_helper+35>
0xfea5c2e6 <ndr_print_printf_helper+71>: lea 0x10(%ebp),%eax
0xfea5c2e9 <ndr_print_printf_helper+74>: mov %eax,-0x8(%ebp)
0xfea5c2ec <ndr_print_printf_helper+77>: sub $0x8,%esp
0xfea5c2ef <ndr_print_printf_helper+80>: pushl -0x8(%ebp)
0xfea5c2f2 <ndr_print_printf_helper+83>: pushl 0xc(%ebp)
0xfea5c2f5 <ndr_print_printf_helper+86>: call 0xfea54d48 <vprintf@plt>
0xfea5c2fa <ndr_print_printf_helper+91>: add $0x10,%esp
<------- Here it is (take 1) ...
0xfea5c2fd <ndr_print_printf_helper+94>: mov 0x8(%ebp),%eax
0xfea5c300 <ndr_print_printf_helper+97>: cmpb $0x0,0x14(%eax)
0xfea5c304 <ndr_print_printf_helper+101>: jne 0xfea5c318
<ndr_print_printf_helper+121>
0xfea5c306 <ndr_print_printf_helper+103>: sub $0xc,%esp
0xfea5c309 <ndr_print_printf_helper+106>: lea -0x10fda(%ebx),%eax
0xfea5c30f <ndr_print_printf_helper+112>: push %eax
0xfea5c310 <ndr_print_printf_helper+113>: call 0xfea54d38 <printf@plt>
0xfea5c315 <ndr_print_printf_helper+118>: add $0x10,%esp
0xfea5c318 <ndr_print_printf_helper+121>: mov -0x4(%ebp),%ebx
0xfea5c31b <ndr_print_printf_helper+124>: leave
0xfea5c31c <ndr_print_printf_helper+125>: ret
End of assembler dump.
(gdb)
--------
A quick look at the function itself, and unsurprisingly the problem obviously
isn't here - all the function does is print out indentation then "vprintf()"
its' arguments.
So where did those arguments come from? Hmmmm...
Given what you said below about the codeset which introduced the problem,
looking at "ndr_print_PAC_UPN_DNS_INFO ()" seems promising:
--------
(gdb) disas 0xfeac540d
Dump of assembler code for function ndr_print_PAC_UPN_DNS_INFO:
0xfeac5289 <ndr_print_PAC_UPN_DNS_INFO+0>: push %ebp
0xfeac528a <ndr_print_PAC_UPN_DNS_INFO+1>: mov %esp,%ebp
0xfeac528c <ndr_print_PAC_UPN_DNS_INFO+3>: push %ebx
0xfeac528d <ndr_print_PAC_UPN_DNS_INFO+4>: sub $0x14,%esp
0xfeac5290 <ndr_print_PAC_UPN_DNS_INFO+7>: call 0xfeac5295
<ndr_print_PAC_UPN_DNS_INFO+12>
0xfeac5295 <ndr_print_PAC_UPN_DNS_INFO+12>: pop %ebx
0xfeac5296 <ndr_print_PAC_UPN_DNS_INFO+13>: add $0x145df,%ebx
0xfeac529c <ndr_print_PAC_UPN_DNS_INFO+19>: sub $0x4,%esp
...
0xfeac53f5 <ndr_print_PAC_UPN_DNS_INFO+364>: sub $0x4,%esp
0xfeac53f8 <ndr_print_PAC_UPN_DNS_INFO+367>: mov 0x10(%ebp),%eax
0xfeac53fb <ndr_print_PAC_UPN_DNS_INFO+370>: pushl 0x14(%eax)
0xfeac53fe <ndr_print_PAC_UPN_DNS_INFO+373>: lea -0x10845(%ebx),%eax
0xfeac5404 <ndr_print_PAC_UPN_DNS_INFO+379>: push %eax
0xfeac5405 <ndr_print_PAC_UPN_DNS_INFO+380>: pushl 0x8(%ebp)
0xfeac5408 <ndr_print_PAC_UPN_DNS_INFO+383>: call 0xfeac2b98
<ndr_print_string@plt>
0xfeac540d <ndr_print_PAC_UPN_DNS_INFO+388>: add $0x10,%esp
<------- Here it is (take 2) ...
0xfeac5410 <ndr_print_PAC_UPN_DNS_INFO+391>: mov 0x8(%ebp),%eax
0xfeac5413 <ndr_print_PAC_UPN_DNS_INFO+394>: decl 0x4(%eax)
0xfeac5416 <ndr_print_PAC_UPN_DNS_INFO+397>: mov -0x4(%ebp),%ebx
0xfeac5419 <ndr_print_PAC_UPN_DNS_INFO+400>: leave
0xfeac541a <ndr_print_PAC_UPN_DNS_INFO+401>: ret
End of assembler dump.
(gdb)
--------
OK, so it's dying in:
--------
ndr_print_string(ndr, "domain_name", r->domain_name);
--------
So let's see what's *actually* in "r->domain_name":
A trivial hack ...
--------
*** ndr_krb5pac-ORIG.c Wed Feb 27 14:25:10 2013
--- ndr_krb5pac.c Wed Feb 27 14:49:49 2013
***************
*** 351,357 ****
--- 351,411 ----
ndr_print_upn_dns_info_flags(ndr, "flags", r->flags);
ndr_print_uint32(ndr, "padding", r->padding);
ndr_print_string(ndr, "upn_name", r->upn_name);
+ #if 0
ndr_print_string(ndr, "domain_name", r->domain_name);
+ #else /* 0 */
+ /* ##########################################################################
*/
+ /*
+ * Quick hack: Dump out the domain name as a length constrained string, re-
+ * placing any non-printables with "\xXX" (4 char.s). We'll go with a len-
+ * gth of 20 as that's more than enough for our domain name in this test...
+ */
+ {
+ int i, j;
+ char wombat[(20 << 2)+1];
+
+ j = sizeof (wombat) - 1;
+ if (r->domain_name == (char *) 0)
+ {
+ (void) strncpy (wombat,
+ (const char *) "<aadvark>",
+ (size_t) j);
+ wombat[j] = '\0';
+ } else
+ {
+ char *s, *t;
+
+ for (i = j >> 2,
+ s = wombat,
+ t = r->domain_name;
+ i > 0;
+ i--,
+ j--,
+ s++,
+ t++)
+ {
+ char c;
+
+ c = t[0];
+ if (isprnt (c))
+ {
+ s[0] = c;
+ } else
+ {
+ (void) snprintf (s,
+ (size_t) j,
+ "\\x%02X",
+ (((int) c) & 0x00ff));
+ j -= (4 - 1);
+ s += (4 - 1);
+ }
+ }
+ s[0] = '\0';
+ }
+ ndr_print_string(ndr, "domain_name", wombat);
+ }
+ /* ##########################################################################
*/
+ #endif /* 0 */
ndr->depth--;
}
--------
... and a CC (Compilation Coffee) later, and we get ...
... the sinking realisation that you can't just modify that 'C' source file, as
it's automatically built from the PAC structure definition file.
Smooth - nice coding - like it ...
After a little digging around to see whether I could easily inject the
additional code into the automatic build, I gave up and went back to look again
at the "ndrdump" output:
--------
buffers: struct PAC_BUFFER
type : PAC_TYPE_UPN_DNS_INFO (12)
_ndr_size : 0x00000058 (88)
info : *
info : union PAC_INFO(case 12)
upn_dns_info: struct PAC_UPN_DNS_INFO
upn_size : 0x0028 (40)
upn_offset : 0x0010 (16)
domain_size : 0x0020 (32)
domain_offset : 0x0038 (56)
flags : 0x00000000 (0)
0: UDI_ACCT_HAS_NO_UPN
padding : 0x00000000 (0)
upn_name :
'dmz<at>Firstgrade.Co.UKF'
===============================================================
INTERNAL ERROR: Signal 11 in pid 4217 (4.1.0pre1-DEVELOPERBUILD)
Please read the Trouble-Shooting section of the Samba HOWTO
===============================================================
PANIC: internal error
Abort (core dumped)
--------
Hel-looooow...
What's with the 'F' on the end of "dmz<at>Firstgrade.Co.UK" ("upn_name") - that
shouldn't be there. Don't know why I didn't notice that before - sorry ...
A quick look at the decode code would seem to suggest that the PAC structures
are padded, if needed, to cause alignment.
So I'm guessing, and this is a *complete* guess:
The length of the UPN is exactly 20 characters;
So there's no padding required;
The length decode is going wrong somewhere as a result (maybe allowing
for padding which isn't there);
So the decode of the UPN is finishing at the wrong place;
So the decode of the DNS domain name is starting at the wrong place;
So it all goes south, badly and rapidly - subsequent data is being
decoded as lengths, and everything ends up in a heap.
I think. Maybe.
That would explain why it's just this one user - none of our other usernames,
when combined with the DNS domain name (I.e., their UPN), hit a length which
would trip the same hypothetical padding decoding fault.
It doesn't explain why renaming the user didn't eliminate the problem, unless
"Windows" AD keeps some internal UPN history and somehow returns the user's
original UPN in the PAC even if their account has been renamed (is there
perhaps a "Previous UPN(s)" optional block in the PAC?). That could probably
be established with relatively little digging, but is perhaps largely a moot
point.
> Even a bad decode should never crash Samba, so something is going quite wrong
> here, hopefully only in a manual parser.
Quite right. But if I'm right, what's happening here is that what is expected
to be a NULL-terminated string isn't being NULL-terminated, because of an
incorrect length decode. So the string (when "ndrdump" is run on this machine)
is wandering off into the wild blue yonder, and trying to "vprintf()" its'
contents as a string is going to cause havoc. I don't know how you ensure
strings are always NULL-terminated (so the bad decode shouldn't crash Samba)
but if the lengths are being decoded incorrectly then that terminating NULL
could have been written literally anywhere, possibly even before the start of
the string (if someone forgot to make the length unsigned, but I don't consider
that likely given the general extremely high quality of the code).
If that is the case, it was always going to end in tears ...
That would also potentially explain why I get a SIGSEGV and you don't - perhaps
the way memory had previously been used on your machine (could be different
libraries, different executable because of being on a different OS - I doubt
you're running "OpenSolaris" on your big server, different architecture,
different phase-of-the-moon ...) meant there happened to be a convenient NULL
lying around in the memory block into which the DNS domain name was decoded -
you might have received garbage in the output, but at least not a large and
obnoxious SIGSEGV hitting you in the back.
> I got it, and I ran an automated git bisect on our big server. The problem
> commit is apparently:
>
> a6be8a97f705247c1b1cbb0595887d8924740a71 is the first bad commit commit
> a6be8a97f705247c1b1cbb0595887d8924740a71
> Author: Simo Sorce <[email protected]>
> Date: Thu Sep 27 14:12:06 2012 -0400
>
> Support UPN_DNS_INFO in the PAC
>
> Previously marked as UNKNOWN_12 the UPN_DNS_INFO is defined in MS-PAC
>
> Autobuild-User(master): Simo Sorce <[email protected]>
> Autobuild-Date(master): Fri Sep 28 01:13:44 CEST 2012 on sn-devel-104
Oh you *so* have to love "git bisect" ...
I'm an old SCCS die-hard, who has (recently and reluctantly) been forced into
using SVN. Now I see what "git" is capable of, I might just take a leap into
the 21st century ...
> I've CC'ed Simo who made the change, and GD who is working on improving our
> PAC handling (he has a series of patches
> under development, which may already solve your issue). Hopefully they can
> sort things out from here.
Excellent - hopefully the information above is of use then. Thanks - much
appreciated.
> If you can contribute your PAC to the public realm (the PAC itself does not
> contain passwords, but you will need to read the full dump
> to determine if any other details might be a problem), then I know it will be
> very much appreciated, as we can add tests to show that
> these exotic samples continue to work.
>From what I can see (can't, of course, read the dump past the SIGSEGV :-),
>there's not anything in there about which particularly to be concerned.
OK, so normally I don't even like letting an e-mail address out into the wild,
but that's just because I'm paranoid (What, me? Paranoid? Who said that?!
...).
So yes, do please feel free to consider the dump contributed to the public
domain as a future potential test case.
Of course, if it is that the UPN length trips a padding problem, you can
probably much more simply generate a test PAC. However you're still welcome to
the one I sent you anyway.
> Thanks,
Likewise!
> Andrew Bartlett
Tris Mabbs.
--
To unsubscribe from this list go to the following URL and read the
instructions: https://lists.samba.org/mailman/options/samba
