Simplify direct nested group processing

[ondrejv2]

Simplifies direct nested group processing - now single LDAP request is needed to parse nested groups
Replaces PR #7596

[ondrejv2]

Can anyone shed some light into the tests? I do not quite understand why they fail. It works for me with AD.

[sumit-bose]

Can anyone shed some light into the tests? I do not quite understand why they fail. It works for me with AD.

Hi,

please call make check in the SSSD source tree after compiling SSSD, the nestedgroups-tests should fail. You can nestedgroups-tests individually as well and nestedgroups-tests -d 10 might give some helpful debug output.

HTH

bye,
Sumit

[ondrejv2]

please call make check in the SSSD source tree after compiling SSSD, the nestedgroups-tests should fail. You can nestedgroups-tests individually as well and nestedgroups-tests -d 10 might give some helpful debug output.

Hi,
Well I can't even compile tests myself for some reason:

  CC       src/tests/cmocka/nestedgroups_tests-common_mock_be.o
  CC       src/providers/ldap/nestedgroups_tests-sdap_async_nested_groups.o
  CC       src/providers/ldap/nestedgroups_tests-sdap_ad_groups.o
  CC       src/providers/ipa/nestedgroups_tests-ipa_dn.o
  CCLD     nestedgroups-tests
/usr/bin/ld: cannot find ./.libs/libdlopen_test_providers.a: No such file or directory
collect2: error: ld returned 1 exit status
make[3]: *** [Makefile:16460: nestedgroups-tests] Error 1
make[3]: Leaving directory '/opt/sssd'

any idea?

[sumit-bose]

Hi,

are you using any configure options or CFLAGS, LDFLAGS in the environment? It looks like you are trying to build a static version of SSSD which is not supported.

bye,
Sumit

[ondrejv2]

Hi,
I figured eventually - I was missing some dependencies. Can run checks now and they fail because these checks leave the objectClass attribute of the nested group member empty - hence my code fails.
Now I have two options:

• fix the checks so they populate objectClass attr correctly.
• fix my code to check value objectCategory attr instead (that will be slightly more tricky I guess so I would prefer option 1).

What do you think?
Converted to draft meanwhile.

Thanks
Ondrej

[ondrejv2]

Hi,
Is anything else needed here? I mean there is certainly a room to improve the code, but I am wondering if conceptually this has any chance to be accepted.
Thanks

[ondrejv2]

Hi @sumit-bose, is anything needed here from my side?
Thanks

[alexey-tikhonov]

@ondrejv2, could you please rebase on top of latest 'master'?

[ondrejv2]

Hi @alexey-tikhonov, rebased.
Intg checks still report a problem, but not group related ones, so I have no idea why they failed. Can anyone help?

[alexey-tikhonov]

Intg checks still report a problem

JFTR all platforms fail this test:

_______________________ test_refresh_after_cleanup_task ________________________
Traceback (most recent call last):
...
  File "/sssd/src/tests/intg/test_ldap.py", line 460, in test_refresh_after_cleanup_task
    ent.assert_group_by_name(
    ~~~~~~~~~~~~~~~~~~~~~~~~^
        "group2",
        ^^^^^^^^^
        dict(mem=ent.contains_only("user1")))
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/sssd/src/tests/intg/ent.py", line 378, in assert_group_by_name
    assert not d, d
           ^^^^^
AssertionError: member list mismatch: 
expected members not found:
['user1']

Taking into account that recent CI run for 'master' branch doesn't have this issue, it is triggered (introduced/uncovered/...) by this PR.

Hint: you can run intg-tests locally https://sssd.io/contrib/tests/integration-tests.html -- this will help to debug the issue.

[ondrejv2]

Following:
https://sssd.io/contrib/building-sssd.html
to build sssd.
then make intgcheck fails:
configure: error: source directory already configured; run "make distclean" there first
but once I do make distclean, I can't no longer run make intgcheck
Any advise?

[alexey-tikhonov]

Following: https://sssd.io/contrib/building-sssd.html to build sssd. then make intgcheck fails: configure: error: source directory already configured; run "make distclean" there first but once I do make distclean, I can't no longer run make intgcheck Any advise?

