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

MAN: Add note about AD Group types #6263

Conversation

justin-stephenson
Copy link
Contributor

Linux admins/users may not know that the AD distribution group type is intended only for email. Per microsoft:[1] Distribution groups are not security enabled, which means that they cannot be listed in discretionary access control lists (DACLs).

[1] https://docs.microsoft.com/en-us/windows/security/identity-protection/access-control/active-directory-security-groups

src/man/sssd-ad.5.xml Outdated Show resolved Hide resolved
@pbrezina
Copy link
Member

Thank you. Ack.

@justin-stephenson
Copy link
Contributor Author

Does this need another reviewer?

@pbrezina
Copy link
Member

Yes, we talked about it and assigned to Sumit in some previous meeting.

<para>
SSSD only resolves Security-enabled Active Directory group types.
(i.e. Not <quote>distribution</quote> groups)
</para>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I think Security-enabled is not common Active Directory speak in this context. I would suggest something like SSSD only resolves Active Directory Security Groups (since Security Group is used in AD User and Computers in the user and group listing) or SSSD only resolves Active Directory groups of group type "Security"(since this is shown in the properties of a group). @abbra, do you agree or do you have other suggestions?

We already have a reference to MSFT documentation in the sssd-ad man page, so I think it might worth to add https://docs.microsoft.com/en-us/windows-server/identity/ad-ds/manage/understand-security-groups here as well.

bye,
Sumit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer Security Groups but you need to add more details around 'Domain-Local' scope, I think. It becomes a bit complex to explain ;)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

thanks, adding some info about how domain-local groups are handled is a good idea. @justin-stephenson, can you add it or do you need more context?

bye,
Sumit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is mention of how SSSD handles domain local groups in the option description for ad_allow_remote_domain_local_groups. Is it enough or should it be mentioned in this part as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

I would copy the content into this part as well.

bye,
Sumit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, please check. Thank you.

forest. By default they are filtered out e.g. when following a
nested group hierarchy in remote domains because they are not valid
in the local domain. This behavior is dependent on the value for
the <quote>ad_allow_remote_domain_local_groups</quote> option
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

thanks for the update but please remove the last sentence. This option should not get any additional attention. And I would like to ask you to add a sentence like "This is done to be in agreement with Active Directory's group-membership assignment which can e.g. be seen in the PAC of the Kerberos ticket of a user issued by Active Directory."

bye,
Sumit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, thanks.

Linux admins/users may not know that the AD distribution group type
is intended only for email. Per microsoft: Distribution groups are
not security enabled, which means that they cannot be listed in
discretionary access control lists (DACLs).
Copy link
Contributor

@sumit-bose sumit-bose left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, ACK.

bye,
Sumit

@pbrezina
Copy link
Member

pbrezina commented Sep 7, 2022

Backport/no backport?

@justin-stephenson
Copy link
Contributor Author

Backport/no backport?

Forgive the ignorant question, but do I determine if it needs to be backported or not? This PR originates from a RHEL9 BZ

@pbrezina
Copy link
Member

pbrezina commented Sep 7, 2022

If there is rhel9 BZ that is fixed by this commit then it should be backported to 2.7 so Alexey can push to rhel9. If it is just a side job that is not going to rhel9 then backport is not required.

@justin-stephenson justin-stephenson added the backport-to-stable Targets also latest stable branch label Sep 7, 2022
@justin-stephenson
Copy link
Contributor Author

If there is rhel9 BZ that is fixed by this commit then it should be backported to 2.7 so Alexey can push to rhel9. If it is just a side job that is not going to rhel9 then backport is not required.

backport set, thank you.

@alexey-tikhonov
Copy link
Member

@justin-stephenson, could you please add a link to the RHBZ in the comment of this PR?
But I think it only targets 9.2+, so imo backport to 2-7 branch isn't needed.

@justin-stephenson justin-stephenson added no-backport This should go to target branch only. and removed backport-to-stable Targets also latest stable branch labels Sep 13, 2022
@justin-stephenson
Copy link
Contributor Author

justin-stephenson commented Sep 13, 2022

@justin-stephenson
Copy link
Contributor Author

@justin-stephenson, could you please add a link to the RHBZ in the comment of this PR? But I think it only targets 9.2+, so imo backport to 2-7 branch isn't needed.

Done and set 'no-backport'

@pbrezina pbrezina added the Ready to push Ready to push label Sep 16, 2022
@pbrezina
Copy link
Member

Pushed PR: #6263

  • master
    • 794fd13 - MAN: Add note about AD Group types

@pbrezina pbrezina added Pushed and removed Accepted Ready to push Ready to push labels Sep 16, 2022
@pbrezina pbrezina closed this Sep 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugzilla no-backport This should go to target branch only. Pushed Trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants