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(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.
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(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.
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(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel