[PATCH] Add missing KDC "status" on some fail conditions

[Samuel Cabrero]

Hi Greg,

yes, I triggered the assertion by testing samba 4.7rc1 built to use MIT 
instead bundled Heimdal.

Once a Windows 7 machine is joined to the domain, every time it powers 
up sends a TGS-REQ which triggers it. This is the backtrace:

#0  __GI_raise (sig=sig at entry=6) at 
../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007f16a602d0c6 in __GI_abort () at abort.c:78
#2  0x00007f16a602471a in __assert_fail_base (fmt=0x7f16a6163e80 
"%s%s%s:%u: %s%sAssertion `%s' failed.\n%n",
    assertion=assertion at entry=0x55a129c524f6 "status != NULL", 
file=file at entry=0x55a129c523b8 "do_tgs_req.c",
    line=line at entry=826, function=function at entry=0x55a129c525d0 
<__PRETTY_FUNCTION__.13702> "process_tgs_req")
    at assert.c:92
#3  0x00007f16a6024792 in __GI___assert_fail 
(assertion=assertion at entry=0x55a129c524f6 "status != NULL",
    file=file at entry=0x55a129c523b8 "do_tgs_req.c", line=line at entry=826,
    function=function at entry=0x55a129c525d0 <__PRETTY_FUNCTION__.13702> 
"process_tgs_req") at assert.c:101
#4  0x000055a129c41212 in process_tgs_req 
(handle=handle at entry=0x55a129e592c0 <shandle>,
    pkt=pkt at entry=0x55a12b325b38, from=from at entry=0x55a12be8a728, 
response=response at entry=0x7ffe74820f18)
    at do_tgs_req.c:826
#5  0x000055a129c3eb5e in dispatch (cb=0x55a129e592c0 <shandle>, 
local_saddr=<optimized out>,
    from=from at entry=0x55a12be8a728, pkt=pkt at entry=0x55a12b325b38, 
is_tcp=is_tcp at entry=1,
    vctx=vctx at entry=0x55a12b0307e0, respond=0x55a129c4fd60 
<process_tcp_response>, arg=0x55a12b325ab0)
    at dispatch.c:180
#6  0x000055a129c50019 in process_tcp_connection_read 
(ctx=0x55a12b0307e0, ev=0x55a12bc59cd0) at net-server.c:1409
#7  0x00007f16a639fcb8 in verto_fire () from /usr/lib64/libverto.so.1
#8  0x00007f168e472736 in ev_invoke_pending (loop=0x7f168e67ca40 
<default_loop_struct>) at ev.c:3288
#9  0x00007f168e475d48 in ev_run (loop=0x7f168e67ca40 
<default_loop_struct>, flags=0) at ev.c:3688
#10 0x000055a129c3d98f in main (argc=2, argv=0x7ffe74821288) at 
main.c:1065

And the involved data structures:

(gdb) p *request->server
$10 = {magic = 0, realm = {magic = 0, length = 7, data = 0x55a12afa88f0 
"SUSE.AD"}, data = 0x55a12afa8770,
  length = 1, type = 10}
(gdb) p *header_ticket->enc_part2->client
$11 = {magic = 0, realm = {magic = 0, length = 7, data = 0x55a12b3ab880 
"SUSE.AD"}, data = 0x55a12be0ab40,
  length = 1, type = 0}
(gdb) p *header_ticket->enc_part2->client->data
$12 = {magic = 0, length = 5, data = 0x55a12afaa970 "WIN7$U"}
(gdb) p *request->server->data
$13 = {magic = 0, length = 13, data = 0x55a12afa8a90 "win7$@SUSE.ADU"}
(gdb) printf "%x", request->kdc_options
40830000
(gdb) print *request
$14 = {magic = 0, msg_type = 12, padata = 0x55a12afadc00, kdc_options = 
1082327040, client = 0x0,
  server = 0x55a12b7eb890, from = 0, till = 1499852047, rtime = 0, 
nonce = 1809206369, nktypes = 5,
  ktype = 0x55a12afa8cd0, addresses = 0x0, authorization_data = {magic 
= 0, enctype = 0, kvno = 0, ciphertext = {
      magic = 0, length = 0, data = 0x0}}, unenc_authdata = 0x0, 
second_ticket = 0x55a12afa8b50}

I have traced it down and the function kdc_process_s4u2proxy_req 
returns KRB5KDC_ERR_SERVER_NOMATCH without setting the status.

After your comment, this second version version of the patch is more 
convenient as it respect the status if already set.

Also, just for reference, I have attached network captures with and 
without patch.

Samuel Cabrero / SUSE Labs Samba Team
GPG: D7D6 E259 F91C F0B3 2E61 1239 3655 6EC9 7051 0856
scabrero at suse.com
scabrero at suse.de

On mar, jul 11, 2017 at 9:20 , Greg Hudson <ghudson at mit.edu> wrote:
> On 07/11/2017 01:48 PM, Samuel Cabrero wrote:
>>  The process_tgs_req function requires the status string to be set
>>  for all fail conditions before jump to "cleanup" label, otherwise
>>  it will cause an assertion failure.
> 
> In all of these cases, the status string is supposed to be set by the
> subsidiary function, and as far as I know always is.  Did you run 
> into a
> case where the assertion failure triggered?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-missing-KDC-status-on-some-fail-conditions.patch
Type: text/x-patch
Size: 4132 bytes
Desc: not available
Url : http://mailman.mit.edu/pipermail/krbdev/attachments/20170712/19214c91/attachment.bin