From 82177956daeb425a0df25866c2345647bb6c5b28 Mon Sep 17 00:00:00 2001
From: Simon Tatham <anakin@pobox.com>
Date: Sun, 12 Sep 2021 09:52:46 +0100
Subject: [PATCH] Divide seat_set_trust_status into query and update.

This complicates the API in one sense (more separate functions), but
in another sense, simplifies it (each function does something
simpler). When I start putting one Seat in front of another during SSH
proxying, the latter will be more important - in particular, it means
you can find out _whether_ a seat can support changing trust status
without having to actually attempt a destructive modification.
---
 pscp.c                   |  3 ++-
 psftp.c                  |  3 ++-
 putty.h                  | 22 ++++++++++++++++------
 ssh/connection1-client.c |  3 ++-
 ssh/connection2-client.c |  3 ++-
 ssh/server.c             |  1 +
 ssh/sesschan.c           |  1 +
 ssh/userauth2-client.c   |  3 ++-
 sshproxy.c               | 25 +++++++++++++++----------
 unix/console.c           | 15 ++++++++++++---
 unix/plink.c             |  1 +
 unix/window.c            | 10 ++++++++--
 utils/nullseat.c         |  5 +++--
 windows/console.c        | 15 ++++++++++++---
 windows/plink.c          |  1 +
 windows/window.c         | 10 ++++++++--
 16 files changed, 88 insertions(+), 33 deletions(-)

