Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
bye, Sumit
On Tue, 2014-07-22 at 14:55 +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
The patches look good to me, quite simple and direct. It would be nice to avoid the sshd annoying restrictions in RHEL/Fedora indeed, and possibly elsewhere.
Idea: Can we add a distributors.README file where we start listing all the common issues we find in packages and warn distributors to patches they may want to adopt in related packages that we asked Fedora/RHEL maintainers to add/backport ?
Simo.
On Mon, 28 Jul 2014, Simo Sorce wrote:
On Tue, 2014-07-22 at 14:55 +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
The patches look good to me, quite simple and direct. It would be nice to avoid the sshd annoying restrictions in RHEL/Fedora indeed, and possibly elsewhere.
Idea: Can we add a distributors.README file where we start listing all the common issues we find in packages and warn distributors to patches they may want to adopt in related packages that we asked Fedora/RHEL maintainers to add/backport ?
It is a good idea overall for both SSSD and FreeIPA to carry a list of such dependencies/fixes. We get regularly questions about backports of a newer code and answering them would be better with a structured list.
On Tue, Aug 12, 2014 at 03:30:02PM +0300, Alexander Bokovoy wrote:
On Mon, 28 Jul 2014, Simo Sorce wrote:
On Tue, 2014-07-22 at 14:55 +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
The patches look good to me, quite simple and direct. It would be nice to avoid the sshd annoying restrictions in RHEL/Fedora indeed, and possibly elsewhere.
Idea: Can we add a distributors.README file where we start listing all the common issues we find in packages and warn distributors to patches they may want to adopt in related packages that we asked Fedora/RHEL maintainers to add/backport ?
It is a good idea overall for both SSSD and FreeIPA to carry a list of such dependencies/fixes. We get regularly questions about backports of a newer code and answering them would be better with a structured list.
I don't object to the idea of the list, but as with any such list, how do you maintain it and ensure it's not obsolete?
On Tue, 12 Aug 2014, Jakub Hrozek wrote:
On Tue, Aug 12, 2014 at 03:30:02PM +0300, Alexander Bokovoy wrote:
On Mon, 28 Jul 2014, Simo Sorce wrote:
On Tue, 2014-07-22 at 14:55 +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
The patches look good to me, quite simple and direct. It would be nice to avoid the sshd annoying restrictions in RHEL/Fedora indeed, and possibly elsewhere.
Idea: Can we add a distributors.README file where we start listing all the common issues we find in packages and warn distributors to patches they may want to adopt in related packages that we asked Fedora/RHEL maintainers to add/backport ?
It is a good idea overall for both SSSD and FreeIPA to carry a list of such dependencies/fixes. We get regularly questions about backports of a newer code and answering them would be better with a structured list.
I don't object to the idea of the list, but as with any such list, how do you maintain it and ensure it's not obsolete?
Since it is a historical information, it wouldn't become obsolete.
"In order to have single sign-on for users from FreeIPA trusted AD domains, one have to set up libkrb5:
In libkrb5 prior to 1.12: auth_to_local rules must be set explicitly on every machine
In libkrb5 1.12, in Fedora 21, in RHEL 7.x (where .x is specific number where we get libkrb5 1.12 merged): set up SSSD localauth plugin instead of auth_to_local rules. "
and so on, the idea is that information doesn't expire because it reflects old code which is released out wild and not going to change.
On Tue, Jul 22, 2014 at 02:55:15PM +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
bye, Sumit
Hi,
The code works fine. I tested with commenting out the auth_to_local rules and adding the [plugins] section as you suggested. Then login as an AD user worked fine with w/o a password prompt: ssh $(hostname) -l tuser@win.example.com
About documentation of the plugin framework, I found a PDF in the krb5-1.12 tarball very helpful. It's located in doc/pdf/plugindev.pdf in the tarball.
See couple of questions inline. With the exception of the unparse_name they are mostly questions and I don't insist on any of them.
From 7270f9583d6e0bb1bd881be0fc3ee49965d40193 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Wed, 16 Jul 2014 21:43:30 +0200 Subject: [PATCH 1/2] Add conditional build for MIT Kerberos localauth plugin
This patch adds everything what is needed to build the MIT Kerberos localauth plugin if the used version of MIT Kerberos supports it. It does not implement the plugin.
Makefile.am | 19 ++++++++++++++++++ configure.ac | 1 + contrib/sssd.spec.in | 11 +++++++++++ src/conf_macros.m4 | 14 ++++++++++++++ src/external/krb5.m4 | 15 ++++++++++++++ src/krb5_plugin/sssd_krb5_localauth_plugin.c | 29 ++++++++++++++++++++++++++++ src/tests/dlopen-tests.c | 4 ++++ 7 files changed, 93 insertions(+) create mode 100644 src/krb5_plugin/sssd_krb5_localauth_plugin.c
[...]
--- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -48,6 +48,10 @@ %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin %endif
+%if (0%{?fedora} >= 21)
Do you think it's time to add a rule for RHEL-7.1 already? I'm just concerned that we'll forget later. Alternatively we can just file a ticket.
- %global with_krb5_localauth_plugin 1
+%endif
Name: @PACKAGE_NAME@ Version: @PACKAGE_VERSION@ Release: 0@PRERELEASE_VERSION@%{?dist} @@ -113,7 +117,11 @@ BuildRequires: pcre-devel BuildRequires: libxslt BuildRequires: libxml2 BuildRequires: docbook-style-xsl +%if (0%{?with_krb5_localauth_plugin} == 1) +BuildRequires: krb5-devel >= 1.12 +%else BuildRequires: krb5-devel +%endif BuildRequires: c-ares-devel BuildRequires: python-devel BuildRequires: check-devel @@ -746,6 +754,9 @@ rm -rf $RPM_BUILD_ROOT %{_libdir}/cifs-utils/cifs_idmap_sss.so %ghost %{_sysconfdir}/cifs-utils/idmap-plugin %endif +%if (0%{?with_krb5_localauth_plugin} == 1) +%{_libdir}/%{name}/sssd_krb5_localauth_plugin.so +%endif %{_mandir}/man8/pam_sss.8* %{_mandir}/man8/sssd_krb5_locator_plugin.8*
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index 3c9827b..368820c 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -274,6 +274,20 @@ AC_DEFUN([WITH_KRB5_PLUGIN_PATH], AC_SUBST(krb5pluginpath) ])
+AC_DEFUN([WITH_KRB5_LOCALAUTH_PLUGIN_PATH],
- [ AC_ARG_WITH([krb5-localauth-plugin-path],
[AC_HELP_STRING([--with-krb5-localauth-plugin-path=PATH],[Path to kerberos localauth plugin store [/usr/lib/sssd]]
/usr/lib/sssd/ is used by internal sssd libraries, such as the providers. Would it make sense to set the default location to /usr/lib/sssd/modules which is where we put the autofs plugin currently and we also have a patch to move the sudo plugin there.
)])- krb5localauth_pluginpath="${libdir}/sssd"
- if test x"$with_krb5_localauth_plugin_path" != x; then
krb5localauth_pluginpath=$with_krb5_localauth_plugin_path- fi
- AC_SUBST(krb5localauth_pluginpath)
- ])
AC_DEFUN([WITH_CIFS_PLUGIN_PATH], [ AC_ARG_WITH([cifs-plugin-path], [AC_HELP_STRING([--with-cifs-plugin-path=PATH],
From 6bd15ceb7b0991a5ad0747d1ce2b29e6c4772e5c Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 17 Jul 2014 21:08:20 +0200 Subject: [PATCH 2/2] Implement MIT Kerberos localauth plugin
The MIT Kerberos localauth pluing interface defines two different calls.
The first checks if a given Kerberos principal relates to a given name of a local user (userok). The implementation lets SSSD resolve the principal and the user name and if the returned user entries both have the same UID success is returned.
The second translates a given Kerberos principal to a local user name (a2l). Here SSSD is only called once to resolve the principal and the user name is returned.
Resolves https://fedorahosted.org/sssd/ticket/1835
[...]
diff --git a/src/krb5_plugin/sssd_krb5_localauth_plugin.c b/src/krb5_plugin/sssd_krb5_localauth_plugin.c index 5e38ca5..f3dacc2 100644 --- a/src/krb5_plugin/sssd_krb5_localauth_plugin.c +++ b/src/krb5_plugin/sssd_krb5_localauth_plugin.c @@ -18,12 +18,151 @@ along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <nss.h> +#include <sys/types.h> +#include <pwd.h> +#include <string.h> +#include <strings.h> +#include <errno.h>
#include <krb5/localauth_plugin.h>
+enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
char *buffer, size_t buflen, int *errnop);+#define DEFAULT_BUFSIZE 4096
OK, this should be more than enough, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024 on F-20.
+static krb5_error_code sss_userok(krb5_context context,
krb5_localauth_moddata data,krb5_const_principal aname,const char *lname)+{
- krb5_error_code kerr;
- char *princ_str;
- struct passwd pwd = { 0 };
- char *buffer = NULL;
- size_t buflen;
- enum nss_status nss_status;
- int nss_errno;
- uid_t princ_uid;
- int ret;
- kerr = krb5_unparse_name(context, aname, &princ_str);
Looks like you're leaking princ_str here, I think you should call krb5_free_unparsed_name() in the done: handler.
- if (kerr != 0) {
return kerr;- }
- if (strcasecmp(princ_str, lname) == 0) {
return 0;- }
- buflen = DEFAULT_BUFSIZE;
- buffer = malloc(buflen);
- if (buffer == NULL) {
return ENOMEM;- }
- nss_status = _nss_sss_getpwnam_r(princ_str, &pwd, buffer, buflen,
&nss_errno);- if (nss_status != NSS_STATUS_SUCCESS) {
Question -- if the getpwnam_r call returns that sssd doesn't know the username, does it make sense to return KRB5_PLUGIN_NO_HANDLE to let another plugin handle the user?
ret = EIO;goto done;- }
- princ_uid = pwd.pw_uid;
- nss_status = _nss_sss_getpwnam_r(lname, &pwd, buffer, buflen, &nss_errno);
- if (nss_status != NSS_STATUS_SUCCESS) {
Same question here.
ret = EIO;goto done;- }
- if (princ_uid != pwd.pw_uid) {
ret = EPERM;
OK, documentation says EPERM is the error code krb5 expects.
goto done;- }
- ret = 0;
+done:
- free(buffer);
- return ret;
+}
+static krb5_error_code sss_an2ln(krb5_context context,
krb5_localauth_moddata data,const char *type, const char *residual,krb5_const_principal aname, char **lname_out)+{
- krb5_error_code kerr;
- char *princ_str;
- struct passwd pwd = { 0 };
- char *buffer = NULL;
- size_t buflen;
- enum nss_status nss_status;
- int nss_errno;
- int ret;
- char *str;
- kerr = krb5_unparse_name(context, aname, &princ_str);
Looks like another leak here.
- if (kerr != 0) {
return kerr;- }
- buflen = DEFAULT_BUFSIZE;
- buffer = malloc(buflen);
- if (buffer == NULL) {
return ENOMEM;- }
- nss_status = _nss_sss_getpwnam_r(princ_str, &pwd, buffer, buflen,
&nss_errno);- if (nss_status != NSS_STATUS_SUCCESS) {
ret = EIO;goto done;- }
- str = strdup(pwd.pw_name);
- if (str == NULL) {
ret = ENOMEM;goto done;- }
- *lname_out = str;
- ret = 0;
+done:
- free(buffer);
- return ret;
+}
+static void sss_freestr(krb5_context context,
krb5_localauth_moddata data, char *str)+{
- free(str);
ok, documentation says that whenever an2ln is implemented, free_string must be implemented do deallocate whatever was allocated with an2ln.
+}
krb5_error_code localauth_sssd_initvt(krb5_context context, int maj_ver, int min_ver, krb5_plugin_vtable vtable) {
- return KRB5_PLUGIN_VER_NOTSUPP;
- if (maj_ver != 1 || min_ver != 1) {
return KRB5_PLUGIN_VER_NOTSUPP;- }
Why do you also check min_ver here? All the plugins currently in tree only check maj_ver as far as I can see. Or rather, what is the semantics of the min_ver number?
- krb5_localauth_vtable vt = (krb5_localauth_vtable)vtable;
- vt->init = NULL;
- vt->fini = NULL;
- vt->name = "sssd";
- vt->an2ln = sss_an2ln;
- vt->userok = sss_userok;
- vt->free_string = sss_freestr;
- return 0;
}
-- 1.8.3.1
On Tue, 12 Aug 2014, Jakub Hrozek wrote:
On Tue, Jul 22, 2014 at 02:55:15PM +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
bye, Sumit
Hi,
The code works fine. I tested with commenting out the auth_to_local rules and adding the [plugins] section as you suggested. Then login as an AD user worked fine with w/o a password prompt: ssh $(hostname) -l tuser@win.example.com
About documentation of the plugin framework, I found a PDF in the krb5-1.12 tarball very helpful. It's located in doc/pdf/plugindev.pdf in the tarball.
See couple of questions inline. With the exception of the unparse_name they are mostly questions and I don't insist on any of them.
From 7270f9583d6e0bb1bd881be0fc3ee49965d40193 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Wed, 16 Jul 2014 21:43:30 +0200 Subject: [PATCH 1/2] Add conditional build for MIT Kerberos localauth plugin
This patch adds everything what is needed to build the MIT Kerberos localauth plugin if the used version of MIT Kerberos supports it. It does not implement the plugin.
Makefile.am | 19 ++++++++++++++++++ configure.ac | 1 + contrib/sssd.spec.in | 11 +++++++++++ src/conf_macros.m4 | 14 ++++++++++++++ src/external/krb5.m4 | 15 ++++++++++++++ src/krb5_plugin/sssd_krb5_localauth_plugin.c | 29 ++++++++++++++++++++++++++++ src/tests/dlopen-tests.c | 4 ++++ 7 files changed, 93 insertions(+) create mode 100644 src/krb5_plugin/sssd_krb5_localauth_plugin.c
[...]
--- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -48,6 +48,10 @@ %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin %endif
+%if (0%{?fedora} >= 21)
Do you think it's time to add a rule for RHEL-7.1 already? I'm just concerned that we'll forget later. Alternatively we can just file a ticket.
- %global with_krb5_localauth_plugin 1
+%endif
Name: @PACKAGE_NAME@ Version: @PACKAGE_VERSION@ Release: 0@PRERELEASE_VERSION@%{?dist} @@ -113,7 +117,11 @@ BuildRequires: pcre-devel BuildRequires: libxslt BuildRequires: libxml2 BuildRequires: docbook-style-xsl +%if (0%{?with_krb5_localauth_plugin} == 1) +BuildRequires: krb5-devel >= 1.12 +%else BuildRequires: krb5-devel +%endif BuildRequires: c-ares-devel BuildRequires: python-devel BuildRequires: check-devel @@ -746,6 +754,9 @@ rm -rf $RPM_BUILD_ROOT %{_libdir}/cifs-utils/cifs_idmap_sss.so %ghost %{_sysconfdir}/cifs-utils/idmap-plugin %endif +%if (0%{?with_krb5_localauth_plugin} == 1) +%{_libdir}/%{name}/sssd_krb5_localauth_plugin.so +%endif %{_mandir}/man8/pam_sss.8* %{_mandir}/man8/sssd_krb5_locator_plugin.8*
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index 3c9827b..368820c 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -274,6 +274,20 @@ AC_DEFUN([WITH_KRB5_PLUGIN_PATH], AC_SUBST(krb5pluginpath) ])
+AC_DEFUN([WITH_KRB5_LOCALAUTH_PLUGIN_PATH],
- [ AC_ARG_WITH([krb5-localauth-plugin-path],
[AC_HELP_STRING([--with-krb5-localauth-plugin-path=PATH],[Path to kerberos localauth plugin store [/usr/lib/sssd]]/usr/lib/sssd/ is used by internal sssd libraries, such as the providers. Would it make sense to set the default location to /usr/lib/sssd/modules which is where we put the autofs plugin currently and we also have a patch to move the sudo plugin there.
)])- krb5localauth_pluginpath="${libdir}/sssd"
- if test x"$with_krb5_localauth_plugin_path" != x; then
krb5localauth_pluginpath=$with_krb5_localauth_plugin_path- fi
- AC_SUBST(krb5localauth_pluginpath)
- ])
AC_DEFUN([WITH_CIFS_PLUGIN_PATH], [ AC_ARG_WITH([cifs-plugin-path], [AC_HELP_STRING([--with-cifs-plugin-path=PATH],
From 6bd15ceb7b0991a5ad0747d1ce2b29e6c4772e5c Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 17 Jul 2014 21:08:20 +0200 Subject: [PATCH 2/2] Implement MIT Kerberos localauth plugin
The MIT Kerberos localauth pluing interface defines two different calls.
The first checks if a given Kerberos principal relates to a given name of a local user (userok). The implementation lets SSSD resolve the principal and the user name and if the returned user entries both have the same UID success is returned.
The second translates a given Kerberos principal to a local user name (a2l). Here SSSD is only called once to resolve the principal and the user name is returned.
Resolves https://fedorahosted.org/sssd/ticket/1835
[...]
diff --git a/src/krb5_plugin/sssd_krb5_localauth_plugin.c b/src/krb5_plugin/sssd_krb5_localauth_plugin.c index 5e38ca5..f3dacc2 100644 --- a/src/krb5_plugin/sssd_krb5_localauth_plugin.c +++ b/src/krb5_plugin/sssd_krb5_localauth_plugin.c @@ -18,12 +18,151 @@ along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <nss.h> +#include <sys/types.h> +#include <pwd.h> +#include <string.h> +#include <strings.h> +#include <errno.h>
#include <krb5/localauth_plugin.h>
+enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
char *buffer, size_t buflen, int *errnop);+#define DEFAULT_BUFSIZE 4096
OK, this should be more than enough, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024 on F-20.
+static krb5_error_code sss_userok(krb5_context context,
krb5_localauth_moddata data,krb5_const_principal aname,const char *lname)+{
- krb5_error_code kerr;
- char *princ_str;
- struct passwd pwd = { 0 };
- char *buffer = NULL;
- size_t buflen;
- enum nss_status nss_status;
- int nss_errno;
- uid_t princ_uid;
- int ret;
- kerr = krb5_unparse_name(context, aname, &princ_str);
Looks like you're leaking princ_str here, I think you should call krb5_free_unparsed_name() in the done: handler.
- if (kerr != 0) {
return kerr;- }
- if (strcasecmp(princ_str, lname) == 0) {
return 0;- }
- buflen = DEFAULT_BUFSIZE;
- buffer = malloc(buflen);
- if (buffer == NULL) {
return ENOMEM;- }
- nss_status = _nss_sss_getpwnam_r(princ_str, &pwd, buffer, buflen,
&nss_errno);- if (nss_status != NSS_STATUS_SUCCESS) {
Question -- if the getpwnam_r call returns that sssd doesn't know the username, does it make sense to return KRB5_PLUGIN_NO_HANDLE to let another plugin handle the user?
Yes, plugins should be stacked, so if we don't know name, we should allow another plugin or auth_to_local rule to apply.
On Tue, Aug 12, 2014 at 02:24:38PM +0200, Jakub Hrozek wrote:
On Tue, Jul 22, 2014 at 02:55:15PM +0200, Sumit Bose wrote:
Hi,
these two patches implement the MIT Kerberos localauth plugin for SSSD. Since it uses the new plugin style (http://k5wiki.kerberos.org/wiki/Projects/Plugin_support_improvements) it has to be activated explicitly. A section like
[plugins] localauth = { module = sssd:/path/to/plugin/sssd_krb5_localauth_plugin.so enable_only = sssd }
Should work. Please note that this example deactivates any other mechanism, e.g. .k5login files. See krb5.conf man page of MIT Kerberos 1.12 how to active the mechanism as well.
One of the main use cases for this plugin is an IPA environment with trust to AD. Currently AD user who want to use SSO with IPA client needed a .k5login file in their home directory containing their Kerberos principal. Alternatively krb5.conf has to be edited but here the names user at the login prompt has to follow a fixed format and are case sensitive. If the localauth plugin is activated the mapping of Kerberos principal and user name is done by SSSD.
While I was testing the plugin with ssh I found that st least the Fedora and RHEL versions of the sshd do not rely completely in the Kerberos libraries here but do some checks on their own, especially they check for the existence of the .k5login file in the default configuration. This check can be disabled by setting KerberosUseKuserok to 'no' but then sshd does not call krb5_userok() but the more restrictive krb5_aname_to_localname() and does case sensitive checks on the related names which won't help much in out case. As a result a .k5login file is still needed when testing with ssh but it can be empty or contain random content. I will investigate why OpenSSH is patched in this way on Fedora and RHEL.
bye, Sumit
Hi,
The code works fine. I tested with commenting out the auth_to_local rules and adding the [plugins] section as you suggested. Then login as an AD user worked fine with w/o a password prompt: ssh $(hostname) -l tuser@win.example.com
About documentation of the plugin framework, I found a PDF in the krb5-1.12 tarball very helpful. It's located in doc/pdf/plugindev.pdf in the tarball.
See couple of questions inline. With the exception of the unparse_name they are mostly questions and I don't insist on any of them.
From 7270f9583d6e0bb1bd881be0fc3ee49965d40193 Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Wed, 16 Jul 2014 21:43:30 +0200 Subject: [PATCH 1/2] Add conditional build for MIT Kerberos localauth plugin
This patch adds everything what is needed to build the MIT Kerberos localauth plugin if the used version of MIT Kerberos supports it. It does not implement the plugin.
Makefile.am | 19 ++++++++++++++++++ configure.ac | 1 + contrib/sssd.spec.in | 11 +++++++++++ src/conf_macros.m4 | 14 ++++++++++++++ src/external/krb5.m4 | 15 ++++++++++++++ src/krb5_plugin/sssd_krb5_localauth_plugin.c | 29 ++++++++++++++++++++++++++++ src/tests/dlopen-tests.c | 4 ++++ 7 files changed, 93 insertions(+) create mode 100644 src/krb5_plugin/sssd_krb5_localauth_plugin.c
[...]
--- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -48,6 +48,10 @@ %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin %endif
+%if (0%{?fedora} >= 21)
Do you think it's time to add a rule for RHEL-7.1 already? I'm just concerned that we'll forget later. Alternatively we can just file a ticket.
Checking minor versions is a bit tricky, I hope you agree with my solution.
- %global with_krb5_localauth_plugin 1
+%endif
Name: @PACKAGE_NAME@ Version: @PACKAGE_VERSION@ Release: 0@PRERELEASE_VERSION@%{?dist} @@ -113,7 +117,11 @@ BuildRequires: pcre-devel BuildRequires: libxslt BuildRequires: libxml2 BuildRequires: docbook-style-xsl +%if (0%{?with_krb5_localauth_plugin} == 1) +BuildRequires: krb5-devel >= 1.12 +%else BuildRequires: krb5-devel +%endif BuildRequires: c-ares-devel BuildRequires: python-devel BuildRequires: check-devel @@ -746,6 +754,9 @@ rm -rf $RPM_BUILD_ROOT %{_libdir}/cifs-utils/cifs_idmap_sss.so %ghost %{_sysconfdir}/cifs-utils/idmap-plugin %endif +%if (0%{?with_krb5_localauth_plugin} == 1) +%{_libdir}/%{name}/sssd_krb5_localauth_plugin.so +%endif %{_mandir}/man8/pam_sss.8* %{_mandir}/man8/sssd_krb5_locator_plugin.8*
diff --git a/src/conf_macros.m4 b/src/conf_macros.m4 index 3c9827b..368820c 100644 --- a/src/conf_macros.m4 +++ b/src/conf_macros.m4 @@ -274,6 +274,20 @@ AC_DEFUN([WITH_KRB5_PLUGIN_PATH], AC_SUBST(krb5pluginpath) ])
+AC_DEFUN([WITH_KRB5_LOCALAUTH_PLUGIN_PATH],
- [ AC_ARG_WITH([krb5-localauth-plugin-path],
[AC_HELP_STRING([--with-krb5-localauth-plugin-path=PATH],[Path to kerberos localauth plugin store [/usr/lib/sssd]]/usr/lib/sssd/ is used by internal sssd libraries, such as the providers. Would it make sense to set the default location to /usr/lib/sssd/modules which is where we put the autofs plugin currently and we also have a patch to move the sudo plugin there.
good idea, done.
)])- krb5localauth_pluginpath="${libdir}/sssd"
- if test x"$with_krb5_localauth_plugin_path" != x; then
krb5localauth_pluginpath=$with_krb5_localauth_plugin_path- fi
- AC_SUBST(krb5localauth_pluginpath)
- ])
AC_DEFUN([WITH_CIFS_PLUGIN_PATH], [ AC_ARG_WITH([cifs-plugin-path], [AC_HELP_STRING([--with-cifs-plugin-path=PATH],
From 6bd15ceb7b0991a5ad0747d1ce2b29e6c4772e5c Mon Sep 17 00:00:00 2001 From: Sumit Bose sbose@redhat.com Date: Thu, 17 Jul 2014 21:08:20 +0200 Subject: [PATCH 2/2] Implement MIT Kerberos localauth plugin
The MIT Kerberos localauth pluing interface defines two different calls.
The first checks if a given Kerberos principal relates to a given name of a local user (userok). The implementation lets SSSD resolve the principal and the user name and if the returned user entries both have the same UID success is returned.
The second translates a given Kerberos principal to a local user name (a2l). Here SSSD is only called once to resolve the principal and the user name is returned.
Resolves https://fedorahosted.org/sssd/ticket/1835
[...]
diff --git a/src/krb5_plugin/sssd_krb5_localauth_plugin.c b/src/krb5_plugin/sssd_krb5_localauth_plugin.c index 5e38ca5..f3dacc2 100644 --- a/src/krb5_plugin/sssd_krb5_localauth_plugin.c +++ b/src/krb5_plugin/sssd_krb5_localauth_plugin.c @@ -18,12 +18,151 @@ along with this program. If not, see http://www.gnu.org/licenses/. */
+#include <nss.h> +#include <sys/types.h> +#include <pwd.h> +#include <string.h> +#include <strings.h> +#include <errno.h>
#include <krb5/localauth_plugin.h>
+enum nss_status _nss_sss_getpwnam_r(const char *name, struct passwd *result,
char *buffer, size_t buflen, int *errnop);+#define DEFAULT_BUFSIZE 4096
OK, this should be more than enough, sysconf(_SC_GETPW_R_SIZE_MAX) returns 1024 on F-20.
+static krb5_error_code sss_userok(krb5_context context,
krb5_localauth_moddata data,krb5_const_principal aname,const char *lname)+{
- krb5_error_code kerr;
- char *princ_str;
- struct passwd pwd = { 0 };
- char *buffer = NULL;
- size_t buflen;
- enum nss_status nss_status;
- int nss_errno;
- uid_t princ_uid;
- int ret;
- kerr = krb5_unparse_name(context, aname, &princ_str);
Looks like you're leaking princ_str here, I think you should call krb5_free_unparsed_name() in the done: handler.
fixed
- if (kerr != 0) {
return kerr;- }
- if (strcasecmp(princ_str, lname) == 0) {
return 0;- }
- buflen = DEFAULT_BUFSIZE;
- buffer = malloc(buflen);
- if (buffer == NULL) {
return ENOMEM;- }
- nss_status = _nss_sss_getpwnam_r(princ_str, &pwd, buffer, buflen,
&nss_errno);- if (nss_status != NSS_STATUS_SUCCESS) {
Question -- if the getpwnam_r call returns that sssd doesn't know the username, does it make sense to return KRB5_PLUGIN_NO_HANDLE to let another plugin handle the user?
fixed
ret = EIO;goto done;- }
- princ_uid = pwd.pw_uid;
- nss_status = _nss_sss_getpwnam_r(lname, &pwd, buffer, buflen, &nss_errno);
- if (nss_status != NSS_STATUS_SUCCESS) {
Same question here.
fixed
ret = EIO;goto done;- }
- if (princ_uid != pwd.pw_uid) {
ret = EPERM;OK, documentation says EPERM is the error code krb5 expects.
goto done;- }
- ret = 0;
+done:
- free(buffer);
- return ret;
+}
+static krb5_error_code sss_an2ln(krb5_context context,
krb5_localauth_moddata data,const char *type, const char *residual,krb5_const_principal aname, char **lname_out)+{
- krb5_error_code kerr;
- char *princ_str;
- struct passwd pwd = { 0 };
- char *buffer = NULL;
- size_t buflen;
- enum nss_status nss_status;
- int nss_errno;
- int ret;
- char *str;
- kerr = krb5_unparse_name(context, aname, &princ_str);
Looks like another leak here.
fixed
- if (kerr != 0) {
return kerr;- }
- buflen = DEFAULT_BUFSIZE;
- buffer = malloc(buflen);
- if (buffer == NULL) {
return ENOMEM;- }
- nss_status = _nss_sss_getpwnam_r(princ_str, &pwd, buffer, buflen,
&nss_errno);- if (nss_status != NSS_STATUS_SUCCESS) {
ret = EIO;goto done;- }
- str = strdup(pwd.pw_name);
- if (str == NULL) {
ret = ENOMEM;goto done;- }
- *lname_out = str;
- ret = 0;
+done:
- free(buffer);
- return ret;
+}
+static void sss_freestr(krb5_context context,
krb5_localauth_moddata data, char *str)+{
- free(str);
ok, documentation says that whenever an2ln is implemented, free_string must be implemented do deallocate whatever was allocated with an2ln.
+}
krb5_error_code localauth_sssd_initvt(krb5_context context, int maj_ver, int min_ver, krb5_plugin_vtable vtable) {
- return KRB5_PLUGIN_VER_NOTSUPP;
- if (maj_ver != 1 || min_ver != 1) {
return KRB5_PLUGIN_VER_NOTSUPP;- }
Why do you also check min_ver here? All the plugins currently in tree only check maj_ver as far as I can see. Or rather, what is the semantics of the min_ver number?
If I understood the doc correctly the minor version can be used to enable/disable elements in the vtable.
- krb5_localauth_vtable vt = (krb5_localauth_vtable)vtable;
- vt->init = NULL;
- vt->fini = NULL;
- vt->name = "sssd";
- vt->an2ln = sss_an2ln;
- vt->userok = sss_userok;
- vt->free_string = sss_freestr;
- return 0;
}
-- 1.8.3.1
New version attached.
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
I'm sorry about the delay in review. I had to rebuild my test infrastructure...
On Thu, Aug 21, 2014 at 06:36:13PM +0200, Sumit Bose wrote:
--- a/contrib/sssd.spec.in +++ b/contrib/sssd.spec.in @@ -48,6 +48,10 @@ %global with_cifs_utils_plugin_option --disable-cifs-idmap-plugin %endif
+%if (0%{?fedora} >= 21)
Do you think it's time to add a rule for RHEL-7.1 already? I'm just concerned that we'll forget later. Alternatively we can just file a ticket.
Checking minor versions is a bit tricky, I hope you agree with my solution.
I did test by manually editing /etc/redhat-release and it seemed to work fine. Thanks!
[...]
Why do you also check min_ver here? All the plugins currently in tree only check maj_ver as far as I can see. Or rather, what is the semantics of the min_ver number?
If I understood the doc correctly the minor version can be used to enable/disable elements in the vtable.
Ah, I see. Thanks for the explanation.
- krb5_localauth_vtable vt = (krb5_localauth_vtable)vtable;
- vt->init = NULL;
- vt->fini = NULL;
- vt->name = "sssd";
- vt->an2ln = sss_an2ln;
- vt->userok = sss_userok;
- vt->free_string = sss_freestr;
- return 0;
}
-- 1.8.3.1
New version attached.
bye, Sumit
This version looks good to me.
ACK.
sssd-devel@lists.fedorahosted.org
-
Alexander Bokovoy -
Jakub Hrozek -
Simo Sorce -
Sumit Bose