From fb663d4761c221fed090ff98d891a9883e76cbb3 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Thu, 16 Sep 2021 11:43:02 +0100 Subject: [PATCH] Promote ssh2_userauth_antispoof_msg into utils. It doesn't actually do anything specific to the userauth layer; it's just a helper function that deals with the mechanics of printing an unspoofable message on various kinds of front end, and the only parameters it needs are a Seat and a message. Currently, it's used for 'here is the start/end of the server banner' only. But it's also got all the right functionality to be used for the (still missing) messages about which proxy SSH server the next set of login prompts are going to refer to. --- putty.h | 4 ++++ ssh/userauth2-client.c | 47 +++++++++--------------------------------- utils/CMakeLists.txt | 1 + utils/antispoof.c | 28 +++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 37 deletions(-) create mode 100644 utils/antispoof.c diff --git a/putty.h b/putty.h index 5a765771..caca09f3 100644 --- a/putty.h +++ b/putty.h @@ -1250,6 +1250,10 @@ static inline size_t seat_banner(Seat *seat, const void *data, size_t len) static inline size_t seat_banner_pl(Seat *seat, ptrlen data) { return seat_output(seat, SEAT_OUTPUT_AUTH_BANNER, 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 */ +void seat_antispoof_msg(Seat *seat, const char *msg); + /* * Stub methods for seat implementations that want to use the obvious * null handling for a given method. diff --git a/ssh/userauth2-client.c b/ssh/userauth2-client.c index 4c908048..e82abe8a 100644 --- a/ssh/userauth2-client.c +++ b/ssh/userauth2-client.c @@ -114,8 +114,6 @@ static void ssh2_userauth_add_session_id( static PktOut *ssh2_userauth_gss_packet( struct ssh2_userauth_state *s, const char *authtype); #endif -static void ssh2_userauth_antispoof_msg( - struct ssh2_userauth_state *s, const char *msg); static const PacketProtocolLayerVtable ssh2_userauth_vtable = { .free = ssh2_userauth_free, @@ -522,8 +520,9 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) if (bufchain_size(&s->banner) && (seat_verbose(s->ppl.seat) || seat_interactive(s->ppl.seat))) { if (s->banner_scc) { - ssh2_userauth_antispoof_msg( - s, "Pre-authentication banner message from server:"); + seat_antispoof_msg( + s->ppl.seat, + "Pre-authentication banner message from server:"); seat_set_trust_status(s->ppl.seat, false); } @@ -542,8 +541,8 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) if (s->banner_scc) { seat_set_trust_status(s->ppl.seat, true); - ssh2_userauth_antispoof_msg( - s, "End of banner message from server"); + seat_antispoof_msg(s->ppl.seat, + "End of banner message from server"); } } @@ -1343,8 +1342,8 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) */ if (!s->ki_printed_header && s->ki_scc && (s->num_prompts || name.len || inst.len)) { - ssh2_userauth_antispoof_msg( - s, "Keyboard-interactive authentication " + seat_antispoof_msg( + s->ppl.seat, "Keyboard-interactive authentication " "prompts from server:"); s->ki_printed_header = true; seat_set_trust_status(s->ppl.seat, false); @@ -1446,8 +1445,9 @@ static void ssh2_userauth_process_queue(PacketProtocolLayer *ppl) */ if (s->ki_printed_header) { seat_set_trust_status(s->ppl.seat, true); - ssh2_userauth_antispoof_msg( - s, "End of keyboard-interactive prompts from server"); + seat_antispoof_msg( + s->ppl.seat, + "End of keyboard-interactive prompts from server"); } /* @@ -1895,30 +1895,3 @@ static void ssh2_userauth_reconfigure(PacketProtocolLayer *ppl, Conf *conf) container_of(ppl, struct ssh2_userauth_state, ppl); ssh_ppl_reconfigure(s->successor_layer, conf); } - -static void ssh2_userauth_antispoof_msg( - struct ssh2_userauth_state *s, const char *msg) -{ - strbuf *sb = strbuf_new(); - 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 - * unmodified as an unspoofable header. - */ - put_datapl(sb, ptrlen_from_asciz(msg)); - } else { - /* - * Otherwise, add enough padding around it that the server - * wouldn't be able to mimic it within our line-length - * constraint. - */ - strbuf_catf(sb, "-- %s ", msg); - while (sb->len < 78) - put_byte(sb, '-'); - } - put_datapl(sb, PTRLEN_LITERAL("\r\n")); - seat_banner_pl(s->ppl.seat, ptrlen_from_strbuf(sb)); - strbuf_free(sb); -} diff --git a/utils/CMakeLists.txt b/utils/CMakeLists.txt index 4b10442b..a209eb58 100644 --- a/utils/CMakeLists.txt +++ b/utils/CMakeLists.txt @@ -1,4 +1,5 @@ add_sources_from_current_dir(utils + antispoof.c base64_decode_atom.c base64_encode_atom.c bufchain.c diff --git a/utils/antispoof.c b/utils/antispoof.c new file mode 100644 index 00000000..d8599455 --- /dev/null +++ b/utils/antispoof.c @@ -0,0 +1,28 @@ +#include "putty.h" +#include "misc.h" + +void seat_antispoof_msg(Seat *seat, const char *msg) +{ + strbuf *sb = strbuf_new(); + seat_set_trust_status(seat, true); + if (seat_can_set_trust_status(seat)) { + /* + * If the seat can directly indicate that this message is + * generated by the client, then we can just use the message + * unmodified as an unspoofable header. + */ + put_datapl(sb, ptrlen_from_asciz(msg)); + } else { + /* + * Otherwise, add enough padding around it that the server + * wouldn't be able to mimic it within our line-length + * constraint. + */ + strbuf_catf(sb, "-- %s ", msg); + while (sb->len < 78) + put_byte(sb, '-'); + } + put_datapl(sb, PTRLEN_LITERAL("\r\n")); + seat_banner_pl(seat, ptrlen_from_strbuf(sb)); + strbuf_free(sb); +}