[PATCH] winbindd: error handling in rpc_try_lookup_sids3()

[Jeremy Allison]

On Fri, Mar 31, 2017 at 10:53:30PM +0200, Ralph Böhme via samba-technical wrote:
> Hi!
> 
> Attached is another winbindd fix:
> <https://bugzilla.samba.org/show_bug.cgi?id=12728>
> 
> Please review & push if ok. Thanks!

Can I ask a question about this one please ?

NT_STATUS_SOME_NOT_MAPPED == NT_STATUS(0x107)
NT_STATUS_NONE_MAPPED == NT_STATUS(0xc0000073)

and:

#define NT_STATUS_IS_ERR(x) (unlikely((NT_STATUS_V(x) & 0xc0000000) == 0xc0000000))

So looking at the NT_STATUS_IS_ERR check we have:

NT_STATUS_IS_ERR(NT_STATUS_SOME_NOT_MAPPED) == false
NT_STATUS_IS_ERR(NT_STATUS_NONE_MAPPED) == true

i.e. Only NT_STATUS_NONE_MAPPED would return an
error here.

Can you explain why the change now treats
NT_STATUS_NONE_MAPPED as equal to NT_STATUS_SOME_NOT_MAPPED ?

Should I go look in the MS-LSA doc ? :-).

Also, the function below:

rpc_lookup_sids() has exactly the same logic as that
in rpc_try_lookup_sids3(). Should we do the same
change there ? If not, why not ?

Feel free to tell me to go read MS-LSA :-).

Cheers,

	Jeremy.

> From 2bd779b0f7b20ff7fcf5c0fb57509bcce053a62d Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Sun, 26 Mar 2017 08:22:13 +0200
> Subject: [PATCH] winbindd: error handling in rpc_try_lookup_sids3()
> 
> NT_STATUS_NONE_MAPPED and NT_STATUS_SOME_NOT_MAPPED should not be
> treated as fatal error. We should continue processing the results and
> not bail out.
> 
> Bug: https://bugzilla.samba.org/show_bug.cgi?id=12728
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/winbindd/winbindd_rpc.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
> index 3dd4f77..3763738 100644
> --- a/source3/winbindd/winbindd_rpc.c
> +++ b/source3/winbindd/winbindd_rpc.c
> @@ -981,9 +981,14 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
>  	if (!NT_STATUS_IS_OK(status)) {
>  		return status;
>  	}
> -	if (NT_STATUS_IS_ERR(result)) {
> -		return result;
> +
> +	if (!NT_STATUS_EQUAL(result, NT_STATUS_NONE_MAPPED) &&
> +	    !NT_STATUS_EQUAL(result, NT_STATUS_SOME_NOT_MAPPED)) {
> +		if (NT_STATUS_IS_ERR(result)) {
> +			return result;
> +		}
>  	}
> +
>  	if (sids->num_sids != lsa_names2.count) {
>  		return NT_STATUS_INVALID_NETWORK_RESPONSE;
>  	}
> @@ -1010,7 +1015,7 @@ static NTSTATUS rpc_try_lookup_sids3(TALLOC_CTX *mem_ctx,
>  			return NT_STATUS_INVALID_NETWORK_RESPONSE;
>  		}
>  	}
> -	return result;
> +	return NT_STATUS_OK;
>  }
>  
>  NTSTATUS rpc_lookup_sids(TALLOC_CTX *mem_ctx,
> -- 
> 2.9.3
>