Skip to content

Commit

Permalink
Give BPPs a Frontend, so they can do their own logging.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sgtatham
sgtatham committed Oct 7, 2018
1 parent 36caf03 commit 2e7ced6
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 64 deletions.
6 changes: 3 additions & 3 deletions ssh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions ssh.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
10 changes: 9 additions & 1 deletion ssh1bpp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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)
Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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"));
}

/*
Expand Down
2 changes: 0 additions & 2 deletions ssh1login.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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"));
Expand Down
3 changes: 2 additions & 1 deletion ssh2bpp-bare.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
34 changes: 33 additions & 1 deletion ssh2bpp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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;
}
Expand All @@ -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(
Expand All @@ -145,13 +164,23 @@ 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;
}
s->in.etm_mode = etm_mode;
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;
}
Expand All @@ -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. */
Expand Down
27 changes: 0 additions & 27 deletions ssh2transport.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
*/
Expand Down
8 changes: 5 additions & 3 deletions sshbpp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
Loading

0 comments on commit 2e7ced6

Please sign in to comment.