Bug 10087 - ntlm_auth sometimes returns the wrong username to mod_ntlm_auth_winbind
Summary: ntlm_auth sometimes returns the wrong username to mod_ntlm_auth_winbind
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.0
Classification: Unclassified
Component: Tools (show other bugs)
Version: 4.0.8
Hardware: All Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-15 06:33 UTC by Man Min Yan
Modified: 2014-02-14 19:01 UTC (History)
2 users (show)

See Also:
bjacke: review+
jra: review+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Man Min Yan 2013-08-15 06:33:34 UTC
I'm using apache 2.4 (on linux) with mod_auth_ntlm_winbind to do authentication in a windows environment. I've noticed that sometimes, the username returned by mod_auth_ntlm_winbind is corrupted, even though the authentication itself succeeds. I've done some debugging and I think I found the cause. In source3/utils/ntlm_auth.c, in the function manage_gensec_request, near line 1681:

...
        } else {
            reply_code = "AF";
            reply_arg = session_info->unix_info->unix_name;
            talloc_free(session_info);
        }
...

    case GSS_SPNEGO_SERVER:
        x_fprintf(x_stdout, "%s %s %s\n", reply_code,
                 out_base64 ? out_base64 : "*",
                 reply_arg ? reply_arg : "*");

I think the problem is that session_info->unix_info->unix_name (via reply_arg) is being used after it's been freed. I've hacked the source on my system to do

char replyArgBuffer[80]; /* somewhere near the beginning of the function */
...
        } else {
            reply_code = "AF";
            strncpy(replyArgBuffer, session_info->unix_info->unix_name, sizeof(replyArgBuffer));
            reply_arg = replyArgBuffer;
            talloc_free(session_info);
        }

and that seems to have fixed the problem.
Comment 1 Alistair Leslie-Hughes 2013-09-28 00:45:31 UTC
A patch has been accepted (to master) to corrected this issue.
6bf9a774718917c3429fa1492f5b0268ae5e01c3
Comment 2 Björn Jacke 2014-01-28 23:57:53 UTC
I think we should get this in the 4.x release branches, too.

git cherry-pick -x 6bf9a774718917c3429fa1492f5b0268ae5e01c3

cleanly applies to v4-0-test and v4-1-test.
Comment 3 Jeremy Allison 2014-01-29 00:00:40 UTC
LGTM. Karolin please pick for 4.0.next and 4.1.next.
Jeremy.
Comment 4 Karolin Seeger 2014-02-05 10:16:28 UTC
Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 5 Karolin Seeger 2014-02-06 10:42:22 UTC
(In reply to comment #4)
> Pushed to autobuild-v4-1-test and autobuild-v4-0-test.

Pushed to v4-1-test.
Autobuild-v4-0-test failed, re-trying...
Comment 6 Karolin Seeger 2014-02-11 15:19:47 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
> 
> Pushed to v4-1-test.
> Autobuild-v4-0-test failed, re-trying...

Autobuild-v4-0-test failed again, re-trying...
Comment 7 Karolin Seeger 2014-02-14 19:01:57 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > (In reply to comment #4)
> > > Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
> > 
> > Pushed to v4-1-test.
> > Autobuild-v4-0-test failed, re-trying...
> 
> Autobuild-v4-0-test failed again, re-trying...

Pushed to v4-0-test.
Closing out bug report.

Thanks!