Are you building right in working tree?

I use https://github.com/SSSD/sssd/blob/master/contrib/fedora/bashrc_sssd#L62

reconfig
make intg-check

[alexey-tikhonov]

make check is failing:

[  FAILED  ] nested_groups_test_one_group_unique_group_members
[  FAILED  ] nested_groups_test_one_group_dup_group_members
[  FAILED  ] nested_groups_test_nested_chain
[  FAILED  ] nested_group_external_member_test
...

[ondrejv2]

I know, will have to figure out why - interesting thing is that intg checks are now all fine

[alexey-tikhonov]

interesting thing is that intg checks are now all fine

Those do not run automatically for patches provided by external contributors. I will start it now.

[ondrejv2]

Those do not run automatically for patches provided by external contributors. I will start it now.

I mean they did run fine on my system - so guessing they should pass here, too

[ondrejv2]

Just wanted to rebase my past commit fc486b3, but looks like it wasn't a bright idea.
What shall I do now? Sorry for the noise

[alexey-tikhonov]

How do you "rebase"?

Typically you update local copy of 'master' branch, switch to your branch, and then git rebase master

[ondrejv2]

I did:

git add test_nested_groups.c 
git commit --fixup=bdedd9838f688b0d396d0468dede6af8f04865a5
git rebase -i --autosquash bdedd9838f688b0d396d0468dede6af8f04865a5~1
git push --force

Now I see this PR has like 50 commits and 30 participants, making it a decent spam source.
Is there any chance of a remedy (other than closing this PR and starting a new one)?

[alexey-tikhonov]

I would try something like:

• get up-to-date 'master' branch
• note hashes of your actual patches
• make a copy of your branch (just in case)
• reset this branch (say, git reset HEAD~150 --hard)
• git rebase master ( <-- at this point your branch should be equal 'master' branch)
• git cherry-pick hash1 ... - cherry pick your patches back
• git push --force

So idea is to reset your 'groups' branch to mint condition and cherry pick your patches back on top (maybe resolving conflicts if there are any).

Maybe this can be done easier, but this should work.

[ondrejv2]

Hello, has this PR any chance for adoption? I would like to come up with some more PR based on this one, but don't want to waste my time if this one is rejected

[alexey-tikhonov]

@sumit-bose is busy this week, so unfortunately this PR will have to wait.

[ondrejv2]

Hi @sumit-bose ,
I am thinking about reusing the SID from foreign security principal we get in this PR. We can try to look it up in other domains SSSD is defined for (assuming there are at least two defined).
Now since they are handled by a different backend process, we would have to look them up either via:

• NSS services - there is the getnamebysid() function interface. Bad news is that the NSS client backend is not exactly tevent friendly so it might block
• SBUS interface - it seems to me a better approach, but there is no FindBySID method so so we would need to extend SBUS interface for this.

General question: would you be happy to support this activity? If so, which way you would prefer?

[sumit-bose]

Hi @sumit-bose , I am thinking about reusing the SID from foreign security principal we get in this PR. We can try to look it up in other domains SSSD is defined for (assuming there are at least two defined).

Hi,

what would be the use case of looking up the related object?

bye,
Sumit

[ondrejv2]

what would be the use case of looking up the related object?

Hi,
Well, currently, group enumeration silently ignores Foreign security principals from trusted domains/forests. The use case of this would be that SSSD could recognize group defined in domain A that has member belonging to domain B, provided that both domains would be defined in [domain] section in SSSD. Only members from domain A are now listed.
Currently every domain defined in SSSD yields into separate backend daemon started and these can't communicate between each other (for group enumeration for example).

[sumit-bose]

Hi,

I'm very sorry but I basically forgot about this PR.

In general I agree with your approach to just read the object and then determine what type it is. But the way you are doing the attribute name mapping does not match how it is done in other places. There is already code which can handle searches which can return multiple different types of objects. Unfortunately these are dereference searches which can not be easily applied here.

