SSSD / sssd

A daemon to manage identity, authentication and authorization for centrally-managed systems.
https://sssd.io
GNU General Public License v3.0
575 stars 235 forks source link

sudo plugin and ipa_hostname option not documented #7387

Open scabrero opened 1 month ago

scabrero commented 1 month ago

Hi,

currently sssd's sudo plugin [1] checks for the ipa_hostname option in its own and if it is set, its value overrides the system hostname returned by gethostname(2) when evaluating the sudo rules delivered by the daemon to the plugin.

This behavior is not documented in sssd-ipa(5) manpage and there is no sudo_plugin_sssd(5) manpage.

I propose:

  1. Document the behavior
    • Add a sudo_plugin_sssd(5) mangape
    • Extend sssd-ipa(5) manpage section.
  2. Add an alias of ipa_hostname like override_hostname for the [sudo] section of sssd.conf. This is not limited to IPA, it can be useful when using plain LDAP or AD, e.g. [2].

I am not familiar with IPA nor its use case requiring this option, so I open this issue before sending patches. Comments?

[1] https://github.com/sudo-project/sudo/blob/main/plugins/sudoers/sssd.c#L632 [2] https://serverfault.com/questions/301841/sudo-ldap-with-fqdn-instead-of-short-hostname

pbrezina commented 1 month ago

Hi Samuel,

1. Document the behavior

   * Add a `sudo_plugin_sssd(5)` mangape

sudo_plugin_sssd would have to be contributed to sudo. What would you like to see there?

   * Extend `sssd-ipa(5)` manpage section.

Ok, there is no problem with that. ipa_hostname allows you to set how the client identifies to the IPA domain, it defaults to system hostname but it can be different. If it is different, then yes, it is used instead of gethostname().

2. Add an alias of `ipa_hostname` like `override_hostname` for the [sudo] section of sssd.conf. This is not limited to IPA, it can be useful when using plain LDAP or AD, e.g. [2].

There is also ad_hostname.

There is already ldap_sudo_hostnames which allows you to set which hostnames should be used to download the rules.

I am not familiar with IPA nor its use case requiring this option, so I open this issue before sending patches. Comments?

[1] https://github.com/sudo-project/sudo/blob/main/plugins/sudoers/sssd.c#L632 [2] https://serverfault.com/questions/301841/sudo-ldap-with-fqdn-instead-of-short-hostname

Hmm, this is wrong. I don't remember why and how this got to the plugin or if it was even consulted with us, but it is definitely wrong at this moment since it does not reflect conf.d, nor ad_hostname. It would probably even fail on multidomain environment. The good way would be to extend the protocol and pass the sssd's hostname to sudo.

scabrero commented 1 month ago

Hi Samuel,

1. Document the behavior

   * Add a `sudo_plugin_sssd(5)` mangape

sudo_plugin_sssd would have to be contributed to sudo. What would you like to see there?

Mainly how the ipa_hostname option from sssd.conf affects to the plugin behavior. I found it by chance reading the sources while tracing a related issue.

   * Extend `sssd-ipa(5)` manpage section.

Ok, there is no problem with that. ipa_hostname allows you to set how the client identifies to the IPA domain, it defaults to system hostname but it can be different. If it is different, then yes, it is used instead of gethostname().

2. Add an alias of `ipa_hostname` like `override_hostname` for the [sudo] section of sssd.conf. This is not limited to IPA, it can be useful when using plain LDAP or AD, e.g. [2].

There is also ad_hostname.

Ah, right. I would expect the plugin to check for this option too in addition to ipa_hostname...

There is already ldap_sudo_hostnames which allows you to set which hostnames should be used to download the rules.

This option seems to serve other purpose. It filters the rules to download from LDAP, the ipa_hostname is used on the client side to override the hostname returned from gethostname(2).

I am not familiar with IPA nor its use case requiring this option, so I open this issue before sending patches. Comments? [1] https://github.com/sudo-project/sudo/blob/main/plugins/sudoers/sssd.c#L632 [2] https://serverfault.com/questions/301841/sudo-ldap-with-fqdn-instead-of-short-hostname

Hmm, this is wrong. I don't remember why and how this got to the plugin or if it was even consulted with us, but it is definitely wrong at this moment since it does not reflect conf.d, nor ad_hostname. It would probably even fail on multidomain environment. The good way would be to extend the protocol and pass the sssd's hostname to sudo.

Ok, so the idea would be to pass the ldap_sudo_hostnames to the plugin to override gethostname(2), right?

pbrezina commented 1 month ago

I am not familiar with IPA nor its use case requiring this option, so I open this issue before sending patches. Comments? [1] https://github.com/sudo-project/sudo/blob/main/plugins/sudoers/sssd.c#L632 [2] https://serverfault.com/questions/301841/sudo-ldap-with-fqdn-instead-of-short-hostname

Hmm, this is wrong. I don't remember why and how this got to the plugin or if it was even consulted with us, but it is definitely wrong at this moment since it does not reflect conf.d, nor ad_hostname. It would probably even fail on multidomain environment. The good way would be to extend the protocol and pass the sssd's hostname to sudo.

Ok, so the idea would be to pass the ldap_sudo_hostnames to the plugin to override gethostname(2), right?

Essentially, yes. And collaborate with sudo upstream to extend the protocol. The protocol must be versioned so sssd is able to serve rules to both patched and unpatched sudo. We have done something similar in autofs, please look at sss_autofs.c for _protocol.