- djm@cvs.openbsd.org 2012/11/04 10:38:43

     [auth2-pubkey.c sshd.c sshd_config.5]
     Remove default of AuthorizedCommandUser. Administrators are now expected
     to explicitly specify a user. feedback and ok markus@

4 files changed, 31 insertions, 17 deletions

diff --git a/ChangeLog b/ChangeLog
index f1d0580fc..120c132af 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,10 @@
    - jmc@cvs.openbsd.org 2012/10/31 08:04:50
      [sshd_config.5]
      tweak previous;
+   - djm@cvs.openbsd.org 2012/11/04 10:38:43
+     [auth2-pubkey.c sshd.c sshd_config.5]
+     Remove default of AuthorizedCommandUser. Administrators are now expected
+     to explicitly specify a user. feedback and ok markus@
 
 20121030
  - (djm) OpenBSD CVS Sync
diff --git a/auth2-pubkey.c b/auth2-pubkey.c
index ec8f75d57..6a6217017 100644
--- a/auth2-pubkey.c
+++ b/auth2-pubkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-pubkey.c,v 1.31 2012/10/30 21:29:54 djm Exp $ */
+/* $OpenBSD: auth2-pubkey.c,v 1.32 2012/11/04 10:38:43 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -462,23 +462,27 @@ user_key_command_allowed2(struct passwd *user_pw, Key *key)
         struct stat st;
         int status, devnull, p[2], i;
         pid_t pid;
-        char errmsg[512];
+        char *username, errmsg[512];
 
         if (options.authorized_keys_command == NULL ||
             options.authorized_keys_command[0] != '/')
                 return 0;
 
-        /* If no user specified to run commands the default to target user */
-        if (options.authorized_keys_command_user == NULL)
-                pw = user_pw;
-        else {
-                pw = getpwnam(options.authorized_keys_command_user);
-                if (pw == NULL) {
-                        error("AuthorizedKeyCommandUser \"%s\" not found: %s",
-                            options.authorized_keys_command, strerror(errno));
-                        return 0;
-                }
+        if (options.authorized_keys_command_user == NULL) {
+                error("No user for AuthorizedKeysCommand specified, skipping");
+                return 0;
+        }
+
+        username = percent_expand(options.authorized_keys_command_user,
+            "u", user_pw->pw_name, (char *)NULL);
+        pw = getpwnam(username);
+        if (pw == NULL) {
+                error("AuthorizedKeyCommandUser \"%s\" not found: %s",
+                    options.authorized_keys_command, strerror(errno));
+                free(username);
+                return 0;
         }
+        free(username);
 
         temporarily_use_uid(pw);
 
@@ -517,6 +521,7 @@ user_key_command_allowed2(struct passwd *user_pw, Key *key)
                 for (i = 0; i < NSIG; i++)
                         signal(i, SIG_DFL);
 
+                closefrom(STDERR_FILENO + 1);
                 /* Don't use permanently_set_uid() here to avoid fatal() */
                 if (setresgid(pw->pw_gid, pw->pw_gid, pw->pw_gid) != 0) {
                         error("setresgid %u: %s", (u_int)pw->pw_gid,
@@ -541,7 +546,6 @@ user_key_command_allowed2(struct passwd *user_pw, Key *key)
                         error("%s: dup2: %s", __func__, strerror(errno));
                         _exit(1);
                 }
-                closefrom(STDERR_FILENO + 1);
 
                 execl(options.authorized_keys_command,
                     options.authorized_keys_command, pw->pw_name, NULL);
diff --git a/sshd.c b/sshd.c
index eff0290b0..4ad1a4bd1 100644
--- a/sshd.c
+++ b/sshd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshd.c,v 1.394 2012/10/30 21:29:55 djm Exp $ */
+/* $OpenBSD: sshd.c,v 1.395 2012/11/04 10:38:43 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1559,6 +1559,13 @@ main(int ac, char **av)
         if (options.challenge_response_authentication)
                 options.kbd_interactive_authentication = 1;
 
+        /* Check that options are sensible */
+        if (options.authorized_keys_command_user == NULL &&
+            (options.authorized_keys_command != NULL &&
+            strcasecmp(options.authorized_keys_command, "none") != 0))
+                fatal("AuthorizedKeysCommand set without "
+                    "AuthorizedKeysCommandUser");
+
         /* set default channel AF */
         channel_set_af(options.address_family);
 
diff --git a/sshd_config.5 b/sshd_config.5
index 3cc901a82..0fb0b837d 100644
--- a/sshd_config.5
+++ b/sshd_config.5
@@ -33,8 +33,8 @@
 .\" (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
 .\" THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.\" $OpenBSD: sshd_config.5,v 1.147 2012/10/31 08:04:50 jmc Exp $
-.Dd $Mdocdate: October 31 2012 $
+.\" $OpenBSD: sshd_config.5,v 1.148 2012/11/04 10:38:43 djm Exp $
+.Dd $Mdocdate: November 4 2012 $
 .Dt SSHD_CONFIG 5
 .Os
 .Sh NAME
@@ -166,7 +166,6 @@ files.
 By default, no AuthorizedKeysCommand is run.
 .It Cm AuthorizedKeysCommandUser
 Specifies the user under whose account the AuthorizedKeysCommand is run.
-The default is the user being authenticated.
 It is recommended to use a dedicated user that has no other role on the host
 than running authorized keys commands.
 .It Cm AuthorizedKeysFile