tighten pledge(2) after session establishment

feedback, ok & testing in snaps deraadt@

diff --git a/usr.bin/ssh/clientloop.c b/usr.bin/ssh/clientloop.c
index d17e3aa..f747202 100644 (file)
--- a/usr.bin/ssh/clientloop.c
+++ b/usr.bin/ssh/clientloop.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: clientloop.c,v 1.383 2022/11/28 01:37:36 djm Exp $ */
+/* $OpenBSD: clientloop.c,v 1.384 2022/11/28 01:38:22 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -151,6 +151,8 @@ static int need_rekeying;   /* Set to non-zero if rekeying is requested. */
 static int session_closed;     /* In SSH2: login session closed. */
 static u_int x11_refuse_time;  /* If >0, refuse x11 opens after this time. */
 static time_t server_alive_time;       /* Time to do server_alive_check */
+static int hostkeys_update_complete;
+static int session_setup_complete;
 
 static void client_init_dispatch(struct ssh *ssh);
 int    session_ident = -1;
@@ -748,6 +750,72 @@ client_register_global_confirm(global_confirm_cb *cb, void *ctx)
        TAILQ_INSERT_TAIL(&global_confirms, gc, entry);
 }
 
+/*
+ * Returns non-zero if the client is able to handle a hostkeys-00@openssh.com
+ * hostkey update request.
+ */
+static int
+can_update_hostkeys(void)
+{
+       if (hostkeys_update_complete)
+               return 0;
+       if (options.update_hostkeys == SSH_UPDATE_HOSTKEYS_ASK &&
+           options.batch_mode)
+               return 0; /* won't ask in batchmode, so don't even try */
+       if (!options.update_hostkeys || options.num_user_hostfiles <= 0)
+               return 0;
+       return 1;
+}
+
+void
+client_repledge()
+{
+       debug3_f("enter");
+
+       /* Might be able to tighten pledge now that session is established */
+       if (options.control_master || options.control_path != NULL ||
+           options.forward_x11 || options.fork_after_authentication ||
+           can_update_hostkeys() ||
+           (session_ident != -1 && !session_setup_complete)) {
+               /* Can't tighten */
+               return;
+       }
+       /*
+        * LocalCommand and UpdateHostkeys have finished, so can get rid of
+        * filesystem.
+        *
+        * XXX protocol allows a server can to change hostkeys during the
+        *     connection at rekey time that could trigger a hostkeys update
+        *     but AFAIK no implementations support this. Could improve by
+        *     forcing known_hosts to be read-only or via unveil(2).
+        */
+       if (options.num_local_forwards != 0 ||
+           options.num_remote_forwards != 0 ||
+           options.num_permitted_remote_opens != 0 ||
+           options.enable_escape_commandline != 0) {
+               /* rfwd needs inet */
+               debug("pledge: network");
+               if (pledge("stdio unix inet dns proc tty", NULL) == -1)
+                       fatal_f("pledge(): %s", strerror(errno));
+       } else if (options.forward_agent != 0) {
+               /* agent forwarding needs to open $SSH_AUTH_SOCK at will */
+               debug("pledge: agent");
+               if (pledge("stdio unix proc tty", NULL) == -1)
+                       fatal_f("pledge(): %s", strerror(errno));
+       } else {
+               debug("pledge: fork");
+               if (pledge("stdio proc tty", NULL) == -1)
+                       fatal_f("pledge(): %s", strerror(errno));
+       }
+       /* XXX further things to do:
+        *
+        * - might be able to get rid of proc if we kill ~^Z
+        * - ssh -N (no session)
+        * - stdio forwarding
+        * - sessions without tty
+        */
+}
+
 static void
 process_cmdline(struct ssh *ssh)
 {
@@ -1229,6 +1297,7 @@ client_loop(struct ssh *ssh, int have_pty, int escape_char_arg,
        int conn_in_ready, conn_out_ready;
 
        debug("Entering interactive session.");
+       session_ident = ssh2_chan_id;
 
        if (options.control_master &&
            !option_clear_or_none(options.control_path)) {
@@ -1261,6 +1330,9 @@ client_loop(struct ssh *ssh, int have_pty, int escape_char_arg,
                        fatal_f("pledge(): %s", strerror(errno));
        }
 
+       /* might be able to tighten now */
+       client_repledge();
+
        start_time = monotime_double();
 
        /* Initialize variables. */
@@ -1294,7 +1366,6 @@ client_loop(struct ssh *ssh, int have_pty, int escape_char_arg,
        if (have_pty)
                enter_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
 
-       session_ident = ssh2_chan_id;
        if (session_ident != -1) {
                if (escape_char_arg != SSH_ESCAPECHAR_NONE) {
                        channel_register_filter(ssh, session_ident,
@@ -2194,6 +2265,8 @@ client_global_hostkeys_prove_confirm(struct ssh *ssh, int type,
        update_known_hosts(ctx);
  out:
        hostkeys_update_ctx_free(ctx);
+       hostkeys_update_complete = 1;
+       client_repledge();
 }
 
 /*
@@ -2227,7 +2300,7 @@ client_input_hostkeys(struct ssh *ssh)
        size_t i, len = 0;
        struct sshbuf *buf = NULL;
        struct sshkey *key = NULL, **tmp;
-       int r;
+       int r, prove_sent = 0;
        char *fp;
        static int hostkeys_seen = 0; /* XXX use struct ssh */
        extern struct sockaddr_storage hostaddr; /* XXX from ssh.c */
@@ -2236,11 +2309,9 @@ client_input_hostkeys(struct ssh *ssh)
 
        if (hostkeys_seen)
                fatal_f("server already sent hostkeys");
-       if (options.update_hostkeys == SSH_UPDATE_HOSTKEYS_ASK &&
-           options.batch_mode)
-               return 1; /* won't ask in batchmode, so don't even try */
-       if (!options.update_hostkeys || options.num_user_hostfiles <= 0)
+       if (!can_update_hostkeys())
                return 1;
+       hostkeys_seen = 1;
 
        ctx = xcalloc(1, sizeof(*ctx));
        while (ssh_packet_remaining(ssh) > 0) {
@@ -2409,12 +2480,18 @@ client_input_hostkeys(struct ssh *ssh)
        client_register_global_confirm(
            client_global_hostkeys_prove_confirm, ctx);
        ctx = NULL;  /* will be freed in callback */
+       prove_sent = 1;
 
        /* Success */
  out:
        hostkeys_update_ctx_free(ctx);
        sshkey_free(key);
        sshbuf_free(buf);
+       if (!prove_sent) {
+               /* UpdateHostkeys handling completed */
+               hostkeys_update_complete = 1;
+               client_repledge();
+       }
        /*
         * NB. Return success for all cases. The server doesn't need to know
         * what the client does with its hosts file.
@@ -2570,6 +2647,9 @@ client_session2_setup(struct ssh *ssh, int id, int want_tty, int want_subsystem,
                if ((r = sshpkt_send(ssh)) != 0)
                        fatal_fr(r, "send shell");
        }
+
+       session_setup_complete = 1;
+       client_repledge();
 }
 
 static void