From 4249b39ed369e3ee948d4cb33391742b9d6965f5 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Tue, 13 Sep 2022 08:49:38 +0100 Subject: [PATCH] New Seat method, seat_nonfatal(). This is like the seat-independent nonfatal(), but specifies a Seat, which allows the GUI dialog box to have the right terminal window as its parent (if there are multiple ones). Changed over all the nonfatal() calls in the code base that could be localised to a Seat, which means all the ones that come up if something goes horribly wrong in host key storage. To make that possible, I've added a 'seat' parameter to store_host_key(); it turns out that all its call sites had one available already. --- console.c | 5 +++++ proxy/sshproxy.c | 9 +++++++++ pscp.c | 1 + psftp.c | 1 + psocks.c | 2 +- putty.h | 17 +++++++++++++---- ssh/kex2-client.c | 2 +- ssh/server.c | 1 + ssh/sesschan.c | 1 + storage.h | 4 +++- stubs/null-seat.c | 1 + unix/console.c | 2 +- unix/dialog.c | 3 ++- unix/plink.c | 1 + unix/storage.c | 14 +++++++------- unix/window.c | 7 +++++++ utils/CMakeLists.txt | 1 + utils/seat_nonfatal.c | 19 +++++++++++++++++++ utils/tempseat.c | 12 ++++++++++++ windows/console.c | 2 +- windows/dialog.c | 2 +- windows/plink.c | 1 + windows/storage.c | 2 +- windows/window.c | 13 +++++++++++++ 24 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 utils/seat_nonfatal.c diff --git a/console.c b/console.c index 62c64aff..78c2da1b 100644 --- a/console.c +++ b/console.c @@ -79,6 +79,11 @@ void console_connection_fatal(Seat *seat, const char *msg) cleanup_exit(1); } +void console_nonfatal(Seat *seat, const char *msg) +{ + console_print_error_msg("ERROR", msg); +} + /* * Console front ends redo their select() or equivalent every time, so * they don't need separate timer handling. diff --git a/proxy/sshproxy.c b/proxy/sshproxy.c index 165c6c0a..8a902e08 100644 --- a/proxy/sshproxy.c +++ b/proxy/sshproxy.c @@ -405,6 +405,14 @@ static void sshproxy_connection_fatal(Seat *seat, const char *message) } } +static void sshproxy_nonfatal(Seat *seat, const char *message) +{ + SshProxy *sp = container_of(seat, SshProxy, seat); + if (sp->clientseat) + seat_nonfatal(sp->clientseat, "error in proxy SSH connection: %s", + message); +} + static SeatPromptResult sshproxy_confirm_ssh_host_key( Seat *seat, const char *host, int port, const char *keytype, char *keystr, SeatDialogText *text, HelpCtx helpctx, @@ -541,6 +549,7 @@ static const SeatVtable SshProxy_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = sshproxy_notify_remote_disconnect, .connection_fatal = sshproxy_connection_fatal, + .nonfatal = sshproxy_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/pscp.c b/pscp.c index 77de1cd9..87fb1722 100644 --- a/pscp.c +++ b/pscp.c @@ -71,6 +71,7 @@ static const SeatVtable pscp_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = console_connection_fatal, + .nonfatal = console_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/psftp.c b/psftp.c index d8b5c400..bf8782d3 100644 --- a/psftp.c +++ b/psftp.c @@ -52,6 +52,7 @@ static const SeatVtable psftp_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = console_connection_fatal, + .nonfatal = console_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/psocks.c b/psocks.c index eb7c8f01..f9e76949 100644 --- a/psocks.c +++ b/psocks.c @@ -529,7 +529,7 @@ int check_stored_host_key(const char *hostname, int port, unreachable("host keys not handled in this tool"); } -void store_host_key(const char *hostname, int port, +void store_host_key(Seat *seat, const char *hostname, int port, const char *keytype, const char *key) { unreachable("host keys not handled in this tool"); diff --git a/putty.h b/putty.h index 5c3adfe9..9eb6c0b5 100644 --- a/putty.h +++ b/putty.h @@ -1214,9 +1214,15 @@ struct SeatVtable { void (*notify_remote_disconnect)(Seat *seat); /* - * Notify the seat that the connection has suffered a fatal error. + * Notify the seat that the connection has suffered an error, + * either fatal to the whole connection or not. + * + * The latter kind of error is expected to be things along the + * lines of 'I/O error storing the new host key', which has + * traditionally been presented via a dialog box or similar. */ void (*connection_fatal)(Seat *seat, const char *message); + void (*nonfatal)(Seat *seat, const char *message); /* * Notify the seat that the list of special commands available @@ -1481,10 +1487,11 @@ static inline bool seat_interactive(Seat *seat) static inline bool seat_get_cursor_position(Seat *seat, int *x, int *y) { return seat->vt->get_cursor_position(seat, x, y); } -/* Unlike the seat's actual method, the public entry point - * seat_connection_fatal is a wrapper function with a printf-like API, - * defined in utils. */ +/* Unlike the seat's actual method, the public entry points + * seat_connection_fatal and seat_nonfatal are wrapper functions with + * a printf-like API, defined in utils. */ void seat_connection_fatal(Seat *seat, const char *fmt, ...) PRINTF_LIKE(2, 3); +void seat_nonfatal(Seat *seat, const char *fmt, ...) PRINTF_LIKE(2, 3); /* Handy aliases for seat_output which set is_stderr to a fixed value. */ static inline size_t seat_stdout(Seat *seat, const void *data, size_t len) @@ -1528,6 +1535,7 @@ void nullseat_notify_session_started(Seat *seat); void nullseat_notify_remote_exit(Seat *seat); void nullseat_notify_remote_disconnect(Seat *seat); void nullseat_connection_fatal(Seat *seat, const char *message); +void nullseat_nonfatal(Seat *seat, const char *message); void nullseat_update_specials_menu(Seat *seat); char *nullseat_get_ttymode(Seat *seat, const char *mode); void nullseat_set_busy_status(Seat *seat, BusyStatus status); @@ -1567,6 +1575,7 @@ bool nullseat_get_cursor_position(Seat *seat, int *x, int *y); */ void console_connection_fatal(Seat *seat, const char *message); +void console_nonfatal(Seat *seat, const char *message); SeatPromptResult console_confirm_ssh_host_key( Seat *seat, const char *host, int port, const char *keytype, char *keystr, SeatDialogText *text, HelpCtx helpctx, diff --git a/ssh/kex2-client.c b/ssh/kex2-client.c index b890c023..8ca8cd40 100644 --- a/ssh/kex2-client.c +++ b/ssh/kex2-client.c @@ -1021,7 +1021,7 @@ void ssh2kex_coroutine(struct ssh2_transport_state *s, bool *aborted) ppl_logevent("%s", fingerprint); sfree(fingerprint); - store_host_key(s->savedhost, s->savedport, + store_host_key(s->ppl.seat, s->savedhost, s->savedport, ssh_key_cache_id(s->hkey), s->keystr); /* * Don't forget to store the new key as the one we'll be diff --git a/ssh/server.c b/ssh/server.c index 188426b3..4e315874 100644 --- a/ssh/server.c +++ b/ssh/server.c @@ -116,6 +116,7 @@ static const SeatVtable server_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = nullseat_connection_fatal, + .nonfatal = nullseat_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/ssh/sesschan.c b/ssh/sesschan.c index 9776eaf2..b500f77d 100644 --- a/ssh/sesschan.c +++ b/ssh/sesschan.c @@ -193,6 +193,7 @@ static const SeatVtable sesschan_seat_vt = { .notify_remote_exit = sesschan_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = sesschan_connection_fatal, + .nonfatal = nullseat_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/storage.h b/storage.h index e9138f40..09e61368 100644 --- a/storage.h +++ b/storage.h @@ -89,8 +89,10 @@ int check_stored_host_key(const char *hostname, int port, /* * Write a host key into the database, overwriting any previous * entry that might have been there. + * + * A Seat is provided for error-reporting purposes. */ -void store_host_key(const char *hostname, int port, +void store_host_key(Seat *seat, const char *hostname, int port, const char *keytype, const char *key); /* ---------------------------------------------------------------------- diff --git a/stubs/null-seat.c b/stubs/null-seat.c index 37cb0f4c..80d8b27f 100644 --- a/stubs/null-seat.c +++ b/stubs/null-seat.c @@ -17,6 +17,7 @@ void nullseat_notify_session_started(Seat *seat) {} void nullseat_notify_remote_exit(Seat *seat) {} void nullseat_notify_remote_disconnect(Seat *seat) {} void nullseat_connection_fatal(Seat *seat, const char *message) {} +void nullseat_nonfatal(Seat *seat, const char *message) {} void nullseat_update_specials_menu(Seat *seat) {} char *nullseat_get_ttymode(Seat *seat, const char *mode) { return NULL; } void nullseat_set_busy_status(Seat *seat, BusyStatus status) {} diff --git a/unix/console.c b/unix/console.c index 286ecf29..495783b1 100644 --- a/unix/console.c +++ b/unix/console.c @@ -191,7 +191,7 @@ SeatPromptResult console_confirm_ssh_host_key( if (line[0] != '\0' && line[0] != '\r' && line[0] != '\n' && line[0] != 'q' && line[0] != 'Q') { if (line[0] == 'y' || line[0] == 'Y') - store_host_key(host, port, keytype, keystr); + store_host_key(seat, host, port, keytype, keystr); postmsg(&cf); return SPR_OK; } else { diff --git a/unix/dialog.c b/unix/dialog.c index 0d9d9573..2ff51ede 100644 --- a/unix/dialog.c +++ b/unix/dialog.c @@ -3529,7 +3529,8 @@ static void confirm_ssh_host_key_result_callback(void *vctx, int result) * doesn't care whether we saved the host key or not). */ if (result == 2) { - store_host_key(ctx->host, ctx->port, ctx->keytype, ctx->keystr); + store_host_key(ctx->seat, ctx->host, ctx->port, + ctx->keytype, ctx->keystr); logical_result = SPR_OK; } else if (result == 1) { logical_result = SPR_OK; diff --git a/unix/plink.c b/unix/plink.c index f0c73754..c1738e10 100644 --- a/unix/plink.c +++ b/unix/plink.c @@ -409,6 +409,7 @@ static const SeatVtable plink_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = console_connection_fatal, + .nonfatal = console_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = plink_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/unix/storage.c b/unix/storage.c index ca225732..35a276c2 100644 --- a/unix/storage.c +++ b/unix/storage.c @@ -828,7 +828,7 @@ bool have_ssh_host_key(const char *hostname, int port, return check_stored_host_key(hostname, port, keytype, "") != 1; } -void store_host_key(const char *hostname, int port, +void store_host_key(Seat *seat, const char *hostname, int port, const char *keytype, const char *key) { FILE *rfp, *wfp; @@ -846,7 +846,7 @@ void store_host_key(const char *hostname, int port, dir = make_filename(INDEX_DIR, NULL); if ((errmsg = make_dir_path(dir, 0700)) != NULL) { - nonfatal("Unable to store host key: %s", errmsg); + seat_nonfatal(seat, "Unable to store host key: %s", errmsg); sfree(errmsg); sfree(dir); sfree(tmpfilename); @@ -857,8 +857,8 @@ void store_host_key(const char *hostname, int port, wfp = fopen(tmpfilename, "w"); } if (!wfp) { - nonfatal("Unable to store host key: open(\"%s\") " - "returned '%s'", tmpfilename, strerror(errno)); + seat_nonfatal(seat, "Unable to store host key: open(\"%s\") " + "returned '%s'", tmpfilename, strerror(errno)); sfree(tmpfilename); return; } @@ -889,9 +889,9 @@ void store_host_key(const char *hostname, int port, fclose(wfp); if (rename(tmpfilename, filename) < 0) { - nonfatal("Unable to store host key: rename(\"%s\",\"%s\")" - " returned '%s'", tmpfilename, filename, - strerror(errno)); + seat_nonfatal(seat, "Unable to store host key: rename(\"%s\",\"%s\")" + " returned '%s'", tmpfilename, filename, + strerror(errno)); } sfree(tmpfilename); diff --git a/unix/window.c b/unix/window.c index 1958edde..de75a1c2 100644 --- a/unix/window.c +++ b/unix/window.c @@ -290,6 +290,12 @@ static void gtk_seat_connection_fatal(Seat *seat, const char *msg) queue_toplevel_callback(connection_fatal_callback, inst); } +static void gtk_seat_nonfatal(Seat *seat, const char *msg) +{ + GtkFrontend *inst = container_of(seat, GtkFrontend, seat); + nonfatal_message_box(inst->window, msg); +} + /* * Default settings that are specific to pterm. */ @@ -423,6 +429,7 @@ static const SeatVtable gtk_seat_vt = { .notify_remote_exit = gtk_seat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = gtk_seat_connection_fatal, + .nonfatal = gtk_seat_nonfatal, .update_specials_menu = gtk_seat_update_specials_menu, .get_ttymode = gtk_seat_get_ttymode, .set_busy_status = gtk_seat_set_busy_status, diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 6ef33c8d..c09027f4 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -51,6 +51,7 @@ add_sources_from_current_dir(utils read_file_into.c seat_connection_fatal.c seat_dialog_text.c + seat_nonfatal.c sessprep.c sk_free_peer_info.c smemclr.c diff --git a/utils/seat_nonfatal.c b/utils/seat_nonfatal.c new file mode 100644 index 00000000..d21e4c17 --- /dev/null +++ b/utils/seat_nonfatal.c @@ -0,0 +1,19 @@ +/* + * Wrapper function for the nonfatal() method of a Seat, + * providing printf-style formatting. + */ + +#include "putty.h" + +void seat_nonfatal(Seat *seat, const char *fmt, ...) +{ + va_list ap; + char *msg; + + va_start(ap, fmt); + msg = dupvprintf(fmt, ap); + va_end(ap); + + seat->vt->nonfatal(seat, msg); + sfree(msg); +} diff --git a/utils/tempseat.c b/utils/tempseat.c index ad37b0e3..a8d99cc2 100644 --- a/utils/tempseat.c +++ b/utils/tempseat.c @@ -288,6 +288,17 @@ static void tempseat_connection_fatal(Seat *seat, const char *message) unreachable("connection_fatal should never be called on TempSeat"); } +static void tempseat_nonfatal(Seat *seat, const char *message) +{ + /* + * Non-fatal errors specific to a Seat should also not occur, + * because those will be for things like I/O errors writing the + * host key collection, and a backend's not _doing_ that when we + * haven't connected it to the host yet. + */ + unreachable("nonfatal should never be called on TempSeat"); +} + static bool tempseat_eof(Seat *seat) { /* @@ -327,6 +338,7 @@ static const struct SeatVtable tempseat_vt = { .notify_remote_exit = tempseat_notify_remote_exit, .notify_remote_disconnect = tempseat_notify_remote_disconnect, .connection_fatal = tempseat_connection_fatal, + .nonfatal = tempseat_nonfatal, .update_specials_menu = tempseat_update_specials_menu, .get_ttymode = tempseat_get_ttymode, .set_busy_status = tempseat_set_busy_status, diff --git a/windows/console.c b/windows/console.c index 2fd572b4..b6ecb203 100644 --- a/windows/console.c +++ b/windows/console.c @@ -119,7 +119,7 @@ SeatPromptResult console_confirm_ssh_host_key( if (line[0] != '\0' && line[0] != '\r' && line[0] != '\n' && line[0] != 'q' && line[0] != 'Q') { if (line[0] == 'y' || line[0] == 'Y') - store_host_key(host, port, keytype, keystr); + store_host_key(seat, host, port, keytype, keystr); return SPR_OK; } else { fputs(console_abandoned_msg, stderr); diff --git a/windows/dialog.c b/windows/dialog.c index c34703f6..68d88df8 100644 --- a/windows/dialog.c +++ b/windows/dialog.c @@ -1155,7 +1155,7 @@ SeatPromptResult win_seat_confirm_ssh_host_key( wgs->term_hwnd, HostKeyDialogProc, ctx); assert(mbret==IDC_HK_ACCEPT || mbret==IDC_HK_ONCE || mbret==IDCANCEL); if (mbret == IDC_HK_ACCEPT) { - store_host_key(host, port, keytype, keystr); + store_host_key(seat, host, port, keytype, keystr); return SPR_OK; } else if (mbret == IDC_HK_ONCE) { return SPR_OK; diff --git a/windows/plink.c b/windows/plink.c index 05c25073..48429ba2 100644 --- a/windows/plink.c +++ b/windows/plink.c @@ -97,6 +97,7 @@ static const SeatVtable plink_seat_vt = { .notify_remote_exit = nullseat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = console_connection_fatal, + .nonfatal = console_nonfatal, .update_specials_menu = nullseat_update_specials_menu, .get_ttymode = nullseat_get_ttymode, .set_busy_status = nullseat_set_busy_status, diff --git a/windows/storage.c b/windows/storage.c index f2c33feb..7736b3db 100644 --- a/windows/storage.c +++ b/windows/storage.c @@ -357,7 +357,7 @@ bool have_ssh_host_key(const char *hostname, int port, return check_stored_host_key(hostname, port, keytype, "") != 1; } -void store_host_key(const char *hostname, int port, +void store_host_key(Seat *seat, const char *hostname, int port, const char *keytype, const char *key) { strbuf *regname = strbuf_new(); diff --git a/windows/window.c b/windows/window.c index bd26e733..35236f2d 100644 --- a/windows/window.c +++ b/windows/window.c @@ -321,6 +321,7 @@ static bool win_seat_eof(Seat *seat); static SeatPromptResult win_seat_get_userpass_input(Seat *seat, prompts_t *p); static void win_seat_notify_remote_exit(Seat *seat); static void win_seat_connection_fatal(Seat *seat, const char *msg); +static void win_seat_nonfatal(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 void win_seat_set_trust_status(Seat *seat, bool trusted); @@ -338,6 +339,7 @@ static const SeatVtable win_seat_vt = { .notify_remote_exit = win_seat_notify_remote_exit, .notify_remote_disconnect = nullseat_notify_remote_disconnect, .connection_fatal = win_seat_connection_fatal, + .nonfatal = win_seat_nonfatal, .update_specials_menu = win_seat_update_specials_menu, .get_ttymode = win_seat_get_ttymode, .set_busy_status = win_seat_set_busy_status, @@ -1188,6 +1190,17 @@ static void win_seat_connection_fatal(Seat *seat, const char *msg) } } +/* + * Print a message box and don't close the connection. + */ +static void win_seat_nonfatal(Seat *seat, const char *msg) +{ + char *title = dupprintf("%s Error", appname); + show_mouseptr(true); + MessageBox(wgs.term_hwnd, msg, title, MB_ICONERROR | MB_OK); + sfree(title); +} + /* * Report an error at the command-line parsing stage. */