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(a)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(a)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(a)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