Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subdomains: allow to inherit case_sensitive=Preserving #5251

Closed
wants to merge 5 commits into from

Conversation

pbrezina
Copy link
Member

The first patch is just man page update to reflect current state.

I think it makes sense to be able to show subdomain names in
their original casing. Patches 2-3 make it work for AD provider.

Patch 4 makes it work for IPA provider. There is apparantely a bug
in winbind, but there is no link the any bugzilla so I do not know
if it was already fixed. The commit is four years old. This patch
requires case_sensitive=Preserving to be set also on the server,
otherwise it does not work. It can be enabled without the server setting
but we need to make nss_cmd_getpwnam_ex (and other _ex commands) to
always return case preserving name. So before I continue the work
I'd like to ask @sumit-bose if we can do it like this.

Resolves:
#5250

@pbrezina
Copy link
Member Author

@sumit-bose bump

@sumit-bose
Copy link
Contributor

Hi,

about the winbind comments, they are coming from a time where the extdom plugin on the IPA servers was using winbind for the SID-to-name (and reverse) lookups. This was changed 7 years ago, see https://pagure.io/freeipa/issue/3637 for details. However, IPA servers on RHEL6 might still be affected.

Would it be possible to check in s2n_response_to_attrs() is Preserving is requested and keep the lower-case version if not? Since this feature requires that Preserving is set on the server side as well this more or less implies that it cannot be used wiht very old versions of IPA.

Do you think it would be worth to ask IPA team if they can add a ipa-config option to switch Preserving on to make the configuration more easy and consistent?

bye,
Sumit

@pbrezina
Copy link
Member Author

pbrezina commented Sep 1, 2020

RHEL-6 servers should not be affected since it requires changes on both client and server side SSSD. So RHEL-6 servers will just reply with lowercased names (since the client will not be updated there) and that's what the client will use. Thanks for the clarification, I'll see what we can do about it.

@sumit-bose
Copy link
Contributor

RHEL-6 servers should not be affected since it requires changes on both client and server side SSSD. So RHEL-6 servers will just reply with lowercased names (since the client will not be updated there) and that's what the client will use. Thanks for the clarification, I'll see what we can do about it.

Hi,

I think it is a bit different. On RHEL-6 there is no SSSD ipa-server mode since all lookups in AD where still done by winbind. And iirc this unconditional lower-casing was added becasue depending on the type of operation winbind might have returned an all-lowercase name or the original spelling and SSSD at this time got confused and considered those as different users and tried to add them twice. Hence the ad-hoc fix to unconditionally lower-case the names.

I'm not sure if current winbind on RHEL-6 still acts in the same way or if a recent version of SSSD on an IPA client would still have an issue if a RHEL-6 IPA server would still return the name of the same user sometimes all lower-case and other times in the original spelling with maybe some upper-case characters. I hope that using a RHEL-8 IPA client with a RHEL-6 IPA server will be as rare as a proton decay, but who knows.

bye,
Sumit

@pbrezina pbrezina changed the title [wip] subdomains: allow to inherit case_sensitive=Preserving subdomains: allow to inherit case_sensitive=Preserving Nov 12, 2020
@pbrezina
Copy link
Member Author

See updated patches. I added s2n_response_to_attrs_fqname() that return lowercased name for oldest protocol version which can use winbind underneath. Newer protocols use ipa server mode which returns lowercased name (without these patches) and original name (with these patches applied on the server side). case_sensitive=Preserving now works for AD provider as well as IPA provider without the need to set anything on the server side. SSSD needs to be updated on the server side as well though.

@pbrezina
Copy link
Member Author

Rebased on top of master branch.

@pbrezina
Copy link
Member Author

@SSSD/developers can some of you review these patches? It would be good to include this in the next release.

@sumit-bose
Copy link
Contributor

Hi,

thanks for the rebase. I'm not sure I like the last patch. Why would you want to set case_sensitive=Preserving only on some clients and especially not on the server? Wouldn't this cause confusion? I would even say that it the SSSD side is fixed it might be better to ask FreeIPA to add a ipa config option to set case_sensitive for the whole domain and the SSSD use this new option.