diff --git a/pscp.c b/pscp.c
index f8a6965c..a200484f 100644
--- a/pscp.c
+++ b/pscp.c
@@ -82,7 +82,8 @@ static const SeatVtable pscp_seat_vt = {
     .get_windowid = nullseat_get_windowid,
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = console_stripctrl_new,
-    .set_trust_status = nullseat_set_trust_status_vacuously,
+    .set_trust_status = nullseat_set_trust_status,
+    .can_set_trust_status = nullseat_can_set_trust_status_yes,
     .verbose = cmdline_seat_verbose,
     .interactive = nullseat_interactive_no,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/psftp.c b/psftp.c
index 1fff45ef..ea52b23e 100644
--- a/psftp.c
+++ b/psftp.c
@@ -64,7 +64,8 @@ static const SeatVtable psftp_seat_vt = {
     .get_windowid = nullseat_get_windowid,
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = console_stripctrl_new,
-    .set_trust_status = nullseat_set_trust_status_vacuously,
+    .set_trust_status = nullseat_set_trust_status,
+    .can_set_trust_status = nullseat_can_set_trust_status_yes,
     .verbose = cmdline_seat_verbose,
     .interactive = nullseat_interactive_yes,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/putty.h b/putty.h
index 2b544e54..382c010d 100644
--- a/putty.h
+++ b/putty.h
@@ -1097,13 +1097,19 @@ struct SeatVtable {
      * (and hence, can be trusted if it's asking you for secrets such
      * as your passphrase); false means output is coming from the
      * server.
+     */
+    void (*set_trust_status)(Seat *seat, bool trusted);
+
+    /*
+     * Query whether this Seat can do anything user-visible in
+     * response to set_trust_status.
      *
      * Returns true if the seat has a way to indicate this
      * distinction. Returns false if not, in which case the backend
      * should use a fallback defence against spoofing of PuTTY's local
      * prompts by malicious servers.
      */
-    bool (*set_trust_status)(Seat *seat, bool trusted);
+    bool (*can_set_trust_status)(Seat *seat);
 
     /*
      * Ask the seat whether it would like verbose messages.
@@ -1169,8 +1175,10 @@ static inline bool seat_get_window_pixel_size(Seat *seat, int *w, int *h)
 static inline StripCtrlChars *seat_stripctrl_new(
     Seat *seat, BinarySink *bs, SeatInteractionContext sic)
 { return seat->vt->stripctrl_new(seat, bs, sic); }
-static inline bool seat_set_trust_status(Seat *seat, bool trusted)
-{ return  seat->vt->set_trust_status(seat, trusted); }
+static inline void seat_set_trust_status(Seat *seat, bool trusted)
+{ seat->vt->set_trust_status(seat, trusted); }
+static inline bool seat_can_set_trust_status(Seat *seat)
+{ return seat->vt->can_set_trust_status(seat); }
 static inline bool seat_verbose(Seat *seat)
 { return seat->vt->verbose(seat); }
 static inline bool seat_interactive(Seat *seat)
@@ -1229,8 +1237,9 @@ bool nullseat_get_windowid(Seat *seat, long *id_out);
 bool nullseat_get_window_pixel_size(Seat *seat, int *width, int *height);
 StripCtrlChars *nullseat_stripctrl_new(
         Seat *seat, BinarySink *bs_out, SeatInteractionContext sic);
-bool nullseat_set_trust_status(Seat *seat, bool trusted);
-bool nullseat_set_trust_status_vacuously(Seat *seat, bool trusted);
+void nullseat_set_trust_status(Seat *seat, bool trusted);
+bool nullseat_can_set_trust_status_yes(Seat *seat);
+bool nullseat_can_set_trust_status_no(Seat *seat);
 bool nullseat_verbose_no(Seat *seat);
 bool nullseat_verbose_yes(Seat *seat);
 bool nullseat_interactive_no(Seat *seat);
@@ -1255,7 +1264,8 @@ int console_confirm_weak_cached_hostkey(
     void (*callback)(void *ctx, int result), void *ctx);
 StripCtrlChars *console_stripctrl_new(
         Seat *seat, BinarySink *bs_out, SeatInteractionContext sic);
-bool console_set_trust_status(Seat *seat, bool trusted);
+void console_set_trust_status(Seat *seat, bool trusted);
+bool console_can_set_trust_status(Seat *seat);
 
 /*
  * Other centralised seat functions.
diff --git a/ssh/connection1-client.c b/ssh/connection1-client.c
index f32fefcd..7429b892 100644
--- a/ssh/connection1-client.c
+++ b/ssh/connection1-client.c
@@ -543,5 +543,6 @@ SshChannel *ssh1_serverside_agent_open(ConnectionLayer *cl, Channel *chan)
 
 bool ssh1_connection_need_antispoof_prompt(struct ssh1_connection_state *s)
 {
-    return !seat_set_trust_status(s->ppl.seat, false);
+    seat_set_trust_status(s->ppl.seat, false);
+    return !seat_can_set_trust_status(s->ppl.seat);
 }
diff --git a/ssh/connection2-client.c b/ssh/connection2-client.c
index b07e1eb2..88338500 100644
--- a/ssh/connection2-client.c
+++ b/ssh/connection2-client.c
@@ -500,6 +500,7 @@ void ssh2channel_send_terminal_size_change(SshChannel *sc, int w, int h)
 
 bool ssh2_connection_need_antispoof_prompt(struct ssh2_connection_state *s)
 {
-    bool success = seat_set_trust_status(s->ppl.seat, false);
+    seat_set_trust_status(s->ppl.seat, false);
+    bool success = seat_can_set_trust_status(s->ppl.seat);
     return (!success && !ssh_is_bare(s->ppl.ssh));
 }
diff --git a/ssh/server.c b/ssh/server.c
index 6abbf332..e6d7f605 100644
--- a/ssh/server.c
+++ b/ssh/server.c
@@ -125,6 +125,7 @@ static const SeatVtable server_seat_vt = {
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = nullseat_stripctrl_new,
     .set_trust_status = nullseat_set_trust_status,
+    .can_set_trust_status = nullseat_can_set_trust_status_no,
     .verbose = nullseat_verbose_no,
     .interactive = nullseat_interactive_no,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/ssh/sesschan.c b/ssh/sesschan.c
index f16faad2..7a2062a8 100644
--- a/ssh/sesschan.c
+++ b/ssh/sesschan.c
@@ -204,6 +204,7 @@ static const SeatVtable sesschan_seat_vt = {
     .get_window_pixel_size = sesschan_get_window_pixel_size,
     .stripctrl_new = nullseat_stripctrl_new,
     .set_trust_status = nullseat_set_trust_status,
+    .can_set_trust_status = nullseat_can_set_trust_status_no,
     .verbose = nullseat_verbose_no,
     .interactive = nullseat_interactive_no,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/ssh/userauth2-client.c b/ssh/userauth2-client.c
index ddce251a..0fa1df06 100644
--- a/ssh/userauth2-client.c
+++ b/ssh/userauth2-client.c
@@ -1950,7 +1950,8 @@ static void ssh2_userauth_antispoof_msg(
     struct ssh2_userauth_state *s, const char *msg)
 {
     strbuf *sb = strbuf_new();
-    if (seat_set_trust_status(s->ppl.seat, true)) {
+    seat_set_trust_status(s->ppl.seat, true);
+    if (seat_can_set_trust_status(s->ppl.seat)) {
         /*
          * If the seat can directly indicate that this message is
          * generated by the client, then we can just use the message
diff --git a/sshproxy.c b/sshproxy.c
index 18a1b9da..955012a8 100644
--- a/sshproxy.c
+++ b/sshproxy.c
@@ -405,20 +405,24 @@ static int sshproxy_confirm_weak_cached_hostkey(
     return 0;
 }
 
-static bool sshproxy_set_trust_status(Seat *seat, bool trusted)
+static void sshproxy_set_trust_status(Seat *seat, bool trusted)
 {
     /*
      * This is called by the proxy SSH connection, to set our Seat
-     * into a given trust status. We can safely do nothing here and
-     * return true to claim we did something (effectively eliminating
-     * the spoofing defences completely, by suppressing the 'press
-     * Return to begin session' prompt and not providing anything in
-     * place of it), on the basis that session I/O from the proxy SSH
-     * connection is never passed directly on to the end user, so a
-     * malicious proxy SSH server wouldn't be able to spoof our human
-     * in any case.
+     * into a given trust status. We can safely do nothing here, and
+     * have can_set_trust_status return true to claim we did something
+     * (effectively eliminating the spoofing defences completely, by
+     * suppressing the 'press Return to begin session' prompt and not
+     * providing anything in place of it), on the basis that session
+     * I/O from the proxy SSH connection is never passed directly on
+     * to the end user, so a malicious proxy SSH server wouldn't be
+     * able to spoof our human in any case.
      */
-    return true;
+}
+
+static bool sshproxy_can_set_trust_status(Seat *seat)
+{
+    return true; /* see comment above */
 }
 
 static const SeatVtable SshProxy_seat_vt = {
@@ -442,6 +446,7 @@ static const SeatVtable SshProxy_seat_vt = {
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = nullseat_stripctrl_new,
     .set_trust_status = sshproxy_set_trust_status,
+    .can_set_trust_status = sshproxy_can_set_trust_status,
     .verbose = nullseat_verbose_no,
     .interactive = nullseat_interactive_no,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/unix/console.c b/unix/console.c
index 90e73a98..ffe777fd 100644
--- a/unix/console.c
+++ b/unix/console.c
@@ -321,7 +321,16 @@ int console_askappend(LogPolicy *lp, Filename *filename,
 }
 
 bool console_antispoof_prompt = true;
-bool console_set_trust_status(Seat *seat, bool trusted)
+
+void console_set_trust_status(Seat *seat, bool trusted)
+{
+    /* Do nothing in response to a change of trust status, because
+     * there's nothing we can do in a console environment. However,
+     * the query function below will make a fiddly decision about
+     * whether to tell the backend to enable fallback handling. */
+}
+
+bool console_can_set_trust_status(Seat *seat)
 {
     if (console_batch_mode || !is_interactive() || !console_antispoof_prompt) {
         /*
@@ -334,8 +343,8 @@ bool console_set_trust_status(Seat *seat, bool trusted)
          * prompt, the user couldn't respond to it via the terminal
          * anyway.
          *
-         * We also vacuously return success if the user has purposely
-         * disabled the antispoof prompt.
+         * We also return true without enabling any defences if the
+         * user has purposely disabled the antispoof prompt.
          */
         return true;
     }
diff --git a/unix/plink.c b/unix/plink.c
index 7663b976..a1f640a1 100644
--- a/unix/plink.c
+++ b/unix/plink.c
@@ -407,6 +407,7 @@ static const SeatVtable plink_seat_vt = {
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = console_stripctrl_new,
     .set_trust_status = console_set_trust_status,
+    .can_set_trust_status = console_can_set_trust_status,
     .verbose = cmdline_seat_verbose,
     .interactive = plink_seat_interactive,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/unix/window.c b/unix/window.c
index 8582777b..e5b541c8 100644
--- a/unix/window.c
+++ b/unix/window.c
@@ -383,7 +383,8 @@ static const char *gtk_seat_get_x_display(Seat *seat);
 #ifndef NOT_X_WINDOWS
 static bool gtk_seat_get_windowid(Seat *seat, long *id);
 #endif
-static bool gtk_seat_set_trust_status(Seat *seat, bool trusted);
+static void gtk_seat_set_trust_status(Seat *seat, bool trusted);
+static bool gtk_seat_can_set_trust_status(Seat *seat);
 static bool gtk_seat_get_cursor_position(Seat *seat, int *x, int *y);
 
 static const SeatVtable gtk_seat_vt = {
@@ -411,6 +412,7 @@ static const SeatVtable gtk_seat_vt = {
     .get_window_pixel_size = gtk_seat_get_window_pixel_size,
     .stripctrl_new = gtk_seat_stripctrl_new,
     .set_trust_status = gtk_seat_set_trust_status,
+    .can_set_trust_status = gtk_seat_can_set_trust_status,
     .verbose = nullseat_verbose_yes,
     .interactive = nullseat_interactive_yes,
     .get_cursor_position = gtk_seat_get_cursor_position,
@@ -5415,10 +5417,14 @@ void new_session_window(Conf *conf, const char *geometry_string)
         ldisc_echoedit_update(inst->ldisc); /* cause ldisc to notice changes */
 }
 
-static bool gtk_seat_set_trust_status(Seat *seat, bool trusted)
+static void gtk_seat_set_trust_status(Seat *seat, bool trusted)
 {
     GtkFrontend *inst = container_of(seat, GtkFrontend, seat);
     term_set_trust_status(inst->term, trusted);
+}
+
+static bool gtk_seat_can_set_trust_status(Seat *seat)
+{
     return true;
 }
 
diff --git a/utils/nullseat.c b/utils/nullseat.c
index 0c773af0..5e601669 100644
--- a/utils/nullseat.c
+++ b/utils/nullseat.c
@@ -35,8 +35,9 @@ bool nullseat_get_window_pixel_size(
     Seat *seat, int *width, int *height) { return false; }
 StripCtrlChars *nullseat_stripctrl_new(
     Seat *seat, BinarySink *bs_out, SeatInteractionContext sic) {return NULL;}
-bool nullseat_set_trust_status(Seat *seat, bool tr) { return false; }
-bool nullseat_set_trust_status_vacuously(Seat *seat, bool tr) { return true; }
+void nullseat_set_trust_status(Seat *seat, bool trusted) {}
+bool nullseat_can_set_trust_status_yes(Seat *seat) { return true; }
+bool nullseat_can_set_trust_status_no(Seat *seat) { return false; }
 bool nullseat_verbose_no(Seat *seat) { return false; }
 bool nullseat_verbose_yes(Seat *seat) { return true; }
 bool nullseat_interactive_no(Seat *seat) { return false; }
diff --git a/windows/console.c b/windows/console.c
index 414167b4..9bdfb6f4 100644
--- a/windows/console.c
+++ b/windows/console.c
@@ -187,7 +187,16 @@ bool is_interactive(void)
 }
 
 bool console_antispoof_prompt = true;
-bool console_set_trust_status(Seat *seat, bool trusted)
+
+void console_set_trust_status(Seat *seat, bool trusted)
+{
+    /* Do nothing in response to a change of trust status, because
+     * there's nothing we can do in a console environment. However,
+     * the query function below will make a fiddly decision about
+     * whether to tell the backend to enable fallback handling. */
+}
+
+bool console_can_set_trust_status(Seat *seat)
 {
     if (console_batch_mode || !is_interactive() || !console_antispoof_prompt) {
         /*
@@ -200,8 +209,8 @@ bool console_set_trust_status(Seat *seat, bool trusted)
          * prompt, the user couldn't respond to it via the terminal
          * anyway.
          *
-         * We also vacuously return success if the user has purposely
-         * disabled the antispoof prompt.
+         * We also return true without enabling any defences if the
+         * user has purposely disabled the antispoof prompt.
          */
         return true;
     }
diff --git a/windows/plink.c b/windows/plink.c
index ac68ce7d..2c68fb63 100644
--- a/windows/plink.c
+++ b/windows/plink.c
@@ -101,6 +101,7 @@ static const SeatVtable plink_seat_vt = {
     .get_window_pixel_size = nullseat_get_window_pixel_size,
     .stripctrl_new = console_stripctrl_new,
     .set_trust_status = console_set_trust_status,
+    .can_set_trust_status = console_can_set_trust_status,
     .verbose = cmdline_seat_verbose,
     .interactive = plink_seat_interactive,
     .get_cursor_position = nullseat_get_cursor_position,
diff --git a/windows/window.c b/windows/window.c
index b00b910c..9a273d83 100644
--- a/windows/window.c
+++ b/windows/window.c
@@ -323,7 +323,8 @@ static void win_seat_notify_remote_exit(Seat *seat);
 static void win_seat_connection_fatal(Seat *seat, const char *msg);
 static void win_seat_update_specials_menu(Seat *seat);
 static void win_seat_set_busy_status(Seat *seat, BusyStatus status);
-static bool win_seat_set_trust_status(Seat *seat, bool trusted);
+static void win_seat_set_trust_status(Seat *seat, bool trusted);
+static bool win_seat_can_set_trust_status(Seat *seat);
 static bool win_seat_get_cursor_position(Seat *seat, int *x, int *y);
 static bool win_seat_get_window_pixel_size(Seat *seat, int *x, int *y);
 
@@ -348,6 +349,7 @@ static const SeatVtable win_seat_vt = {
     .get_window_pixel_size = win_seat_get_window_pixel_size,
     .stripctrl_new = win_seat_stripctrl_new,
     .set_trust_status = win_seat_set_trust_status,
+    .can_set_trust_status = win_seat_can_set_trust_status,
     .verbose = nullseat_verbose_yes,
     .interactive = nullseat_interactive_yes,
     .get_cursor_position = win_seat_get_cursor_position,
@@ -5753,9 +5755,13 @@ static int win_seat_get_userpass_input(
     return ret;
 }
 
-static bool win_seat_set_trust_status(Seat *seat, bool trusted)
+static void win_seat_set_trust_status(Seat *seat, bool trusted)
 {
     term_set_trust_status(term, trusted);
+}
+
+static bool win_seat_can_set_trust_status(Seat *seat)
+{
     return true;
 }