To make more clear what I mean I created a small patch at sumit-bose@bb70f43 which adds a new call sdap_get_and_multi_parse_generic_send() where you not just give a single map but a pointer to an array of struct sdap_attr_map_info which can hold multiple different maps. What is missing is the implementation of the callback sdap_get_and_multi_parse_generic_parse_entry() but I think here you can use a copy of sdap_asq_search_parse_entry() but with a change in the type of the state and you have to decide if unknown types should be skipped or be returned without any attribute mapping.[1] If you now use sdap_get_and_multi_parse_generic_send() instead of sdap_get_generic_send() with a maps array which contains the maps for users and groups (and maybe in the future for the foreign security principals) the search will be more aligned to the other searches in SSSD and you do not need the post-processing.

bye,
Sumit

[1] Yes, this will cause some duplication but this is an issue we already have. sdap_get_and_parse_generic_parse_entry() and sdap_sd_search_parse_entry() are the same just with a different type of the state. Here some refactoring is needed to make the handling of the private data of the callback more flexible but this should not be part of this effort here.

[ondrejv2]

Hello,
I know it is being done differently on other places, but due to unresolved dependencies (see the ldapoptions you still referring in your code - why is that needed for just attribute parsing? Still not getting it) I could not find a way to do it similarly here - hence I came up with my map parser (quite a piece of a nice code, bit pity to see it does not fit).

Anyway, I see the rework would not be really straightforward and I can't work on this PR anymore.
Feel free to modify/adjust the code as needed or reject completely.

[sumit-bose]

Hi,

I created #8127 which contains the unmodified version of your patches surrounded by some of my patches. I wonder if you can test one of the builds from https://copr.fedorainfracloud.org/coprs/packit/SSSD-sssd-8127 to see if it works as expected for you.

Thanks

bye,
Sumit

[ondrejv2]

This PR has been superceded by PR #8127 so just added my last work on it - it was never finished attempt to try to actually resolve Foreign Security Principals in Forest trusts.
I did not want to discuss forests in AD Global catalogs or anything like that - it was more like attempt to try to resolve foreign SIDs using already defined providers - see my example here where "diasemi.com" one way trusts "adwin.renesas.com" forest:

[sssd]
services = autofs, nss, pam
config_file_version = 2
debug_level = 7

domains = diasemi
[nss]
debug_level = 9

[domain/diasemi]
debug_level = 9
ldap_id_mapping = False
ad_domain = diasemi.com
ad_enabled_domains = diasemi.com
id_provider = ad
auth_provider = ad
chpass_provider = ad
autofs_provider = ad
ad_enable_gc = False
cache_credentials = True
# krb5_realm = ADWIN.RENESAS.COM
krb5_canonicalize = False
ldap_user_name = uid
# ldap_group_name = displayName
krb5_renew_interval = 3600
# request renewable Kerberos tickets
krb5_renewable_lifetime = 30d
ldap_sasl_mech = GSSAPI
ldap_referrals = False
ldap_autofs_search_base = OU=sl,OU=SSSD,OU=Unix,OU=Global,DC=diasemi,DC=com
ldap_sasl_authid = SLSRVADM-02V$@ADWIN.RENESAS.COM
fallback_homedir = /users/%u
default_shell = /bin/bash
ldap_ignore_unreadable_references = True
# force direct lookups
ldap_deref_threshold = 0
# auto_private_groups = true
# ldap_use_tokengroups = false

[domain/renesas]
ad_update_samba_machine_account_password = True
ldap_id_mapping = False
ad_domain = adwin.renesas.com
ad_enabled_domains = adwin.renesas.com
id_provider = ad
auth_provider = ad
chpass_provider = ad
ad_enable_gc = False
cache_credentials = True
krb5_canonicalize = False
ldap_user_name = uid
ldap_user_gecos = displayName
# ldap_group_name = displayName
krb5_renew_interval = 3600
# request renewable Kerberos tickets
krb5_renewable_lifetime = 30d
ldap_sasl_mech = GSSAPI
ldap_referrals = False
fallback_homedir = /users/%u
default_shell = /bin/bash

[ifp]
user_attributes = +mail
allowed_uids = root

feel free to grab parts of this if you find it useful