[PATCH] fix dom_sid_in_domain()

[Jeremy Allison]

On Tue, Apr 11, 2017 at 02:15:27PM +0200, Ralph Böhme via samba-technical wrote:
> Hi!
> 
> Attached is a fix for dom_sid_in_domain(). It just passed a private autobuild.
> 
> Already carries two reviews, will push later if noone objects.

Oh amazingly good catch. How did you find that one ?

RB+.


> From c56e868b722cf1c9c0a62d189d3adb8833331734 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Mon, 10 Apr 2017 16:14:45 +0200
> Subject: [PATCH] libcli/security: fix dom_sid_in_domain()
> 
> Ensure the SID has exactly one component more then the domain SID, eg
> 
> Domain SID: S-1-5-21-1-2-3
> SID:        S-1-5-21-1-2-3-4
> 
> This will return true. If the SID has more components, eg
> 
> SID: S-1-5-21-1-2-3-4-5, or
> SID: S-1-5-21-1-2-3-4-5-6-7-8
> 
> dom_sid_in_domain() must return false.
> 
> This was verified against Windows:
> 
>      lsa_LookupSids: struct lsa_LookupSids
>         out: struct lsa_LookupSids
>             domains                  : *
>                 domains                  : *
>                     domains: struct lsa_RefDomainList
>                         count                    : 0x00000002 (2)
>                         domains                  : *
>                             domains: ARRAY(2)
>                                 domains: struct lsa_DomainInfo
>                                     name: struct lsa_StringLarge
>                                         length                   : 0x000e (14)
>                                         size                     : 0x0010 (16)
>                                         string                   : *
>                                             string                   : 'BUILTIN'
>                                     sid                      : *
>                                         sid                      : S-1-5-32
>                                 domains: struct lsa_DomainInfo
>                                     name: struct lsa_StringLarge
>                                         length                   : 0x0012 (18)
>                                         size                     : 0x0014 (20)
>                                         string                   : *
>                                             string                   : 'W4EDOM-L4'
>                                     sid                      : *
>                                         sid                      : S-1-5-21-278041429-3399921908-1452754838
>                         max_size                 : 0x00000020 (32)
>             names                    : *
>                 names: struct lsa_TransNameArray
>                     count                    : 0x00000004 (4)
>                     names                    : *
>                         names: ARRAY(4)
>                             names: struct lsa_TranslatedName
>                                 sid_type                 : SID_NAME_USER (1)
>                                 name: struct lsa_String
>                                     length                   : 0x001a (26)
>                                     size                     : 0x001a (26)
>                                     string                   : *
>                                         string                   : 'Administrator'
>                                 sid_index                : 0x00000001 (1)
>                             names: struct lsa_TranslatedName
>                                 sid_type                 : SID_NAME_UNKNOWN (8)
>                                 name: struct lsa_String
>                                     length                   : 0x005c (92)
>                                     size                     : 0x005e (94)
>                                     string                   : *
>                                         string                   : 'S-1-5-21-278041429-3399921908-1452754838-500-1'
>                                 sid_index                : 0xffffffff (4294967295)
>                             names: struct lsa_TranslatedName
>                                 sid_type                 : SID_NAME_ALIAS (4)
>                                 name: struct lsa_String
>                                     length                   : 0x001c (28)
>                                     size                     : 0x001c (28)
>                                     string                   : *
>                                         string                   : 'Administrators'
>                                 sid_index                : 0x00000000 (0)
>                             names: struct lsa_TranslatedName
>                                 sid_type                 : SID_NAME_UNKNOWN (8)
>                                 name: struct lsa_String
>                                     length                   : 0x001c (28)
>                                     size                     : 0x001e (30)
>                                     string                   : *
>                                         string                   : 'S-1-5-32-544-9'
>                                 sid_index                : 0xffffffff (4294967295)
>             count                    : *
>                 count                    : 0x00000002 (2)
>             result                   : STATUS_SOME_UNMAPPED
> 
> Pair-Programmed-With: Stefan Metzmacher <metze at samba.org>
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> Signed-off-by: Stefan Metzmacher <metze at samba.org>
> ---
>  libcli/security/dom_sid.c | 6 +++++-
>  selftest/knownfail        | 1 -
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
> index 5454c51..e6beff1 100644
> --- a/libcli/security/dom_sid.c
> +++ b/libcli/security/dom_sid.c
> @@ -341,7 +341,11 @@ bool dom_sid_in_domain(const struct dom_sid *domain_sid,
>  		return false;
>  	}
>  
> -	if (domain_sid->num_auths > sid->num_auths) {
> +	if (sid->num_auths < 2) {
> +		return false;
> +	}
> +
> +	if (domain_sid->num_auths != (sid->num_auths - 1)) {
>  		return false;
>  	}
>  
> diff --git a/selftest/knownfail b/selftest/knownfail
> index 07b4cdb..9c3138e 100644
> --- a/selftest/knownfail
> +++ b/selftest/knownfail
> @@ -38,7 +38,6 @@
>  ^samba3.base.delete.deltest16a
>  ^samba3.base.delete.deltest17a
>  ^samba3.unix.whoami anonymous connection.whoami\(ad_dc\) # We need to resolve if we should be including SID_NT_WORLD and SID_NT_NETWORK in this token
> -^samba3.unix.whoami anonymous connection.whoami\(ad_member\) # smbd maps anonymous logins to domain guest in the local domain, not SID_NT_ANONYMOUS
>  # these show that we still have some differences between our system
>  # with our internal iconv because it passes except when we bypass our
>  # internal iconv modules
> -- 
> 2.9.3
>