From 74a0be9c5647ed033b7ef7b58d074a3510bcc071 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sat, 30 Oct 2021 17:06:00 +0100 Subject: [PATCH] Split seat_banner from seat_output. Previously, SSH authentication banners were displayed by calling the ordinary seat_output function, and passing it a special value in the SeatOutputType enumeration indicating an auth banner. The awkwardness of this was already showing a little in SshProxy's implementation of seat_output, where it had to check for that special value and do totally different things for SEAT_OUTPUT_AUTH_BANNER and everything else. Further work in that area is going to make it more and more awkward if I keep the two output systems unified. So let's split them up. Now, Seat has separate output() and banner() methods, which each implementation can override differently if it wants to. All the 'end user' Seat implementations use the centralised implementation function nullseat_banner_to_stderr(), which turns banner text straight back into SEAT_OUTPUT_STDERR and passes it on to seat_output. So I didn't have to tediously implement a boring version of this function in GTK, Windows GUI, consoles, file transfer etc. --- proxy/sshproxy.c | 29 ++++++++++++++++------------- pscp.c | 1 + psftp.c | 1 + putty.h | 29 ++++++++++++++++++++--------- ssh/server.c | 1 + ssh/sesschan.c | 6 +----- unix/plink.c | 1 + unix/window.c | 1 + utils/nullseat.c | 3 +++ utils/tempseat.c | 6 ++++++ windows/plink.c | 1 + windows/window.c | 1 + 12 files changed, 53 insertions(+), 27 deletions(-) diff --git a/proxy/sshproxy.c b/proxy/sshproxy.c index ad274d57..d113cb04 100644 --- a/proxy/sshproxy.c +++ b/proxy/sshproxy.c @@ -262,20 +262,22 @@ static size_t sshproxy_output(Seat *seat, SeatOutputType type, const void *data, size_t len) { SshProxy *sp = container_of(seat, SshProxy, seat); - if (type == SEAT_OUTPUT_AUTH_BANNER) { - if (sp->clientseat) { - /* - * If we have access to the outer Seat, pass the SSH login - * banner on to it. - */ - return seat_output(sp->clientseat, type, data, len); - } else { - return 0; - } + bufchain_add(&sp->ssh_to_socket, data, len); + try_send_ssh_to_socket(sp); + return bufchain_size(&sp->ssh_to_socket); +} + +static size_t sshproxy_banner(Seat *seat, const void *data, size_t len) +{ + SshProxy *sp = container_of(seat, SshProxy, seat); + if (sp->clientseat) { + /* + * If we have access to the outer Seat, pass the SSH login + * banner on to it. + */ + return seat_banner(sp->clientseat, data, len); } else { - bufchain_add(&sp->ssh_to_socket, data, len); - try_send_ssh_to_socket(sp); - return bufchain_size(&sp->ssh_to_socket); + return 0; } } @@ -452,6 +454,7 @@ static const SeatVtable SshProxy_seat_vt = { .output = sshproxy_output, .eof = sshproxy_eof, .sent = sshproxy_sent, + .banner = sshproxy_banner, .get_userpass_input = sshproxy_get_userpass_input, .notify_session_started = sshproxy_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/pscp.c b/pscp.c index 0b091085..13e89812 100644 --- a/pscp.c +++ b/pscp.c @@ -67,6 +67,7 @@ static const SeatVtable pscp_seat_vt = { .output = pscp_output, .eof = pscp_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = filexfer_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/psftp.c b/psftp.c index e00ebed1..14483569 100644 --- a/psftp.c +++ b/psftp.c @@ -48,6 +48,7 @@ static const SeatVtable psftp_seat_vt = { .output = psftp_output, .eof = psftp_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = filexfer_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/putty.h b/putty.h index 1d55f91e..4e1f9dde 100644 --- a/putty.h +++ b/putty.h @@ -925,7 +925,7 @@ typedef enum SeatInteractionContext { } SeatInteractionContext; typedef enum SeatOutputType { - SEAT_OUTPUT_STDOUT, SEAT_OUTPUT_STDERR, SEAT_OUTPUT_AUTH_BANNER + SEAT_OUTPUT_STDOUT, SEAT_OUTPUT_STDERR } SeatOutputType; /* @@ -941,11 +941,10 @@ struct Seat { struct SeatVtable { /* * Provide output from the remote session. 'type' indicates the - * type of the output (stdout, stderr or SSH auth banner), which - * can be used to split the output into separate message channels, - * if the seat wants to handle them differently. But combining the - * channels into one is OK too; that's what terminal-window based - * seats do. + * type of the output (stdout or stderr), which can be used to + * split the output into separate message channels, if the seat + * wants to handle them differently. But combining the channels + * into one is OK too; that's what terminal-window based seats do. * * The return value is the current size of the output backlog. */ @@ -971,6 +970,14 @@ struct SeatVtable { */ void (*sent)(Seat *seat, size_t new_sendbuffer); + /* + * Provide authentication-banner output from the session setup. + * End-user Seats can treat this as very similar to 'output', but + * intermediate Seats in complex proxying situations will want to + * implement this and 'output' differently. + */ + size_t (*banner)(Seat *seat, const void *data, size_t len); + /* * Try to get answers from a set of interactive login prompts. The * prompts are provided in 'p'. @@ -1227,6 +1234,8 @@ static inline bool seat_eof(Seat *seat) { return seat->vt->eof(seat); } static inline void seat_sent(Seat *seat, size_t bufsize) { seat->vt->sent(seat, bufsize); } +static inline size_t seat_banner(Seat *seat, const void *data, size_t len) +{ return seat->vt->banner(seat, data, len); } static inline int seat_get_userpass_input(Seat *seat, prompts_t *p) { return seat->vt->get_userpass_input(seat, p); } static inline void seat_notify_session_started(Seat *seat) @@ -1293,10 +1302,10 @@ static inline size_t seat_stderr(Seat *seat, const void *data, size_t len) { return seat_output(seat, SEAT_OUTPUT_STDERR, data, len); } static inline size_t seat_stderr_pl(Seat *seat, ptrlen data) { return seat_output(seat, SEAT_OUTPUT_STDERR, data.ptr, data.len); } -static inline size_t seat_banner(Seat *seat, const void *data, size_t len) -{ return seat_output(seat, SEAT_OUTPUT_AUTH_BANNER, data, len); } + +/* Alternative API for seat_banner taking a ptrlen */ static inline size_t seat_banner_pl(Seat *seat, ptrlen data) -{ return seat_output(seat, SEAT_OUTPUT_AUTH_BANNER, data.ptr, data.len); } +{ return seat->vt->banner(seat, data.ptr, data.len); } /* In the utils subdir: print a message to the Seat which can't be * spoofed by server-supplied auth-time output such as SSH banners */ @@ -1313,6 +1322,8 @@ size_t nullseat_output( Seat *seat, SeatOutputType type, const void *data, size_t len); bool nullseat_eof(Seat *seat); void nullseat_sent(Seat *seat, size_t bufsize); +size_t nullseat_banner(Seat *seat, const void *data, size_t len); +size_t nullseat_banner_to_stderr(Seat *seat, const void *data, size_t len); int nullseat_get_userpass_input(Seat *seat, prompts_t *p); void nullseat_notify_session_started(Seat *seat); void nullseat_notify_remote_exit(Seat *seat); diff --git a/ssh/server.c b/ssh/server.c index 63a68f62..724abaf4 100644 --- a/ssh/server.c +++ b/ssh/server.c @@ -108,6 +108,7 @@ static const SeatVtable server_seat_vt = { .output = nullseat_output, .eof = nullseat_eof, .sent = nullseat_sent, + .banner = nullseat_banner, .get_userpass_input = nullseat_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/ssh/sesschan.c b/ssh/sesschan.c index 6951edc4..56ee37e1 100644 --- a/ssh/sesschan.c +++ b/ssh/sesschan.c @@ -187,6 +187,7 @@ static const SeatVtable sesschan_seat_vt = { .output = sesschan_seat_output, .eof = sesschan_seat_eof, .sent = nullseat_sent, + .banner = nullseat_banner, .get_userpass_input = nullseat_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = sesschan_notify_remote_exit, @@ -614,11 +615,6 @@ static size_t sesschan_seat_output( Seat *seat, SeatOutputType type, const void *data, size_t len) { sesschan *sess = container_of(seat, sesschan, seat); - - /* We don't expect anything but stdout and stderr to come here, - * because the pty backend doesn't generate auth banners */ - assert(type != SEAT_OUTPUT_AUTH_BANNER); - return sshfwd_write_ext(sess->c, type == SEAT_OUTPUT_STDERR, data, len); } diff --git a/unix/plink.c b/unix/plink.c index 524e4545..dee0cbec 100644 --- a/unix/plink.c +++ b/unix/plink.c @@ -391,6 +391,7 @@ static const SeatVtable plink_seat_vt = { .output = plink_output, .eof = plink_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = plink_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/unix/window.c b/unix/window.c index c3ddb515..c5894ffa 100644 --- a/unix/window.c +++ b/unix/window.c @@ -390,6 +390,7 @@ static const SeatVtable gtk_seat_vt = { .output = gtk_seat_output, .eof = gtk_seat_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = gtk_seat_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = gtk_seat_notify_remote_exit, diff --git a/utils/nullseat.c b/utils/nullseat.c index 19fad59d..a53f563e 100644 --- a/utils/nullseat.c +++ b/utils/nullseat.c @@ -8,6 +8,9 @@ size_t nullseat_output( Seat *seat, SeatOutputType type, const void *data, size_t len) {return 0;} bool nullseat_eof(Seat *seat) { return true; } void nullseat_sent(Seat *seat, size_t bufsize) {} +size_t nullseat_banner(Seat *seat, const void *data, size_t len) {return 0;} +size_t nullseat_banner_to_stderr(Seat *seat, const void *data, size_t len) +{ return seat_output(seat, SEAT_OUTPUT_STDERR, data, len); } int nullseat_get_userpass_input(Seat *seat, prompts_t *p) { return 0; } void nullseat_notify_session_started(Seat *seat) {} void nullseat_notify_remote_exit(Seat *seat) {} diff --git a/utils/tempseat.c b/utils/tempseat.c index f877dd1d..eac10f8d 100644 --- a/utils/tempseat.c +++ b/utils/tempseat.c @@ -224,6 +224,11 @@ static int tempseat_get_userpass_input(Seat *seat, prompts_t *p) unreachable("get_userpass_input should never be called on TempSeat"); } +static size_t tempseat_banner(Seat *seat, const void *data, size_t len) +{ + unreachable("banner should never be called on TempSeat"); +} + static int tempseat_confirm_ssh_host_key( Seat *seat, const char *host, int port, const char *keytype, char *keystr, const char *keydisp, char **key_fingerprints, bool mismatch, @@ -299,6 +304,7 @@ static const struct SeatVtable tempseat_vt = { .output = tempseat_output, .eof = tempseat_eof, .sent = nullseat_sent, + .banner = tempseat_banner, .get_userpass_input = tempseat_get_userpass_input, .notify_session_started = tempseat_notify_session_started, .notify_remote_exit = tempseat_notify_remote_exit, diff --git a/windows/plink.c b/windows/plink.c index 7506a13f..ef982be1 100644 --- a/windows/plink.c +++ b/windows/plink.c @@ -85,6 +85,7 @@ static const SeatVtable plink_seat_vt = { .output = plink_output, .eof = plink_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = plink_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = nullseat_notify_remote_exit, diff --git a/windows/window.c b/windows/window.c index 4c67009b..1aa83743 100644 --- a/windows/window.c +++ b/windows/window.c @@ -332,6 +332,7 @@ static const SeatVtable win_seat_vt = { .output = win_seat_output, .eof = win_seat_eof, .sent = nullseat_sent, + .banner = nullseat_banner_to_stderr, .get_userpass_input = win_seat_get_userpass_input, .notify_session_started = nullseat_notify_session_started, .notify_remote_exit = win_seat_notify_remote_exit,