From 2e7ced64801514e0f8c72849e281032418d96ef7 Mon Sep 17 00:00:00 2001 From: Simon Tatham Date: Sun, 7 Oct 2018 08:16:44 +0100 Subject: [PATCH] Give BPPs a Frontend, so they can do their own logging. The sshverstring quasi-frontend is passed a Frontend pointer at setup time, so that it can generate Event Log entries containing the local and remote version strings and the results of remote bug detection. I'm promoting that field of sshverstring to a field of the public BPP structure, so now all BPPs have the right to talk directly to the frontend if they want to. This means I can move all the log messages of the form 'Initialised so-and-so cipher/MAC/compression' down into the BPPs themselves, where they can live exactly alongside the actual initialisation of those primitives. It also means BPPs will be able to log interesting things they detect at any point in the packet stream, which is about to come in useful for another purpose. --- ssh.c | 6 +++--- ssh.h | 2 ++ ssh1bpp.c | 10 +++++++++- ssh1login.c | 2 -- ssh2bpp-bare.c | 3 ++- ssh2bpp.c | 34 ++++++++++++++++++++++++++++++++- ssh2transport.c | 27 -------------------------- sshbpp.h | 8 +++++--- sshverstring.c | 51 ++++++++++++++++++++++++------------------------- 9 files changed, 79 insertions(+), 64 deletions(-) diff --git a/ssh.c b/ssh.c index 5227b4de..433a2572 100644 --- a/ssh.c +++ b/ssh.c @@ -172,7 +172,7 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv, int is_simple = (conf_get_int(ssh->conf, CONF_ssh_simple) && !ssh->connshare); - ssh->bpp = ssh2_bpp_new(&ssh->stats); + ssh->bpp = ssh2_bpp_new(ssh->frontend, &ssh->stats); ssh_connect_bpp(ssh); #ifndef NO_GSSAPI @@ -247,7 +247,7 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv, } else { - ssh->bpp = ssh1_bpp_new(); + ssh->bpp = ssh1_bpp_new(ssh->frontend); ssh_connect_bpp(ssh); connection_layer = ssh1_connection_new(ssh, ssh->conf, &ssh->cl); @@ -260,7 +260,7 @@ static void ssh_got_ssh_version(struct ssh_version_receiver *rcv, } } else { - ssh->bpp = ssh2_bare_bpp_new(); + ssh->bpp = ssh2_bare_bpp_new(ssh->frontend); ssh_connect_bpp(ssh); connection_layer = ssh2_connection_new( diff --git a/ssh.h b/ssh.h index 62b8b6ad..3e14aa4c 100644 --- a/ssh.h +++ b/ssh.h @@ -765,10 +765,12 @@ struct ssh_compression_alg { #define ssh_compressor_free(comp) ((comp)->vt->compress_free(comp)) #define ssh_compressor_compress(comp, in, inlen, out, outlen, minlen) \ ((comp)->vt->compress(comp, in, inlen, out, outlen, minlen)) +#define ssh_compressor_alg(comp) ((comp)->vt) #define ssh_decompressor_new(alg) ((alg)->decompress_new()) #define ssh_decompressor_free(comp) ((comp)->vt->decompress_free(comp)) #define ssh_decompressor_decompress(comp, in, inlen, out, outlen) \ ((comp)->vt->decompress(comp, in, inlen, out, outlen)) +#define ssh_decompressor_alg(comp) ((comp)->vt) struct ssh2_userkey { ssh_key *key; /* the key itself */ diff --git a/ssh1bpp.c b/ssh1bpp.c index 36afcd24..33482a2c 100644 --- a/ssh1bpp.c +++ b/ssh1bpp.c @@ -43,11 +43,12 @@ static const struct BinaryPacketProtocolVtable ssh1_bpp_vtable = { ssh1_bpp_queue_disconnect, }; -BinaryPacketProtocol *ssh1_bpp_new(void) +BinaryPacketProtocol *ssh1_bpp_new(Frontend *frontend) { struct ssh1_bpp_state *s = snew(struct ssh1_bpp_state); memset(s, 0, sizeof(*s)); s->bpp.vt = &ssh1_bpp_vtable; + s->bpp.frontend = frontend; ssh_bpp_common_setup(&s->bpp); return &s->bpp; } @@ -67,6 +68,9 @@ static void ssh1_bpp_free(BinaryPacketProtocol *bpp) sfree(s); } +#define bpp_logevent(printf_args) \ + logevent_and_free(s->bpp.frontend, dupprintf printf_args) + void ssh1_bpp_new_cipher(BinaryPacketProtocol *bpp, const struct ssh1_cipheralg *cipher, const void *session_key) @@ -83,6 +87,8 @@ void ssh1_bpp_new_cipher(BinaryPacketProtocol *bpp, assert(!s->crcda_ctx); s->crcda_ctx = crcda_make_context(); + + bpp_logevent(("Initialised %s encryption", cipher->text_name)); } } @@ -223,6 +229,8 @@ static void ssh1_bpp_handle_input(BinaryPacketProtocol *bpp) s->compctx = ssh_compressor_new(&ssh_zlib); s->decompctx = ssh_decompressor_new(&ssh_zlib); + + bpp_logevent(("Started zlib (RFC1950) compression")); } /* diff --git a/ssh1login.c b/ssh1login.c index e64bc834..0b8ea448 100644 --- a/ssh1login.c +++ b/ssh1login.c @@ -406,7 +406,6 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) (s->cipher_type == SSH_CIPHER_BLOWFISH ? &ssh1_blowfish : s->cipher_type == SSH_CIPHER_DES ? &ssh1_des : &ssh1_3des); ssh1_bpp_new_cipher(s->ppl.bpp, cipher, s->session_key); - ppl_logevent(("Initialised %s encryption", cipher->text_name)); } if (s->servkey.modulus) { @@ -1114,7 +1113,6 @@ static void ssh1_login_process_queue(PacketProtocolLayer *ppl) * easiest way to avoid race conditions if other packets * cross in transit.) */ - ppl_logevent(("Started zlib (RFC1950) compression")); } else if (pktin->type == SSH1_SMSG_FAILURE) { ppl_logevent(("Server refused to enable compression")); ppl_printf(("Server refused to compress\r\n")); diff --git a/ssh2bpp-bare.c b/ssh2bpp-bare.c index 213acf0c..e7c8f8ca 100644 --- a/ssh2bpp-bare.c +++ b/ssh2bpp-bare.c @@ -33,11 +33,12 @@ static const struct BinaryPacketProtocolVtable ssh2_bare_bpp_vtable = { ssh2_bpp_queue_disconnect, /* in sshcommon.c */ }; -BinaryPacketProtocol *ssh2_bare_bpp_new(void) +BinaryPacketProtocol *ssh2_bare_bpp_new(Frontend *frontend) { struct ssh2_bare_bpp_state *s = snew(struct ssh2_bare_bpp_state); memset(s, 0, sizeof(*s)); s->bpp.vt = &ssh2_bare_bpp_vtable; + s->bpp.frontend = frontend; ssh_bpp_common_setup(&s->bpp); return &s->bpp; } diff --git a/ssh2bpp.c b/ssh2bpp.c index f0b6fd22..67745278 100644 --- a/ssh2bpp.c +++ b/ssh2bpp.c @@ -51,11 +51,13 @@ static const struct BinaryPacketProtocolVtable ssh2_bpp_vtable = { ssh2_bpp_queue_disconnect, /* in sshcommon.c */ }; -BinaryPacketProtocol *ssh2_bpp_new(struct DataTransferStats *stats) +BinaryPacketProtocol *ssh2_bpp_new( + Frontend *frontend, struct DataTransferStats *stats) { struct ssh2_bpp_state *s = snew(struct ssh2_bpp_state); memset(s, 0, sizeof(*s)); s->bpp.vt = &ssh2_bpp_vtable; + s->bpp.frontend = frontend; s->stats = stats; ssh_bpp_common_setup(&s->bpp); return &s->bpp; @@ -81,6 +83,9 @@ static void ssh2_bpp_free(BinaryPacketProtocol *bpp) sfree(s); } +#define bpp_logevent(printf_args) \ + logevent_and_free(s->bpp.frontend, dupprintf printf_args) + void ssh2_bpp_new_outgoing_crypto( BinaryPacketProtocol *bpp, const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv, @@ -106,6 +111,9 @@ void ssh2_bpp_new_outgoing_crypto( s->cbc_ignore_workaround = ( (ssh2_cipher_alg(s->out.cipher)->flags & SSH_CIPHER_IS_CBC) && !(s->bpp.remote_bugs & BUG_CHOKES_ON_SSH2_IGNORE)); + + bpp_logevent(("Initialised %.200s client->server encryption", + ssh2_cipher_alg(s->out.cipher)->text_name)); } else { s->out.cipher = NULL; s->cbc_ignore_workaround = FALSE; @@ -114,6 +122,14 @@ void ssh2_bpp_new_outgoing_crypto( if (mac) { s->out.mac = ssh2_mac_new(mac, s->out.cipher); mac->setkey(s->out.mac, mac_key); + + bpp_logevent(("Initialised %.200s client->server" + " MAC algorithm%s%s", + ssh2_mac_alg(s->out.mac)->text_name, + etm_mode ? " (in ETM mode)" : "", + (s->out.cipher && + ssh2_cipher_alg(s->out.cipher)->required_mac ? + " (required by cipher)" : ""))); } else { s->out.mac = NULL; } @@ -122,6 +138,9 @@ void ssh2_bpp_new_outgoing_crypto( * indicated by ssh_comp_none. But this setup call may return a * null out_comp. */ s->out_comp = ssh_compressor_new(compression); + if (s->out_comp) + bpp_logevent(("Initialised %s compression", + ssh_compressor_alg(s->out_comp)->text_name)); } void ssh2_bpp_new_incoming_crypto( @@ -145,6 +164,9 @@ void ssh2_bpp_new_incoming_crypto( s->in.cipher = ssh2_cipher_new(cipher); ssh2_cipher_setkey(s->in.cipher, ckey); ssh2_cipher_setiv(s->in.cipher, iv); + + bpp_logevent(("Initialised %.200s server->client encryption", + ssh2_cipher_alg(s->in.cipher)->text_name)); } else { s->in.cipher = NULL; } @@ -152,6 +174,13 @@ void ssh2_bpp_new_incoming_crypto( if (mac) { s->in.mac = ssh2_mac_new(mac, s->in.cipher); mac->setkey(s->in.mac, mac_key); + + bpp_logevent(("Initialised %.200s server->client MAC algorithm%s%s", + ssh2_mac_alg(s->in.mac)->text_name, + etm_mode ? " (in ETM mode)" : "", + (s->in.cipher && + ssh2_cipher_alg(s->in.cipher)->required_mac ? + " (required by cipher)" : ""))); } else { s->in.mac = NULL; } @@ -160,6 +189,9 @@ void ssh2_bpp_new_incoming_crypto( * indicated by ssh_comp_none. But this setup call may return a * null in_decomp. */ s->in_decomp = ssh_decompressor_new(compression); + if (s->in_decomp) + bpp_logevent(("Initialised %s decompression", + ssh_decompressor_alg(s->in_decomp)->text_name)); /* Clear the pending_newkeys flag, so that handle_input below will * start consuming the input data again. */ diff --git a/ssh2transport.c b/ssh2transport.c index bec55dac..60626403 100644 --- a/ssh2transport.c +++ b/ssh2transport.c @@ -2153,20 +2153,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) strbuf_free(mac_key); } - if (s->out.cipher) - ppl_logevent(("Initialised %.200s client->server encryption", - s->out.cipher->text_name)); - if (s->out.mac) - ppl_logevent(("Initialised %.200s client->server" - " MAC algorithm%s%s", - s->out.mac->text_name, - s->out.etm_mode ? " (in ETM mode)" : "", - (s->out.cipher->required_mac ? - " (required by cipher)" : ""))); - if (s->out.comp->text_name) - ppl_logevent(("Initialised %s compression", - s->out.comp->text_name)); - /* * Now our end of the key exchange is complete, we can send all * our queued higher-layer packets. Transfer the whole of the next @@ -2222,19 +2208,6 @@ static void ssh2_transport_process_queue(PacketProtocolLayer *ppl) strbuf_free(mac_key); } - if (s->in.cipher) - ppl_logevent(("Initialised %.200s server->client encryption", - s->in.cipher->text_name)); - if (s->in.mac) - ppl_logevent(("Initialised %.200s server->client MAC algorithm%s%s", - s->in.mac->text_name, - s->in.etm_mode ? " (in ETM mode)" : "", - (s->in.cipher->required_mac ? - " (required by cipher)" : ""))); - if (s->in.comp->text_name) - ppl_logevent(("Initialised %s decompression", - s->in.comp->text_name)); - /* * Free shared secret. */ diff --git a/sshbpp.h b/sshbpp.h index ee5c1c12..3914eeb3 100644 --- a/sshbpp.h +++ b/sshbpp.h @@ -23,6 +23,7 @@ struct BinaryPacketProtocol { PacketLogSettings *pls; LogContext *logctx; Ssh *ssh; + Frontend *frontend; /* ic_in_raw is filled in by the BPP (probably by calling * ssh_bpp_common_setup). The BPP's owner triggers it when data is @@ -52,7 +53,7 @@ struct BinaryPacketProtocol { * does centralised parts of the freeing too. */ void ssh_bpp_free(BinaryPacketProtocol *bpp); -BinaryPacketProtocol *ssh1_bpp_new(void); +BinaryPacketProtocol *ssh1_bpp_new(Frontend *frontend); void ssh1_bpp_new_cipher(BinaryPacketProtocol *bpp, const struct ssh1_cipheralg *cipher, const void *session_key); @@ -96,7 +97,8 @@ struct DataTransferStats { ((stats)->direction.running = FALSE, TRUE) : \ ((stats)->direction.remaining -= (size), FALSE)) -BinaryPacketProtocol *ssh2_bpp_new(struct DataTransferStats *stats); +BinaryPacketProtocol *ssh2_bpp_new( + Frontend *frontend, struct DataTransferStats *stats); void ssh2_bpp_new_outgoing_crypto( BinaryPacketProtocol *bpp, const struct ssh2_cipheralg *cipher, const void *ckey, const void *iv, @@ -108,7 +110,7 @@ void ssh2_bpp_new_incoming_crypto( const struct ssh2_macalg *mac, int etm_mode, const void *mac_key, const struct ssh_compression_alg *compression); -BinaryPacketProtocol *ssh2_bare_bpp_new(void); +BinaryPacketProtocol *ssh2_bare_bpp_new(Frontend *frontend); /* * The initial code to handle the SSH version exchange is also diff --git a/sshverstring.c b/sshverstring.c index 060587f4..66db98b6 100644 --- a/sshverstring.c +++ b/sshverstring.c @@ -17,7 +17,6 @@ struct ssh_verstring_state { int crState; Conf *conf; - Frontend *frontend; ptrlen prefix_wanted; char *our_protoversion; struct ssh_version_receiver *receiver; @@ -88,7 +87,7 @@ BinaryPacketProtocol *ssh_verstring_new( assert(s->prefix_wanted.len <= PREFIX_MAXLEN); s->conf = conf_copy(conf); - s->frontend = frontend; + s->bpp.frontend = frontend; s->our_protoversion = dupstr(protoversion); s->receiver = rcv; @@ -146,8 +145,8 @@ static int ssh_version_includes_v2(const char *ver) return ssh_versioncmp(ver, "1.99") >= 0; } -#define vs_logevent(printf_args) \ - logevent_and_free(s->frontend, dupprintf printf_args) +#define bpp_logevent(printf_args) \ + logevent_and_free(s->bpp.frontend, dupprintf printf_args) static void ssh_verstring_send(struct ssh_verstring_state *s) { @@ -198,7 +197,7 @@ static void ssh_verstring_send(struct ssh_verstring_state *s) bufchain_add(s->bpp.out_raw, "\015", 1); bufchain_add(s->bpp.out_raw, "\012", 1); - vs_logevent(("We claim version: %s", s->our_vstring)); + bpp_logevent(("We claim version: %s", s->our_vstring)); } #define BPP_WAITFOR(minlen) do \ @@ -308,7 +307,7 @@ void ssh_verstring_handle_input(BinaryPacketProtocol *bpp) s->vslen--; s->vstring[s->vslen] = '\0'; - vs_logevent(("Remote version: %s", s->vstring)); + bpp_logevent(("Remote version: %s", s->vstring)); /* * Pick out the protocol version and software version. The former @@ -374,7 +373,7 @@ void ssh_verstring_handle_input(BinaryPacketProtocol *bpp) crStopV; } - vs_logevent(("Using SSH protocol version %d", s->major_protoversion)); + bpp_logevent(("Using SSH protocol version %d", s->major_protoversion)); if (!s->send_early) { /* @@ -443,7 +442,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * sniffing. */ s->remote_bugs |= BUG_CHOKES_ON_SSH1_IGNORE; - vs_logevent(("We believe remote version has SSH-1 ignore bug")); + bpp_logevent(("We believe remote version has SSH-1 ignore bug")); } if (conf_get_int(s->conf, CONF_sshbug_plainpw1) == FORCE_ON || @@ -455,8 +454,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * the password. */ s->remote_bugs |= BUG_NEEDS_SSH1_PLAIN_PASSWORD; - vs_logevent(("We believe remote version needs a " - "plain SSH-1 password")); + bpp_logevent(("We believe remote version needs a " + "plain SSH-1 password")); } if (conf_get_int(s->conf, CONF_sshbug_rsa1) == FORCE_ON || @@ -468,8 +467,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * an AUTH_RSA message. */ s->remote_bugs |= BUG_CHOKES_ON_RSA; - vs_logevent(("We believe remote version can't handle SSH-1 " - "RSA authentication")); + bpp_logevent(("We believe remote version can't handle SSH-1 " + "RSA authentication")); } if (conf_get_int(s->conf, CONF_sshbug_hmac2) == FORCE_ON || @@ -482,7 +481,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * These versions have the HMAC bug. */ s->remote_bugs |= BUG_SSH2_HMAC; - vs_logevent(("We believe remote version has SSH-2 HMAC bug")); + bpp_logevent(("We believe remote version has SSH-2 HMAC bug")); } if (conf_get_int(s->conf, CONF_sshbug_derivekey2) == FORCE_ON || @@ -495,8 +494,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * generate the keys). */ s->remote_bugs |= BUG_SSH2_DERIVEKEY; - vs_logevent(("We believe remote version has SSH-2 " - "key-derivation bug")); + bpp_logevent(("We believe remote version has SSH-2 " + "key-derivation bug")); } if (conf_get_int(s->conf, CONF_sshbug_rsapad2) == FORCE_ON || @@ -509,7 +508,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * These versions have the SSH-2 RSA padding bug. */ s->remote_bugs |= BUG_SSH2_RSA_PADDING; - vs_logevent(("We believe remote version has SSH-2 RSA padding bug")); + bpp_logevent(("We believe remote version has SSH-2 RSA padding bug")); } if (conf_get_int(s->conf, CONF_sshbug_pksessid2) == FORCE_ON || @@ -520,8 +519,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * public-key authentication. */ s->remote_bugs |= BUG_SSH2_PK_SESSIONID; - vs_logevent(("We believe remote version has SSH-2 " - "public-key-session-ID bug")); + bpp_logevent(("We believe remote version has SSH-2 " + "public-key-session-ID bug")); } if (conf_get_int(s->conf, CONF_sshbug_rekey2) == FORCE_ON || @@ -537,7 +536,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * These versions have the SSH-2 rekey bug. */ s->remote_bugs |= BUG_SSH2_REKEY; - vs_logevent(("We believe remote version has SSH-2 rekey bug")); + bpp_logevent(("We believe remote version has SSH-2 rekey bug")); } if (conf_get_int(s->conf, CONF_sshbug_maxpkt2) == FORCE_ON || @@ -548,8 +547,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * This version ignores our makpkt and needs to be throttled. */ s->remote_bugs |= BUG_SSH2_MAXPKT; - vs_logevent(("We believe remote version ignores SSH-2 " - "maximum packet size")); + bpp_logevent(("We believe remote version ignores SSH-2 " + "maximum packet size")); } if (conf_get_int(s->conf, CONF_sshbug_ignore2) == FORCE_ON) { @@ -558,7 +557,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * none detected automatically. */ s->remote_bugs |= BUG_CHOKES_ON_SSH2_IGNORE; - vs_logevent(("We believe remote version has SSH-2 ignore bug")); + bpp_logevent(("We believe remote version has SSH-2 ignore bug")); } if (conf_get_int(s->conf, CONF_sshbug_oldgex2) == FORCE_ON || @@ -570,7 +569,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * we use the newer version. */ s->remote_bugs |= BUG_SSH2_OLDGEX; - vs_logevent(("We believe remote version has outdated SSH-2 GEX")); + bpp_logevent(("We believe remote version has outdated SSH-2 GEX")); } if (conf_get_int(s->conf, CONF_sshbug_winadj) == FORCE_ON) { @@ -579,7 +578,7 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * reason or another. Currently, none detected automatically. */ s->remote_bugs |= BUG_CHOKES_ON_WINADJ; - vs_logevent(("We believe remote version has winadj bug")); + bpp_logevent(("We believe remote version has winadj bug")); } if (conf_get_int(s->conf, CONF_sshbug_chanreq) == FORCE_ON || @@ -596,8 +595,8 @@ static void ssh_detect_bugs(struct ssh_verstring_state *s) * https://secure.ucc.asn.au/hg/dropbear/rev/cd02449b709c */ s->remote_bugs |= BUG_SENDS_LATE_REQUEST_REPLY; - vs_logevent(("We believe remote version has SSH-2 " - "channel request bug")); + bpp_logevent(("We believe remote version has SSH-2 " + "channel request bug")); } }