samba - Nov 2023 - LDAP_MATCHING_RULE_IN_CHAIN no longer working after upgrade?

On Fri, 10 Nov 2023 at 02:57, Andrew Bartlett <abartlet at samba.org>
wrote:
>
> On Thu, 2023-11-09 at 23:29 +0000, Jonathan Hunter via samba wrote:
> > Hi Andrew,
> >
> > Sorry for the couple of days silence; I've been creating a bash
> > script to use with 'git bisect' (it's been a little slow
in my testing
>
> No worries!  Most folks just run away when I suggest it, but is a good
> way to get a lead on a problem that doesn't involve deep diagnostics on
> my side, so is an efficient way that I can get users to help, without
stretching me too thin.
Indeed.

Whilst I have no expectation that my test script is efficient or
optimal in any way, I couldn't see an existing guide on the samba wiki
so I created a page that should hopefully help others, using my script
as an initial example

https://wiki.samba.org/index.php?title=Using_git_bisect_to_locate_a_Samba_issue
> > As of 4.18.5:
> > - ldbsearch -H ldap:// - FAIL
> > - ldbsearch -H sam.ldb - PASS
> > - ldapsearch -H ldap:// - FAIL
>
> OK, so it most likely the permissions handling.
>
> If your automated bisect becomes a pain, or you want to debug in the
> traditional way, look into permissions and ensure your connecting user
> can see all the way down the chain, and check if specifying the matched
> attribute helps.
Thank you.

The git bisect has now finished, and you may share my lack of surprise
at the eventual commit it landed on :)

0776ce5caedf18aa8cc1d1dddb1a425f3d0c926c is the first bad commit
commit 0776ce5caedf18aa8cc1d1dddb1a425f3d0c926c
   CVE-2023-0614 lib/ldb-samba Ensure ACLs are evaluated on
SAMBA_LDAP_MATCH_RULE_TRANSITIVE_EVAL / L
DAP_MATCHING_RULE_IN_CHAIN

I've created a bug for this in bugzilla, hope that's helpful:
https://bugzilla.samba.org/show_bug.cgi?id=15515

Let me know how I can help next,

Thanks

Jonathan

One small point to add below regarding permissions - the query still
fails even if I run it as Administrator.

On Fri, 10 Nov 2023 at 15:50, Jonathan Hunter <jmhunter1 at gmail.com>
wrote:
> Whilst I have no expectation that my test script is efficient or
> optimal in any way, I couldn't see an existing guide on the samba wiki
> so I created a page that should hopefully help others, using my script
> as an initial example
(For anyone else looking for this page - it's not yet live as it needs
approval since it contains external links)
> > OK, so it most likely the permissions handling.
> >
> > If your automated bisect becomes a pain, or you want to debug in the
> > traditional way, look into permissions and ensure your connecting user
> > can see all the way down the chain, and check if specifying the
matched
> > attribute helps.
I'm was running the query from a DC on the commandline as the domain
Administrator user. Whilst I do have at least one OU in the domain
where permissions are locked down (a few years back I think I did set
custom permissions so that only specific groups can access this), the
group being queried is not in this part of the tree.

It is possible that some of the group members also have access to the
locked-down section of the tree though; I wonder if that has any
bearing on things..

Hi Andrew

On Fri, 10 Nov 2023 at 15:50, Jonathan Hunter <jmhunter1 at gmail.com>
wrote:
> 0776ce5caedf18aa8cc1d1dddb1a425f3d0c926c is the first bad commit
> commit 0776ce5caedf18aa8cc1d1dddb1a425f3d0c926c
>    CVE-2023-0614 lib/ldb-samba Ensure ACLs are evaluated on
> SAMBA_LDAP_MATCH_RULE_TRANSITIVE_EVAL / L
> DAP_MATCHING_RULE_IN_CHAIN
>
> I've created a bug for this in bugzilla, hope that's helpful:
> https://bugzilla.samba.org/show_bug.cgi?id=15515
Is there anything I can do to help with this?

Looking through git changes, I found this commit with the same commit
message as returned by my 'git bisect' (I am not sure why the commit
IDs are different to the output of my 'git bisect'?), that looks like
a very simple change:
https://gitlab.com/samba-team/samba/-/commit/dfe7b05730425e9f1b0616bb7757dbf77bae6cd2
(if the view I get from gitlab is correct, it's a one-line change to
lib/ldb-samba/ldb_matching_rules.c )

I checked out revision samba-4.19.2 and reverted just this one line
change, and can confirm that my LDAP query works correctly again in
that scenario.

I'm sure the fix isn't as simple as "revert the change", as it
was
added for a reason - but it seems to have led to a regression for me
and has broken my LDAP searches that use LDAP_MATCHING_RULE_IN_CHAIN.
Is there any sensible route I can help move this forward?

Thanks!

Jonathan
-- 
"If we knew what it was we were doing, it would not be called
research, would it?"
      - Albert Einstein