[SSSD] [PATCHES] Implement MIT Kerberos localauth plugin

Jakub Hrozek jhrozek at redhat.com
Tue Aug 12 12:24:38 UTC 2014


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 at 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 at 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 at 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 at 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
> 



More information about the sssd-devel mailing list