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

feat: add anonymousdse_enabled config option #79

Merged

Conversation

NucciTheBoss
Copy link
Contributor

anonymousdse must be enabled for applications like SSSD to successfully bind to the Glauth server, but it's recommended to keep the option set to false if you're not using applications that need to anonymously query the root DSE. See the Glauth Security documentation for more info.

This option is needed for the SSSD operator in Charmed HPC to successfully bind to the Glauth service, otherwise the SSSD service will silently fail in background with the error message Insufficient permissions (50). Once anonymousdse is enabled, SSSD successfully binds to Glauth if also using the patches provided in #78.

`anonymousdse` must be enabled for applications like SSSD to successfully
bind to the Glauth server, but it's recommended to keep off if not using
applications that need to anonymously query the root DSE.

Signed-off-by: Jason C. Nucciarone <[email protected]>
@NucciTheBoss NucciTheBoss requested a review from a team as a code owner December 17, 2024 04:49
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, lgtm. Only 1 comment:
Should we add an anonymousdse_enabled field in the ldap provider databag so that the requirer charm can block if it requires it in order to work?
Alternatively we could add anonymousdse_required to the ldap requirer databag so that we can log an error/block on glauth side.

(I am fine with merging this PR as it is and implementing this change at a different PR)

@wood-push-melon what do you think?

@wood-push-melon
Copy link
Contributor

Thanks for the contribution, lgtm. Only 1 comment: Should we add an anonymousdse_enabled field in the ldap provider databag so that the requirer charm can block if it requires it in order to work? Alternatively we could add anonymousdse_required to the ldap requirer databag so that we can log an error/block on glauth side.

(I am fine with merging this PR as it is and implementing this change at a different PR)

@wood-push-melon what do you think?

yep, good suggestion to me.

The only tiny concern about the configuration is the case when multiple requirer charms are integrate with a single glauth charm. In this case, if the configuration is turned on, that means true for all requirers (correct me if I'm wrong). We probably would need to let users aware that and carefully evaluate it before they turn it on.

@NucciTheBoss
Copy link
Contributor Author

The only tiny concern about the configuration is the case when multiple requirer charms are integrate with a single glauth charm. In this case, if the configuration is turned on, that means true for all requirers (correct me if I'm wrong). We probably would need to let users aware that and carefully evaluate it before they turn it on.

I do have anonymousdse disabled by default, with documentation mentioning why you would want to have it set to true or vice versa depending on your needs.

anonymousdse must be explicitly enabled by the cluster admin when they deploy glauth-k8s. One benefit of using glauth as the LDAP server implementation is that you can have multiple glauth instances running that all point to the same backend database, but function as different endpoints. You could have a separate glauth application deployed with anonymousdse enabled for applications that need it like SSSD, and use the other glauth instance for applications that don't need anonymousdse enabled. One thing to note is that you also still need to share the certificates between applications for StartTLS as well.

@NucciTheBoss
Copy link
Contributor Author

Should we add an anonymousdse_enabled field in the ldap provider databag so that the requirer charm can block if it requires it in order to work?

This feature sounds like a good idea to me! That way SSSD doesn't silently fail in the backend on the machine charms. The sssd doesn't throw an error code if it's unable to query the root DSE; it just writes a "not so fun" log entry in the /var/log/sssd/sss.log_ file.

I would agree that this should be a separate PR however as it requires changing both integrations.py and the ldap charm library. Doesn't require a major version release of ldap however as I do not see us needing to break the existing API.

@wood-push-melon
Copy link
Contributor

The only tiny concern about the configuration is the case when multiple requirer charms are integrate with a single glauth charm. In this case, if the configuration is turned on, that means true for all requirers (correct me if I'm wrong). We probably would need to let users aware that and carefully evaluate it before they turn it on.

I do have anonymousdse disabled by default, with documentation mentioning why you would want to have it set to true or vice versa depending on your needs.

anonymousdse must be explicitly enabled by the cluster admin when they deploy glauth-k8s. One benefit of using glauth as the LDAP server implementation is that you can have multiple glauth instances running that all point to the same backend database, but function as different endpoints. You could have a separate glauth application deployed with anonymousdse enabled for applications that need it like SSSD, and use the other glauth instance for applications that don't need anonymousdse enabled. One thing to note is that you also still need to share the certificates between applications for StartTLS as well.

yeah, what I meant was that we probably need to put something somewhere (maybe security.MD?) to let users be aware of how to properly use this configuration, the risk if they would like to turn it on, and what they probably need to do (depending their use cases and deployments).

We can do it later, definitely not in this PR.

Copy link
Contributor

@wood-push-melon wood-push-melon left a comment

Choose a reason for hiding this comment

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

LGTM

@nsklikas nsklikas merged commit c16ed41 into canonical:main Dec 19, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants