Shell overrides

[John Hodrien]

I've just been playing with the allowed_shells stuff.  I'm using a very large
LDAP tree.  It contains mostly good data, but has some users shells set to
/bin/sh and /bin/csh.  For these users I want to override the shell to
something more sane (like /bin/bash).  But I don't want to actually remove
those shells from systems, as I want them to be available for local accounts
and scripts.

So on the client I have local accounts that make use of /bin/sh and /bin/csh
and an /etc/shells of:

/bin/sh
/bin/bash
/sbin/nologin
/bin/tcsh
/bin/csh

If I don't edit /etc/shells it'll use those shells, which isn't what I want.
If I remove it from /etc/shells, and set allowed_shells to an approved list,
it'll switch their shell to nologin.

Is there a way I can deal with this using the current setup?  I couldn't see a
way.

I was wondering about adding a new config option to allow you to mark certain
shells as being vetoed, such that the fallback shell is used instead.  Is this
in any way sane?

Ignoring the trimmings, the meat of it is just:

diff -ur sssd-1.5.11.orig/src/responder/nss/nsssrv_cmd.c
sssd-1.5.11/src/responder/nss/nsssrv_cmd.c
--- sssd-1.5.11.orig/src/responder/nss/nsssrv_cmd.c     2011-07-05 18:41:13.000000000
+0100
+++ sssd-1.5.11/src/responder/nss/nsssrv_cmd.c  2011-07-28 15:46:10.787877792 +0100
@@ -314,7 +314,18 @@

      user_shell = ldb_msg_find_attr_as_string(msg, SYSDB_SHELL, NULL);
      if (!user_shell) return NULL;
-    if (!nctx->allowed_shells) return talloc_strdup(mem_ctx, user_shell);
+    if (!nctx->allowed_shells && !nctx->vetoed_shells) return
talloc_strdup(mem_ctx, user_shell);
+
+    if (nctx->vetoed_shells)
+    {
+        for (i=0; nctx->vetoed_shells[i]; i++) {
+            if (strcmp(nctx->vetoed_shells[i], user_shell) == 0) {
+                DEBUG(5, ("The shell '%s' is vetoed. "
+                         "Using fallback\n", user_shell));
+                return talloc_strdup(mem_ctx, nctx->shell_fallback);
+            }
+        }
+    }

By all means tell me I'm mad and there are much better ways of achieving what
I'm trying to do.  I'm new to sssd, but it looks like it offers a whole raft
of advantages over nss_ldap (which I'm currently using) that makes it look
irresistable.

Thanks,

jh

[Jakub Hrozek]

On 07/29/2011 12:56 PM, John Hodrien wrote:
>  Is there a way I can deal with this using the current setup?  I
> couldn't see a
>  way. 
No, not currently. The logic works like this:

1) if the user's shell is on the machine, use it
2) if the user's shell is not installed, but is allowed, map to
fallback, usually /bin/sh
3) else /sbin/nologin

This feature was meant to cover the case where a user with a fancy shell
would try to log into a machine that does not have his shell available.

[John Hodrien]

On Fri, 29 Jul 2011, Jakub Hrozek wrote:

>  On 07/29/2011 12:56 PM, John Hodrien wrote:
> > Is there a way I can deal with this using the current setup?  I couldn't see a
> > way.
>
>  No, not currently. The logic works like this:
>
>  1) if the user's shell is on the machine, use it
>  2) if the user's shell is not installed, but is allowed, map to
>  fallback, usually /bin/sh
>  3) else /sbin/nologin
>
>  This feature was meant to cover the case where a user with a fancy shell
>  would try to log into a machine that does not have his shell available. 
Yep.  So am I a strange corner case, or is this something other people would
be interested in?

At the very least to aid people migrating from nss_ldap, it'd be good to have
the nss_ldap equivalent of:

nss_override_attribute_value loginShell /bin/bash

For my uses the additional config option actually brings sssd up a notch on
what I was able to acheive with nss_ldap, but I can understand that our setup
isn't entirely sane.

jh

[Stephen Gallagher]

On Fri, 2011-07-29 at 13:05 +0100, John Hodrien wrote:
>  On Fri, 29 Jul 2011, Jakub Hrozek wrote:
>  
>  > On 07/29/2011 12:56 PM, John Hodrien wrote:
>  >> Is there a way I can deal with this using the current setup?  I couldn't see
> a
>  >> way.
>  >
>  > No, not currently. The logic works like this:
>  >
>  > 1) if the user's shell is on the machine, use it
>  > 2) if the user's shell is not installed, but is allowed, map to
>  > fallback, usually /bin/sh
>  > 3) else /sbin/nologin
>  >
>  > This feature was meant to cover the case where a user with a fancy shell
>  > would try to log into a machine that does not have his shell available.
>  
>  Yep.  So am I a strange corner case, or is this something other people would
>  be interested in?
>  
>  At the very least to aid people migrating from nss_ldap, it'd be good to have
>  the nss_ldap equivalent of:
>  
>  nss_override_attribute_value loginShell /bin/bash
>  
>  For my uses the additional config option actually brings sssd up a notch on
>  what I was able to acheive with nss_ldap, but I can understand that our setup
>  isn't entirely sane. 

Well, it seems to me that you're probably describing a very unusual
corner case. The basic idea we work from in SSSD is that the central
data store should always be authoritative.

Phrased differently, our assumption is that if a user has set their
preferred shell in LDAP, we should do our best to honor that on any
client that supports it, but we should also allow a fallback if their
preferred shell is unavailable. (On nss_ldap without override_attribute,
the user would simply be denied).

The problem with the override_attribute option in nss_ldap is that it
forces ALL users to use the same shell on the system. It ignores their
central selection, even if that shell was available.

So my view is that your LDAP entries are wrong and should be fixed. That
said, I do see at least two possibly valid case for being able to veto a
shell as a login shell. If dash (a variant of the bourne shell used for
scripts but not logins) is present in /etc/shells for support of some
scripts on the system, we probably want to be able to ensure that it is
never used as a login shell by any user (since it's not designed to
handle this well). Granted, that's still probably another case of the
user in LDAP being misconfigured.

The second possibility is that some platforms may have an implementation
of a particular shell with serious known issues (for example, a copy
of /bin/tcsh that does not in fact include the full tcsh extensions to
csh). If an OS contains such a known-broken shell that it cannot remove
for one reason or another (such as support for a broken legacy script),
we'd want to be able to forcibly veto this as well.

So, I suppose I'm willing to see this added. John, would you be willing
to do the legwork on this if we show you where things need to be added?

[John Hodrien]

On Fri, 29 Jul 2011, Stephen Gallagher wrote:

>  So my view is that your LDAP entries are wrong and should be fixed.
> That
>  said, I do see at least two possibly valid case for being able to veto a
>  shell as a login shell. If dash (a variant of the bourne shell used for
>  scripts but not logins) is present in /etc/shells for support of some
>  scripts on the system, we probably want to be able to ensure that it is
>  never used as a login shell by any user (since it's not designed to
>  handle this well). Granted, that's still probably another case of the
>  user in LDAP being misconfigured.
>
>  The second possibility is that some platforms may have an implementation
>  of a particular shell with serious known issues (for example, a copy
>  of /bin/tcsh that does not in fact include the full tcsh extensions to
>  csh). If an OS contains such a known-broken shell that it cannot remove
>  for one reason or another (such as support for a broken legacy script),
>  we'd want to be able to forcibly veto this as well. 
I agree in our case the LDAP tree contains duff information and should be
fixed.  That's a separate line of attack I'm pursuing.

>  So, I suppose I'm willing to see this added. John, would you be
> willing
>  to do the legwork on this if we show you where things need to be added? 
I'd be happy to help on this, yes.

This is the patch I'd produced internally based on what was added already for
the override_shell stuff:

diff -ur sssd-1.5.11.orig/src/confdb/confdb.h sssd-1.5.11/src/confdb/confdb.h
--- sssd-1.5.11.orig/src/confdb/confdb.h	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/confdb/confdb.h	2011-07-28 15:39:55.455556158 +0100
@@ -74,6 +74,7 @@
  #define CONFDB_NSS_FILTER_GROUPS "filter_groups"
  #define CONFDB_NSS_PWFIELD  "pwfield"
  #define CONFDB_NSS_OVERRIDE_HOMEDIR "override_homedir"
+#define CONFDB_NSS_VETOED_SHELL  "vetoed_shells"
  #define CONFDB_NSS_ALLOWED_SHELL "allowed_shells"
  #define CONFDB_NSS_SHELL_FALLBACK "shell_fallback"

diff -ur sssd-1.5.11.orig/src/config/SSSDConfig.py sssd-1.5.11/src/config/SSSDConfig.py
--- sssd-1.5.11.orig/src/config/SSSDConfig.py	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/config/SSSDConfig.py	2011-07-28 15:41:35.106569862 +0100
@@ -60,6 +60,7 @@
      'pwfield' : _('The value of the password field the NSS provider should
return'),
      'override_homedir' : _('Override homedir value from the identity
provider with this value'),
      'allowed_shells' : _('The list of shells users are allowed to log in
with'),
+    'vetoed_shells' : _('The list of shells that will be ignored, and
replaced with the fallback shell'),
      'shell_fallback' : _('If a shell stored in central directory is allowed
but not available, use this fallback'),

      # [pam]
diff -ur sssd-1.5.11.orig/src/man/sssd.conf.5.xml sssd-1.5.11/src/man/sssd.conf.5.xml
--- sssd-1.5.11.orig/src/man/sssd.conf.5.xml	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/man/sssd.conf.5.xml	2011-07-28 15:48:23.780656479 +0100
@@ -424,6 +424,14 @@
                      </listitem>
                  </varlistentry>
                  <varlistentry>
+                    <term>vetoed_shells (string)</term>
+                    <listitem>
+                        <para>
+                            Replace any instance of these shells with the fallback_shell
+                        </para>
+                    </listitem>
+                </varlistentry>
+                <varlistentry>
                      <term>shell_fallback (string)</term>
                      <listitem>
                          <para>
diff -ur sssd-1.5.11.orig/src/responder/nss/nsssrv.c
sssd-1.5.11/src/responder/nss/nsssrv.c
--- sssd-1.5.11.orig/src/responder/nss/nsssrv.c	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/responder/nss/nsssrv.c	2011-07-28 15:47:05.574269965 +0100
@@ -188,6 +188,10 @@
                                      &nctx->allowed_shells);
      if (ret != EOK && ret != ENOENT) goto done;

+    ret = confdb_get_string_as_list(cdb, nctx, CONFDB_NSS_CONF_ENTRY,
+                                    CONFDB_NSS_VETOED_SHELL,
+                                    &nctx->vetoed_shells);
+    if (ret != EOK && ret != ENOENT) goto done;
      ret = nss_get_etc_shells(nctx, &nctx->etc_shells);
      if (ret != EOK) goto done;

diff -ur sssd-1.5.11.orig/src/responder/nss/nsssrv_cmd.c
sssd-1.5.11/src/responder/nss/nsssrv_cmd.c
--- sssd-1.5.11.orig/src/responder/nss/nsssrv_cmd.c	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/responder/nss/nsssrv_cmd.c	2011-07-28 15:46:10.787877792 +0100
@@ -314,7 +314,18 @@

      user_shell = ldb_msg_find_attr_as_string(msg, SYSDB_SHELL, NULL);
      if (!user_shell) return NULL;
-    if (!nctx->allowed_shells) return talloc_strdup(mem_ctx, user_shell);
+    if (!nctx->allowed_shells && !nctx->vetoed_shells) return
talloc_strdup(mem_ctx, user_shell);
+
+    if (nctx->vetoed_shells)
+    {
+        for (i=0; nctx->vetoed_shells[i]; i++) {
+            if (strcmp(nctx->vetoed_shells[i], user_shell) == 0) {
+                DEBUG(5, ("The shell '%s' is vetoed. "
+                         "Using fallback\n", user_shell));
+                return talloc_strdup(mem_ctx, nctx->shell_fallback);
+            }
+        }
+    }

      for (i=0; nctx->etc_shells[i]; i++) {
          if (strcmp(user_shell, nctx->etc_shells[i]) == 0) {
diff -ur sssd-1.5.11.orig/src/responder/nss/nsssrv.h
sssd-1.5.11/src/responder/nss/nsssrv.h
--- sssd-1.5.11.orig/src/responder/nss/nsssrv.h	2011-07-05 18:41:13.000000000 +0100
+++ sssd-1.5.11/src/responder/nss/nsssrv.h	2011-07-28 15:42:00.332070919 +0100
@@ -60,6 +60,7 @@

      char *override_homedir;
      char **allowed_shells;
+    char **vetoed_shells;
      char **etc_shells;
      char *shell_fallback;
  };

[Stephen Gallagher]

On Fri, 2011-07-29 at 13:52 +0100, John Hodrien wrote:
>  On Fri, 29 Jul 2011, Stephen Gallagher wrote:
>  > So, I suppose I'm willing to see this added. John, would you be willing
>  > to do the legwork on this if we show you where things need to be added?
>  
>  I'd be happy to help on this, yes.
>  
>  This is the patch I'd produced internally based on what was added already for
>  the override_shell stuff: 
<snip>

This patch looks pretty good, John, but would you mind submitting it
following the instructions at
https://fedorahosted.org/sssd/wiki/Contribute#SourceCodeRepository and
https://fedorahosted.org/sssd/wiki/BugLifecycle#Whattodowhenyoureworkingo... please?

It makes it easy to maintain attribution. Also, please ensure that your
code corresponds to the style guidelines at
http://www.freeipa.org/page/Coding_Style (I can't tell from the
copy-pasted patch in the email).

[John Hodrien]

On Fri, 29 Jul 2011, Stephen Gallagher wrote:

>  This patch looks pretty good, John, but would you mind submitting it
>  following the instructions at
>  https://fedorahosted.org/sssd/wiki/Contribute#SourceCodeRepository and
>  https://fedorahosted.org/sssd/wiki/BugLifecycle#Whattodowhenyoureworkingo... please?
>
>  It makes it easy to maintain attribution. Also, please ensure that your
>  code corresponds to the style guidelines at
>  http://www.freeipa.org/page/Coding_Style (I can't tell from the
>  copy-pasted patch in the email). 
Sure, will try to get this done today, but if I don't, it'll have to wait a
week as I'm away.

jh

[John Hodrien]

On Fri, 29 Jul 2011, Stephen Gallagher wrote:

>  This patch looks pretty good, John, but would you mind submitting it
>  following the instructions at
>  https://fedorahosted.org/sssd/wiki/Contribute#SourceCodeRepository and
>  https://fedorahosted.org/sssd/wiki/BugLifecycle#Whattodowhenyoureworkingo... please?
>
>  It makes it easy to maintain attribution. Also, please ensure that your
>  code corresponds to the style guidelines at
>  http://www.freeipa.org/page/Coding_Style (I can't tell from the
>  copy-pasted patch in the email). 
Ignoring me being a dozy fool and making a bit meal of creating it correctly,
I've hopefully done this as #945.

As far as I could tell it's compliant with the coding style.

Thanks,

jh

[Stephen Gallagher]

On Fri, 2011-07-29 at 14:59 +0100, John Hodrien wrote:
>  On Fri, 29 Jul 2011, Stephen Gallagher wrote:
>  
>  > This patch looks pretty good, John, but would you mind submitting it
>  > following the instructions at
>  > https://fedorahosted.org/sssd/wiki/Contribute#SourceCodeRepository and
>  > https://fedorahosted.org/sssd/wiki/BugLifecycle#Whattodowhenyoureworkingo...
> please?
>  >
>  > It makes it easy to maintain attribution. Also, please ensure that your
>  > code corresponds to the style guidelines at
>  > http://www.freeipa.org/page/Coding_Style (I can't tell from the
>  > copy-pasted patch in the email).
>  
>  Ignoring me being a dozy fool and making a bit meal of creating it correctly,
>  I've hopefully done this as #945.
>  
>  As far as I could tell it's compliant with the coding style. 

Yup, I saw the bug. I'd still have preferred you to create the patch
with 'git format-patch -M -C --patience --full-index -1', but I can
handle that for you.

Thanks for the patch!

[John Hodrien]

On Fri, 29 Jul 2011, Stephen Gallagher wrote:

>  Yup, I saw the bug. I'd still have preferred you to create the
> patch
>  with 'git format-patch -M -C --patience --full-index -1', but I can
>  handle that for you. 
Sorry, that's blindness on my part, as I'd skimmed and seen that as git
commands that I wasn't doing it.  Obviously I see that bit now you've
mentioned it...

>  Thanks for the patch! 
Thanks for SSSD ;)

