From 03a0d11c4de6da9c244f677a04ce948fce67e59b Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 21 Mar 2013 23:33:12 +0800 Subject: [PATCH 01/33] Added tag DROPBEAR_2013.56 for changeset 1b8b2b9d6e94 --- .hgtags | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgtags b/.hgtags index 761d71f..71a20fe 100644 --- a/.hgtags +++ b/.hgtags @@ -35,3 +35,4 @@ d354464b2aa6f6ba0bf44d43bcae5aa798435393 DROPBEAR_2012.55 0000000000000000000000000000000000000000 t:ltc-0.95-orig d7da3b1e15401eb234ec866d5eac992fc4cd5878 t:ltc-0.95-db-merge1 0000000000000000000000000000000000000000 t:ltc-0.95-db-merge1 +1b8b2b9d6e94bc3cc5e61b620476ea36cc466e1b DROPBEAR_2013.56 From c172fb3b326ec796a1e81656548653f188a8d62c Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 21 Mar 2013 23:35:07 +0800 Subject: [PATCH 02/33] Added signature for changeset 9b80981212fe --- .hgsigs | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgsigs b/.hgsigs index 506e078..5dcb50e 100644 --- a/.hgsigs +++ b/.hgsigs @@ -1,3 +1,4 @@ aa2f51a6b81d33de5e9898a7f27c792a173d9b26 0 iD8DBQBOuADmjPn4sExkf7wRAv/fAJ9FJFvjDoF+wd1ipDx1wkzdeBQNqgCgykUrSbXv76FBbxKntVbk9oS3GjI= 3f12086c2ef2b9ffe36a822fdb3ff647fcec1831 0 iD8DBQBOuSlQjPn4sExkf7wRAvkbAKCgE1e8xEMQ16CGeoywhIQ0QR4eNgCfZdYYlzjb/+521Uvh5/7FRYEmrho= 85f835f2fe0ac2c503c50a414de127222fb0a57c 0 iD8DBQBPRkMUjPn4sExkf7wRAvM4AJ9mw2OAkyjhSbamM1MizlEJUX18HACgoFKQkYf6BnYxN34Nv2HhM0cmzUc= +9b80981212fe6c01b7c16b3ca7c4e66af56f12f1 0 iEYEABECAAYFAlFLKKcACgkQjPn4sExkf7xK7wCfcioCmJPsysSbQO6+4qZMVe0mmLwAn2/o+wRf4MrUXlohrr7aXEF9vdSB From 5996c3824c52425c2d4f3d7cb69f0c9fa81a33b8 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 23 Mar 2013 23:16:06 +0800 Subject: [PATCH 03/33] Add ~. and ~^Z handling to exit/suspend dbclient --- channel.h | 4 ++- cli-chansession.c | 63 +++++++++++++++++++++++++++++++++++++++++++++-- common-channel.c | 13 +++++++++- session.h | 3 +++ sysoptions.h | 2 ++ 5 files changed, 81 insertions(+), 4 deletions(-) diff --git a/channel.h b/channel.h index 950d4b3..7146697 100644 --- a/channel.h +++ b/channel.h @@ -83,8 +83,10 @@ struct Channel { int flushing; - const struct ChanType* type; + /* Used by client chansession to handle ~ escaping, NULL ignored otherwise */ + void (*read_mangler)(struct Channel*, unsigned char* bytes, int *len); + const struct ChanType* type; }; struct ChanType { diff --git a/cli-chansession.c b/cli-chansession.c index 126c32f..65d1f4f 100644 --- a/cli-chansession.c +++ b/cli-chansession.c @@ -38,9 +38,10 @@ static void cli_closechansess(struct Channel *channel); static int cli_initchansess(struct Channel *channel); static void cli_chansessreq(struct Channel *channel); - static void send_chansess_pty_req(struct Channel *channel); static void send_chansess_shell_req(struct Channel *channel); +static void cli_escape_handler(struct Channel *channel, unsigned char* buf, int *len); + static void cli_tty_setup(); @@ -374,7 +375,9 @@ static int cli_initchansess(struct Channel *channel) { if (cli_opts.wantpty) { cli_tty_setup(); - } + channel->read_mangler = cli_escape_handler; + cli_ses.last_char = '\r'; + } return 0; /* Success */ } @@ -429,3 +432,59 @@ void cli_send_chansess_request() { TRACE(("leave cli_send_chansess_request")) } + +// returns 1 if the character should be consumed, 0 to pass through +static int +do_escape(unsigned char c) { + switch (c) { + case '.': + dropbear_exit("Terminated"); + return 1; + break; + case 0x1a: + // ctrl-z + cli_tty_cleanup(); + kill(getpid(), SIGTSTP); + // after continuation + cli_tty_setup(); + cli_ses.winchange = 1; + return 1; + break; + } + return 0; +} + +static +void cli_escape_handler(struct Channel *channel, unsigned char* buf, int *len) { + char c; + int skip_char = 0; + + // only handle escape characters if they are read one at a time. simplifies + // the code and avoids nasty people putting ~. at the start of a line to paste + if (*len != 1) { + cli_ses.last_char = 0x0; + return; + } + + c = buf[0]; + + if (cli_ses.last_char == DROPBEAR_ESCAPE_CHAR) { + skip_char = do_escape(c); + cli_ses.last_char = 0x0; + } else { + if (c == DROPBEAR_ESCAPE_CHAR) { + if (cli_ses.last_char == '\r') { + cli_ses.last_char = DROPBEAR_ESCAPE_CHAR; + skip_char = 1; + } else { + cli_ses.last_char = 0x0; + } + } else { + cli_ses.last_char = c; + } + } + + if (skip_char) { + *len = 0; + } +} diff --git a/common-channel.c b/common-channel.c index 9328a2e..05b9d11 100644 --- a/common-channel.c +++ b/common-channel.c @@ -643,6 +643,7 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) { /* read the data */ len = read(fd, buf_getwriteptr(ses.writepayload, maxlen), maxlen); + if (len <= 0) { if (len == 0 || errno != EINTR) { /* This will also get hit in the case of EAGAIN. The only @@ -650,12 +651,22 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) { in which case it can be treated the same as EOF */ close_chan_fd(channel, fd, SHUT_RD); } - ses.writepayload->len = ses.writepayload->pos = 0; + buf_setpos(ses.writepayload, 0); + buf_setlen(ses.writepayload, 0); TRACE(("leave send_msg_channel_data: len %d read err %d or EOF for fd %d", len, errno, fd)) return; } + if (channel->read_mangler) { + channel->read_mangler(channel, buf_getwriteptr(ses.writepayload, len), &len); + if (len == 0) { + buf_setpos(ses.writepayload, 0); + buf_setlen(ses.writepayload, 0); + return; + } + } + TRACE(("send_msg_channel_data: len %d fd %d", len, fd)) buf_incrwritepos(ses.writepayload, len); /* ... real size here */ diff --git a/session.h b/session.h index 158e209..0719e34 100644 --- a/session.h +++ b/session.h @@ -259,6 +259,9 @@ struct clientsession { int stderrcopy; int stderrflags; + /* for escape char handling */ + int last_char; + int winchange; /* Set to 1 when a windowchange signal happens */ int lastauthtype; /* either AUTH_TYPE_PUBKEY or AUTH_TYPE_PASSWORD, diff --git a/sysoptions.h b/sysoptions.h index 2d93e7b..8c591ea 100644 --- a/sysoptions.h +++ b/sysoptions.h @@ -54,6 +54,8 @@ #define _PATH_CP "/bin/cp" +#define DROPBEAR_ESCAPE_CHAR '~' + /* success/failure defines */ #define DROPBEAR_SUCCESS 0 #define DROPBEAR_FAILURE -1 From c4861340e93280dce97bbfa3b421c7819bc7864e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 23 Mar 2013 23:17:01 +0800 Subject: [PATCH 04/33] Fix a few compile warnings --- cli-agentfwd.c | 3 --- cli-kex.c | 1 - svr-main.c | 2 +- svr-x11fwd.c | 2 +- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/cli-agentfwd.c b/cli-agentfwd.c index a821305..c661455 100644 --- a/cli-agentfwd.c +++ b/cli-agentfwd.c @@ -156,8 +156,6 @@ static buffer * agent_request(unsigned char type, buffer *data) { goto out; } - TRACE(("agent_request readlen is %d", readlen)) - buf_resize(inbuf, readlen); buf_setpos(inbuf, 0); ret = atomicio(read, fd, buf_getwriteptr(inbuf, readlen), readlen); @@ -167,7 +165,6 @@ static buffer * agent_request(unsigned char type, buffer *data) { } buf_incrwritepos(inbuf, readlen); buf_setpos(inbuf, 0); - TRACE(("agent_request success, length %d", readlen)) out: if (payload) diff --git a/cli-kex.c b/cli-kex.c index 0d5a9d2..9dadb3c 100644 --- a/cli-kex.c +++ b/cli-kex.c @@ -309,7 +309,6 @@ static void checkhostkey(unsigned char* keyblob, unsigned int keybloblen) { buf_putbytes(line, algoname, algolen); buf_putbyte(line, ' '); len = line->size - line->pos; - TRACE(("keybloblen %d, len %d", keybloblen, len)) /* The only failure with base64 is buffer_overflow, but buf_getwriteptr * will die horribly in the case anyway */ base64_encode(keyblob, keybloblen, buf_getwriteptr(line, len), &len); diff --git a/svr-main.c b/svr-main.c index 43d6656..461aeaf 100644 --- a/svr-main.c +++ b/svr-main.c @@ -271,7 +271,7 @@ void main_noinetd() { goto out; } - addrandom(&fork_ret, sizeof(fork_ret)); + addrandom((void*)&fork_ret, sizeof(fork_ret)); if (fork_ret > 0) { diff --git a/svr-x11fwd.c b/svr-x11fwd.c index 92dadd5..f6368d7 100644 --- a/svr-x11fwd.c +++ b/svr-x11fwd.c @@ -175,7 +175,7 @@ void x11cleanup(struct ChanSess *chansess) { m_free(chansess->x11authprot); m_free(chansess->x11authcookie); - TRACE(("chansess %x", chansess)) + TRACE(("chansess %p", chansess)) if (chansess->x11listener != NULL) { remove_listener(chansess->x11listener); chansess->x11listener = NULL; From bbf6d5f2f5375c3d227f7618c04552f77a352b4c Mon Sep 17 00:00:00 2001 From: Mike Frysinger Date: Sun, 24 Mar 2013 00:00:39 +0800 Subject: [PATCH 05/33] rename configure.in -> configure.ac Latest autotools warn now if the file is named configure.in --HG-- rename : configure.in => configure.ac --- configure.in => configure.ac | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename configure.in => configure.ac (100%) diff --git a/configure.in b/configure.ac similarity index 100% rename from configure.in rename to configure.ac From 9be0d6b53d6b85a8f273ee28876405fbca49b9fd Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 24 Mar 2013 00:02:20 +0800 Subject: [PATCH 06/33] Define _GNU_SOURCE for vasprintf --- scpmisc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/scpmisc.c b/scpmisc.c index c3b09a5..ec4df35 100644 --- a/scpmisc.c +++ b/scpmisc.c @@ -40,6 +40,7 @@ /*RCSID("OpenBSD: xmalloc.c,v 1.16 2001/07/23 18:21:46 stevesk Exp ");*/ +#define _GNU_SOURCE #include "includes.h" #include "scpmisc.h" From 4f62da0f0d5da78b6c7a0cf507307aeacc8ac842 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 29 Mar 2013 20:44:13 +0800 Subject: [PATCH 07/33] first_kex_packet_follows working, needs tidying --HG-- branch : kexguess --- cli-kex.c | 5 +++-- cli-session.c | 21 ++++++++++++++------- common-kex.c | 34 ++++++++++++++++++++++++++-------- kex.h | 4 +++- session.h | 2 ++ 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/cli-kex.c b/cli-kex.c index 9dadb3c..828af9a 100644 --- a/cli-kex.c +++ b/cli-kex.c @@ -42,7 +42,7 @@ static void checkhostkey(unsigned char* keyblob, unsigned int keybloblen); #define MAX_KNOWNHOSTS_LINE 4500 void send_msg_kexdh_init() { - + TRACE(("send_msg_kexdh_init()")) cli_ses.dh_e = (mp_int*)m_malloc(sizeof(mp_int)); cli_ses.dh_x = (mp_int*)m_malloc(sizeof(mp_int)); m_mp_init_multi(cli_ses.dh_e, cli_ses.dh_x, NULL); @@ -53,7 +53,8 @@ void send_msg_kexdh_init() { buf_putbyte(ses.writepayload, SSH_MSG_KEXDH_INIT); buf_putmpint(ses.writepayload, cli_ses.dh_e); encrypt_packet(); - ses.requirenext = SSH_MSG_KEXDH_REPLY; + // XXX fixme + //ses.requirenext = SSH_MSG_KEXDH_REPLY; } /* Handle a diffie-hellman key exchange reply. */ diff --git a/cli-session.c b/cli-session.c index e58fdbd..cc514bb 100644 --- a/cli-session.c +++ b/cli-session.c @@ -109,6 +109,12 @@ void cli_session(int sock_in, int sock_out) { } +static void cli_send_kex_first_guess() { + send_msg_kexdh_init(); + dropbear_log(LOG_INFO, "kexdh_init guess sent"); + //cli_ses.kex_state = KEXDH_INIT_SENT; +} + static void cli_session_init() { cli_ses.state = STATE_NOTHING; @@ -148,6 +154,9 @@ static void cli_session_init() { ses.packettypes = cli_packettypes; ses.isserver = 0; + + ses.send_kex_first_guess = cli_send_kex_first_guess; + } /* This function drives the progress of the session - it initiates KEX, @@ -157,15 +166,13 @@ static void cli_sessionloop() { TRACE(("enter cli_sessionloop")) if (ses.lastpacket == SSH_MSG_KEXINIT && cli_ses.kex_state == KEX_NOTHING) { - cli_ses.kex_state = KEXINIT_RCVD; - } - - if (cli_ses.kex_state == KEXINIT_RCVD) { - /* We initiate the KEXDH. If DH wasn't the correct type, the KEXINIT * negotiation would have failed. */ - send_msg_kexdh_init(); - cli_ses.kex_state = KEXDH_INIT_SENT; + if (!ses.kexstate.our_first_follows_matches) { + dropbear_log(LOG_INFO, "kexdh_init after remote's kexinit"); + send_msg_kexdh_init(); + } + cli_ses.kex_state = KEXDH_INIT_SENT; TRACE(("leave cli_sessionloop: done with KEXINIT_RCVD")) return; } diff --git a/common-kex.c b/common-kex.c index 56b206d..e4b4c02 100644 --- a/common-kex.c +++ b/common-kex.c @@ -131,8 +131,8 @@ void send_msg_kexinit() { /* languages_server_to_client */ buf_putstring(ses.writepayload, "", 0); - /* first_kex_packet_follows - unimplemented for now */ - buf_putbyte(ses.writepayload, 0x00); + /* first_kex_packet_follows */ + buf_putbyte(ses.writepayload, (ses.send_kex_first_guess != NULL)); /* reserved unit32 */ buf_putint(ses.writepayload, 0); @@ -144,9 +144,19 @@ void send_msg_kexinit() { encrypt_packet(); ses.dataallowed = 0; /* don't send other packets during kex */ + ses.kexstate.sentkexinit = 1; + + ses.newkeys = (struct key_context*)m_malloc(sizeof(struct key_context)); + + if (ses.send_kex_first_guess) { + ses.newkeys->algo_kex = sshkex[0].val; + ses.newkeys->algo_hostkey = sshhostkey[0].val; + ses.send_kex_first_guess(); + } + TRACE(("DATAALLOWED=0")) TRACE(("-> KEXINIT")) - ses.kexstate.sentkexinit = 1; + } /* *** NOTE regarding (send|recv)_msg_newkeys *** @@ -236,11 +246,13 @@ static void kexinitialise() { ses.kexstate.sentnewkeys = 0; /* first_packet_follows */ - ses.kexstate.firstfollows = 0; + ses.kexstate.them_firstfollows = 0; ses.kexstate.datatrans = 0; ses.kexstate.datarecv = 0; + ses.kexstate.our_first_follows_matches = 0; + ses.kexstate.lastkextime = time(NULL); } @@ -555,7 +567,7 @@ void gen_kexdh_vals(mp_int *dh_pub, mp_int *dh_priv) { DEF_MP_INT(dh_q); DEF_MP_INT(dh_g); - TRACE(("enter send_msg_kexdh_reply")) + TRACE(("enter gen_kexdh_vals")) m_mp_init_multi(&dh_g, &dh_p, &dh_q, NULL); @@ -678,7 +690,7 @@ static void read_kex_algos() { buf_incrpos(ses.payload, 16); /* start after the cookie */ - ses.newkeys = (struct key_context*)m_malloc(sizeof(struct key_context)); + memset(ses.newkeys, 0x0, sizeof(*ses.newkeys)); /* kex_algorithms */ algo = ses.buf_match_algo(ses.payload, sshkex, &goodguess); @@ -754,9 +766,10 @@ static void read_kex_algos() { /* languages_server_to_client */ buf_eatstring(ses.payload); - /* first_kex_packet_follows */ + /* their first_kex_packet_follows */ if (buf_getbool(ses.payload)) { - ses.kexstate.firstfollows = 1; + TRACE(("them kex firstfollows. allgood %d", allgood)) + ses.kexstate.them_firstfollows = 1; /* if the guess wasn't good, we ignore the packet sent */ if (!allgood) { ses.ignorenext = 1; @@ -799,6 +812,11 @@ static void read_kex_algos() { /* reserved for future extensions */ buf_getint(ses.payload); + + if (ses.send_kex_first_guess && allgood) { + TRACE(("our_first_follows_matches 1")) + ses.kexstate.our_first_follows_matches = 1; + } return; error: diff --git a/kex.h b/kex.h index c89b0a3..dc5f46b 100644 --- a/kex.h +++ b/kex.h @@ -51,13 +51,15 @@ struct KEXState { unsigned sentkexinit : 1; /*set when we've sent/recv kexinit packet */ unsigned recvkexinit : 1; - unsigned firstfollows : 1; /* true when first_kex_packet_follows is set */ + unsigned them_firstfollows : 1; /* true when first_kex_packet_follows is set */ unsigned sentnewkeys : 1; /* set once we've send MSG_NEWKEYS (will be cleared once we have also received */ unsigned recvnewkeys : 1; /* set once we've received MSG_NEWKEYS (cleared once we have also sent */ unsigned donefirstkex : 1; /* Set to 1 after the first kex has completed, ie the transport layer has been set up */ + unsigned our_first_follows_matches : 1; + time_t lastkextime; /* time of the last kex */ unsigned int datatrans; /* data transmitted since last kex */ unsigned int datarecv; /* data received since last kex */ diff --git a/session.h b/session.h index 0719e34..6b106f5 100644 --- a/session.h +++ b/session.h @@ -178,6 +178,8 @@ struct sshsession { void(*remoteclosed)(); /* A callback to handle closure of the remote connection */ + void(*send_kex_first_guess)(); + struct AuthState authstate; /* Common amongst client and server, since most struct elements are common */ From 99d9cf500b30c77107bf8477dd55831626f9beaf Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 29 Mar 2013 23:29:48 +0800 Subject: [PATCH 08/33] Add kexguess2 behaviour --HG-- branch : kexguess --- algo.h | 14 ++++++++++++-- cli-algo.c | 32 +++++++++++++++++++++++++++----- common-algo.c | 1 + common-kex.c | 21 ++++++++++++--------- debug.h | 4 ++-- kex.h | 1 + options.h | 4 ++-- session.h | 1 + svr-algo.c | 32 ++++++++++++++++++++++++++++---- 9 files changed, 86 insertions(+), 24 deletions(-) diff --git a/algo.h b/algo.h index ad57037..755cfcd 100644 --- a/algo.h +++ b/algo.h @@ -83,10 +83,20 @@ void crypto_init(); int have_algo(char* algo, size_t algolen, algo_type algos[]); void buf_put_algolist(buffer * buf, algo_type localalgos[]); +enum kexguess2_used { + KEXGUESS2_LOOK, + KEXGUESS2_NO, + KEXGUESS2_YES, +}; + +#define KEXGUESS2_ALGO_NAME "kexguess2@matt.ucc.asn.au" +#define KEXGUESS2_ALGO_ID 99 + + algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], - int *goodguess); + enum kexguess2_used *kexguess2, int *goodguess); algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], - int *goodguess); + enum kexguess2_used *kexguess2, int *goodguess); #ifdef ENABLE_USER_ALGO_LIST int check_user_algos(const char* user_algo_list, algo_type * algos, diff --git a/cli-algo.c b/cli-algo.c index 09da41a..ec8c541 100644 --- a/cli-algo.c +++ b/cli-algo.c @@ -34,7 +34,7 @@ * that is also on the server's list. */ algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], - int *goodguess) { + enum kexguess2_used *kexguess2, int *goodguess) { unsigned char * algolist = NULL; unsigned char * remotealgos[MAX_PROPOSED_ALGO]; @@ -42,7 +42,9 @@ algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], unsigned int count, i, j; algo_type * ret = NULL; - *goodguess = 0; + if (goodguess) { + *goodguess = 0; + } /* get the comma-separated list from the buffer ie "algo1,algo2,algo3" */ algolist = buf_getstring(buf, &len); @@ -72,6 +74,19 @@ algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], } } + if (kexguess2 && *kexguess2 == KEXGUESS2_LOOK) { + for (i = 0; i < count; i++) + { + if (strcmp(remotealgos[i], KEXGUESS2_ALGO_NAME) == 0) { + *kexguess2 = KEXGUESS2_YES; + break; + } + } + if (*kexguess2 == KEXGUESS2_LOOK) { + *kexguess2 = KEXGUESS2_NO; + } + } + /* iterate and find the first match */ for (j = 0; localalgos[j].name != NULL; j++) { @@ -81,9 +96,16 @@ algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], if (len == strlen(remotealgos[i]) && strncmp(localalgos[j].name, remotealgos[i], len) == 0) { - if (i == 0 && j == 0) { - /* was a good guess */ - *goodguess = 1; + if (goodguess && kexguess2) { + if (*kexguess2 == KEXGUESS2_YES) { + if (j == 0) { + *goodguess = 1; + } + } else { + if (i == 0 && j == 0) { + *goodguess = 1; + } + } } ret = &localalgos[j]; goto out; diff --git a/common-algo.c b/common-algo.c index 4a14651..46dbe74 100644 --- a/common-algo.c +++ b/common-algo.c @@ -215,6 +215,7 @@ algo_type sshhostkey[] = { algo_type sshkex[] = { {"diffie-hellman-group1-sha1", DROPBEAR_KEX_DH_GROUP1, NULL, 1, NULL}, {"diffie-hellman-group14-sha1", DROPBEAR_KEX_DH_GROUP14, NULL, 1, NULL}, + {KEXGUESS2_ALGO_NAME, KEXGUESS2_ALGO_ID, NULL, 1, NULL}, {NULL, 0, NULL, 0, NULL} }; diff --git a/common-kex.c b/common-kex.c index e4b4c02..0640b9f 100644 --- a/common-kex.c +++ b/common-kex.c @@ -692,18 +692,21 @@ static void read_kex_algos() { memset(ses.newkeys, 0x0, sizeof(*ses.newkeys)); + enum kexguess2_used kexguess2 = KEXGUESS2_LOOK; + /* kex_algorithms */ - algo = ses.buf_match_algo(ses.payload, sshkex, &goodguess); + algo = ses.buf_match_algo(ses.payload, sshkex, &kexguess2, &goodguess); allgood &= goodguess; - if (algo == NULL) { + if (algo == NULL || algo->val == KEXGUESS2_ALGO_ID) { erralgo = "kex"; goto error; } + TRACE(("kexguess2 %d", kexguess2)) TRACE(("kex algo %s", algo->name)) ses.newkeys->algo_kex = algo->val; /* server_host_key_algorithms */ - algo = ses.buf_match_algo(ses.payload, sshhostkey, &goodguess); + algo = ses.buf_match_algo(ses.payload, sshhostkey, &kexguess2, &goodguess); allgood &= goodguess; if (algo == NULL) { erralgo = "hostkey"; @@ -713,7 +716,7 @@ static void read_kex_algos() { ses.newkeys->algo_hostkey = algo->val; /* encryption_algorithms_client_to_server */ - c2s_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, &goodguess); + c2s_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, NULL, NULL); if (c2s_cipher_algo == NULL) { erralgo = "enc c->s"; goto error; @@ -721,7 +724,7 @@ static void read_kex_algos() { TRACE(("enc c2s is %s", c2s_cipher_algo->name)) /* encryption_algorithms_server_to_client */ - s2c_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, &goodguess); + s2c_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, NULL, NULL); if (s2c_cipher_algo == NULL) { erralgo = "enc s->c"; goto error; @@ -729,7 +732,7 @@ static void read_kex_algos() { TRACE(("enc s2c is %s", s2c_cipher_algo->name)) /* mac_algorithms_client_to_server */ - c2s_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, &goodguess); + c2s_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, NULL, NULL); if (c2s_hash_algo == NULL) { erralgo = "mac c->s"; goto error; @@ -737,7 +740,7 @@ static void read_kex_algos() { TRACE(("hash c2s is %s", c2s_hash_algo->name)) /* mac_algorithms_server_to_client */ - s2c_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, &goodguess); + s2c_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, NULL, NULL); if (s2c_hash_algo == NULL) { erralgo = "mac s->c"; goto error; @@ -745,7 +748,7 @@ static void read_kex_algos() { TRACE(("hash s2c is %s", s2c_hash_algo->name)) /* compression_algorithms_client_to_server */ - c2s_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, &goodguess); + c2s_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); if (c2s_comp_algo == NULL) { erralgo = "comp c->s"; goto error; @@ -753,7 +756,7 @@ static void read_kex_algos() { TRACE(("hash c2s is %s", c2s_comp_algo->name)) /* compression_algorithms_server_to_client */ - s2c_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, &goodguess); + s2c_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); if (s2c_comp_algo == NULL) { erralgo = "comp s->c"; goto error; diff --git a/debug.h b/debug.h index b20e685..02c100f 100644 --- a/debug.h +++ b/debug.h @@ -39,7 +39,7 @@ * Caution: Don't use this in an unfriendly environment (ie unfirewalled), * since the printing may not sanitise strings etc. This will add a reasonable * amount to your executable size. */ -/*#define DEBUG_TRACE */ +#define DEBUG_TRACE /* All functions writing to the cleartext payload buffer call * CHECKCLEARTOWRITE() before writing. This is only really useful if you're @@ -69,7 +69,7 @@ /* To debug with GDB it is easier to run with no forking of child processes. You will need to pass "-F" as well. */ -/* #define DEBUG_NOFORK */ +#define DEBUG_NOFORK /* For testing as non-root on shadowed systems, include the crypt of a password diff --git a/kex.h b/kex.h index dc5f46b..72430e9 100644 --- a/kex.h +++ b/kex.h @@ -66,6 +66,7 @@ struct KEXState { }; + #define MAX_KEXHASHBUF 2000 #endif /* _KEX_H_ */ diff --git a/options.h b/options.h index c52d6c2..71d39cc 100644 --- a/options.h +++ b/options.h @@ -174,9 +174,9 @@ much traffic. */ * PAM challenge/response. * You can't enable both PASSWORD and PAM. */ -#define ENABLE_SVR_PASSWORD_AUTH +//#define ENABLE_SVR_PASSWORD_AUTH /* PAM requires ./configure --enable-pam */ -/*#define ENABLE_SVR_PAM_AUTH*/ +#define ENABLE_SVR_PAM_AUTH #define ENABLE_SVR_PUBKEY_AUTH /* Whether to take public key options in diff --git a/session.h b/session.h index 6b106f5..3b9e957 100644 --- a/session.h +++ b/session.h @@ -170,6 +170,7 @@ struct sshsession { struct packetlist *reply_queue_head, *reply_queue_tail; algo_type*(*buf_match_algo)(buffer*buf, algo_type localalgos[], + enum kexguess2_used *kexguess2, int *goodguess); /* The function to use to choose which algorithm to use from the ones presented by the remote side. Is specific to the client/server mode, diff --git a/svr-algo.c b/svr-algo.c index f8f9055..620cfeb 100644 --- a/svr-algo.c +++ b/svr-algo.c @@ -33,7 +33,7 @@ * 0 otherwise. This is used for checking if the kexalgo/hostkeyalgos are * guessed correctly */ algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], - int *goodguess) + enum kexguess2_used *kexguess2, int *goodguess) { unsigned char * algolist = NULL; @@ -42,7 +42,9 @@ algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], unsigned int count, i, j; algo_type * ret = NULL; - *goodguess = 0; + if (goodguess) { + *goodguess = 0; + } /* get the comma-separated list from the buffer ie "algo1,algo2,algo3" */ algolist = buf_getstring(buf, &len); @@ -73,6 +75,19 @@ algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], } } + if (kexguess2 && *kexguess2 == KEXGUESS2_LOOK) { + for (i = 0; i < count; i++) + { + if (strcmp(remotealgos[i], KEXGUESS2_ALGO_NAME) == 0) { + *kexguess2 = KEXGUESS2_YES; + break; + } + } + if (*kexguess2 == KEXGUESS2_LOOK) { + *kexguess2 = KEXGUESS2_NO; + } + } + /* iterate and find the first match */ for (i = 0; i < count; i++) { @@ -83,8 +98,17 @@ algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], if (len == strlen(localalgos[j].name) && strncmp(localalgos[j].name, remotealgos[i], len) == 0) { /* set if it was a good guess */ - if (i == 0 && j == 0) { - *goodguess = 1; + if (goodguess && kexguess2) { + if (*kexguess2 == KEXGUESS2_YES) { + if (i == 0) { + *goodguess = 1; + } + + } else { + if (i == 0 && j == 0) { + *goodguess = 1; + } + } } /* set the algo to return */ ret = &localalgos[j]; From 9c7485331a581d1ff32f9caf005e7b13fa1c051e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sat, 30 Mar 2013 23:55:05 +0800 Subject: [PATCH 09/33] Get rid of client/server specific buf_match_algo, use single function with a couple of if statements instead --HG-- branch : kexguess --- Makefile.in | 4 +- algo.h | 4 +- cli-algo.c | 121 ------------------------------------------------ cli-session.c | 1 - common-algo.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++ common-kex.c | 16 +++---- session.h | 7 --- svr-algo.c | 124 -------------------------------------------------- svr-session.c | 1 - 9 files changed, 128 insertions(+), 267 deletions(-) delete mode 100644 cli-algo.c delete mode 100644 svr-algo.c diff --git a/Makefile.in b/Makefile.in index cec35f1..108566c 100644 --- a/Makefile.in +++ b/Makefile.in @@ -28,12 +28,12 @@ COMMONOBJS=dbutil.o buffer.o \ queue.o \ atomicio.o compat.o fake-rfc2553.o -SVROBJS=svr-kex.o svr-algo.o svr-auth.o sshpty.o \ +SVROBJS=svr-kex.o svr-auth.o sshpty.o \ svr-authpasswd.o svr-authpubkey.o svr-authpubkeyoptions.o svr-session.o svr-service.o \ svr-chansession.o svr-runopts.o svr-agentfwd.o svr-main.o svr-x11fwd.o\ svr-tcpfwd.o svr-authpam.o -CLIOBJS=cli-algo.o cli-main.o cli-auth.o cli-authpasswd.o cli-kex.o \ +CLIOBJS=cli-main.o cli-auth.o cli-authpasswd.o cli-kex.o \ cli-session.o cli-service.o cli-runopts.o cli-chansession.o \ cli-authpubkey.o cli-tcpfwd.o cli-channel.o cli-authinteract.o \ cli-agentfwd.o list.o diff --git a/algo.h b/algo.h index 755cfcd..ad40c0d 100644 --- a/algo.h +++ b/algo.h @@ -93,9 +93,7 @@ enum kexguess2_used { #define KEXGUESS2_ALGO_ID 99 -algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], - enum kexguess2_used *kexguess2, int *goodguess); -algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], +algo_type * buf_match_algo(buffer* buf, algo_type localalgos[], enum kexguess2_used *kexguess2, int *goodguess); #ifdef ENABLE_USER_ALGO_LIST diff --git a/cli-algo.c b/cli-algo.c deleted file mode 100644 index ec8c541..0000000 --- a/cli-algo.c +++ /dev/null @@ -1,121 +0,0 @@ -/* - * Dropbear - a SSH2 server - * SSH client implementation - * - * Copyright (c) 2002,2003 Matt Johnston - * Copyright (c) 2004 by Mihnea Stoenescu - * All rights reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. */ - -#include "algo.h" -#include "dbutil.h" - - -/* - * The chosen [encryption | MAC | compression] algorithm to each - * direction MUST be the first algorithm on the client's list - * that is also on the server's list. - */ -algo_type * cli_buf_match_algo(buffer* buf, algo_type localalgos[], - enum kexguess2_used *kexguess2, int *goodguess) { - - unsigned char * algolist = NULL; - unsigned char * remotealgos[MAX_PROPOSED_ALGO]; - unsigned int len; - unsigned int count, i, j; - algo_type * ret = NULL; - - if (goodguess) { - *goodguess = 0; - } - - /* get the comma-separated list from the buffer ie "algo1,algo2,algo3" */ - algolist = buf_getstring(buf, &len); - TRACE(("cli_buf_match_algo: %s", algolist)) - if (len > MAX_PROPOSED_ALGO*(MAX_NAME_LEN+1)) { - goto out; /* just a sanity check, no other use */ - } - - /* remotealgos will contain a list of the strings parsed out */ - /* We will have at least one string (even if it's just "") */ - remotealgos[0] = algolist; - count = 1; - /* Iterate through, replacing ','s with NULs, to split it into - * words. */ - for (i = 0; i < len; i++) { - if (algolist[i] == '\0') { - /* someone is trying something strange */ - goto out; - } - if (algolist[i] == ',') { - algolist[i] = '\0'; - remotealgos[count] = &algolist[i+1]; - count++; - } - if (count >= MAX_PROPOSED_ALGO) { - break; - } - } - - if (kexguess2 && *kexguess2 == KEXGUESS2_LOOK) { - for (i = 0; i < count; i++) - { - if (strcmp(remotealgos[i], KEXGUESS2_ALGO_NAME) == 0) { - *kexguess2 = KEXGUESS2_YES; - break; - } - } - if (*kexguess2 == KEXGUESS2_LOOK) { - *kexguess2 = KEXGUESS2_NO; - } - } - - /* iterate and find the first match */ - - for (j = 0; localalgos[j].name != NULL; j++) { - if (localalgos[j].usable) { - len = strlen(localalgos[j].name); - for (i = 0; i < count; i++) { - if (len == strlen(remotealgos[i]) - && strncmp(localalgos[j].name, - remotealgos[i], len) == 0) { - if (goodguess && kexguess2) { - if (*kexguess2 == KEXGUESS2_YES) { - if (j == 0) { - *goodguess = 1; - } - } else { - if (i == 0 && j == 0) { - *goodguess = 1; - } - } - } - ret = &localalgos[j]; - goto out; - } - } - } - } - -out: - m_free(algolist); - return ret; -} - diff --git a/cli-session.c b/cli-session.c index cc514bb..590bfcc 100644 --- a/cli-session.c +++ b/cli-session.c @@ -148,7 +148,6 @@ static void cli_session_init() { /* For printing "remote host closed" for the user */ ses.remoteclosed = cli_remoteclosed; - ses.buf_match_algo = cli_buf_match_algo; /* packet handlers */ ses.packettypes = cli_packettypes; diff --git a/common-algo.c b/common-algo.c index 46dbe74..8267852 100644 --- a/common-algo.c +++ b/common-algo.c @@ -24,6 +24,7 @@ * SOFTWARE. */ #include "algo.h" +#include "session.h" #include "dbutil.h" /* This file (algo.c) organises the ciphers which can be used, and is used to @@ -308,6 +309,122 @@ void buf_put_algolist(buffer * buf, algo_type localalgos[]) { buf_free(algolist); } +/* match the first algorithm in the comma-separated list in buf which is + * also in localalgos[], or return NULL on failure. + * (*goodguess) is set to 1 if the preferred client/server algos match, + * 0 otherwise. This is used for checking if the kexalgo/hostkeyalgos are + * guessed correctly */ +algo_type * buf_match_algo(buffer* buf, algo_type localalgos[], + enum kexguess2_used *kexguess2, int *goodguess) +{ + + unsigned char * algolist = NULL; + const unsigned char *remotenames[MAX_PROPOSED_ALGO], *localnames[MAX_PROPOSED_ALGO]; + unsigned int len; + unsigned int remotecount, localcount, clicount, servcount, i, j; + algo_type * ret = NULL; + const unsigned char **clinames, **servnames; + + if (goodguess) { + *goodguess = 0; + } + + /* get the comma-separated list from the buffer ie "algo1,algo2,algo3" */ + algolist = buf_getstring(buf, &len); + TRACE(("buf_match_algo: %s", algolist)) + if (len > MAX_PROPOSED_ALGO*(MAX_NAME_LEN+1)) { + goto out; + } + + /* remotenames will contain a list of the strings parsed out */ + /* We will have at least one string (even if it's just "") */ + remotenames[0] = algolist; + remotecount = 1; + for (i = 0; i < len; i++) { + if (algolist[i] == '\0') { + /* someone is trying something strange */ + goto out; + } + if (algolist[i] == ',') { + algolist[i] = '\0'; + remotenames[remotecount] = &algolist[i+1]; + remotecount++; + } + if (remotecount >= MAX_PROPOSED_ALGO) { + break; + } + } + if (kexguess2 && *kexguess2 == KEXGUESS2_LOOK) { + for (i = 0; i < remotecount; i++) + { + if (strcmp(remotenames[i], KEXGUESS2_ALGO_NAME) == 0) { + *kexguess2 = KEXGUESS2_YES; + break; + } + } + if (*kexguess2 == KEXGUESS2_LOOK) { + *kexguess2 = KEXGUESS2_NO; + } + } + + for (i = 0; localalgos[i].name != NULL; i++) { + if (localalgos[i].usable) { + localnames[i] = localalgos[i].name; + } else { + localnames[i] = NULL; + } + } + localcount = i; + + if (IS_DROPBEAR_SERVER) { + clinames = remotenames; + clicount = remotecount; + servnames = localnames; + servcount = localcount; + } else { + clinames = localnames; + clicount = localcount; + servnames = remotenames; + servcount = remotecount; + } + + /* iterate and find the first match */ + for (i = 0; i < clicount; i++) { + for (j = 0; j < servcount; j++) { + if (!(servnames[j] && clinames[i])) { + // unusable algos are NULL + continue; + } + if (strcmp(servnames[j], clinames[i]) == 0) { + /* set if it was a good guess */ + if (goodguess && kexguess2) { + if (*kexguess2 == KEXGUESS2_YES) { + if (i == 0) { + *goodguess = 1; + } + + } else { + if (i == 0 && j == 0) { + *goodguess = 1; + } + } + } + /* set the algo to return */ + if (IS_DROPBEAR_SERVER) { + ret = &localalgos[j]; + } else { + ret = &localalgos[i]; + } + goto out; + } + } + } + +out: + m_free(algolist); + return ret; +} + #ifdef DROPBEAR_NONE_CIPHER void diff --git a/common-kex.c b/common-kex.c index 0640b9f..eb1fd7b 100644 --- a/common-kex.c +++ b/common-kex.c @@ -695,7 +695,7 @@ static void read_kex_algos() { enum kexguess2_used kexguess2 = KEXGUESS2_LOOK; /* kex_algorithms */ - algo = ses.buf_match_algo(ses.payload, sshkex, &kexguess2, &goodguess); + algo = buf_match_algo(ses.payload, sshkex, &kexguess2, &goodguess); allgood &= goodguess; if (algo == NULL || algo->val == KEXGUESS2_ALGO_ID) { erralgo = "kex"; @@ -706,7 +706,7 @@ static void read_kex_algos() { ses.newkeys->algo_kex = algo->val; /* server_host_key_algorithms */ - algo = ses.buf_match_algo(ses.payload, sshhostkey, &kexguess2, &goodguess); + algo = buf_match_algo(ses.payload, sshhostkey, &kexguess2, &goodguess); allgood &= goodguess; if (algo == NULL) { erralgo = "hostkey"; @@ -716,7 +716,7 @@ static void read_kex_algos() { ses.newkeys->algo_hostkey = algo->val; /* encryption_algorithms_client_to_server */ - c2s_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, NULL, NULL); + c2s_cipher_algo = buf_match_algo(ses.payload, sshciphers, NULL, NULL); if (c2s_cipher_algo == NULL) { erralgo = "enc c->s"; goto error; @@ -724,7 +724,7 @@ static void read_kex_algos() { TRACE(("enc c2s is %s", c2s_cipher_algo->name)) /* encryption_algorithms_server_to_client */ - s2c_cipher_algo = ses.buf_match_algo(ses.payload, sshciphers, NULL, NULL); + s2c_cipher_algo = buf_match_algo(ses.payload, sshciphers, NULL, NULL); if (s2c_cipher_algo == NULL) { erralgo = "enc s->c"; goto error; @@ -732,7 +732,7 @@ static void read_kex_algos() { TRACE(("enc s2c is %s", s2c_cipher_algo->name)) /* mac_algorithms_client_to_server */ - c2s_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, NULL, NULL); + c2s_hash_algo = buf_match_algo(ses.payload, sshhashes, NULL, NULL); if (c2s_hash_algo == NULL) { erralgo = "mac c->s"; goto error; @@ -740,7 +740,7 @@ static void read_kex_algos() { TRACE(("hash c2s is %s", c2s_hash_algo->name)) /* mac_algorithms_server_to_client */ - s2c_hash_algo = ses.buf_match_algo(ses.payload, sshhashes, NULL, NULL); + s2c_hash_algo = buf_match_algo(ses.payload, sshhashes, NULL, NULL); if (s2c_hash_algo == NULL) { erralgo = "mac s->c"; goto error; @@ -748,7 +748,7 @@ static void read_kex_algos() { TRACE(("hash s2c is %s", s2c_hash_algo->name)) /* compression_algorithms_client_to_server */ - c2s_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); + c2s_comp_algo = buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); if (c2s_comp_algo == NULL) { erralgo = "comp c->s"; goto error; @@ -756,7 +756,7 @@ static void read_kex_algos() { TRACE(("hash c2s is %s", c2s_comp_algo->name)) /* compression_algorithms_server_to_client */ - s2c_comp_algo = ses.buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); + s2c_comp_algo = buf_match_algo(ses.payload, ses.compress_algos, NULL, NULL); if (s2c_comp_algo == NULL) { erralgo = "comp s->c"; goto error; diff --git a/session.h b/session.h index 3b9e957..8e71bdc 100644 --- a/session.h +++ b/session.h @@ -169,13 +169,6 @@ struct sshsession { concluded (ie, while dataallowed was unset)*/ struct packetlist *reply_queue_head, *reply_queue_tail; - algo_type*(*buf_match_algo)(buffer*buf, algo_type localalgos[], - enum kexguess2_used *kexguess2, - int *goodguess); /* The function to use to choose which algorithm - to use from the ones presented by the remote - side. Is specific to the client/server mode, - hence the function-pointer callback.*/ - void(*remoteclosed)(); /* A callback to handle closure of the remote connection */ diff --git a/svr-algo.c b/svr-algo.c deleted file mode 100644 index 620cfeb..0000000 --- a/svr-algo.c +++ /dev/null @@ -1,124 +0,0 @@ -/* - * Dropbear - a SSH2 server - * SSH client implementation - * - * Copyright (c) 2002,2003 Matt Johnston - * Copyright (c) 2004 by Mihnea Stoenescu - * All rights reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. */ - -#include "algo.h" -#include "dbutil.h" - -/* match the first algorithm in the comma-separated list in buf which is - * also in localalgos[], or return NULL on failure. - * (*goodguess) is set to 1 if the preferred client/server algos match, - * 0 otherwise. This is used for checking if the kexalgo/hostkeyalgos are - * guessed correctly */ -algo_type * svr_buf_match_algo(buffer* buf, algo_type localalgos[], - enum kexguess2_used *kexguess2, int *goodguess) -{ - - unsigned char * algolist = NULL; - unsigned char * remotealgos[MAX_PROPOSED_ALGO]; - unsigned int len; - unsigned int count, i, j; - algo_type * ret = NULL; - - if (goodguess) { - *goodguess = 0; - } - - /* get the comma-separated list from the buffer ie "algo1,algo2,algo3" */ - algolist = buf_getstring(buf, &len); - /* Debug this */ - TRACE(("buf_match_algo: %s", algolist)) - if (len > MAX_PROPOSED_ALGO*(MAX_NAME_LEN+1)) { - goto out; /* just a sanity check, no other use */ - } - - /* remotealgos will contain a list of the strings parsed out */ - /* We will have at least one string (even if it's just "") */ - remotealgos[0] = algolist; - count = 1; - /* Iterate through, replacing ','s with NULs, to split it into - * words. */ - for (i = 0; i < len; i++) { - if (algolist[i] == '\0') { - /* someone is trying something strange */ - goto out; - } - if (algolist[i] == ',') { - algolist[i] = '\0'; - remotealgos[count] = &algolist[i+1]; - count++; - } - if (count >= MAX_PROPOSED_ALGO) { - break; - } - } - - if (kexguess2 && *kexguess2 == KEXGUESS2_LOOK) { - for (i = 0; i < count; i++) - { - if (strcmp(remotealgos[i], KEXGUESS2_ALGO_NAME) == 0) { - *kexguess2 = KEXGUESS2_YES; - break; - } - } - if (*kexguess2 == KEXGUESS2_LOOK) { - *kexguess2 = KEXGUESS2_NO; - } - } - - /* iterate and find the first match */ - for (i = 0; i < count; i++) { - - len = strlen(remotealgos[i]); - - for (j = 0; localalgos[j].name != NULL; j++) { - if (localalgos[j].usable) { - if (len == strlen(localalgos[j].name) && - strncmp(localalgos[j].name, remotealgos[i], len) == 0) { - /* set if it was a good guess */ - if (goodguess && kexguess2) { - if (*kexguess2 == KEXGUESS2_YES) { - if (i == 0) { - *goodguess = 1; - } - - } else { - if (i == 0 && j == 0) { - *goodguess = 1; - } - } - } - /* set the algo to return */ - ret = &localalgos[j]; - goto out; - } - } - } - } - -out: - m_free(algolist); - return ret; -} diff --git a/svr-session.c b/svr-session.c index cf82289..a564525 100644 --- a/svr-session.c +++ b/svr-session.c @@ -106,7 +106,6 @@ void svr_session(int sock, int childpipe) { /* packet handlers */ ses.packettypes = svr_packettypes; - ses.buf_match_algo = svr_buf_match_algo; ses.isserver = 1; From a0e931005b6f9de407ba2ac5b5df9e34d4a7fb7e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 31 Mar 2013 00:40:00 +0800 Subject: [PATCH 10/33] send out our kexinit packet before blocking to read the SSH version string --- cli-session.c | 2 +- common-session.c | 29 +++++++++++++++++++---------- session.h | 7 +++++-- svr-session.c | 2 +- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/cli-session.c b/cli-session.c index e58fdbd..f862bc8 100644 --- a/cli-session.c +++ b/cli-session.c @@ -99,7 +99,7 @@ void cli_session(int sock_in, int sock_out) { sessinitdone = 1; /* Exchange identification */ - session_identification(); + send_session_identification(); send_msg_kexinit(); diff --git a/common-session.c b/common-session.c index f4fa579..ec5c9ed 100644 --- a/common-session.c +++ b/common-session.c @@ -39,6 +39,7 @@ static void checktimeouts(); static long select_timeout(); static int ident_readln(int fd, char* buf, int count); +static void read_session_identification(); struct sshsession ses; /* GLOBAL */ @@ -141,7 +142,10 @@ void session_loop(void(*loophandler)()) { FD_ZERO(&writefd); FD_ZERO(&readfd); dropbear_assert(ses.payload == NULL); - if (ses.sock_in != -1) { + + /* during initial setup we flush out the KEXINIT packet before + * attempting to read the remote version string, which might block */ + if (ses.sock_in != -1 && (ses.remoteident || isempty(&ses.writequeue))) { FD_SET(ses.sock_in, &readfd); } if (ses.sock_out != -1 && !isempty(&ses.writequeue)) { @@ -195,7 +199,12 @@ void session_loop(void(*loophandler)()) { if (ses.sock_in != -1) { if (FD_ISSET(ses.sock_in, &readfd)) { - read_packet(); + if (!ses.remoteident) { + /* blocking read of the version string */ + read_session_identification(); + } else { + read_packet(); + } } /* Process the decrypted packet. After this, the read buffer @@ -245,20 +254,20 @@ void common_session_cleanup() { } -void session_identification() { - - /* max length of 255 chars */ - char linebuf[256]; - int len = 0; - char done = 0; - int i; - +void send_session_identification() { /* write our version string, this blocks */ if (atomicio(write, ses.sock_out, LOCAL_IDENT "\r\n", strlen(LOCAL_IDENT "\r\n")) == DROPBEAR_FAILURE) { ses.remoteclosed(); } +} +static void read_session_identification() { + /* max length of 255 chars */ + char linebuf[256]; + int len = 0; + char done = 0; + int i; /* If they send more than 50 lines, something is wrong */ for (i = 0; i < 50; i++) { len = ident_readln(ses.sock_in, linebuf, sizeof(linebuf)); diff --git a/session.h b/session.h index 0719e34..d524226 100644 --- a/session.h +++ b/session.h @@ -45,7 +45,7 @@ extern int exitflag; void common_session_init(int sock_in, int sock_out); void session_loop(void(*loophandler)()); void common_session_cleanup(); -void session_identification(); +void send_session_identification(); void send_msg_ignore(); const char* get_user_shell(); @@ -111,7 +111,10 @@ struct sshsession { int sock_in; int sock_out; - unsigned char *remoteident; + /* remotehost will be initially NULL as we delay + * reading the remote version string. it will be set + * by the time any recv_() packet methods are called */ + unsigned char *remoteident; int maxfd; /* the maximum file descriptor to check with select() */ diff --git a/svr-session.c b/svr-session.c index cf82289..7234f4a 100644 --- a/svr-session.c +++ b/svr-session.c @@ -114,7 +114,7 @@ void svr_session(int sock, int childpipe) { sessinitdone = 1; /* exchange identification, version etc */ - session_identification(); + send_session_identification(); /* start off with key exchange */ send_msg_kexinit(); From 36526700a983e09e77f03e404f207d6fbc220306 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 31 Mar 2013 21:38:17 +0800 Subject: [PATCH 11/33] Don't bother waiting for a ssh-connection service reply - the server will disconnect if it wasn't accepted --- Makefile.in | 2 +- cli-service.c | 85 --------------------------------------------------- cli-session.c | 24 +++++++++++---- service.h | 2 -- session.h | 4 --- 5 files changed, 19 insertions(+), 98 deletions(-) delete mode 100644 cli-service.c diff --git a/Makefile.in b/Makefile.in index cec35f1..8e1ba52 100644 --- a/Makefile.in +++ b/Makefile.in @@ -34,7 +34,7 @@ SVROBJS=svr-kex.o svr-algo.o svr-auth.o sshpty.o \ svr-tcpfwd.o svr-authpam.o CLIOBJS=cli-algo.o cli-main.o cli-auth.o cli-authpasswd.o cli-kex.o \ - cli-session.o cli-service.o cli-runopts.o cli-chansession.o \ + cli-session.o cli-runopts.o cli-chansession.o \ cli-authpubkey.o cli-tcpfwd.o cli-channel.o cli-authinteract.o \ cli-agentfwd.o list.o diff --git a/cli-service.c b/cli-service.c deleted file mode 100644 index f763103..0000000 --- a/cli-service.c +++ /dev/null @@ -1,85 +0,0 @@ -/* - * Dropbear SSH - * - * Copyright (c) 2002,2003 Matt Johnston - * Copyright (c) 2004 by Mihnea Stoenescu - * All rights reserved. - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE - * SOFTWARE. */ - -#include "includes.h" -#include "service.h" -#include "dbutil.h" -#include "packet.h" -#include "buffer.h" -#include "session.h" -#include "ssh.h" - -void send_msg_service_request(char* servicename) { - - TRACE(("enter send_msg_service_request: servicename='%s'", servicename)) - - CHECKCLEARTOWRITE(); - - buf_putbyte(ses.writepayload, SSH_MSG_SERVICE_REQUEST); - buf_putstring(ses.writepayload, servicename, strlen(servicename)); - - encrypt_packet(); - TRACE(("leave send_msg_service_request")) -} - -/* This just sets up the state variables right for the main client session loop - * to deal with */ -void recv_msg_service_accept() { - - unsigned char* servicename; - unsigned int len; - - TRACE(("enter recv_msg_service_accept")) - - servicename = buf_getstring(ses.payload, &len); - - /* ssh-userauth */ - if (cli_ses.state == SERVICE_AUTH_REQ_SENT - && len == SSH_SERVICE_USERAUTH_LEN - && strncmp(SSH_SERVICE_USERAUTH, servicename, len) == 0) { - - cli_ses.state = SERVICE_AUTH_ACCEPT_RCVD; - m_free(servicename); - TRACE(("leave recv_msg_service_accept: done ssh-userauth")) - return; - } - - /* ssh-connection */ - if (cli_ses.state == SERVICE_CONN_REQ_SENT - && len == SSH_SERVICE_CONNECTION_LEN - && strncmp(SSH_SERVICE_CONNECTION, servicename, len) == 0) { - - if (ses.authstate.authdone != 1) { - dropbear_exit("Request for connection before auth"); - } - - cli_ses.state = SERVICE_CONN_ACCEPT_RCVD; - m_free(servicename); - TRACE(("leave recv_msg_service_accept: done ssh-connection")) - return; - } - - dropbear_exit("Unrecognised service accept"); -} diff --git a/cli-session.c b/cli-session.c index f862bc8..81aa8c9 100644 --- a/cli-session.c +++ b/cli-session.c @@ -41,6 +41,7 @@ static void cli_remoteclosed(); static void cli_sessionloop(); static void cli_session_init(); static void cli_finished(); +static void recv_msg_service_accept(void); struct clientsession cli_ses; /* GLOBAL */ @@ -150,6 +151,23 @@ static void cli_session_init() { ses.isserver = 0; } +static void send_msg_service_request(char* servicename) { + + TRACE(("enter send_msg_service_request: servicename='%s'", servicename)) + + CHECKCLEARTOWRITE(); + + buf_putbyte(ses.writepayload, SSH_MSG_SERVICE_REQUEST); + buf_putstring(ses.writepayload, servicename, strlen(servicename)); + + encrypt_packet(); + TRACE(("leave send_msg_service_request")) +} + +static void recv_msg_service_accept(void) { + // do nothing, if it failed then the server MUST have disconnected +} + /* This function drives the progress of the session - it initiates KEX, * service, userauth and channel requests */ static void cli_sessionloop() { @@ -195,12 +213,6 @@ static void cli_sessionloop() { /* We've got the transport layer sorted, we now need to request * userauth */ send_msg_service_request(SSH_SERVICE_USERAUTH); - cli_ses.state = SERVICE_AUTH_REQ_SENT; - TRACE(("leave cli_sessionloop: sent userauth service req")) - return; - - /* userauth code */ - case SERVICE_AUTH_ACCEPT_RCVD: cli_auth_getmethods(); cli_ses.state = USERAUTH_REQ_SENT; TRACE(("leave cli_sessionloop: sent userauth methods req")) diff --git a/service.h b/service.h index 197d8d1..9c60c09 100644 --- a/service.h +++ b/service.h @@ -26,7 +26,5 @@ #define _SERVICE_H_ void recv_msg_service_request(); /* Server */ -void send_msg_service_request(); /* Client */ -void recv_msg_service_accept(); /* Client */ #endif /* _SERVICE_H_ */ diff --git a/session.h b/session.h index d524226..39104a3 100644 --- a/session.h +++ b/session.h @@ -236,10 +236,6 @@ typedef enum { typedef enum { STATE_NOTHING, - SERVICE_AUTH_REQ_SENT, - SERVICE_AUTH_ACCEPT_RCVD, - SERVICE_CONN_REQ_SENT, - SERVICE_CONN_ACCEPT_RCVD, USERAUTH_REQ_SENT, USERAUTH_FAIL_RCVD, USERAUTH_SUCCESS_RCVD, From f6b304250bda59f7dcc56ffa7c4c5f482e66d32f Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 31 Mar 2013 23:15:35 +0800 Subject: [PATCH 12/33] Try using writev() for writing packets out to tcp --- configure.ac | 4 ++-- includes.h | 4 ++++ packet.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++------ queue.h | 2 +- 4 files changed, 66 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index 3b82bb7..05461f3 100644 --- a/configure.ac +++ b/configure.ac @@ -211,7 +211,7 @@ AC_ARG_ENABLE(shadow, # Checks for header files. AC_HEADER_STDC AC_HEADER_SYS_WAIT -AC_CHECK_HEADERS([fcntl.h limits.h netinet/in.h netinet/tcp.h stdlib.h string.h sys/socket.h sys/time.h termios.h unistd.h crypt.h pty.h ioctl.h libutil.h libgen.h inttypes.h stropts.h utmp.h utmpx.h lastlog.h paths.h util.h netdb.h security/pam_appl.h pam/pam_appl.h netinet/in_systm.h]) +AC_CHECK_HEADERS([fcntl.h limits.h netinet/in.h netinet/tcp.h stdlib.h string.h sys/socket.h sys/time.h termios.h unistd.h crypt.h pty.h ioctl.h libutil.h libgen.h inttypes.h stropts.h utmp.h utmpx.h lastlog.h paths.h util.h netdb.h security/pam_appl.h pam/pam_appl.h netinet/in_systm.h, sys/uio.h]) # Checks for typedefs, structures, and compiler characteristics. AC_C_CONST @@ -616,7 +616,7 @@ AC_PROG_GCC_TRADITIONAL AC_FUNC_MEMCMP AC_FUNC_SELECT_ARGTYPES AC_TYPE_SIGNAL -AC_CHECK_FUNCS([dup2 getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork]) +AC_CHECK_FUNCS([dup2 getspnam getusershell memset putenv select socket strdup clearenv strlcpy strlcat daemon basename _getpty getaddrinfo freeaddrinfo getnameinfo fork writev]) AC_SEARCH_LIBS(basename, gen, AC_DEFINE(HAVE_BASENAME)) diff --git a/includes.h b/includes.h index 571a3be..203ccfd 100644 --- a/includes.h +++ b/includes.h @@ -120,6 +120,10 @@ #include #endif +#ifdef HAVE_SYS_UIO_H +#include +#endif + #ifdef BUNDLED_LIBTOM #include "libtomcrypt/src/headers/tomcrypt.h" #include "libtommath/tommath.h" diff --git a/packet.c b/packet.c index f979cae..5cd60c2 100644 --- a/packet.c +++ b/packet.c @@ -55,10 +55,60 @@ void write_packet() { buffer * writebuf = NULL; time_t now; unsigned packet_type; + int all_ignore = 1; +#ifdef HAVE_WRITEV + struct iovec *iov = NULL; + int i; + struct Link *l; +#endif TRACE(("enter write_packet")) dropbear_assert(!isempty(&ses.writequeue)); +#ifdef HAVE_WRITEV + iov = m_malloc(sizeof(*iov) * ses.writequeue.count); + for (l = ses.writequeue.head, i = 0; l; l = l->link, i++) + { + writebuf = (buffer*)l->item; + packet_type = writebuf->data[writebuf->len-1]; + len = writebuf->len - 1 - writebuf->pos; + dropbear_assert(len > 0); + all_ignore &= (packet_type == SSH_MSG_IGNORE); + iov[i].iov_base = buf_getptr(writebuf, len); + iov[i].iov_len = len; + } + written = writev(ses.sock_out, iov, ses.writequeue.count); + if (written < 0) { + if (errno == EINTR) { + m_free(iov); + TRACE(("leave writepacket: EINTR")) + return; + } else { + dropbear_exit("Error writing"); + } + } + + if (written == 0) { + ses.remoteclosed(); + } + + while (written > 0) { + writebuf = (buffer*)examine(&ses.writequeue); + len = writebuf->len - 1 - writebuf->pos; + if (len > written) { + // partial buffer write + buf_incrpos(writebuf, written); + written = 0; + } else { + written -= len; + dequeue(&ses.writequeue); + buf_free(writebuf); + } + } + + m_free(iov); + +#else /* Get the next buffer in the queue of encrypted packets to write*/ writebuf = (buffer*)examine(&ses.writequeue); @@ -78,13 +128,7 @@ void write_packet() { dropbear_exit("Error writing"); } } - - now = time(NULL); - ses.last_trx_packet_time = now; - - if (packet_type != SSH_MSG_IGNORE) { - ses.last_packet_time = now; - } + all_ignore = (packet_type == SSH_MSG_IGNORE); if (written == 0) { ses.remoteclosed(); @@ -100,6 +144,14 @@ void write_packet() { buf_incrpos(writebuf, written); } +#endif + now = time(NULL); + ses.last_trx_packet_time = now; + + if (!all_ignore) { + ses.last_packet_time = now; + } + TRACE(("leave write_packet")) } diff --git a/queue.h b/queue.h index 80fbb9d..8cffab7 100644 --- a/queue.h +++ b/queue.h @@ -36,7 +36,7 @@ struct Queue { struct Link* head; struct Link* tail; - unsigned int count; /* safety value */ + unsigned int count; }; From 5abe22d1a514d1f98f88927a23701b51bd7c134e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 31 Mar 2013 23:29:03 +0800 Subject: [PATCH 13/33] Fix incorrect logic for USE_VFORK and calling arg_setup() --- scp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scp.c b/scp.c index 8715726..114d095 100644 --- a/scp.c +++ b/scp.c @@ -230,7 +230,7 @@ do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int argc) close(pin[0]); close(pout[1]); -#ifdef USE_VFORK +#ifndef USE_VFORK arg_setup(host, remuser, cmd); #endif From 484516da516037d37747319e9b7cff91bffb6a4a Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 31 Mar 2013 23:48:25 +0800 Subject: [PATCH 14/33] Send an auth packet straight away, save another roundtrip This needs a bit of testing to make sure it doesn't have side-effects. --- cli-auth.c | 9 +++++---- sysoptions.h | 3 +++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cli-auth.c b/cli-auth.c index 321cbf3..7dc101f 100644 --- a/cli-auth.c +++ b/cli-auth.c @@ -40,11 +40,12 @@ void cli_authinitialise() { /* Send a "none" auth request to get available methods */ void cli_auth_getmethods() { - TRACE(("enter cli_auth_getmethods")) - +#ifdef CLI_IMMEDIATE_AUTH + ses.authstate.authtypes = AUTH_TYPE_PUBKEY | AUTH_TYPE_PASSWORD | AUTH_TYPE_INTERACT; + cli_auth_try(); +#else CHECKCLEARTOWRITE(); - buf_putbyte(ses.writepayload, SSH_MSG_USERAUTH_REQUEST); buf_putstring(ses.writepayload, cli_opts.username, strlen(cli_opts.username)); @@ -53,8 +54,8 @@ void cli_auth_getmethods() { buf_putstring(ses.writepayload, "none", 4); /* 'none' method */ encrypt_packet(); +#endif TRACE(("leave cli_auth_getmethods")) - } void recv_msg_userauth_banner() { diff --git a/sysoptions.h b/sysoptions.h index 8c591ea..6e60294 100644 --- a/sysoptions.h +++ b/sysoptions.h @@ -188,6 +188,9 @@ #define DROPBEAR_KEY_LINES /* ie we're using authorized_keys or known_hosts */ #endif +/* Send an auth request straight away rather than trying "none" type to get a list */ +#define CLI_IMMEDIATE_AUTH + /* Changing this is inadvisable, it appears to have problems * with flushing compressed data */ #define DROPBEAR_ZLIB_MEM_LEVEL 8 From 90cf7f012cdc6143752464bc9bb2b4a9f94f7132 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 1 Apr 2013 00:07:26 +0800 Subject: [PATCH 15/33] Move the more verbose TRACE() statements into TRACE2() --- buffer.c | 4 ++-- cli-kex.c | 2 +- cli-session.c | 4 ++-- common-channel.c | 4 ++-- dbutil.c | 20 +++++++++++++++++--- dbutil.h | 1 + debug.h | 4 +++- dss.c | 6 +++--- packet.c | 40 +++++++++++++++++++--------------------- process-packet.c | 4 ++-- queue.c | 2 -- rsa.c | 6 +++--- signkey.c | 18 +++++++++--------- 13 files changed, 64 insertions(+), 51 deletions(-) diff --git a/buffer.c b/buffer.c index 13fa1ce..facee24 100644 --- a/buffer.c +++ b/buffer.c @@ -282,7 +282,7 @@ void buf_putbytes(buffer *buf, const unsigned char *bytes, unsigned int len) { void buf_putmpint(buffer* buf, mp_int * mp) { unsigned int len, pad = 0; - TRACE(("enter buf_putmpint")) + TRACE2(("enter buf_putmpint")) dropbear_assert(mp != NULL); @@ -318,7 +318,7 @@ void buf_putmpint(buffer* buf, mp_int * mp) { buf_incrwritepos(buf, len-pad); } - TRACE(("leave buf_putmpint")) + TRACE2(("leave buf_putmpint")) } /* Retrieve an mp_int from the buffer. diff --git a/cli-kex.c b/cli-kex.c index 9dadb3c..e039071 100644 --- a/cli-kex.c +++ b/cli-kex.c @@ -246,7 +246,7 @@ static void checkhostkey(unsigned char* keyblob, unsigned int keybloblen) { /* Compare hostnames */ if (strncmp(cli_opts.remotehost, buf_getptr(line, hostlen), hostlen) != 0) { - TRACE(("hosts don't match")) + TRACE2(("hosts don't match")) continue; } diff --git a/cli-session.c b/cli-session.c index 81aa8c9..3adec73 100644 --- a/cli-session.c +++ b/cli-session.c @@ -172,7 +172,7 @@ static void recv_msg_service_accept(void) { * service, userauth and channel requests */ static void cli_sessionloop() { - TRACE(("enter cli_sessionloop")) + TRACE2(("enter cli_sessionloop")) if (ses.lastpacket == SSH_MSG_KEXINIT && cli_ses.kex_state == KEX_NOTHING) { cli_ses.kex_state = KEXINIT_RCVD; @@ -286,7 +286,7 @@ static void cli_sessionloop() { break; } - TRACE(("leave cli_sessionloop: fell out")) + TRACE2(("leave cli_sessionloop: fell out")) } diff --git a/common-channel.c b/common-channel.c index 05b9d11..331ea60 100644 --- a/common-channel.c +++ b/common-channel.c @@ -273,10 +273,10 @@ static unsigned int write_pending(struct Channel * channel) { static void check_close(struct Channel *channel) { int close_allowed = 0; - TRACE(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d", + TRACE2(("check_close: writefd %d, readfd %d, errfd %d, sent_close %d, recv_close %d", channel->writefd, channel->readfd, channel->errfd, channel->sent_close, channel->recv_close)) - TRACE(("writebuf size %d extrabuf size %d", + TRACE2(("writebuf size %d extrabuf size %d", channel->writebuf ? cbuf_getused(channel->writebuf) : 0, channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0)) diff --git a/dbutil.c b/dbutil.c index 044388a..8c48a24 100644 --- a/dbutil.c +++ b/dbutil.c @@ -151,6 +151,20 @@ void dropbear_trace(const char* format, ...) { fprintf(stderr, "\n"); va_end(param); } +void dropbear_trace2(const char* format, ...) { + + va_list param; + + if (!(debug_trace && getenv("DROPBEAR_TRACE2"))) { + return; + } + + va_start(param, format); + fprintf(stderr, "TRACE2 (%d): ", getpid()); + vfprintf(stderr, format, param); + fprintf(stderr, "\n"); + va_end(param); +} #endif /* DEBUG_TRACE */ static void set_sock_priority(int sock) { @@ -725,7 +739,7 @@ int buf_getline(buffer * line, FILE * authfile) { int c = EOF; - TRACE(("enter buf_getline")) + TRACE2(("enter buf_getline")) buf_setpos(line, 0); buf_setlen(line, 0); @@ -750,10 +764,10 @@ out: /* if we didn't read anything before EOF or error, exit */ if (c == EOF && line->pos == 0) { - TRACE(("leave buf_getline: failure")) + TRACE2(("leave buf_getline: failure")) return DROPBEAR_FAILURE; } else { - TRACE(("leave buf_getline: success")) + TRACE2(("leave buf_getline: success")) buf_setpos(line, 0); return DROPBEAR_SUCCESS; } diff --git a/dbutil.h b/dbutil.h index 0f16bf3..fc01251 100644 --- a/dbutil.h +++ b/dbutil.h @@ -57,6 +57,7 @@ void fail_assert(const char* expr, const char* file, int line) ATTRIB_NORETURN; #ifdef DEBUG_TRACE void dropbear_trace(const char* format, ...) ATTRIB_PRINTF(1,2); +void dropbear_trace2(const char* format, ...) ATTRIB_PRINTF(1,2); void printhex(const char * label, const unsigned char * buf, int len); extern int debug_trace; #endif diff --git a/debug.h b/debug.h index b20e685..289c577 100644 --- a/debug.h +++ b/debug.h @@ -39,7 +39,7 @@ * Caution: Don't use this in an unfriendly environment (ie unfirewalled), * since the printing may not sanitise strings etc. This will add a reasonable * amount to your executable size. */ -/*#define DEBUG_TRACE */ +/* #define DEBUG_TRACE */ /* All functions writing to the cleartext payload buffer call * CHECKCLEARTOWRITE() before writing. This is only really useful if you're @@ -63,8 +63,10 @@ /* you don't need to touch this block */ #ifdef DEBUG_TRACE #define TRACE(X) dropbear_trace X; +#define TRACE2(X) dropbear_trace2 X; #else /*DEBUG_TRACE*/ #define TRACE(X) +#define TRACE2(X) #endif /*DEBUG_TRACE*/ /* To debug with GDB it is easier to run with no forking of child processes. diff --git a/dss.c b/dss.c index d984669..75dc0d0 100644 --- a/dss.c +++ b/dss.c @@ -101,9 +101,9 @@ int buf_get_dss_priv_key(buffer* buf, dropbear_dss_key *key) { /* Clear and free the memory used by a public or private key */ void dss_key_free(dropbear_dss_key *key) { - TRACE(("enter dsa_key_free")) + TRACE2(("enter dsa_key_free")) if (key == NULL) { - TRACE(("enter dsa_key_free: key == NULL")) + TRACE2(("enter dsa_key_free: key == NULL")) return; } if (key->p) { @@ -127,7 +127,7 @@ void dss_key_free(dropbear_dss_key *key) { m_free(key->x); } m_free(key); - TRACE(("leave dsa_key_free")) + TRACE2(("leave dsa_key_free")) } /* put the dss public key into the buffer in the required format: diff --git a/packet.c b/packet.c index 5cd60c2..4a3a53a 100644 --- a/packet.c +++ b/packet.c @@ -62,7 +62,7 @@ void write_packet() { struct Link *l; #endif - TRACE(("enter write_packet")) + TRACE2(("enter write_packet")) dropbear_assert(!isempty(&ses.writequeue)); #ifdef HAVE_WRITEV @@ -81,7 +81,7 @@ void write_packet() { if (written < 0) { if (errno == EINTR) { m_free(iov); - TRACE(("leave writepacket: EINTR")) + TRACE2(("leave writepacket: EINTR")) return; } else { dropbear_exit("Error writing"); @@ -122,7 +122,7 @@ void write_packet() { if (written < 0) { if (errno == EINTR) { - TRACE(("leave writepacket: EINTR")) + TRACE2(("leave writepacket: EINTR")) return; } else { dropbear_exit("Error writing"); @@ -152,7 +152,7 @@ void write_packet() { ses.last_packet_time = now; } - TRACE(("leave write_packet")) + TRACE2(("leave write_packet")) } /* Non-blocking function reading available portion of a packet into the @@ -164,7 +164,7 @@ void read_packet() { unsigned int maxlen; unsigned char blocksize; - TRACE(("enter read_packet")) + TRACE2(("enter read_packet")) blocksize = ses.keys->recv.algo_crypt->blocksize; if (ses.readbuf == NULL || ses.readbuf->len < blocksize) { @@ -177,7 +177,7 @@ void read_packet() { if (ret == DROPBEAR_FAILURE) { /* didn't read enough to determine the length */ - TRACE(("leave read_packet: packetinit done")) + TRACE2(("leave read_packet: packetinit done")) return; } } @@ -199,7 +199,7 @@ void read_packet() { if (len < 0) { if (errno == EINTR || errno == EAGAIN) { - TRACE(("leave read_packet: EINTR or EAGAIN")) + TRACE2(("leave read_packet: EINTR or EAGAIN")) return; } else { dropbear_exit("Error reading: %s", strerror(errno)); @@ -215,7 +215,7 @@ void read_packet() { /* The main select() loop process_packet() to * handle the packet contents... */ } - TRACE(("leave read_packet")) + TRACE2(("leave read_packet")) } /* Function used to read the initial portion of a packet, and determine the @@ -249,7 +249,7 @@ static int read_packet_init() { } if (slen < 0) { if (errno == EINTR) { - TRACE(("leave read_packet_init: EINTR")) + TRACE2(("leave read_packet_init: EINTR")) return DROPBEAR_FAILURE; } dropbear_exit("Error reading: %s", strerror(errno)); @@ -273,7 +273,7 @@ static int read_packet_init() { } len = buf_getint(ses.readbuf) + 4 + macsize; - TRACE(("packet size is %d, block %d mac %d", len, blocksize, macsize)) + TRACE2(("packet size is %d, block %d mac %d", len, blocksize, macsize)) /* check packet length */ @@ -299,7 +299,7 @@ void decrypt_packet() { unsigned int padlen; unsigned int len; - TRACE(("enter decrypt_packet")) + TRACE2(("enter decrypt_packet")) blocksize = ses.keys->recv.algo_crypt->blocksize; macsize = ses.keys->recv.algo_mac->hashsize; @@ -356,7 +356,7 @@ void decrypt_packet() { ses.recvseq++; - TRACE(("leave decrypt_packet")) + TRACE2(("leave decrypt_packet")) } /* Checks the mac at the end of a decrypted readbuf. @@ -455,7 +455,7 @@ static void enqueue_reply_packet() { ses.reply_queue_head = new_item; } ses.reply_queue_tail = new_item; - TRACE(("leave enqueue_reply_packet")) + TRACE2(("leave enqueue_reply_packet")) } void maybe_flush_reply_queue() { @@ -492,13 +492,13 @@ void encrypt_packet() { unsigned int len, encrypt_buf_size; unsigned char mac_bytes[MAX_MAC_LEN]; - TRACE(("enter encrypt_packet()")) + TRACE2(("enter encrypt_packet()")) buf_setpos(ses.writepayload, 0); packet_type = buf_getbyte(ses.writepayload); buf_setpos(ses.writepayload, 0); - TRACE(("encrypt_packet type is %d", packet_type)) + TRACE2(("encrypt_packet type is %d", packet_type)) if ((!ses.dataallowed && !packet_is_okay_kex(packet_type)) || ses.kexstate.sentnewkeys) { @@ -611,7 +611,7 @@ void encrypt_packet() { ses.kexstate.datatrans += writebuf->len; ses.transseq++; - TRACE(("leave encrypt_packet()")) + TRACE2(("leave encrypt_packet()")) } @@ -624,8 +624,6 @@ static void make_mac(unsigned int seqno, const struct key_context_directional * unsigned long bufsize; hmac_state hmac; - TRACE(("enter writemac")) - if (key_state->algo_mac->hashsize > 0) { /* calculate the mac */ if (hmac_init(&hmac, @@ -654,7 +652,7 @@ static void make_mac(unsigned int seqno, const struct key_context_directional * dropbear_exit("HMAC error"); } } - TRACE(("leave writemac")) + TRACE2(("leave writemac")) } #ifndef DISABLE_ZLIB @@ -665,7 +663,7 @@ static void buf_compress(buffer * dest, buffer * src, unsigned int len) { unsigned int endpos = src->pos + len; int result; - TRACE(("enter buf_compress")) + TRACE2(("enter buf_compress")) while (1) { @@ -699,6 +697,6 @@ static void buf_compress(buffer * dest, buffer * src, unsigned int len) { buf_resize(dest, dest->size + ZLIB_COMPRESS_INCR); } - TRACE(("leave buf_compress")) + TRACE2(("leave buf_compress")) } #endif diff --git a/process-packet.c b/process-packet.c index 2ae410d..384e449 100644 --- a/process-packet.c +++ b/process-packet.c @@ -45,7 +45,7 @@ void process_packet() { unsigned char type; unsigned int i; - TRACE(("enter process_packet")) + TRACE2(("enter process_packet")) type = buf_getbyte(ses.payload); TRACE(("process_packet: packet type = %d", type)) @@ -123,7 +123,7 @@ out: buf_free(ses.payload); ses.payload = NULL; - TRACE(("leave process_packet")) + TRACE2(("leave process_packet")) } diff --git a/queue.c b/queue.c index 7a80124..9d00808 100644 --- a/queue.c +++ b/queue.c @@ -70,7 +70,6 @@ void enqueue(struct Queue* queue, void* item) { struct Link* newlink; - TRACE(("enter enqueue")) newlink = (struct Link*)m_malloc(sizeof(struct Link)); newlink->item = item; @@ -85,5 +84,4 @@ void enqueue(struct Queue* queue, void* item) { queue->head = newlink; } queue->count++; - TRACE(("leave enqueue")) } diff --git a/rsa.c b/rsa.c index 91bf59d..520ad84 100644 --- a/rsa.c +++ b/rsa.c @@ -139,10 +139,10 @@ out: /* Clear and free the memory used by a public or private key */ void rsa_key_free(dropbear_rsa_key *key) { - TRACE(("enter rsa_key_free")) + TRACE2(("enter rsa_key_free")) if (key == NULL) { - TRACE(("leave rsa_key_free: key == NULL")) + TRACE2(("leave rsa_key_free: key == NULL")) return; } if (key->d) { @@ -166,7 +166,7 @@ void rsa_key_free(dropbear_rsa_key *key) { m_free(key->q); } m_free(key); - TRACE(("leave rsa_key_free")) + TRACE2(("leave rsa_key_free")) } /* Put the public rsa key into the buffer in the required format: diff --git a/signkey.c b/signkey.c index 1d908f4..f647990 100644 --- a/signkey.c +++ b/signkey.c @@ -98,7 +98,7 @@ int buf_get_pub_key(buffer *buf, sign_key *key, int *type) { int keytype; int ret = DROPBEAR_FAILURE; - TRACE(("enter buf_get_pub_key")) + TRACE2(("enter buf_get_pub_key")) ident = buf_getstring(buf, &len); keytype = signkey_type_from_name(ident, len); @@ -109,7 +109,7 @@ int buf_get_pub_key(buffer *buf, sign_key *key, int *type) { return DROPBEAR_FAILURE; } - TRACE(("buf_get_pub_key keytype is %d", keytype)) + TRACE2(("buf_get_pub_key keytype is %d", keytype)) *type = keytype; @@ -137,7 +137,7 @@ int buf_get_pub_key(buffer *buf, sign_key *key, int *type) { } #endif - TRACE(("leave buf_get_pub_key")) + TRACE2(("leave buf_get_pub_key")) return ret; @@ -153,7 +153,7 @@ int buf_get_priv_key(buffer *buf, sign_key *key, int *type) { int keytype; int ret = DROPBEAR_FAILURE; - TRACE(("enter buf_get_priv_key")) + TRACE2(("enter buf_get_priv_key")) ident = buf_getstring(buf, &len); keytype = signkey_type_from_name(ident, len); @@ -190,7 +190,7 @@ int buf_get_priv_key(buffer *buf, sign_key *key, int *type) { } #endif - TRACE(("leave buf_get_priv_key")) + TRACE2(("leave buf_get_priv_key")) return ret; @@ -201,7 +201,7 @@ void buf_put_pub_key(buffer* buf, sign_key *key, int type) { buffer *pubkeys; - TRACE(("enter buf_put_pub_key")) + TRACE2(("enter buf_put_pub_key")) pubkeys = buf_new(MAX_PUBKEY_SIZE); #ifdef DROPBEAR_DSS @@ -223,7 +223,7 @@ void buf_put_pub_key(buffer* buf, sign_key *key, int type) { pubkeys->len); buf_free(pubkeys); - TRACE(("leave buf_put_pub_key")) + TRACE2(("leave buf_put_pub_key")) } /* type is either DROPBEAR_SIGNKEY_DSS or DROPBEAR_SIGNKEY_RSA */ @@ -251,7 +251,7 @@ void buf_put_priv_key(buffer* buf, sign_key *key, int type) { void sign_key_free(sign_key *key) { - TRACE(("enter sign_key_free")) + TRACE2(("enter sign_key_free")) #ifdef DROPBEAR_DSS dss_key_free(key->dsskey); @@ -265,7 +265,7 @@ void sign_key_free(sign_key *key) { m_free(key->filename); m_free(key); - TRACE(("leave sign_key_free")) + TRACE2(("leave sign_key_free")) } static char hexdig(unsigned char x) { From 5af0d33164261fb9603d1b44f715d63659799373 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 1 Apr 2013 22:26:24 +0800 Subject: [PATCH 16/33] Try password before interactive - bit of a hack --- cli-auth.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cli-auth.c b/cli-auth.c index 7dc101f..42c925b 100644 --- a/cli-auth.c +++ b/cli-auth.c @@ -257,6 +257,16 @@ void cli_auth_try() { } #endif +#ifdef ENABLE_CLI_PASSWORD_AUTH + if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { + fprintf(stderr, "Sorry, I won't let you use password auth unencrypted.\n"); + } else if (!finished && ses.authstate.authtypes & AUTH_TYPE_PASSWORD) { + cli_auth_password(); + finished = 1; + cli_ses.lastauthtype = AUTH_TYPE_PASSWORD; + } +#endif + #ifdef ENABLE_CLI_INTERACT_AUTH if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { fprintf(stderr, "Sorry, I won't let you use interactive auth unencrypted.\n"); @@ -271,16 +281,6 @@ void cli_auth_try() { } #endif -#ifdef ENABLE_CLI_PASSWORD_AUTH - if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { - fprintf(stderr, "Sorry, I won't let you use password auth unencrypted.\n"); - } else if (!finished && ses.authstate.authtypes & AUTH_TYPE_PASSWORD) { - cli_auth_password(); - finished = 1; - cli_ses.lastauthtype = AUTH_TYPE_PASSWORD; - } -#endif - TRACE(("cli_auth_try lastauthtype %d", cli_ses.lastauthtype)) if (!finished) { From 90b5691183f12d348d2651e026e8438ad278c2be Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 1 Apr 2013 22:26:55 +0800 Subject: [PATCH 17/33] Run the cleanup handler also when we close due to TCP connection being closed --- cli-main.c | 3 +-- cli-session.c | 7 ++++--- common-session.c | 6 +++++- session.h | 4 ++-- svr-session.c | 13 +++++++++---- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/cli-main.c b/cli-main.c index 5f72969..1a8b02e 100644 --- a/cli-main.c +++ b/cli-main.c @@ -98,8 +98,7 @@ static void cli_dropbear_exit(int exitcode, const char* format, va_list param) { } /* Do the cleanup first, since then the terminal will be reset */ - cli_session_cleanup(); - common_session_cleanup(); + session_cleanup(); _dropbear_log(LOG_INFO, fmtbuf, param); diff --git a/cli-session.c b/cli-session.c index 3adec73..0c6635a 100644 --- a/cli-session.c +++ b/cli-session.c @@ -42,6 +42,7 @@ static void cli_sessionloop(); static void cli_session_init(); static void cli_finished(); static void recv_msg_service_accept(void); +static void cli_session_cleanup(void); struct clientsession cli_ses; /* GLOBAL */ @@ -143,6 +144,7 @@ static void cli_session_init() { /* For printing "remote host closed" for the user */ ses.remoteclosed = cli_remoteclosed; + ses.extra_session_cleanup = cli_session_cleanup; ses.buf_match_algo = cli_buf_match_algo; /* packet handlers */ @@ -290,7 +292,7 @@ static void cli_sessionloop() { } -void cli_session_cleanup() { +static void cli_session_cleanup(void) { if (!sessinitdone) { return; @@ -308,8 +310,7 @@ void cli_session_cleanup() { static void cli_finished() { - cli_session_cleanup(); - common_session_cleanup(); + session_cleanup(); fprintf(stderr, "Connection to %s@%s:%s closed.\n", cli_opts.username, cli_opts.remotehost, cli_opts.remoteport); exit(cli_ses.retval); diff --git a/common-session.c b/common-session.c index ec5c9ed..b514796 100644 --- a/common-session.c +++ b/common-session.c @@ -234,7 +234,7 @@ void session_loop(void(*loophandler)()) { } /* clean up a session on exit */ -void common_session_cleanup() { +void session_cleanup() { TRACE(("enter session_cleanup")) @@ -243,6 +243,10 @@ void common_session_cleanup() { TRACE(("leave session_cleanup: !sessinitdone")) return; } + + if (ses.extra_session_cleanup) { + ses.extra_session_cleanup(); + } m_free(ses.session_id); m_burn(ses.keys, sizeof(struct key_context)); diff --git a/session.h b/session.h index 39104a3..dd486f0 100644 --- a/session.h +++ b/session.h @@ -44,7 +44,7 @@ extern int exitflag; void common_session_init(int sock_in, int sock_out); void session_loop(void(*loophandler)()); -void common_session_cleanup(); +void session_cleanup(); void send_session_identification(); void send_msg_ignore(); @@ -58,7 +58,6 @@ void svr_dropbear_log(int priority, const char* format, va_list param); /* Client */ void cli_session(int sock_in, int sock_out); -void cli_session_cleanup(); void cleantext(unsigned char* dirtytext); /* crypto parameters that are stored individually for transmit and receive */ @@ -181,6 +180,7 @@ struct sshsession { void(*remoteclosed)(); /* A callback to handle closure of the remote connection */ + void(*extra_session_cleanup)(); /* client or server specific cleanup */ struct AuthState authstate; /* Common amongst client and server, since most struct elements are common */ diff --git a/svr-session.c b/svr-session.c index 7234f4a..c235c8a 100644 --- a/svr-session.c +++ b/svr-session.c @@ -72,6 +72,13 @@ static const struct ChanType *svr_chantypes[] = { NULL /* Null termination is mandatory. */ }; +static void +svr_session_cleanup(void) +{ + /* free potential public key options */ + svr_pubkey_options_cleanup(); +} + void svr_session(int sock, int childpipe) { char *host, *port; size_t len; @@ -103,6 +110,7 @@ void svr_session(int sock, int childpipe) { /* set up messages etc */ ses.remoteclosed = svr_remoteclosed; + ses.extra_session_cleanup = svr_session_cleanup; /* packet handlers */ ses.packettypes = svr_packettypes; @@ -160,11 +168,8 @@ void svr_dropbear_exit(int exitcode, const char* format, va_list param) { if (svr_ses.server_pid == getpid()) #endif { - /* free potential public key options */ - svr_pubkey_options_cleanup(); - /* must be after we've done with username etc */ - common_session_cleanup(); + session_cleanup(); } exit(exitcode); From ff2aa20565df2ed528080288d0102cffb25e4204 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 2 Apr 2013 00:11:53 +0800 Subject: [PATCH 18/33] Be a bit more careful about when we want to use CLI_AUTH_IMMEDIATE Only use it if we have pubkeys to try, or we have $DROPBEAR_PASSWORD set --- auth.h | 2 +- cli-auth.c | 54 +++++++++++++++++++++++++++++---------------------- cli-session.c | 4 +++- 3 files changed, 35 insertions(+), 25 deletions(-) diff --git a/auth.h b/auth.h index 0fd9c73..df6634e 100644 --- a/auth.h +++ b/auth.h @@ -67,7 +67,7 @@ void recv_msg_userauth_pk_ok(); void recv_msg_userauth_info_request(); void cli_get_user(); void cli_auth_getmethods(); -void cli_auth_try(); +int cli_auth_try(); void recv_msg_userauth_banner(); void cli_pubkeyfail(); void cli_auth_password(); diff --git a/cli-auth.c b/cli-auth.c index 42c925b..efa9e9b 100644 --- a/cli-auth.c +++ b/cli-auth.c @@ -42,9 +42,15 @@ void cli_authinitialise() { void cli_auth_getmethods() { TRACE(("enter cli_auth_getmethods")) #ifdef CLI_IMMEDIATE_AUTH - ses.authstate.authtypes = AUTH_TYPE_PUBKEY | AUTH_TYPE_PASSWORD | AUTH_TYPE_INTERACT; - cli_auth_try(); -#else + ses.authstate.authtypes = AUTH_TYPE_PUBKEY; + if (getenv(DROPBEAR_PASSWORD_ENV)) { + ses.authstate.authtypes |= AUTH_TYPE_PASSWORD | AUTH_TYPE_INTERACT; + } + if (cli_auth_try() == DROPBEAR_SUCCESS) { + TRACE(("skipped initial none auth query")) + return; + } +#endif CHECKCLEARTOWRITE(); buf_putbyte(ses.writepayload, SSH_MSG_USERAUTH_REQUEST); buf_putstring(ses.writepayload, cli_opts.username, @@ -54,7 +60,6 @@ void cli_auth_getmethods() { buf_putstring(ses.writepayload, "none", 4); /* 'none' method */ encrypt_packet(); -#endif TRACE(("leave cli_auth_getmethods")) } @@ -241,7 +246,7 @@ void recv_msg_userauth_success() { #endif } -void cli_auth_try() { +int cli_auth_try() { int finished = 0; TRACE(("enter cli_auth_try")) @@ -258,36 +263,39 @@ void cli_auth_try() { #endif #ifdef ENABLE_CLI_PASSWORD_AUTH - if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { - fprintf(stderr, "Sorry, I won't let you use password auth unencrypted.\n"); - } else if (!finished && ses.authstate.authtypes & AUTH_TYPE_PASSWORD) { - cli_auth_password(); - finished = 1; - cli_ses.lastauthtype = AUTH_TYPE_PASSWORD; + if (!finished && (ses.authstate.authtypes & AUTH_TYPE_PASSWORD)) { + if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { + fprintf(stderr, "Sorry, I won't let you use password auth unencrypted.\n"); + } else { + cli_auth_password(); + finished = 1; + cli_ses.lastauthtype = AUTH_TYPE_PASSWORD; + } } #endif #ifdef ENABLE_CLI_INTERACT_AUTH - if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { - fprintf(stderr, "Sorry, I won't let you use interactive auth unencrypted.\n"); - } else if (!finished && ses.authstate.authtypes & AUTH_TYPE_INTERACT) { - if (cli_ses.auth_interact_failed) { - finished = 0; + if (!finished && (ses.authstate.authtypes & AUTH_TYPE_INTERACT)) { + if (ses.keys->trans.algo_crypt->cipherdesc == NULL) { + fprintf(stderr, "Sorry, I won't let you use interactive auth unencrypted.\n"); } else { - cli_auth_interactive(); - cli_ses.lastauthtype = AUTH_TYPE_INTERACT; - finished = 1; + if (!cli_ses.auth_interact_failed) { + cli_auth_interactive(); + cli_ses.lastauthtype = AUTH_TYPE_INTERACT; + finished = 1; + } } } #endif TRACE(("cli_auth_try lastauthtype %d", cli_ses.lastauthtype)) - if (!finished) { - dropbear_exit("No auth methods could be used."); + if (finished) { + TRACE(("leave cli_auth_try success")) + return DROPBEAR_SUCCESS; } - - TRACE(("leave cli_auth_try")) + TRACE(("leave cli_auth_try failure")) + return DROPBEAR_FAILURE; } /* A helper for getpass() that exits if the user cancels. The returned diff --git a/cli-session.c b/cli-session.c index 0c6635a..b13948f 100644 --- a/cli-session.c +++ b/cli-session.c @@ -221,7 +221,9 @@ static void cli_sessionloop() { return; case USERAUTH_FAIL_RCVD: - cli_auth_try(); + if (cli_auth_try() == DROPBEAR_FAILURE) { + dropbear_exit("No auth methods could be used."); + } cli_ses.state = USERAUTH_REQ_SENT; TRACE(("leave cli_sessionloop: cli_auth_try")) return; From e55e4687543793216b501e0de8fd3fbeb4264e1b Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 2 Apr 2013 18:53:18 +0800 Subject: [PATCH 19/33] Fix segfault when /dev/urandom isn't writable --- random.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/random.c b/random.c index 0378e9a..f935e21 100644 --- a/random.c +++ b/random.c @@ -157,6 +157,9 @@ static void write_urandom() /* This is opportunistic, don't worry about failure */ unsigned char buf[INIT_SEED_SIZE]; FILE *f = fopen(DROPBEAR_URANDOM_DEV, "w"); + if (!f) { + return; + } genrandom(buf, sizeof(buf)); fwrite(buf, sizeof(buf), 1, f); fclose(f); From dcd1527a1125eef0e9171f440276522476624fd8 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 2 Apr 2013 18:59:00 +0800 Subject: [PATCH 20/33] fix tabs --- random.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/random.c b/random.c index f935e21..1f687a2 100644 --- a/random.c +++ b/random.c @@ -157,9 +157,9 @@ static void write_urandom() /* This is opportunistic, don't worry about failure */ unsigned char buf[INIT_SEED_SIZE]; FILE *f = fopen(DROPBEAR_URANDOM_DEV, "w"); - if (!f) { - return; - } + if (!f) { + return; + } genrandom(buf, sizeof(buf)); fwrite(buf, sizeof(buf), 1, f); fclose(f); From a6eb824950e6d938fa47c1f9fa1880b652d51833 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 2 Apr 2013 19:11:13 +0800 Subject: [PATCH 21/33] add IUTF8 --- termcodes.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/termcodes.c b/termcodes.c index d59505c..490e6ce 100644 --- a/termcodes.c +++ b/termcodes.c @@ -107,8 +107,14 @@ const struct TermCode termcodes[MAX_TERMCODE+1] = { #else {0, 0}, #endif - {0, 0}, /* 42 */ + /* IUTF8 isn't standardised in rfc4254 but is likely soon. + * Implemented by linux and darwin */ +#ifdef IUTF8 + {IUTF8, TERMCODE_INPUT}, +#else {0, 0}, +#endif + {0, 0}, /* 43 */ {0, 0}, {0, 0}, {0, 0}, From f267ca1f3a51a465ee98d0cfd230c40ddfeda332 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 00:32:05 +0800 Subject: [PATCH 22/33] Add sentinel attribute --HG-- branch : kexguess --- bignum.h | 2 +- dbutil.h | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/bignum.h b/bignum.h index 042f811..cabb4a2 100644 --- a/bignum.h +++ b/bignum.h @@ -28,7 +28,7 @@ #include "includes.h" void m_mp_init(mp_int *mp); -void m_mp_init_multi(mp_int *mp, ...); +void m_mp_init_multi(mp_int *mp, ...) ATTRIB_SENTINEL; void bytes_to_mp(mp_int *mp, const unsigned char* bytes, unsigned int len); void sha1_process_mp(hash_state *hs, mp_int *mp); diff --git a/dbutil.h b/dbutil.h index fc01251..3af79f7 100644 --- a/dbutil.h +++ b/dbutil.h @@ -35,14 +35,12 @@ void startsyslog(); #ifdef __GNUC__ #define ATTRIB_PRINTF(fmt,args) __attribute__((format(printf, fmt, args))) +#define ATTRIB_NORETURN __attribute__((noreturn)) +#define ATTRIB_SENTINEL __attribute__((sentinel)) #else #define ATTRIB_PRINTF(fmt,args) -#endif - -#ifdef __GNUC__ -#define ATTRIB_NORETURN __attribute__((noreturn)) -#else #define ATTRIB_NORETURN +#define ATTRIB_SENTINEL #endif extern void (*_dropbear_exit)(int exitcode, const char* format, va_list param) ATTRIB_NORETURN; From 78fbed8c3eda1d7f3e0ffa41b54cd3c6ae31a0fe Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 00:32:55 +0800 Subject: [PATCH 23/33] Don't usually need to recalculate dh_e for the repeated kexdh_init packet --HG-- branch : kexguess --- cli-kex.c | 17 +++++++++++++---- cli-session.c | 10 +++++++--- common-algo.c | 2 +- session.h | 1 + sysoptions.h | 5 +++-- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/cli-kex.c b/cli-kex.c index 833529a..1158aa6 100644 --- a/cli-kex.c +++ b/cli-kex.c @@ -43,11 +43,19 @@ static void checkhostkey(unsigned char* keyblob, unsigned int keybloblen); void send_msg_kexdh_init() { TRACE(("send_msg_kexdh_init()")) - cli_ses.dh_e = (mp_int*)m_malloc(sizeof(mp_int)); - cli_ses.dh_x = (mp_int*)m_malloc(sizeof(mp_int)); - m_mp_init_multi(cli_ses.dh_e, cli_ses.dh_x, NULL); + if ((cli_ses.dh_e && cli_ses.dh_x + && cli_ses.dh_val_algo == ses.newkeys->algo_kex)) { + TRACE(("reusing existing dh_e from first_kex_packet_follows")) + } else { + if (!cli_ses.dh_e || !cli_ses.dh_e) { + cli_ses.dh_e = (mp_int*)m_malloc(sizeof(mp_int)); + cli_ses.dh_x = (mp_int*)m_malloc(sizeof(mp_int)); + m_mp_init_multi(cli_ses.dh_e, cli_ses.dh_x, NULL); + } - gen_kexdh_vals(cli_ses.dh_e, cli_ses.dh_x); + gen_kexdh_vals(cli_ses.dh_e, cli_ses.dh_x); + cli_ses.dh_val_algo = ses.newkeys->algo_kex; + } CHECKCLEARTOWRITE(); buf_putbyte(ses.writepayload, SSH_MSG_KEXDH_INIT); @@ -99,6 +107,7 @@ void recv_msg_kexdh_reply() { mp_clear_multi(cli_ses.dh_e, cli_ses.dh_x, NULL); m_free(cli_ses.dh_e); m_free(cli_ses.dh_x); + cli_ses.dh_val_algo = DROPBEAR_KEX_NONE; if (buf_verify(ses.payload, hostkey, ses.hash, SHA1_HASH_SIZE) != DROPBEAR_SUCCESS) { diff --git a/cli-session.c b/cli-session.c index 600827f..9e64281 100644 --- a/cli-session.c +++ b/cli-session.c @@ -182,6 +182,11 @@ static void cli_sessionloop() { TRACE2(("enter cli_sessionloop")) + if (ses.lastpacket == 0) { + TRACE2(("exit cli_sessionloop: no real packets yet")) + return; + } + if (ses.lastpacket == SSH_MSG_KEXINIT && cli_ses.kex_state == KEX_NOTHING) { /* We initiate the KEXDH. If DH wasn't the correct type, the KEXINIT * negotiation would have failed. */ @@ -206,10 +211,9 @@ static void cli_sessionloop() { return; } - /* We should exit if we haven't donefirstkex: we shouldn't reach here - * in normal operation */ if (ses.kexstate.donefirstkex == 0) { - TRACE(("XXX XXX might be bad! leave cli_sessionloop: haven't donefirstkex")) + /* We might reach here if we have partial packet reads or have + * received SSG_MSG_IGNORE etc. Just skip it */ return; } diff --git a/common-algo.c b/common-algo.c index 8267852..b698611 100644 --- a/common-algo.c +++ b/common-algo.c @@ -214,8 +214,8 @@ algo_type sshhostkey[] = { }; algo_type sshkex[] = { - {"diffie-hellman-group1-sha1", DROPBEAR_KEX_DH_GROUP1, NULL, 1, NULL}, {"diffie-hellman-group14-sha1", DROPBEAR_KEX_DH_GROUP14, NULL, 1, NULL}, + {"diffie-hellman-group1-sha1", DROPBEAR_KEX_DH_GROUP1, NULL, 1, NULL}, {KEXGUESS2_ALGO_NAME, KEXGUESS2_ALGO_ID, NULL, 1, NULL}, {NULL, 0, NULL, 0, NULL} }; diff --git a/session.h b/session.h index 33c1539..9bbeac4 100644 --- a/session.h +++ b/session.h @@ -241,6 +241,7 @@ typedef enum { struct clientsession { mp_int *dh_e, *dh_x; /* Used during KEX */ + int dh_val_algo; /* KEX algorithm corresponding to current dh_e and dh_x */ cli_kex_state kex_state; /* Used for progressing KEX */ cli_state state; /* Used to progress auth/channelsession etc */ unsigned donefirstkex : 1; /* Set when we set sentnewkeys, never reset */ diff --git a/sysoptions.h b/sysoptions.h index 6e60294..4d648c1 100644 --- a/sysoptions.h +++ b/sysoptions.h @@ -61,8 +61,9 @@ #define DROPBEAR_FAILURE -1 /* various algorithm identifiers */ -#define DROPBEAR_KEX_DH_GROUP1 0 -#define DROPBEAR_KEX_DH_GROUP14 1 +#define DROPBEAR_KEX_NONE 0 +#define DROPBEAR_KEX_DH_GROUP1 1 +#define DROPBEAR_KEX_DH_GROUP14 2 #define DROPBEAR_SIGNKEY_ANY 0 #define DROPBEAR_SIGNKEY_RSA 1 From cbd3d5e3a535111b4cd6736d6aff432a252845d2 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 00:43:31 +0800 Subject: [PATCH 24/33] Put some #ifdef options around first-follows options in case they need to be disabled --HG-- branch : kexguess --- cli-session.c | 5 ++++- common-algo.c | 2 ++ common-kex.c | 4 ++++ sysoptions.h | 9 +++++++++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/cli-session.c b/cli-session.c index 9e64281..32b7ac7 100644 --- a/cli-session.c +++ b/cli-session.c @@ -110,11 +110,12 @@ void cli_session(int sock_in, int sock_out) { } +#ifdef USE_KEX_FIRST_FOLLOWS static void cli_send_kex_first_guess() { send_msg_kexdh_init(); dropbear_log(LOG_INFO, "kexdh_init guess sent"); - //cli_ses.kex_state = KEXDH_INIT_SENT; } +#endif static void cli_session_init() { @@ -155,7 +156,9 @@ static void cli_session_init() { ses.isserver = 0; +#ifdef USE_KEX_FIRST_FOLLOWS ses.send_kex_first_guess = cli_send_kex_first_guess; +#endif } diff --git a/common-algo.c b/common-algo.c index b698611..c74463c 100644 --- a/common-algo.c +++ b/common-algo.c @@ -216,7 +216,9 @@ algo_type sshhostkey[] = { algo_type sshkex[] = { {"diffie-hellman-group14-sha1", DROPBEAR_KEX_DH_GROUP14, NULL, 1, NULL}, {"diffie-hellman-group1-sha1", DROPBEAR_KEX_DH_GROUP1, NULL, 1, NULL}, +#ifdef USE_KEXGUESS2 {KEXGUESS2_ALGO_NAME, KEXGUESS2_ALGO_ID, NULL, 1, NULL}, +#endif {NULL, 0, NULL, 0, NULL} }; diff --git a/common-kex.c b/common-kex.c index eb1fd7b..6c22600 100644 --- a/common-kex.c +++ b/common-kex.c @@ -692,7 +692,11 @@ static void read_kex_algos() { memset(ses.newkeys, 0x0, sizeof(*ses.newkeys)); +#ifdef USE_KEXGUESS2 enum kexguess2_used kexguess2 = KEXGUESS2_LOOK; +#else + enum kexguess2_used kexguess2 = KEXGUESS2_NO; +#endif /* kex_algorithms */ algo = buf_match_algo(ses.payload, sshkex, &kexguess2, &goodguess); diff --git a/sysoptions.h b/sysoptions.h index 4d648c1..22c2a4d 100644 --- a/sysoptions.h +++ b/sysoptions.h @@ -23,6 +23,15 @@ #define AUTH_TIMEOUT 300 /* we choose 5 minutes */ #endif +/* A client should try and send an initial key exchange packet guessing + * the algorithm that will match - saves a round trip connecting, has little + * overhead if the guess was "wrong". */ +#define USE_KEX_FIRST_FOLLOWS +/* Use protocol extension to allow "first follows" to succeed more frequently. + * This is currently Dropbear-specific but will gracefully fallback when connecting + * to other implementations. */ +#define USE_KEXGUESS2 + /* Minimum key sizes for DSS and RSA */ #ifndef MIN_DSS_KEYLEN #define MIN_DSS_KEYLEN 512 From 557d86aa79ec533c816bc062d1263a17b291eb84 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 07:33:47 +0800 Subject: [PATCH 25/33] Fix a few options and headers --- bignum.h | 1 + options.h | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bignum.h b/bignum.h index cabb4a2..ba98db1 100644 --- a/bignum.h +++ b/bignum.h @@ -26,6 +26,7 @@ #define _BIGNUM_H_ #include "includes.h" +#include "dbutil.h" void m_mp_init(mp_int *mp); void m_mp_init_multi(mp_int *mp, ...) ATTRIB_SENTINEL; diff --git a/options.h b/options.h index 71d39cc..88b17ca 100644 --- a/options.h +++ b/options.h @@ -153,7 +153,7 @@ much traffic. */ #endif /* Whether to do reverse DNS lookups. */ -#define DO_HOST_LOOKUP +//#define DO_HOST_LOOKUP /* Whether to print the message of the day (MOTD). This doesn't add much code * size */ @@ -174,9 +174,9 @@ much traffic. */ * PAM challenge/response. * You can't enable both PASSWORD and PAM. */ -//#define ENABLE_SVR_PASSWORD_AUTH +#define ENABLE_SVR_PASSWORD_AUTH /* PAM requires ./configure --enable-pam */ -#define ENABLE_SVR_PAM_AUTH +//#define ENABLE_SVR_PAM_AUTH #define ENABLE_SVR_PUBKEY_AUTH /* Whether to take public key options in From 286fa93a8d898b27fdd451512fde312ca08dce5f Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 07:34:18 +0800 Subject: [PATCH 26/33] fix leftover kexguess debugging --- cli-session.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/cli-session.c b/cli-session.c index 9d4bcde..7adea26 100644 --- a/cli-session.c +++ b/cli-session.c @@ -114,7 +114,6 @@ void cli_session(int sock_in, int sock_out) { #ifdef USE_KEX_FIRST_FOLLOWS static void cli_send_kex_first_guess() { send_msg_kexdh_init(); - dropbear_log(LOG_INFO, "kexdh_init guess sent"); } #endif @@ -197,7 +196,6 @@ static void cli_sessionloop() { /* We initiate the KEXDH. If DH wasn't the correct type, the KEXINIT * negotiation would have failed. */ if (!ses.kexstate.our_first_follows_matches) { - dropbear_log(LOG_INFO, "kexdh_init after remote's kexinit"); send_msg_kexdh_init(); } cli_ses.kex_state = KEXDH_INIT_SENT; From a2f70a3751a60d4843f7d6b8db2a0ce1389e2a99 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 19:23:53 +0800 Subject: [PATCH 27/33] Just put the version string on the queue, don't use atomicio --- common-session.c | 14 +++++--------- process-packet.c | 2 +- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/common-session.c b/common-session.c index b514796..6b357bf 100644 --- a/common-session.c +++ b/common-session.c @@ -33,7 +33,6 @@ #include "random.h" #include "kex.h" #include "channel.h" -#include "atomicio.h" #include "runopts.h" static void checktimeouts(); @@ -50,8 +49,6 @@ int sessinitdone = 0; /* GLOBAL */ /* this is set when we get SIGINT or SIGTERM, the handler is in main.c */ int exitflag = 0; /* GLOBAL */ - - /* called only at the start of a session, set up initial state */ void common_session_init(int sock_in, int sock_out) { @@ -257,13 +254,12 @@ void session_cleanup() { TRACE(("leave session_cleanup")) } - void send_session_identification() { - /* write our version string, this blocks */ - if (atomicio(write, ses.sock_out, LOCAL_IDENT "\r\n", - strlen(LOCAL_IDENT "\r\n")) == DROPBEAR_FAILURE) { - ses.remoteclosed(); - } + buffer *writebuf = buf_new(strlen(LOCAL_IDENT "\r\n") + 1); + buf_putbytes(writebuf, LOCAL_IDENT "\r\n", strlen(LOCAL_IDENT "\r\n")); + buf_putbyte(writebuf, 0x0); // packet type + buf_setpos(writebuf, 0); + enqueue(&ses.writequeue, writebuf); } static void read_session_identification() { diff --git a/process-packet.c b/process-packet.c index 384e449..128eb72 100644 --- a/process-packet.c +++ b/process-packet.c @@ -48,7 +48,7 @@ void process_packet() { TRACE2(("enter process_packet")) type = buf_getbyte(ses.payload); - TRACE(("process_packet: packet type = %d", type)) + TRACE(("process_packet: packet type = %d, len %d", type, ses.payload->len)) ses.lastpacket = type; From e2c813df4d7572df8d905c1bf2aed756c72abaab Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 3 Apr 2013 23:54:58 +0800 Subject: [PATCH 28/33] Fix MAC bug which would prevent asymmetric hashes --- packet.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packet.c b/packet.c index 4a3a53a..366c3d1 100644 --- a/packet.c +++ b/packet.c @@ -74,6 +74,8 @@ void write_packet() { len = writebuf->len - 1 - writebuf->pos; dropbear_assert(len > 0); all_ignore &= (packet_type == SSH_MSG_IGNORE); + TRACE2(("write_packet writev #%d type %d len %d/%d", i, packet_type, + len, writebuf->len-1)) iov[i].iov_base = buf_getptr(writebuf, len); iov[i].iov_len = len; } @@ -366,7 +368,7 @@ static int checkmac() { unsigned char mac_bytes[MAX_MAC_LEN]; unsigned int mac_size, contents_len; - mac_size = ses.keys->trans.algo_mac->hashsize; + mac_size = ses.keys->recv.algo_mac->hashsize; contents_len = ses.readbuf->len - mac_size; buf_setpos(ses.readbuf, 0); @@ -455,7 +457,6 @@ static void enqueue_reply_packet() { ses.reply_queue_head = new_item; } ses.reply_queue_tail = new_item; - TRACE2(("leave enqueue_reply_packet")) } void maybe_flush_reply_queue() { @@ -500,16 +501,12 @@ void encrypt_packet() { TRACE2(("encrypt_packet type is %d", packet_type)) - if ((!ses.dataallowed && !packet_is_okay_kex(packet_type)) - || ses.kexstate.sentnewkeys) { + if ((!ses.dataallowed && !packet_is_okay_kex(packet_type))) { /* During key exchange only particular packets are allowed. Since this packet_type isn't OK we just enqueue it to send after the KEX, see maybe_flush_reply_queue */ - - /* We also enqueue packets here when we have sent a MSG_NEWKEYS - * packet but are yet to received one. For simplicity we just switch - * over all the keys at once. This is the 'ses.kexstate.sentnewkeys' - * case. */ + TRACE2(("Delay sending reply packet. dataallowed %d, type %d, sentnewkeys %d", + ses.dataallowed, packet_type, ses.kexstate.sentnewkeys)) enqueue_reply_packet(); return; } From 7f42096d0fb56c54768f16e93666f8ee420a5424 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 4 Apr 2013 00:18:50 +0800 Subject: [PATCH 29/33] Take transmit and receive keys into use separately --- cli-kex.c | 1 - cli-session.c | 4 +-- common-kex.c | 86 +++++++++++++++++++++++++++------------------------ dbutil.c | 24 ++++++++------ packet.c | 2 -- session.h | 1 + 6 files changed, 64 insertions(+), 54 deletions(-) diff --git a/cli-kex.c b/cli-kex.c index 1158aa6..fd2e48e 100644 --- a/cli-kex.c +++ b/cli-kex.c @@ -256,7 +256,6 @@ static void checkhostkey(unsigned char* keyblob, unsigned int keybloblen) { /* Compare hostnames */ if (strncmp(cli_opts.remotehost, buf_getptr(line, hostlen), hostlen) != 0) { - TRACE2(("hosts don't match")) continue; } diff --git a/cli-session.c b/cli-session.c index 7adea26..9639ffa 100644 --- a/cli-session.c +++ b/cli-session.c @@ -204,8 +204,7 @@ static void cli_sessionloop() { } /* A KEX has finished, so we should go back to our KEX_NOTHING state */ - if (cli_ses.kex_state != KEX_NOTHING && ses.kexstate.recvkexinit == 0 - && ses.kexstate.sentkexinit == 0) { + if (cli_ses.kex_state != KEX_NOTHING && ses.kexstate.sentnewkeys) { cli_ses.kex_state = KEX_NOTHING; } @@ -218,6 +217,7 @@ static void cli_sessionloop() { if (ses.kexstate.donefirstkex == 0) { /* We might reach here if we have partial packet reads or have * received SSG_MSG_IGNORE etc. Just skip it */ + TRACE2(("donefirstkex false\n")) return; } diff --git a/common-kex.c b/common-kex.c index 6c22600..88dcb89 100644 --- a/common-kex.c +++ b/common-kex.c @@ -80,7 +80,7 @@ static const unsigned char dh_p_14[DH_P_14_LEN] = { static const int DH_G_VAL = 2; static void kexinitialise(); -void gen_new_keys(); +static void gen_new_keys(); #ifndef DISABLE_ZLIB static void gen_new_zstreams(); #endif @@ -159,11 +159,39 @@ void send_msg_kexinit() { } -/* *** NOTE regarding (send|recv)_msg_newkeys *** - * Changed by mihnea from the original kex.c to set dataallowed after a - * completed key exchange, no matter the order in which it was performed. - * This enables client mode without affecting server functionality. - */ +void switch_keys() { + TRACE2(("enter switch_keys")) + if (!(ses.kexstate.sentkexinit && ses.kexstate.recvkexinit)) { + dropbear_exit("Unexpected newkeys message"); + } + + if (!ses.keys) { + ses.keys = m_malloc(sizeof(*ses.newkeys)); + } + if (ses.kexstate.recvnewkeys && ses.newkeys->recv.valid) { + TRACE(("switch_keys recv")) + ses.keys->recv = ses.newkeys->recv; + m_burn(&ses.newkeys->recv, sizeof(ses.newkeys->recv)); + ses.newkeys->recv.valid = 0; + } + if (ses.kexstate.sentnewkeys && ses.newkeys->trans.valid) { + TRACE(("switch_keys trans")) + ses.keys->trans = ses.newkeys->trans; + m_burn(&ses.newkeys->trans, sizeof(ses.newkeys->trans)); + ses.newkeys->trans.valid = 0; + } + if (ses.kexstate.sentnewkeys && ses.kexstate.recvnewkeys) + { + TRACE(("switch_keys done")) + ses.keys->algo_kex = ses.newkeys->algo_kex; + ses.keys->algo_hostkey = ses.newkeys->algo_hostkey; + ses.keys->allow_compress = 0; + m_free(ses.newkeys); + ses.newkeys = NULL; + kexinitialise(); + } + TRACE2(("leave switch_keys")) +} /* Bring new keys into use after a key exchange, and let the client know*/ void send_msg_newkeys() { @@ -174,44 +202,25 @@ void send_msg_newkeys() { CHECKCLEARTOWRITE(); buf_putbyte(ses.writepayload, SSH_MSG_NEWKEYS); encrypt_packet(); + - /* set up our state */ - if (ses.kexstate.recvnewkeys) { - TRACE(("while RECVNEWKEYS=1")) - gen_new_keys(); - kexinitialise(); /* we've finished with this kex */ - TRACE((" -> DATAALLOWED=1")) - ses.dataallowed = 1; /* we can send other packets again now */ - ses.kexstate.donefirstkex = 1; - } else { - ses.kexstate.sentnewkeys = 1; - TRACE(("SENTNEWKEYS=1")) - } + ses.kexstate.sentnewkeys = 1; + ses.kexstate.donefirstkex = 1; + ses.dataallowed = 1; /* we can send other packets again now */ + gen_new_keys(); + switch_keys(); - TRACE(("-> MSG_NEWKEYS")) TRACE(("leave send_msg_newkeys")) } /* Bring the new keys into use after a key exchange */ void recv_msg_newkeys() { - TRACE(("<- MSG_NEWKEYS")) TRACE(("enter recv_msg_newkeys")) - /* simply check if we've sent SSH_MSG_NEWKEYS, and if so, - * switch to the new keys */ - if (ses.kexstate.sentnewkeys) { - TRACE(("while SENTNEWKEYS=1")) - gen_new_keys(); - kexinitialise(); /* we've finished with this kex */ - TRACE((" -> DATAALLOWED=1")) - ses.dataallowed = 1; /* we can send other packets again now */ - ses.kexstate.donefirstkex = 1; - } else { - TRACE(("RECVNEWKEYS=1")) - ses.kexstate.recvnewkeys = 1; - } + ses.kexstate.recvnewkeys = 1; + switch_keys(); TRACE(("leave recv_msg_newkeys")) } @@ -293,8 +302,7 @@ static void hashkeys(unsigned char *out, int outlen, * ses.newkeys is the new set of keys which are generated, these are only * taken into use after both sides have sent a newkeys message */ -/* Originally from kex.c, generalized for cli/svr mode --mihnea */ -void gen_new_keys() { +static void gen_new_keys() { unsigned char C2S_IV[MAX_IV_LEN]; unsigned char C2S_key[MAX_KEY_LEN]; @@ -382,11 +390,9 @@ void gen_new_keys() { gen_new_zstreams(); #endif - /* Switch over to the new keys */ - m_burn(ses.keys, sizeof(struct key_context)); - m_free(ses.keys); - ses.keys = ses.newkeys; - ses.newkeys = NULL; + /* Ready to switch over */ + ses.newkeys->trans.valid = 1; + ses.newkeys->recv.valid = 1; m_burn(C2S_IV, sizeof(C2S_IV)); m_burn(C2S_key, sizeof(C2S_key)); diff --git a/dbutil.c b/dbutil.c index 8c48a24..8ee6050 100644 --- a/dbutil.c +++ b/dbutil.c @@ -138,29 +138,39 @@ void dropbear_log(int priority, const char* format, ...) { #ifdef DEBUG_TRACE void dropbear_trace(const char* format, ...) { - va_list param; + struct timeval tv; if (!debug_trace) { return; } + gettimeofday(&tv, NULL); + va_start(param, format); - fprintf(stderr, "TRACE (%d): ", getpid()); + fprintf(stderr, "TRACE (%d) %d.%d: ", getpid(), tv.tv_sec, tv.tv_usec); vfprintf(stderr, format, param); fprintf(stderr, "\n"); va_end(param); } + void dropbear_trace2(const char* format, ...) { - + static int trace_env = -1; va_list param; + struct timeval tv; - if (!(debug_trace && getenv("DROPBEAR_TRACE2"))) { + if (trace_env == -1) { + trace_env = getenv("DROPBEAR_TRACE2") ? 1 : 0; + } + + if (!(debug_trace && trace_env)) { return; } + gettimeofday(&tv, NULL); + va_start(param, format); - fprintf(stderr, "TRACE2 (%d): ", getpid()); + fprintf(stderr, "TRACE2 (%d) %d.%d: ", getpid(), tv.tv_sec, tv.tv_usec); vfprintf(stderr, format, param); fprintf(stderr, "\n"); va_end(param); @@ -739,8 +749,6 @@ int buf_getline(buffer * line, FILE * authfile) { int c = EOF; - TRACE2(("enter buf_getline")) - buf_setpos(line, 0); buf_setlen(line, 0); @@ -764,10 +772,8 @@ out: /* if we didn't read anything before EOF or error, exit */ if (c == EOF && line->pos == 0) { - TRACE2(("leave buf_getline: failure")) return DROPBEAR_FAILURE; } else { - TRACE2(("leave buf_getline: success")) buf_setpos(line, 0); return DROPBEAR_SUCCESS; } diff --git a/packet.c b/packet.c index 366c3d1..09f0600 100644 --- a/packet.c +++ b/packet.c @@ -505,8 +505,6 @@ void encrypt_packet() { /* During key exchange only particular packets are allowed. Since this packet_type isn't OK we just enqueue it to send after the KEX, see maybe_flush_reply_queue */ - TRACE2(("Delay sending reply packet. dataallowed %d, type %d, sentnewkeys %d", - ses.dataallowed, packet_type, ses.kexstate.sentnewkeys)) enqueue_reply_packet(); return; } diff --git a/session.h b/session.h index a76fa99..4ba8ac8 100644 --- a/session.h +++ b/session.h @@ -78,6 +78,7 @@ struct key_context_directional { #endif } cipher_state; unsigned char mackey[MAX_MAC_LEN]; + int valid; }; struct key_context { From 2fdb5fd6ced264fec1594cf674ef125f5dc6a500 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 4 Apr 2013 07:51:13 +0800 Subject: [PATCH 30/33] setup tcp after requesting a channel - might hide some DNS latency --- cli-session.c | 15 ++++++++------- debug.h | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/cli-session.c b/cli-session.c index 9639ffa..401c9e2 100644 --- a/cli-session.c +++ b/cli-session.c @@ -266,13 +266,6 @@ static void cli_sessionloop() { } } -#ifdef ENABLE_CLI_LOCALTCPFWD - setup_localtcp(); -#endif -#ifdef ENABLE_CLI_REMOTETCPFWD - setup_remotetcp(); -#endif - #ifdef ENABLE_CLI_NETCAT if (cli_opts.netcat_host) { cli_send_netcat_request(); @@ -281,6 +274,14 @@ static void cli_sessionloop() { if (!cli_opts.no_cmd) { cli_send_chansess_request(); } + +#ifdef ENABLE_CLI_LOCALTCPFWD + setup_localtcp(); +#endif +#ifdef ENABLE_CLI_REMOTETCPFWD + setup_remotetcp(); +#endif + TRACE(("leave cli_sessionloop: running")) cli_ses.state = SESSION_RUNNING; return; diff --git a/debug.h b/debug.h index 289c577..be09865 100644 --- a/debug.h +++ b/debug.h @@ -39,7 +39,7 @@ * Caution: Don't use this in an unfriendly environment (ie unfirewalled), * since the printing may not sanitise strings etc. This will add a reasonable * amount to your executable size. */ -/* #define DEBUG_TRACE */ +#define DEBUG_TRACE /* All functions writing to the cleartext payload buffer call * CHECKCLEARTOWRITE() before writing. This is only really useful if you're From 3ec467047856eb76945dc7804d1ba7743935fb2d Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 10 Apr 2013 21:32:44 +0800 Subject: [PATCH 31/33] reset terminal modes before printing a message --- cli-chansession.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cli-chansession.c b/cli-chansession.c index 65d1f4f..17e0d53 100644 --- a/cli-chansession.c +++ b/cli-chansession.c @@ -82,14 +82,12 @@ out: /* If the main session goes, we close it up */ static void cli_closechansess(struct Channel *UNUSED(channel)) { + cli_tty_cleanup(); /* Restore tty modes etc */ /* This channel hasn't gone yet, so we have > 1 */ if (ses.chancount > 1) { dropbear_log(LOG_INFO, "Waiting for other channels to close..."); } - - cli_tty_cleanup(); /* Restore tty modes etc */ - } void cli_start_send_channel_request(struct Channel *channel, From c0d7c6693f579a814c50d7965f8e89fa470b2c03 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 10 Apr 2013 21:32:55 +0800 Subject: [PATCH 32/33] run closehandlers on cleanup --- common-channel.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/common-channel.c b/common-channel.c index 331ea60..8b7369c 100644 --- a/common-channel.c +++ b/common-channel.c @@ -561,7 +561,11 @@ static void remove_channel(struct Channel * channel) { TRACE(("CLOSE errfd %d", channel->errfd)) close(channel->errfd); - channel->typedata = NULL; + if (!channel->close_handler_done + && channel->type->closehandler) { + channel->type->closehandler(channel); + channel->close_handler_done = 1; + } ses.channels[channel->index] = NULL; m_free(channel); @@ -625,7 +629,7 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) { * exttype if is extended */ maxlen = MIN(maxlen, ses.writepayload->size - 1 - 4 - 4 - (isextended ? 4 : 0)); - TRACE(("maxlen %d", maxlen)) + TRACE(("maxlen %zd", maxlen)) if (maxlen == 0) { TRACE(("leave send_msg_channel_data: no window")) return; From adeb372a66643000aee8445f8b2f9854fdfb4402 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 11 Apr 2013 23:03:58 +0800 Subject: [PATCH 33/33] Fix zlib for split newkeys --- common-kex.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/common-kex.c b/common-kex.c index 88dcb89..530de0b 100644 --- a/common-kex.c +++ b/common-kex.c @@ -82,7 +82,8 @@ static const int DH_G_VAL = 2; static void kexinitialise(); static void gen_new_keys(); #ifndef DISABLE_ZLIB -static void gen_new_zstreams(); +static void gen_new_zstream_recv(); +static void gen_new_zstream_trans(); #endif static void read_kex_algos(); /* helper function for gen_new_keys */ @@ -159,7 +160,7 @@ void send_msg_kexinit() { } -void switch_keys() { +static void switch_keys() { TRACE2(("enter switch_keys")) if (!(ses.kexstate.sentkexinit && ses.kexstate.recvkexinit)) { dropbear_exit("Unexpected newkeys message"); @@ -170,12 +171,14 @@ void switch_keys() { } if (ses.kexstate.recvnewkeys && ses.newkeys->recv.valid) { TRACE(("switch_keys recv")) + gen_new_zstream_recv(); ses.keys->recv = ses.newkeys->recv; m_burn(&ses.newkeys->recv, sizeof(ses.newkeys->recv)); ses.newkeys->recv.valid = 0; } if (ses.kexstate.sentnewkeys && ses.newkeys->trans.valid) { TRACE(("switch_keys trans")) + gen_new_zstream_trans(); ses.keys->trans = ses.newkeys->trans; m_burn(&ses.newkeys->trans, sizeof(ses.newkeys->trans)); ses.newkeys->trans.valid = 0; @@ -386,10 +389,6 @@ static void gen_new_keys() { ses.newkeys->recv.hash_index = find_hash(ses.newkeys->recv.algo_mac->hashdesc->name); } -#ifndef DISABLE_ZLIB - gen_new_zstreams(); -#endif - /* Ready to switch over */ ses.newkeys->trans.valid = 1; ses.newkeys->recv.valid = 1; @@ -418,7 +417,7 @@ int is_compress_recv() { /* Set up new zlib compression streams, close the old ones. Only * called from gen_new_keys() */ -static void gen_new_zstreams() { +static void gen_new_zstream_recv() { /* create new zstreams */ if (ses.newkeys->recv.algo_comp == DROPBEAR_COMP_ZLIB @@ -433,6 +432,17 @@ static void gen_new_zstreams() { } else { ses.newkeys->recv.zstream = NULL; } + /* clean up old keys */ + if (ses.keys->recv.zstream != NULL) { + if (inflateEnd(ses.keys->recv.zstream) == Z_STREAM_ERROR) { + /* Z_DATA_ERROR is ok, just means that stream isn't ended */ + dropbear_exit("Crypto error"); + } + m_free(ses.keys->recv.zstream); + } +} + +static void gen_new_zstream_trans() { if (ses.newkeys->trans.algo_comp == DROPBEAR_COMP_ZLIB || ses.newkeys->trans.algo_comp == DROPBEAR_COMP_ZLIB_DELAY) { @@ -450,14 +460,6 @@ static void gen_new_zstreams() { ses.newkeys->trans.zstream = NULL; } - /* clean up old keys */ - if (ses.keys->recv.zstream != NULL) { - if (inflateEnd(ses.keys->recv.zstream) == Z_STREAM_ERROR) { - /* Z_DATA_ERROR is ok, just means that stream isn't ended */ - dropbear_exit("Crypto error"); - } - m_free(ses.keys->recv.zstream); - } if (ses.keys->trans.zstream != NULL) { if (deflateEnd(ses.keys->trans.zstream) == Z_STREAM_ERROR) { /* Z_DATA_ERROR is ok, just means that stream isn't ended */