[Samba] "Samba 4" - "smbd"; "can't parse the PAC: NT_STATUS_BUFFER_TOO_SMALL" error but only for a single domain user ("Server 2008 R2" domain, "Server 2008" functional level forest).

Tris Mabbs TM-Samba201302 at Firstgrade.Co.UK
Wed Feb 27 13:28:45 MST 2013


> 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 at 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 at 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 at 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 at 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 <idra at samba.org>
> 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 <idra at samba.org>
>     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.



More information about the samba mailing list