jh

[Stephen Gallagher]

On Fri, 2011-07-29 at 15:13 +0100, John Hodrien wrote:
>  On Fri, 29 Jul 2011, Stephen Gallagher wrote:
>  
>  > Yup, I saw the bug. I'd still have preferred you to create the patch
>  > with 'git format-patch -M -C --patience --full-index -1', but I can
>  > handle that for you.
>  
>  Sorry, that's blindness on my part, as I'd skimmed and seen that as git
>  commands that I wasn't doing it.  Obviously I see that bit now you've
>  mentioned it...
>  
>  > Thanks for the patch!
>  
>  Thanks for SSSD ;) 

Pushed to master.

[Stephen Gallagher]

On Fri, 2011-07-29 at 10:17 -0400, Stephen Gallagher wrote:
>  On Fri, 2011-07-29 at 15:13 +0100, John Hodrien wrote:
>  > On Fri, 29 Jul 2011, Stephen Gallagher wrote:
>  > 
>  > > Yup, I saw the bug. I'd still have preferred you to create the patch
>  > > with 'git format-patch -M -C --patience --full-index -1', but I can
>  > > handle that for you.
>  > 
>  > Sorry, that's blindness on my part, as I'd skimmed and seen that as git
>  > commands that I wasn't doing it.  Obviously I see that bit now you've
>  > mentioned it...
>  > 
>  > > Thanks for the patch!
>  > 
>  > Thanks for SSSD ;)
>  
>  
>  Pushed to master. 
Also backported to sssd-1-5.