[PATCHES] smbd: support NTLM pass-through with \user at realm credentials

Mon Oct 24 21:18:23 UTC 2016

On Sun, Oct 23, 2016 at 10:10:01AM +0300, Uri Simchoni wrote:
> Hi,
> 
> Following previous comments, attached pls find a proposed patch set.
> 
> - user mapping indeed should not be affected
> - the winbindd fix covers a wider set of cases, hopefully it doesn't
> break anything (passes make test)
> - although similar fixes to client side are underway, smbtorture (s4
> client lib) already supports generating such credentials, so the test is
> simple. The bug lists an existing smbtorture command that passes against
> Windows with user at realm credentials.
> 
> Review appreciated.

LGTM. Pushed - thanks !

> From 6dc2f2796e96ddb7b010c86cea07a1499ba284a1 Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Sat, 22 Oct 2016 13:33:42 +0300
> Subject: [PATCH v2 1/3] selftest: test NTLM user at realm authentication
> 
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
>  selftest/knownfail        | 2 ++
>  source3/selftest/tests.py | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 976761b..e3090f4 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -294,3 +294,5 @@
>  #ntvfs server blocks copychunk with execute access on read handle
>  ^samba4.smb2.ioctl.copy_chunk_bad_access
>  ^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
> +#ad_member does not support for user at realm NTLM authentication
> +^samba3.unix.whoami ntlm user at realm.*\(ad_member
> diff --git a/source3/selftest/tests.py b/source3/selftest/tests.py
> index ff61714..eaaeff1 100755
> --- a/source3/selftest/tests.py
> +++ b/source3/selftest/tests.py
> @@ -345,6 +345,7 @@ for t in tests:
>              plansmbtorture4testsuite(t, env, '//$SERVER/tmp -U$DC_USERNAME@$REALM%$DC_PASSWORD --option=torture:addc=$DC_SERVER')
>              plansmbtorture4testsuite(t, env, '//$SERVER/tmp -k yes -U$DC_USERNAME@$REALM%$DC_PASSWORD --option=torture:addc=$DC_SERVER', description='kerberos connection')
>              plansmbtorture4testsuite(t, env, '//$SERVER/tmpguest -U% --option=torture:addc=$DC_SERVER', description='anonymous connection')
> +            plansmbtorture4testsuite(t, env, '//$SERVER/tmp -k no -U$DC_USERNAME@$REALM%$DC_PASSWORD', description='ntlm user at realm')
>      elif t == "raw.samba3posixtimedlock":
>          plansmbtorture4testsuite(t, "nt4_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/nt4_dc/share')
>          plansmbtorture4testsuite(t, "ad_dc", '//$SERVER_IP/tmpguest -U$USERNAME%$PASSWORD --option=torture:localdir=$SELFTEST_PREFIX/ad_dc/share')
> -- 
> 2.9.3
> 
> 
> From 945605f77f2dd4de4e8cc22bd34a901668f81aea Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Sat, 22 Oct 2016 22:40:26 +0300
> Subject: [PATCH v2 2/3] winbindd: do not modify credentials in NTLM
>  passthrough
> 
> When doing NTLM validation of credentials, do not modify the
> credentials - they might be used in the calculation of
> the response.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12375
> 
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
>  source3/winbindd/winbindd_pam_auth_crap.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/winbindd/winbindd_pam_auth_crap.c b/source3/winbindd/winbindd_pam_auth_crap.c
> index ffbc322..e6a47c8 100644
> --- a/source3/winbindd/winbindd_pam_auth_crap.c
> +++ b/source3/winbindd/winbindd_pam_auth_crap.c
> @@ -37,6 +37,7 @@ struct tevent_req *winbindd_pam_auth_crap_send(
>  	struct tevent_req *req, *subreq;
>  	struct winbindd_pam_auth_crap_state *state;
>  	struct winbindd_domain *domain;
> +	const char *auth_domain = NULL;
>  
>  	req = tevent_req_create(mem_ctx, &state,
>  				struct winbindd_pam_auth_crap_state);
> @@ -77,14 +78,12 @@ struct tevent_req *winbindd_pam_auth_crap_send(
>  		return tevent_req_post(req, ev);
>  	}
>  
> -	if ((request->data.auth_crap.domain[0] == '\0')
> -	    && lp_winbind_use_default_domain()) {
> -		fstrcpy(request->data.auth_crap.domain,
> -			lp_workgroup());
> +	auth_domain = request->data.auth_crap.domain;
> +	if (auth_domain[0] == '\0') {
> +		auth_domain = lp_workgroup();
>  	}
>  
> -	domain = find_auth_domain(
> -		request->flags, request->data.auth_crap.domain);
> +	domain = find_auth_domain(request->flags, auth_domain);
>  	if (domain == NULL) {
>  		tevent_req_nterror(req, NT_STATUS_NO_SUCH_USER);
>  		return tevent_req_post(req, ev);
> -- 
> 2.9.3
> 
> 
> From ca4762446499e6090e75e704a8bc90156879e5ac Mon Sep 17 00:00:00 2001
> From: Uri Simchoni <uri at samba.org>
> Date: Sat, 22 Oct 2016 22:47:08 +0300
> Subject: [PATCH v2 3/3] smbd: in ntlm auth, do not map empty domain in case of
>  \user at realm
> 
> When mapping user and domain during NTLM authentication, an empty domain
> is mapped to the local SAM db. However, an empty domain may legitimately
> be used if the user field has both user and domain in upn at realm format.
> 
> BUG: https://bugzilla.samba.org/show_bug.cgi?id=12375
> 
> Signed-off-by: Uri Simchoni <uri at samba.org>
> ---
>  selftest/knownfail       |  2 --
>  source3/auth/auth_util.c | 10 +++++++---
>  2 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/selftest/knownfail b/selftest/knownfail
> index e3090f4..976761b 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -294,5 +294,3 @@
>  #ntvfs server blocks copychunk with execute access on read handle
>  ^samba4.smb2.ioctl.copy_chunk_bad_access
>  ^samba4.drs.getnc_exop.python.*getnc_exop.DrsReplicaPrefixMapTestCase.test_regular_prefix_map_ex_attid.*
> -#ad_member does not support for user at realm NTLM authentication
> -^samba3.unix.whoami ntlm user at realm.*\(ad_member
> diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c
> index 5473fa2..2da2896 100644
> --- a/source3/auth/auth_util.c
> +++ b/source3/auth/auth_util.c
> @@ -107,6 +107,11 @@ NTSTATUS make_user_info_map(TALLOC_CTX *mem_ctx,
>  	NTSTATUS result;
>  	bool was_mapped;
>  	char *internal_username = NULL;
> +	bool upn_form = false;
> +
> +	if (client_domain[0] == '\0' && strchr(smb_name, '@')) {
> +		upn_form = true;
> +	}
>  
>  	was_mapped = map_username(talloc_tos(), smb_name, &internal_username);
>  	if (!internal_username) {
> @@ -126,10 +131,9 @@ NTSTATUS make_user_info_map(TALLOC_CTX *mem_ctx,
>  	 * non-domain member box will also map to WORKSTATION\user.
>  	 * This also deals with the client passing in a "" domain */
>  
> -	if (!is_trusted_domain(domain) &&
> +	if (!upn_form && !is_trusted_domain(domain) &&
>  	    !strequal(domain, my_sam_name()) &&
> -	    !strequal(domain, get_global_sam_name()))
> -	{
> +	    !strequal(domain, get_global_sam_name())) {
>  		if (lp_map_untrusted_to_domain())
>  			domain = my_sam_name();
>  		else
> -- 
> 2.9.3
>