[PATCH] winbindd: error handling in rpc_try_lookup_sids3()

[Jim Brown]

What about changing
     NT_STATUS_NONE_MAPPED == NT_STATUS(0xc0000073)
to
     NT_STATUS_NONE_MAPPED == NT_STATUS(0x73)
to not be an error (like NT_STATUS_SOME_NOT_MAPPED)?

On 4/1/17 08:21, Ralph Böhme via samba-technical wrote:
> On Fri, Mar 31, 2017 at 04:08:36PM -0700, Jeremy Allison wrote:
>> On Fri, Mar 31, 2017 at 10:53:30PM +0200, Ralph Böhme via samba-technical wrote:
>>> Hi!
>>>
>>> Attached is another winbindd fix:
>>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.samba.org_show-5Fbug.cgi-3Fid-3D12728&d=DwIDaQ&c=y2w-uYmhgFWijp_IQN0DhA&r=Ru1enaR19oK8d7j6CTyYvKyHRw2ZzjrSu2PJyn0fKAQ&m=O33NyuJ35pSENWKa-5d_aJBTvKCueC85OiaFLTxDUd0&s=cYKQBp3QB0b__fr72QrTT-ZxgX7vAGhTS22iyY9QjOc&e= >
>>>
>>> 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.
> yes, but it shouldn't.
>
>> Can you explain why the change now treats
>> NT_STATUS_NONE_MAPPED as equal to NT_STATUS_SOME_NOT_MAPPED ?
> Because it *is* equal. :) The only difference is that with NT_STATUS_NONE_MAPPED
> count will be 0 and with NT_STATUS_SOME_NOT_MAPPED count will be at least 1.
>
> Because even with NT_STATUS_NONE_MAPPED we get a lsa_TransNameArray that we
> should process and return to our caller. Eg, running this against a Windows 2016
> server:
>
> $ sudo ./bin/rpcclient -U Administrator%Passw0rd 10.10.11.101 -c "lookupsids S-1-5-21-666" -d 10
> ---8<---
>       lsa_LookupSids: struct lsa_LookupSids
>          in: struct lsa_LookupSids
>              handle                   : *
>                  handle: struct policy_handle
>                      handle_type              : 0x00000000 (0)
>                      uuid                     : f8df0a26-ee79-4c1f-91e2-b56fb8269b44
>              sids                     : *
>                  sids: struct lsa_SidArray
>                      num_sids                 : 0x00000001 (1)
>                      sids                     : *
>                          sids: ARRAY(1)
>                              sids: struct lsa_SidPtr
>                                  sid                      : *
>                                      sid                      : S-1-5-21-666
>
> ...
>
>          out: struct lsa_LookupSids
>              domains                  : *
>                  domains                  : *
>                      domains: struct lsa_RefDomainList
>                          count                    : 0x00000000 (0)
>                          domains                  : NULL
>                          max_size                 : 0x00000000 (0)
>              names                    : *
>                  names: struct lsa_TransNameArray
>                      count                    : 0x00000001 (1)
>                      names                    : *
>                          names: ARRAY(1)
>                              names: struct lsa_TranslatedName
>                                  sid_type                 : SID_NAME_UNKNOWN (8)
>                                  name: struct lsa_String
>                                      length                   : 0x0018 (24)
>                                      size                     : 0x001a (26)
>                                      string                   : *
>                                          string                   : 'S-1-5-21-666'
>                                  sid_index                : 0xffffffff (4294967295)
>              count                    : *
>                  count                    : 0x00000000 (0)
>              result                   : NT_STATUS_NONE_MAPPED
> LSA_LOOKUPSIDS returned status: 'NT_STATUS_OK', result: 'NT_STATUS_NONE_MAPPED', mapped count = 0'
> ---8<---
>
> So we got a lsa_TransNameArray and we must treat this the same way we handle a
> failed mapping in the NT_STATUS_SOME_NOT_MAPPED case.
>
>> Should I go look in the MS-LSA doc ? :-).
> 3.1.4.9 at the very end:
>
> If LookupLevel is LsapLookupWksta, and the return code can be identified as an
> error value other than STATUS_NONE_MAPPED, ReferencedDomains and the Names
> fields in the TranslatedNames structure MUST NOT be returned.
>
>> 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 ?
> Yes, there are more places where it's done wrong: winbindd_lookup_sids() and
> dcerpc_lsa_lookup_sids_noalloc().
>
> Let's see what happens when I try to fix those as well... :)
>
> Cheerio!
> -slow
>