Addtionally, without any flags set SSS_NSS_GETPWNAM_EX should return the same result as SSS_NSS_GETPWNAM, so adding Preserving flag would be a solution, but this would require additional changes on the IPA side.

bye,
Sumit

@abbra
Copy link
Contributor

abbra commented Jan 18, 2021

For what it worth, IPA always lowcases user and group names when storing in LDAP, there is no way to avoid it.

@pbrezina
Copy link
Member Author

Hi,

thanks for the rebase. I'm not sure I like the last patch. Why would you want to set case_sensitive=Preserving only on some clients and especially not on the server? Wouldn't this cause confusion? I would even say that it the SSSD side is fixed it might be better to ask FreeIPA to add a ipa config option to set case_sensitive for the whole domain and the SSSD use this new option.

Do you suggest to add case_sensitive option in IPA similar to what we do with e.g. domain_resolution_order?

Addtionally, without any flags set SSS_NSS_GETPWNAM_EX should return the same result as SSS_NSS_GETPWNAM, so adding Preserving flag would be a solution, but this would require additional changes on the IPA side.

Given IPA lower case what it gets then why it needs to return the same result?

If you don't agree with the patch then I suggest to enable this for AD only for now and see what we can do for IPA later (the customer behind this requests it for AD provider).

@sumit-bose
Copy link
Contributor

Hi,
thanks for the rebase. I'm not sure I like the last patch. Why would you want to set case_sensitive=Preserving only on some clients and especially not on the server? Wouldn't this cause confusion? I would even say that it the SSSD side is fixed it might be better to ask FreeIPA to add a ipa config option to set case_sensitive for the whole domain and the SSSD use this new option.

Do you suggest to add case_sensitive option in IPA similar to what we do with e.g. domain_resolution_order?

Yes, this would be the long term idea. However, in the meantime I think it is ok to require to set case_sensitive=Preserving on the IPA servers as well if you want to use it on the client and hence the last patch is not needed.

Addtionally, without any flags set SSS_NSS_GETPWNAM_EX should return the same result as SSS_NSS_GETPWNAM, so adding Preserving flag would be a solution, but this would require additional changes on the IPA side.

Given IPA lower case what it gets then why it needs to return the same result?

I think Alexander's comment was about IPA user and groups which are always lower case, AD users are not stored in LDAP.

If you don't agree with the patch then I suggest to enable this for AD only for now and see what we can do for IPA later (the customer behind this requests it for AD provider).

See above. If I understand it correctly by setting case_sensitive=Preserving in sssd.conf on the IPA servers the last patch is not needed, SSSD has to be updated anyways to make sure the option is inherited by the sub-domains (trusted AD domains).

bye,
Sumit

Resolves: SSSD#5250

:feature: `case_sensitive` option can be now inherited by subdomains
Resolves: SSSD#5250

:feature: `case_sensitive` can be now set separately for each
  subdomain in `[domain/parent/subdomain]` section
:feature: `case_sensitive=Preserving` can now be set for trusted domains with AD provider
Resolves: SSSD#5250

:feature: `case_sensitive=Preserving` can now be set for trusted domains
  with IPA provider. However, the option needs to be set to `Preserving`
  on both client and the server for it to take effect.
@pbrezina
Copy link
Member Author

Ok, please see new patch set. I dropped last two patches, updated man page and release notes.

@sumit-bose
Copy link
Contributor

Hi,

thanks, I tested with AD and IPA with trust and the patches are working as expected, CI failures are unrelated. ACK.

bye,
Sumit

@pbrezina
Copy link
Member Author

Pushed PR: #5251

  • master
    • 944c47e - man: update case_sensitive documentation to reflect changes for subdomains
    • f6bb31a - subdomains: allow to inherit case_sensitive=Preserving for IPA
    • f265595 - subdomains: allow to set case_sensitive=Preserving in subdomain section
    • 12eb04b - subdomains: allow to inherit case_sensitive=Preserving
    • 0eb0281 - man: add auto_private_groups to subdomain_inherit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants