From 4b36e24735dcc0c81c412375b333073aa2c62323 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 15:09:41 +0800 Subject: [PATCH 01/30] print config.log on failure --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index aa2dee5..f776307 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ before_install: install: - autoconf - autoheader - - ./configure $CONFIGURE_FLAGS CFLAGS="-O2 -Wall -Wno-pointer-sign $WEXTRAFLAGS $EXTRACFLAGS" --prefix="$HOME/inst" + - ./configure $CONFIGURE_FLAGS CFLAGS="-O2 -Wall -Wno-pointer-sign $WEXTRAFLAGS $EXTRACFLAGS" --prefix="$HOME/inst" || (cat config.log; exit 1) - if [ "$NOWRITEV" = "1" ]; then sed -i -e s/HAVE_WRITEV/DONT_HAVE_WRITEV/ config.h ; fi - make -j3 - test -z $DO_FUZZ || make fuzzstandalone From 85eda7d94354c52e34107c5e9b64b55d6f7ee15b Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 15:18:20 +0800 Subject: [PATCH 02/30] try fix travis sanitizer --- .travis.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f776307..4ee211e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ matrix: - os: linux compiler: gcc env: WEXTRAFLAGS=-Werror + sudo: false - env: MULTI=1 WEXTRAFLAGS=-Werror # libtom has some warnings, so no WEXTRAFLAGS - env: CONFIGURE_FLAGS=--enable-bundled-libtom WEXTRAFLAGS="" @@ -21,12 +22,13 @@ matrix: compiler: clang env: WEXTRAFLAGS="" - # TODO: fuzzing malloc wrapper doesn't replace free() in system libtomcrypt + # Note: the fuzzing malloc wrapper doesn't replace free() in system libtomcrypt, so need bundled. - env: DO_FUZZ=1 CONFIGURE_FLAGS="--enable-fuzz --disable-harden --enable-bundled-libtom" WEXTRAFLAGS="" LDFLAGS=-fsanitize=address EXTRACFLAGS=-fsanitize=address compiler: clang + # sanitizers need ptrace which is privileged https://github.com/travis-ci/travis-ci/issues/9033 + sudo: required # container-based builds -sudo: false addons: apt: packages: From 1e1e477d850e325ed9df9981a21a1e1519d4f106 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 17:08:21 +0800 Subject: [PATCH 03/30] set up CXX for fuzzing build --- Makefile.in | 2 ++ configure.ac | 3 +++ 2 files changed, 5 insertions(+) diff --git a/Makefile.in b/Makefile.in index 479e188..124b0e4 100644 --- a/Makefile.in +++ b/Makefile.in @@ -70,6 +70,8 @@ ifeq (@DROPBEAR_FUZZ@, 1) dbclientobjs=$(allobjs) cli-main.o dropbearkeyobjs=$(allobjs) $(KEYOBJS) dropbearconvertobjs=$(allobjs) $(CONVERTOBJS) + # CXX only set when fuzzing + CXX=@CXX@ else dropbearobjs=$(COMMONOBJS) $(CLISVROBJS) $(SVROBJS) dbclientobjs=$(COMMONOBJS) $(CLISVROBJS) $(CLIOBJS) diff --git a/configure.ac b/configure.ac index d1b2602..c0bb8a3 100644 --- a/configure.ac +++ b/configure.ac @@ -329,6 +329,8 @@ AC_ARG_ENABLE(fuzz, AC_DEFINE(DROPBEAR_FUZZ, 1, Fuzzing) AC_MSG_NOTICE(Enabling fuzzing) DROPBEAR_FUZZ=1 + # libfuzzer needs linking with c++ libraries + AC_PROG_CXX ], [ AC_DEFINE(DROPBEAR_FUZZ, 0, Fuzzing) @@ -337,6 +339,7 @@ AC_ARG_ENABLE(fuzz, ) AC_SUBST(DROPBEAR_FUZZ) +AC_SUBST(CXX) # Checks for header files. AC_HEADER_STDC From 2583b180c994ed37e462547e160b1141d9536d76 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 17:17:37 +0800 Subject: [PATCH 04/30] travis fuzz build clang++ --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4ee211e..9bcbce4 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,7 @@ matrix: env: WEXTRAFLAGS="" # Note: the fuzzing malloc wrapper doesn't replace free() in system libtomcrypt, so need bundled. - - env: DO_FUZZ=1 CONFIGURE_FLAGS="--enable-fuzz --disable-harden --enable-bundled-libtom" WEXTRAFLAGS="" LDFLAGS=-fsanitize=address EXTRACFLAGS=-fsanitize=address + - env: DO_FUZZ=1 CONFIGURE_FLAGS="--enable-fuzz --disable-harden --enable-bundled-libtom" WEXTRAFLAGS="" LDFLAGS=-fsanitize=address EXTRACFLAGS=-fsanitize=address CXX=clang++ compiler: clang # sanitizers need ptrace which is privileged https://github.com/travis-ci/travis-ci/issues/9033 sudo: required From abee8093b3f029ea154b81e729b4a523dcfdb56e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 19:19:45 +0800 Subject: [PATCH 05/30] use random keyblob from the fuzzer instead --- fuzzer-pubkey.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/fuzzer-pubkey.c b/fuzzer-pubkey.c index 3dfe7b5..cb648e4 100644 --- a/fuzzer-pubkey.c +++ b/fuzzer-pubkey.c @@ -20,19 +20,22 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { m_malloc_set_epoch(1); - /* choose a keytype based on input */ - uint8_t b = 0; - size_t i; - for (i = 0; i < Size; i++) { - b ^= Data[i]; - } - const char* algoname = fuzz_signkey_names[b%DROPBEAR_SIGNKEY_NUM_NAMED]; - const char* keyblob = "blob"; /* keep short */ - if (setjmp(fuzz.jmp) == 0) { - fuzz_checkpubkey_line(fuzz.input, 5, "/home/me/authorized_keys", - algoname, strlen(algoname), - (unsigned char*)keyblob, strlen(keyblob)); + buffer *line = buf_getstringbuf(fuzz.input); + buffer *keyblob = buf_getstringbuf(fuzz.input); + + unsigned int algolen; + const char* algoname = buf_getstring(keyblob, &algolen); + + if (have_algo(algo, algolen, sshhostkey) == DROPBEAR_FAILURE) { + dropbear_exit("fuzzer imagined a bogus algorithm"); + } + fuzz_checkpubkey_line(line, 5, "/home/me/authorized_keys", + algoname, algolen, + keyblob->data, keyblob->len); + + buf_free(line); + buf_free(keyblob); m_malloc_free_epoch(1, 0); } else { m_malloc_free_epoch(1, 1); From 129c4403622479e7b3d8363b8bbce16010bf3b5c Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 19:38:34 +0800 Subject: [PATCH 06/30] add a dictionary for fuzzer-pubkey --- fuzzer-pubkey.dict | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 fuzzer-pubkey.dict diff --git a/fuzzer-pubkey.dict b/fuzzer-pubkey.dict new file mode 100644 index 0000000..daf12cf --- /dev/null +++ b/fuzzer-pubkey.dict @@ -0,0 +1,12 @@ +"ssh-rsa" +"ssh-dss" +"ecdsa-sha2-nistp256" +"ecdsa-sha2-nistp384" +"ecdsa-sha2-nistp521" +"no-port-forwarding" +"no-port-forwarding" +"no-agent-forwarding" +"no-X11-forwarding" +"no-pty" +"command=\"" +"#" From f7dedab4a7323e0bd5e1fe2b40d17e0d8beefa19 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 21:44:09 +0800 Subject: [PATCH 07/30] - fuzzer-pubkey needs to free algoname, fix build - improve dictionary with lengths --- fuzzer-pubkey.c | 5 +++-- fuzzer-pubkey.dict | 10 +++++----- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/fuzzer-pubkey.c b/fuzzer-pubkey.c index cb648e4..a062e1f 100644 --- a/fuzzer-pubkey.c +++ b/fuzzer-pubkey.c @@ -25,9 +25,9 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { buffer *keyblob = buf_getstringbuf(fuzz.input); unsigned int algolen; - const char* algoname = buf_getstring(keyblob, &algolen); + char* algoname = buf_getstring(keyblob, &algolen); - if (have_algo(algo, algolen, sshhostkey) == DROPBEAR_FAILURE) { + if (have_algo(algoname, algolen, sshhostkey) == DROPBEAR_FAILURE) { dropbear_exit("fuzzer imagined a bogus algorithm"); } fuzz_checkpubkey_line(line, 5, "/home/me/authorized_keys", @@ -36,6 +36,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { buf_free(line); buf_free(keyblob); + m_free(algoname); m_malloc_free_epoch(1, 0); } else { m_malloc_free_epoch(1, 1); diff --git a/fuzzer-pubkey.dict b/fuzzer-pubkey.dict index daf12cf..734629c 100644 --- a/fuzzer-pubkey.dict +++ b/fuzzer-pubkey.dict @@ -1,8 +1,8 @@ -"ssh-rsa" -"ssh-dss" -"ecdsa-sha2-nistp256" -"ecdsa-sha2-nistp384" -"ecdsa-sha2-nistp521" +"\x00\x00\x00\x07ssh-rsa" +"\x00\x00\x00\x07ssh-dss" +"\x00\x00\x00\x13ecdsa-sha2-nistp256" +"\x00\x00\x00\x13ecdsa-sha2-nistp384" +"\x00\x00\x00\x13ecdsa-sha2-nistp521" "no-port-forwarding" "no-port-forwarding" "no-agent-forwarding" From 0233dcebb4c9510eb08ff3c4f36588fcd5fcbdbe Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Sun, 4 Mar 2018 22:11:23 +0800 Subject: [PATCH 08/30] try and improve the odds of useful fuzzer activity --- fuzz-wrapfd.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fuzz-wrapfd.c b/fuzz-wrapfd.c index 39d2b62..313a110 100644 --- a/fuzz-wrapfd.c +++ b/fuzz-wrapfd.c @@ -6,12 +6,12 @@ #define IOWRAP_MAXFD (FD_SETSIZE-1) static const int MAX_RANDOM_IN = 50000; -static const double CHANCE_CLOSE = 1.0 / 300; -static const double CHANCE_INTR = 1.0 / 200; -static const double CHANCE_READ1 = 0.6; -static const double CHANCE_READ2 = 0.3; -static const double CHANCE_WRITE1 = 0.8; -static const double CHANCE_WRITE2 = 0.3; +static const double CHANCE_CLOSE = 1.0 / 600; +static const double CHANCE_INTR = 1.0 / 900; +static const double CHANCE_READ1 = 0.96; +static const double CHANCE_READ2 = 0.5; +static const double CHANCE_WRITE1 = 0.96; +static const double CHANCE_WRITE2 = 0.5; struct fdwrap { enum wrapfd_mode mode; From 9d11cad5dcc1ae98a42133b592971a7997248deb Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 00:59:17 +0800 Subject: [PATCH 09/30] dictionary for fuzzer-preauth --- fuzzer-preauth.dict | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 fuzzer-preauth.dict diff --git a/fuzzer-preauth.dict b/fuzzer-preauth.dict new file mode 100644 index 0000000..0419a1c --- /dev/null +++ b/fuzzer-preauth.dict @@ -0,0 +1,33 @@ +"\x00\x00\x00\x0aaes128-ctr" +"\x00\x00\x00\x0aaes256-ctr" +"\x00\x00\x00\x0aaes128-cbc" +"\x00\x00\x00\x0aaes256-cbc" +"\x00\x00\x00\x083des-ctr" +"\x00\x00\x00\x083des-cbc" +"\x00\x00\x00\x0chmac-sha1-96" +"\x00\x00\x00\x09hmac-sha1" +"\x00\x00\x00\x0dhmac-sha2-256" +"\x00\x00\x00\x10zlib@openssh.com" +"\x00\x00\x00\x04zlib" +"\x00\x00\x00\x04none" +"\x00\x00\x00\x13ecdsa-sha2-nistp256" +"\x00\x00\x00\x13ecdsa-sha2-nistp384" +"\x00\x00\x00\x13ecdsa-sha2-nistp521" +"\x00\x00\x00\x07ssh-rsa" +"\x00\x00\x00\x07ssh-dss" +"\x00\x00\x00\x11curve25519-sha256" +"\x00\x00\x00\x1ccurve25519-sha256@libssh.org" +"\x00\x00\x00\x12ecdh-sha2-nistp521" +"\x00\x00\x00\x12ecdh-sha2-nistp384" +"\x00\x00\x00\x12ecdh-sha2-nistp256" +"\x00\x00\x00\x1ddiffie-hellman-group14-sha256" +"\x00\x00\x00\x1bdiffie-hellman-group14-sha1" +"\x00\x00\x00\x1adiffie-hellman-group1-sha1" +"\x00\x00\x00\x19kexguess2@matt.ucc.asn.au" +"\x00\x00\x00\x0cssh-userauth" +"\x00\x00\x00\x0essh-connection" +"\x00\x00\x00\x09publickey" +"\x00\x00\x00\x08password" +"\x00\x00\x00\x14keyboard-interactive" +# rsa_asn1_magic +"\x00\x30\x21\x30\x09\x06\x05\x2b\x0e\x03\x02\x1a\x05\x00\x04\x14" From 35f479bd87e16e7370aceda3da65bacdcce0c362 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 11:50:31 +0800 Subject: [PATCH 10/30] Add kexdh and kexecdh fuzzers --- Makefile.in | 9 ++++-- fuzz-common.c | 11 +++++++ fuzz-harness.c | 1 + fuzz.h | 1 + fuzzer-kexdh.c | 70 +++++++++++++++++++++++++++++++++++++++++++ fuzzer-kexecdh.c | 78 ++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 168 insertions(+), 2 deletions(-) create mode 100644 fuzzer-kexdh.c create mode 100644 fuzzer-kexecdh.c diff --git a/Makefile.in b/Makefile.in index 124b0e4..6adc2e0 100644 --- a/Makefile.in +++ b/Makefile.in @@ -255,7 +255,7 @@ tidy: ## Fuzzing targets # list of fuzz targets -FUZZ_TARGETS=fuzzer-preauth fuzzer-pubkey fuzzer-verify fuzzer-preauth_nomaths +FUZZ_TARGETS=fuzzer-preauth fuzzer-pubkey fuzzer-verify fuzzer-preauth_nomaths fuzzer-kexdh fuzzer-kexecdh FUZZER_OPTIONS = $(addsuffix .options, $(FUZZ_TARGETS)) @@ -280,13 +280,18 @@ fuzzer-preauth: fuzzer-preauth.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobj fuzzer-preauth_nomaths: fuzzer-preauth_nomaths.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs) $(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@ - fuzzer-pubkey: fuzzer-pubkey.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs) $(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@ fuzzer-verify: fuzzer-verify.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs) $(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@ +fuzzer-kexdh: fuzzer-kexdh.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs) + $(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@ + +fuzzer-kexecdh: fuzzer-kexecdh.o $(HEADERS) $(LIBTOM_DEPS) Makefile $(svrfuzzobjs) + $(CXX) $(CXXFLAGS) $@.o $(LDFLAGS) $(svrfuzzobjs) -o $@$(EXEEXT) $(LIBTOM_LIBS) $(LIBS) $(FUZZLIB) @CRYPTLIB@ + fuzzer-%.options: Makefile echo "[libfuzzer]" > $@ echo "max_len = 50000" >> $@ diff --git a/fuzz-common.c b/fuzz-common.c index f64504f..5c90c45 100644 --- a/fuzz-common.c +++ b/fuzz-common.c @@ -22,6 +22,7 @@ void fuzz_common_setup(void) { fuzz.input = m_malloc(sizeof(buffer)); _dropbear_log = fuzz_dropbear_log; crypto_init(); + fuzz_seed(); /* let any messages get flushed */ setlinebuf(stdout); } @@ -188,3 +189,13 @@ int fuzz_run_preauth(const uint8_t *Data, size_t Size, int skip_kexmaths) { return 0; } + +const void* fuzz_get_algo(const algo_type *algos, const char* name) { + const algo_type *t; + for (t = algos; t->name; t++) { + if (strcmp(t->name, name) == 0) { + return t->data; + } + } + assert(0); +} diff --git a/fuzz-harness.c b/fuzz-harness.c index 00a2ba6..53bc71f 100644 --- a/fuzz-harness.c +++ b/fuzz-harness.c @@ -9,6 +9,7 @@ int main(int argc, char ** argv) { buffer *input = buf_new(100000); for (i = 1; i < argc; i++) { + printf("arg %s\n", argv[i]); #if DEBUG_TRACE if (strcmp(argv[i], "-v") == 0) { debug_trace = 1; diff --git a/fuzz.h b/fuzz.h index 9316a0a..dab6c37 100644 --- a/fuzz.h +++ b/fuzz.h @@ -19,6 +19,7 @@ void fuzz_svr_setup(void); int fuzz_set_input(const uint8_t *Data, size_t Size); int fuzz_run_preauth(const uint8_t *Data, size_t Size, int skip_kexmaths); +const void* fuzz_get_algo(const algo_type *algos, const char* name); // fuzzer functions that intrude into general code void fuzz_kex_fakealgos(void); diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c new file mode 100644 index 0000000..60255dd --- /dev/null +++ b/fuzzer-kexdh.c @@ -0,0 +1,70 @@ +#include "fuzz.h" +#include "session.h" +#include "fuzz-wrapfd.h" +#include "debug.h" +#include "runopts.h" +#include "algo.h" +#include "bignum.h" + +int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { + static int once = 0; + static struct key_context* keep_newkeys = NULL; + #define NUM_PARAMS 800 + static struct kex_dh_param *dh_params[NUM_PARAMS]; + + if (!once) { + fuzz_common_setup(); + fuzz_svr_setup(); + + keep_newkeys = (struct key_context*)m_malloc(sizeof(struct key_context)); + keep_newkeys->algo_kex = fuzz_get_algo(sshkex, "diffie-hellman-group14-sha256"); + keep_newkeys->algo_hostkey = DROPBEAR_SIGNKEY_ECDSA_NISTP256; + ses.newkeys = keep_newkeys; + + /* Pre-generate parameters */ + int i; + for (i = 0; i < NUM_PARAMS; i++) { + dh_params[i] = gen_kexdh_param(); + } + + once = 1; + } + + if (fuzz_set_input(Data, Size) == DROPBEAR_FAILURE) { + return 0; + } + + m_malloc_set_epoch(1); + + if (setjmp(fuzz.jmp) == 0) { + /* Based on recv_msg_kexdh_init()/send_msg_kexdh_reply() + with DROPBEAR_KEX_NORMAL_DH */ + ses.newkeys = keep_newkeys; + + /* Choose from the collection of ecdh params */ + unsigned int e = buf_getint(fuzz.input); + struct kex_dh_param * dh_param = dh_params[e % NUM_PARAMS]; + + DEF_MP_INT(dh_e); + m_mp_init(&dh_e); + if (buf_getmpint(fuzz.input, &dh_e) != DROPBEAR_SUCCESS) { + dropbear_exit("Bad kex value"); + } + + ses.kexhashbuf = buf_new(4); + buf_putint(ses.kexhashbuf, 12345); + kexdh_comb_key(dh_param, &dh_e, svr_opts.hostkey); + + /* kexhashbuf is freed in kexdh_comb_key */ + m_free(ses.dh_K); + mp_clear(&dh_e); + + m_malloc_free_epoch(1, 0); + } else { + m_malloc_free_epoch(1, 1); + TRACE(("dropbear_exit longjmped")) + /* dropbear_exit jumped here */ + } + + return 0; +} diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c new file mode 100644 index 0000000..db08c2b --- /dev/null +++ b/fuzzer-kexecdh.c @@ -0,0 +1,78 @@ +#include "fuzz.h" +#include "session.h" +#include "fuzz-wrapfd.h" +#include "debug.h" +#include "runopts.h" +#include "algo.h" +#include "bignum.h" + +int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { + static int once = 0; + static const struct dropbear_kex *ecdh[3]; /* 256, 384, 521 */ + static struct key_context* keep_newkeys = NULL; + #define NUM_PARAMS 800 + static struct kex_ecdh_param *ecdh_params[NUM_PARAMS]; + + if (!once) { + fuzz_common_setup(); + fuzz_svr_setup(); + + /* ses gets zeroed by fuzz_set_input */ + keep_newkeys = (struct key_context*)m_malloc(sizeof(struct key_context)); + ecdh[0] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp256"); + ecdh[1] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp384"); + ecdh[2] = fuzz_get_algo(sshkex, "ecdh-sha2-nistp521"); + assert(ecdh[0]); + assert(ecdh[1]); + assert(ecdh[2]); + keep_newkeys->algo_hostkey = DROPBEAR_SIGNKEY_ECDSA_NISTP256; + ses.newkeys = keep_newkeys; + + /* Pre-generate parameters */ + int i; + for (i = 0; i < NUM_PARAMS; i++) { + ses.newkeys->algo_kex = ecdh[i % 3]; + ecdh_params[i] = gen_kexecdh_param(); + } + + once = 1; + } + + if (fuzz_set_input(Data, Size) == DROPBEAR_FAILURE) { + return 0; + } + + m_malloc_set_epoch(1); + + if (setjmp(fuzz.jmp) == 0) { + /* Based on recv_msg_kexdh_init()/send_msg_kexdh_reply() + with DROPBEAR_KEX_ECDH */ + ses.newkeys = keep_newkeys; + + /* random choice of ecdh 256, 384, 521 */ + unsigned char b = buf_getbyte(fuzz.input); + ses.newkeys->algo_kex = ecdh[b % 3]; + + /* Choose from the collection of ecdh params */ + unsigned int e = buf_getint(fuzz.input); + struct kex_ecdh_param *ecdh_param = ecdh_params[e % NUM_PARAMS]; + + buffer * ecdh_qs = buf_getstringbuf(fuzz.input); + + ses.kexhashbuf = buf_new(4); + buf_putint(ses.kexhashbuf, 12345); + kexecdh_comb_key(ecdh_param, ecdh_qs, svr_opts.hostkey); + + /* kexhashbuf is freed in kexdh_comb_key */ + m_free(ses.dh_K); + buf_free(ecdh_qs); + + m_malloc_free_epoch(1, 0); + } else { + m_malloc_free_epoch(1, 1); + TRACE(("dropbear_exit longjmped")) + /* dropbear_exit jumped here */ + } + + return 0; +} From 366fc8f335e134be447ed77b6923b085857dc7f3 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 14:07:11 +0800 Subject: [PATCH 11/30] notes on the fuzzer --- FUZZER-NOTES.md | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 FUZZER-NOTES.md diff --git a/FUZZER-NOTES.md b/FUZZER-NOTES.md new file mode 100644 index 0000000..b5b5c97 --- /dev/null +++ b/FUZZER-NOTES.md @@ -0,0 +1,72 @@ +# Fuzzing Dropbear + +Dropbear is process-per-session so it assumes calling `dropbear_exit()` +is fine at any point to clean up. This makes fuzzing a bit trickier. +A few pieces of wrapping infrastructure are used to work around this. + +The [libfuzzer](http://llvm.org/docs/LibFuzzer.html#fuzz-target) harness +expects a long running process to continually run a test function with +a string of crafted input. That process should not leak resources or exit. + +## longjmp + +When dropbear runs in fuzz mode it sets up a +[`setjmp()`](http://man7.org/linux/man-pages/man3/setjmp.3.html) target prior +to launching the code to be fuzzed, and then [`dropbear_exit()`](dbutil.c#L125) +calls `longjmp()` back there. This avoids exiting though it doesn't free +memory or other resources. + +## malloc Wrapper + +Dropbear normally uses a [`m_malloc()`](dbmalloc.c) function that is the same as `malloc()` but +exits if allocation fails. In fuzzing mode this is replaced with a tracking allocator +that stores all allocations in a linked list. After the `longjmp()` occurs the fuzzer target +calls [`m_malloc_free_epoch(1, 1)`](dbmalloc.c) to clean up any unreleased memory. + +If the fuzz target runs to completion it calls `m_malloc_free_epoch(1, 0)` which will reset +the tracked allocations but will not free memory - that allows libfuzzer's leak checking +to detect leaks in normal operation. + +## File Descriptor Input + +As a network process Dropbear reads and writes from a socket. The wrappers for +`read()`/`write()`/`select()` in [fuzz-wrapfd.c](fuzz-wrapfd.c) will read from the +fuzzer input that has been set up with `wrapfd_add()`. `write()` output is +currently discarded. +These also test error paths such as EINTR and short reads with certain probabilities. + +This allows running the entire dropbear server process with network input provided by the +fuzzer, without many modifications to the main code. At the time of writing this +only runs the pre-authentication stages, though post-authentication could be run similarly. + +## Encryption and Randomness + +When running in fuzzing mode Dropbear uses a [fixed seed](dbrandom.c#L185) +every time so that failures can be reproduced. + +Since the fuzzer cannot generate valid encrypted input the packet decryption and +message authentication calls are disabled, see (packet.c)[packet.c]. +MAC failures are set to occur with a low probability to test that error path. + +## Fuzzers + +Current fuzzers are + +- fuzzer-preauth - the fuzzer input is treated as a stream of session input. This will + test key exchange, packet ordering, authentication attempts etc. + +- fuzzer-preauth_nomaths - the same as fuzzer-preauth but with asymmetric crypto + routines replaced with dummies for faster runtime. corpora are shared + between fuzzers by [oss-fuzz](https://github.com/google/oss-fuzz) so this + will help fuzzer-preauth too. + +- fuzzer-verify - read a key and signature from fuzzer input and verify that signature. + It would not be expected to pass, though some keys with bad parameters are + able to validate with a trivial signature - extra checks are added for that. + +- fuzzer-pubkey - test parsing of an `authorized_keys` line. + +- fuzzer-kexdh - test Diffie-Hellman key exchange where the fuzz input is the + public key that would be received over the network. + +- fuzzer-kexecdh - test Elliptic Curve Diffie-Hellman key exchange like fuzzer-kexdh From 6b05aa4275d127d8b0a46643688ecac404bf2bf0 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 14:14:26 +0800 Subject: [PATCH 12/30] fix some links --- FUZZER-NOTES.md | 18 ++++++++++-------- Makefile.in | 2 +- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/FUZZER-NOTES.md b/FUZZER-NOTES.md index b5b5c97..7b88238 100644 --- a/FUZZER-NOTES.md +++ b/FUZZER-NOTES.md @@ -45,28 +45,30 @@ When running in fuzzing mode Dropbear uses a [fixed seed](dbrandom.c#L185) every time so that failures can be reproduced. Since the fuzzer cannot generate valid encrypted input the packet decryption and -message authentication calls are disabled, see (packet.c)[packet.c]. +message authentication calls are disabled, see [packet.c](packet.c). MAC failures are set to occur with a low probability to test that error path. ## Fuzzers Current fuzzers are -- fuzzer-preauth - the fuzzer input is treated as a stream of session input. This will +- [fuzzer-preauth](fuzzer-preauth.c) - the fuzzer input is treated as a stream of session input. This will test key exchange, packet ordering, authentication attempts etc. -- fuzzer-preauth_nomaths - the same as fuzzer-preauth but with asymmetric crypto +- [fuzzer-preauth_nomaths](fuzzer-preauth_nomaths.c) - the same as fuzzer-preauth but with asymmetric crypto routines replaced with dummies for faster runtime. corpora are shared between fuzzers by [oss-fuzz](https://github.com/google/oss-fuzz) so this will help fuzzer-preauth too. -- fuzzer-verify - read a key and signature from fuzzer input and verify that signature. +- [fuzzer-verify](fuzzer-verify.c) - read a key and signature from fuzzer input and verify that signature. It would not be expected to pass, though some keys with bad parameters are able to validate with a trivial signature - extra checks are added for that. -- fuzzer-pubkey - test parsing of an `authorized_keys` line. +- [fuzzer-pubkey](fuzzer-pubkey.c) - test parsing of an `authorized_keys` line. -- fuzzer-kexdh - test Diffie-Hellman key exchange where the fuzz input is the - public key that would be received over the network. +- [fuzzer-kexdh](fuzzer-kexdh.c) - test Diffie-Hellman key exchange where the fuzz input is the + ephemeral public key that would be received over the network. This is testing `mp_expt_mod()` + and and other libtommath routines. -- fuzzer-kexecdh - test Elliptic Curve Diffie-Hellman key exchange like fuzzer-kexdh +- [fuzzer-kexecdh](fuzzer-kexecdh.c) - test Elliptic Curve Diffie-Hellman key exchange like fuzzer-kexdh. + This is testing libtommath ECC routines. diff --git a/Makefile.in b/Makefile.in index 6adc2e0..be2d39e 100644 --- a/Makefile.in +++ b/Makefile.in @@ -270,7 +270,7 @@ fuzzstandalone: fuzz-harness.o fuzz-targets svrfuzzobjs=$(subst svr-main.o, ,$(dropbearobjs)) # build all the fuzzers. This will require fail to link unless built with -# make fuzz-targetsk FUZZLIB=-lFuzzer.a +# make fuzz-targets FUZZLIB=-lFuzzer.a # or similar - the library provides main(). fuzz-targets: $(FUZZ_TARGETS) $(FUZZER_OPTIONS) From 084ff9b4c8adc31e91c6fa6a12a933170a2b7117 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 16:29:57 +0800 Subject: [PATCH 13/30] reduce number of params so it doesn't hit a timeout --- fuzzer-kexdh.c | 3 ++- fuzzer-kexecdh.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c index 60255dd..998a3eb 100644 --- a/fuzzer-kexdh.c +++ b/fuzzer-kexdh.c @@ -9,7 +9,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { static int once = 0; static struct key_context* keep_newkeys = NULL; - #define NUM_PARAMS 800 + /* number of generated parameters is limited by the timeout for the first run */ + #define NUM_PARAMS 300 static struct kex_dh_param *dh_params[NUM_PARAMS]; if (!once) { diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c index db08c2b..e3a648f 100644 --- a/fuzzer-kexecdh.c +++ b/fuzzer-kexecdh.c @@ -10,7 +10,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { static int once = 0; static const struct dropbear_kex *ecdh[3]; /* 256, 384, 521 */ static struct key_context* keep_newkeys = NULL; - #define NUM_PARAMS 800 + /* number of generated parameters is limited by the timeout for the first run */ + #define NUM_PARAMS 300 static struct kex_ecdh_param *ecdh_params[NUM_PARAMS]; if (!once) { From 145fb9698929d247dffb35189397a3daff497431 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 16:50:24 +0800 Subject: [PATCH 14/30] Don't read uninitialised value. From https://github.com/libtom/libtommath/commit/1d03522625f46214733e8e143a4765c01fc146f9 --- libtommath/bn_fast_s_mp_mul_digs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libtommath/bn_fast_s_mp_mul_digs.c b/libtommath/bn_fast_s_mp_mul_digs.c index a1015af..783e7ca 100644 --- a/libtommath/bn_fast_s_mp_mul_digs.c +++ b/libtommath/bn_fast_s_mp_mul_digs.c @@ -87,7 +87,7 @@ int fast_s_mp_mul_digs (mp_int * a, mp_int * b, mp_int * c, int digs) { mp_digit *tmpc; tmpc = c->dp; - for (ix = 0; ix < (pa + 1); ix++) { + for (ix = 0; ix < pa; ix++) { /* now extract the previous digit [below the carry] */ *tmpc++ = W[ix]; } From 149b21d7cf0ab4b68e7b24a42aeb500548b5e50b Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Mon, 5 Mar 2018 21:02:26 +0800 Subject: [PATCH 15/30] ciphers/hashes/kex algorithms won't have string lengths, also use dictionary for fuzzer-preauth_nomaths --- fuzzer-preauth.dict | 42 ++++++++++++++++++------------------- fuzzer-preauth_nomaths.dict | 33 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 21 deletions(-) create mode 100644 fuzzer-preauth_nomaths.dict diff --git a/fuzzer-preauth.dict b/fuzzer-preauth.dict index 0419a1c..b1d15d9 100644 --- a/fuzzer-preauth.dict +++ b/fuzzer-preauth.dict @@ -1,29 +1,29 @@ -"\x00\x00\x00\x0aaes128-ctr" -"\x00\x00\x00\x0aaes256-ctr" -"\x00\x00\x00\x0aaes128-cbc" -"\x00\x00\x00\x0aaes256-cbc" -"\x00\x00\x00\x083des-ctr" -"\x00\x00\x00\x083des-cbc" -"\x00\x00\x00\x0chmac-sha1-96" -"\x00\x00\x00\x09hmac-sha1" -"\x00\x00\x00\x0dhmac-sha2-256" -"\x00\x00\x00\x10zlib@openssh.com" -"\x00\x00\x00\x04zlib" -"\x00\x00\x00\x04none" +"aes128-ctr" +"aes256-ctr" +"aes128-cbc" +"aes256-cbc" +"3des-ctr" +"3des-cbc" +"hmac-sha1-96" +"hmac-sha1" +"hmac-sha2-256" +"zlib@openssh.com" +"zlib" +"none" "\x00\x00\x00\x13ecdsa-sha2-nistp256" "\x00\x00\x00\x13ecdsa-sha2-nistp384" "\x00\x00\x00\x13ecdsa-sha2-nistp521" "\x00\x00\x00\x07ssh-rsa" "\x00\x00\x00\x07ssh-dss" -"\x00\x00\x00\x11curve25519-sha256" -"\x00\x00\x00\x1ccurve25519-sha256@libssh.org" -"\x00\x00\x00\x12ecdh-sha2-nistp521" -"\x00\x00\x00\x12ecdh-sha2-nistp384" -"\x00\x00\x00\x12ecdh-sha2-nistp256" -"\x00\x00\x00\x1ddiffie-hellman-group14-sha256" -"\x00\x00\x00\x1bdiffie-hellman-group14-sha1" -"\x00\x00\x00\x1adiffie-hellman-group1-sha1" -"\x00\x00\x00\x19kexguess2@matt.ucc.asn.au" +"curve25519-sha256" +"curve25519-sha256@libssh.org" +"ecdh-sha2-nistp521" +"ecdh-sha2-nistp384" +"ecdh-sha2-nistp256" +"diffie-hellman-group14-sha256" +"diffie-hellman-group14-sha1" +"diffie-hellman-group1-sha1" +"kexguess2@matt.ucc.asn.au" "\x00\x00\x00\x0cssh-userauth" "\x00\x00\x00\x0essh-connection" "\x00\x00\x00\x09publickey" diff --git a/fuzzer-preauth_nomaths.dict b/fuzzer-preauth_nomaths.dict new file mode 100644 index 0000000..b1d15d9 --- /dev/null +++ b/fuzzer-preauth_nomaths.dict @@ -0,0 +1,33 @@ +"aes128-ctr" +"aes256-ctr" +"aes128-cbc" +"aes256-cbc" +"3des-ctr" +"3des-cbc" +"hmac-sha1-96" +"hmac-sha1" +"hmac-sha2-256" +"zlib@openssh.com" +"zlib" +"none" +"\x00\x00\x00\x13ecdsa-sha2-nistp256" +"\x00\x00\x00\x13ecdsa-sha2-nistp384" +"\x00\x00\x00\x13ecdsa-sha2-nistp521" +"\x00\x00\x00\x07ssh-rsa" +"\x00\x00\x00\x07ssh-dss" +"curve25519-sha256" +"curve25519-sha256@libssh.org" +"ecdh-sha2-nistp521" +"ecdh-sha2-nistp384" +"ecdh-sha2-nistp256" +"diffie-hellman-group14-sha256" +"diffie-hellman-group14-sha1" +"diffie-hellman-group1-sha1" +"kexguess2@matt.ucc.asn.au" +"\x00\x00\x00\x0cssh-userauth" +"\x00\x00\x00\x0essh-connection" +"\x00\x00\x00\x09publickey" +"\x00\x00\x00\x08password" +"\x00\x00\x00\x14keyboard-interactive" +# rsa_asn1_magic +"\x00\x30\x21\x30\x09\x06\x05\x2b\x0e\x03\x02\x1a\x05\x00\x04\x14" From 5d065258da487301f269e795694702b5349de69f Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 6 Mar 2018 21:00:09 +0800 Subject: [PATCH 16/30] reduce number of dh parameters so fuzzer doesn't timeout --- fuzzer-kexdh.c | 2 +- fuzzer-kexecdh.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c index 998a3eb..6a2b329 100644 --- a/fuzzer-kexdh.c +++ b/fuzzer-kexdh.c @@ -10,7 +10,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { static int once = 0; static struct key_context* keep_newkeys = NULL; /* number of generated parameters is limited by the timeout for the first run */ - #define NUM_PARAMS 300 + #define NUM_PARAMS 80 static struct kex_dh_param *dh_params[NUM_PARAMS]; if (!once) { diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c index e3a648f..e97682c 100644 --- a/fuzzer-kexecdh.c +++ b/fuzzer-kexecdh.c @@ -11,7 +11,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { static const struct dropbear_kex *ecdh[3]; /* 256, 384, 521 */ static struct key_context* keep_newkeys = NULL; /* number of generated parameters is limited by the timeout for the first run */ - #define NUM_PARAMS 300 + #define NUM_PARAMS 80 static struct kex_ecdh_param *ecdh_params[NUM_PARAMS]; if (!once) { From a60725740b65be8679afe612f7fec2639ed0eab7 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 6 Mar 2018 21:51:51 +0800 Subject: [PATCH 17/30] workaround memory sanitizer FD_ZERO false positives --- common-session.c | 11 ++++++----- dbrandom.c | 2 +- dbutil.h | 7 +++++++ fuzz-wrapfd.c | 6 ++++-- svr-main.c | 2 +- sysoptions.h | 11 +++++++++++ 6 files changed, 30 insertions(+), 9 deletions(-) diff --git a/common-session.c b/common-session.c index 41bf5b3..96dd4dc 100644 --- a/common-session.c +++ b/common-session.c @@ -152,8 +152,9 @@ void session_loop(void(*loophandler)(void)) { timeout.tv_sec = select_timeout(); timeout.tv_usec = 0; - FD_ZERO(&writefd); - FD_ZERO(&readfd); + DROPBEAR_FD_ZERO(&writefd); + DROPBEAR_FD_ZERO(&readfd); + dropbear_assert(ses.payload == NULL); /* We get woken up when signal handlers write to this pipe. @@ -204,8 +205,8 @@ void session_loop(void(*loophandler)(void)) { * want to iterate over channels etc for reading, to handle * server processes exiting etc. * We don't want to read/write FDs. */ - FD_ZERO(&writefd); - FD_ZERO(&readfd); + DROPBEAR_FD_ZERO(&writefd); + DROPBEAR_FD_ZERO(&readfd); } /* We'll just empty out the pipe if required. We don't do @@ -406,7 +407,7 @@ static int ident_readln(int fd, char* buf, int count) { return -1; } - FD_ZERO(&fds); + DROPBEAR_FD_ZERO(&fds); /* select since it's a non-blocking fd */ diff --git a/dbrandom.c b/dbrandom.c index 838f8ca..0a55bc5 100644 --- a/dbrandom.c +++ b/dbrandom.c @@ -88,7 +88,7 @@ process_file(hash_state *hs, const char *filename, timeout.tv_sec = 2; timeout.tv_usec = 0; - FD_ZERO(&read_fds); + DROPBEAR_FD_ZERO(&read_fds); FD_SET(readfd, &read_fds); res = select(readfd + 1, &read_fds, NULL, NULL, &timeout); if (res == 0) diff --git a/dbutil.h b/dbutil.h index 7d1c3e1..7cb9d68 100644 --- a/dbutil.h +++ b/dbutil.h @@ -88,4 +88,11 @@ char * expand_homedir_path(const char *inpath); void fsync_parent_dir(const char* fn); +#if DROPBEAR_MSAN +/* FD_ZERO seems to leave some memory uninitialized. clear it to avoid false positives */ +#define DROPBEAR_FD_ZERO(fds) do { memset((fds), 0x0, sizeof(fd_set)); FD_ZERO(fds); } while(0) +#else +#define DROPBEAR_FD_ZERO(fds) FD_ZERO(fds) +#endif + #endif /* DROPBEAR_DBUTIL_H_ */ diff --git a/fuzz-wrapfd.c b/fuzz-wrapfd.c index 313a110..ed8968a 100644 --- a/fuzz-wrapfd.c +++ b/fuzz-wrapfd.c @@ -2,6 +2,8 @@ #include "includes.h" #include "fuzz-wrapfd.h" +#include "dbutil.h" + #include "fuzz.h" #define IOWRAP_MAXFD (FD_SETSIZE-1) @@ -195,7 +197,7 @@ int wrapfd_select(int nfds, fd_set *readfds, fd_set *writefds, nset++; } } - FD_ZERO(readfds); + DROPBEAR_FD_ZERO(readfds); if (nset > 0) { /* set one */ @@ -222,7 +224,7 @@ int wrapfd_select(int nfds, fd_set *readfds, fd_set *writefds, nset++; } } - FD_ZERO(writefds); + DROPBEAR_FD_ZERO(writefds); /* set one */ if (nset > 0) { diff --git a/svr-main.c b/svr-main.c index 6f3144b..0a39b70 100644 --- a/svr-main.c +++ b/svr-main.c @@ -178,7 +178,7 @@ static void main_noinetd() { /* incoming connection select loop */ for(;;) { - FD_ZERO(&fds); + DROPBEAR_FD_ZERO(&fds); /* listening sockets */ for (i = 0; i < listensockcount; i++) { diff --git a/sysoptions.h b/sysoptions.h index 0028199..942e724 100644 --- a/sysoptions.h +++ b/sysoptions.h @@ -318,4 +318,15 @@ If you test it please contact the Dropbear author */ #define DROPBEAR_TRACKING_MALLOC (DROPBEAR_FUZZ) +/* Used to work around Memory Sanitizer false positives */ +#if defined(__has_feature) +# if __has_feature(memory_sanitizer) +# define DROPBEAR_MSAN 1 +# endif +#endif +#ifndef DROPBEAR_MSAN +#define DROPBEAR_MSAN 0 +#endif + + /* no include guard for this file */ From 4fd3160179620e26e90b38ec9b093aa893cd0911 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 6 Mar 2018 22:02:19 +0800 Subject: [PATCH 18/30] fix uninitialised memory in fuzzer codepath --- packet.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packet.c b/packet.c index 90470ee..cacc06d 100644 --- a/packet.c +++ b/packet.c @@ -364,9 +364,11 @@ static int checkmac() { #if DROPBEAR_FUZZ if (fuzz.fuzzing) { - /* fail 1 in 2000 times to test error path. - note that mac_bytes is all zero prior to kex, so don't test ==0 ! */ - unsigned int value = *((unsigned int*)&mac_bytes); + /* fail 1 in 2000 times to test error path. */ + unsigned int value = 0; + if (mac_size > sizeof(value)) { + memcpy(&value, mac_bytes, sizeof(value)); + } if (value % 2000 == 99) { return DROPBEAR_FAILURE; } From e9edbe8bb204b00c7f4b4fda7eeee9d0177934ae Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Tue, 6 Mar 2018 22:18:20 +0800 Subject: [PATCH 19/30] avoid leak of pubkey_options --- fuzzer-pubkey.c | 8 +++++++- svr-authpubkey.c | 4 ++++ svr-authpubkeyoptions.c | 1 - 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/fuzzer-pubkey.c b/fuzzer-pubkey.c index a062e1f..033f496 100644 --- a/fuzzer-pubkey.c +++ b/fuzzer-pubkey.c @@ -30,10 +30,16 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { if (have_algo(algoname, algolen, sshhostkey) == DROPBEAR_FAILURE) { dropbear_exit("fuzzer imagined a bogus algorithm"); } - fuzz_checkpubkey_line(line, 5, "/home/me/authorized_keys", + + int ret = fuzz_checkpubkey_line(line, 5, "/home/me/authorized_keys", algoname, algolen, keyblob->data, keyblob->len); + if (ret == DROPBEAR_SUCCESS) { + /* fuzz_checkpubkey_line() should have cleaned up for failure */ + svr_pubkey_options_cleanup(); + } + buf_free(line); buf_free(keyblob); m_free(algoname); diff --git a/svr-authpubkey.c b/svr-authpubkey.c index 0ca0ea4..e97b158 100644 --- a/svr-authpubkey.c +++ b/svr-authpubkey.c @@ -167,6 +167,10 @@ out: sign_key_free(key); key = NULL; } + /* Retain pubkey options only if auth succeeded */ + if (!ses.authstate.authdone) { + svr_pubkey_options_cleanup(); + } TRACE(("leave pubkeyauth")) } diff --git a/svr-authpubkeyoptions.c b/svr-authpubkeyoptions.c index 19f07b9..9498b64 100644 --- a/svr-authpubkeyoptions.c +++ b/svr-authpubkeyoptions.c @@ -113,7 +113,6 @@ void svr_pubkey_options_cleanup() { m_free(ses.authstate.pubkey_options->forced_command); } m_free(ses.authstate.pubkey_options); - ses.authstate.pubkey_options = NULL; } } From ed4c38ba467618a7193f4e5dec1d5f0169e0c227 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 7 Mar 2018 22:14:36 +0800 Subject: [PATCH 20/30] fix leak in option handling --- svr-authpubkeyoptions.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/svr-authpubkeyoptions.c b/svr-authpubkeyoptions.c index 9498b64..ba6f698 100644 --- a/svr-authpubkeyoptions.c +++ b/svr-authpubkeyoptions.c @@ -168,6 +168,12 @@ int svr_add_pubkey_options(buffer *options_buf, int line_num, const char* filena if (match_option(options_buf, "command=\"") == DROPBEAR_SUCCESS) { int escaped = 0; const unsigned char* command_start = buf_getptr(options_buf, 0); + + if (ses.authstate.pubkey_options->forced_command) { + /* multiple command= options */ + goto bad_option; + } + while (options_buf->pos < options_buf->len) { const char c = buf_getbyte(options_buf); if (!escaped && c == '"') { From 27828c742c0c5e024aa5dd1a5333d64cb4a1b16c Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 7 Mar 2018 22:16:21 +0800 Subject: [PATCH 21/30] don't allow null characters in authorized_keys --- svr-authpubkey.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/svr-authpubkey.c b/svr-authpubkey.c index e97b158..ec14ec0 100644 --- a/svr-authpubkey.c +++ b/svr-authpubkey.c @@ -201,7 +201,12 @@ static int checkpubkey_line(buffer* line, int line_num, const char* filename, if (line->len < MIN_AUTHKEYS_LINE || line->len > MAX_AUTHKEYS_LINE) { TRACE(("checkpubkey_line: bad line length %d", line->len)) - return DROPBEAR_FAILURE; + goto out; + } + + if (memchr(line->data, 0x0, line->len) != NULL) { + TRACE(("checkpubkey_line: bad line has null char")) + goto out; } /* compare the algorithm. +3 so we have enough bytes to read a space and some base64 characters too. */ From f82933108ded556e2fbd07315d52bdf4c2d4ad09 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Wed, 7 Mar 2018 22:50:32 +0800 Subject: [PATCH 22/30] reduce fuzzer-kexdh params count again, still hitting timeout --- fuzzer-kexdh.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c index 6a2b329..7d3491c 100644 --- a/fuzzer-kexdh.c +++ b/fuzzer-kexdh.c @@ -9,8 +9,10 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { static int once = 0; static struct key_context* keep_newkeys = NULL; - /* number of generated parameters is limited by the timeout for the first run */ - #define NUM_PARAMS 80 + /* number of generated parameters is limited by the timeout for the first run. + TODO move this to the libfuzzer initialiser function instead if the timeout + doesn't apply there */ + #define NUM_PARAMS 20 static struct kex_dh_param *dh_params[NUM_PARAMS]; if (!once) { From b4b11c8155b09e4d56925d2d4336c10d802adf11 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 22:22:11 +0800 Subject: [PATCH 23/30] #error if no ecc size is chosen --- ecdsa.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecdsa.h b/ecdsa.h index bb3a18e..e86a038 100644 --- a/ecdsa.h +++ b/ecdsa.h @@ -16,7 +16,7 @@ #elif DROPBEAR_ECC_521 #define ECDSA_DEFAULT_SIZE 521 #else -#define ECDSA_DEFAULT_SIZE 0 +#error ECDSA can't be enabled without enabling at least one size (256, 384, 521) #endif ecc_key *gen_ecdsa_priv_key(unsigned int bit_size); From 56855744b8309f6ca672e224ce8f287b5b6ba774 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 22:25:33 +0800 Subject: [PATCH 24/30] Only advertise a single server ecdsa key when -R (generate as required) is specified. Fixes -R now that default ecdsa key size has changed. --- svr-runopts.c | 65 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/svr-runopts.c b/svr-runopts.c index 2c65009..d6c78df 100644 --- a/svr-runopts.c +++ b/svr-runopts.c @@ -526,8 +526,10 @@ static void addhostkey(const char *keyfile) { void load_all_hostkeys() { int i; - int disable_unset_keys = 1; int any_keys = 0; +#ifdef DROPBEAR_ECDSA + int loaded_any_ecdsa = 0; +#endif svr_opts.hostkey = new_sign_key(); @@ -552,14 +554,8 @@ void load_all_hostkeys() { #endif } -#if DROPBEAR_DELAY_HOSTKEY - if (svr_opts.delay_hostkey) { - disable_unset_keys = 0; - } -#endif - #if DROPBEAR_RSA - if (disable_unset_keys && !svr_opts.hostkey->rsakey) { + if (!svr_opts.delay_hostkey && !svr_opts.hostkey->rsakey) { disablekey(DROPBEAR_SIGNKEY_RSA); } else { any_keys = 1; @@ -567,39 +563,54 @@ void load_all_hostkeys() { #endif #if DROPBEAR_DSS - if (disable_unset_keys && !svr_opts.hostkey->dsskey) { + if (!svr_opts.delay_hostkey && !svr_opts.hostkey->dsskey) { disablekey(DROPBEAR_SIGNKEY_DSS); } else { any_keys = 1; } #endif - #if DROPBEAR_ECDSA + /* We want to advertise a single ecdsa algorithm size. + - If there is a ecdsa hostkey at startup we choose that that size. + - If we generate at runtime we choose the default ecdsa size. + - Otherwise no ecdsa keys will be advertised */ + + /* check if any keys were loaded at startup */ + loaded_any_ecdsa = + 0 #if DROPBEAR_ECC_256 - if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 256) - && !svr_opts.hostkey->ecckey256) { - disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP256); - } else { - any_keys = 1; - } + || svr_opts.hostkey->ecckey256 #endif - #if DROPBEAR_ECC_384 - if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 384) - && !svr_opts.hostkey->ecckey384) { - disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP384); - } else { - any_keys = 1; + || svr_opts.hostkey->ecckey384 +#endif +#if DROPBEAR_ECC_521 + || svr_opts.hostkey->ecckey521 +#endif + ; + any_keys |= loaded_any_ecdsa; + + /* Or an ecdsa key could be generated at runtime */ + any_keys |= svr_opts.delay_hostkey; + + /* At most one ecdsa key size will be left enabled */ +#if DROPBEAR_ECC_256 + if (!svr_opts.hostkey->ecckey256 + && (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 256 )) { + disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP256); + } +#endif +#if DROPBEAR_ECC_384 + if (!svr_opts.hostkey->ecckey384 + && (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 384 )) { + disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP384); } #endif - #if DROPBEAR_ECC_521 - if ((disable_unset_keys || ECDSA_DEFAULT_SIZE != 521) - && !svr_opts.hostkey->ecckey521) { + if (!svr_opts.hostkey->ecckey521 + && (!svr_opts.delay_hostkey || loaded_any_ecdsa || ECDSA_DEFAULT_SIZE != 521 )) { disablekey(DROPBEAR_SIGNKEY_ECDSA_NISTP521); - } else { - any_keys = 1; } #endif #endif /* DROPBEAR_ECDSA */ From ba94bcd2e876cf79a66f3f9f4e46473f89c288c8 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 22:37:54 +0800 Subject: [PATCH 25/30] It turns out you can't have a single-quote in an #error --- ecdsa.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ecdsa.h b/ecdsa.h index e86a038..01cb134 100644 --- a/ecdsa.h +++ b/ecdsa.h @@ -16,7 +16,7 @@ #elif DROPBEAR_ECC_521 #define ECDSA_DEFAULT_SIZE 521 #else -#error ECDSA can't be enabled without enabling at least one size (256, 384, 521) +#error ECDSA cannot be enabled without enabling at least one size (256, 384, 521) #endif ecc_key *gen_ecdsa_priv_key(unsigned int bit_size); From 933bc5f8a72342a711ab1b6b41ec0333c4fc55dd Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 23:22:53 +0800 Subject: [PATCH 26/30] Disable wrapfds outside of fuzzed code --- fuzz-harness.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/fuzz-harness.c b/fuzz-harness.c index 53bc71f..be23d4e 100644 --- a/fuzz-harness.c +++ b/fuzz-harness.c @@ -18,6 +18,7 @@ int main(int argc, char ** argv) { #endif } + int old_fuzz_wrapfds = 0; for (i = 1; i < argc; i++) { if (argv[i][0] == '-') { /* ignore arguments */ @@ -29,11 +30,16 @@ int main(int argc, char ** argv) { buf_readfile(input, fn); buf_setpos(input, 0); + fuzz.wrapfds = old_fuzz_wrapfds; printf("Running %s once \n", fn); LLVMFuzzerTestOneInput(input->data, input->len); printf("Running %s twice \n", fn); LLVMFuzzerTestOneInput(input->data, input->len); printf("Done %s\n", fn); + + /* Disable wrapfd so it won't interfere with buf_readfile() above */ + old_fuzz_wrapfds = fuzz.wrapfds; + fuzz.wrapfds = 0; } printf("Finished\n"); From 397af3e6a6ff4041894e2861b5fb0fae5c187997 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 23:23:19 +0800 Subject: [PATCH 27/30] kexhashbuf was much to small in kex fuzzers --- fuzzer-kexdh.c | 3 +-- fuzzer-kexecdh.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c index 7d3491c..f7abea2 100644 --- a/fuzzer-kexdh.c +++ b/fuzzer-kexdh.c @@ -54,8 +54,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { dropbear_exit("Bad kex value"); } - ses.kexhashbuf = buf_new(4); - buf_putint(ses.kexhashbuf, 12345); + ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS); kexdh_comb_key(dh_param, &dh_e, svr_opts.hostkey); /* kexhashbuf is freed in kexdh_comb_key */ diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c index e97682c..693aecb 100644 --- a/fuzzer-kexecdh.c +++ b/fuzzer-kexecdh.c @@ -60,8 +60,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { buffer * ecdh_qs = buf_getstringbuf(fuzz.input); - ses.kexhashbuf = buf_new(4); - buf_putint(ses.kexhashbuf, 12345); + ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS); kexecdh_comb_key(ecdh_param, ecdh_qs, svr_opts.hostkey); /* kexhashbuf is freed in kexdh_comb_key */ From 38c9408cf859525029f6eb4fe00b50259e08e3f2 Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Thu, 8 Mar 2018 23:51:33 +0800 Subject: [PATCH 28/30] avoid leak of ecdh public key --- common-kex.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/common-kex.c b/common-kex.c index 7da3fb7..d4933dd 100644 --- a/common-kex.c +++ b/common-kex.c @@ -694,6 +694,9 @@ void kexecdh_comb_key(struct kex_ecdh_param *param, buffer *pub_them, /* K, the shared secret */ buf_putmpint(ses.kexhashbuf, ses.dh_K); + ecc_free(Q_them); + m_free(Q_them); + /* calculate the hash H to sign */ finish_kexhashbuf(); } From 76933e6c0ab5a72443d1a900d2fb734ec126f97e Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 9 Mar 2018 20:43:11 +0800 Subject: [PATCH 29/30] move dictionaries to corpus repo --- fuzzer-preauth.dict | 33 --------------------------------- fuzzer-preauth_nomaths.dict | 33 --------------------------------- fuzzer-pubkey.dict | 12 ------------ 3 files changed, 78 deletions(-) delete mode 100644 fuzzer-preauth.dict delete mode 100644 fuzzer-preauth_nomaths.dict delete mode 100644 fuzzer-pubkey.dict diff --git a/fuzzer-preauth.dict b/fuzzer-preauth.dict deleted file mode 100644 index b1d15d9..0000000 --- a/fuzzer-preauth.dict +++ /dev/null @@ -1,33 +0,0 @@ -"aes128-ctr" -"aes256-ctr" -"aes128-cbc" -"aes256-cbc" -"3des-ctr" -"3des-cbc" -"hmac-sha1-96" -"hmac-sha1" -"hmac-sha2-256" -"zlib@openssh.com" -"zlib" -"none" -"\x00\x00\x00\x13ecdsa-sha2-nistp256" -"\x00\x00\x00\x13ecdsa-sha2-nistp384" -"\x00\x00\x00\x13ecdsa-sha2-nistp521" -"\x00\x00\x00\x07ssh-rsa" -"\x00\x00\x00\x07ssh-dss" -"curve25519-sha256" -"curve25519-sha256@libssh.org" -"ecdh-sha2-nistp521" -"ecdh-sha2-nistp384" -"ecdh-sha2-nistp256" -"diffie-hellman-group14-sha256" -"diffie-hellman-group14-sha1" -"diffie-hellman-group1-sha1" -"kexguess2@matt.ucc.asn.au" -"\x00\x00\x00\x0cssh-userauth" -"\x00\x00\x00\x0essh-connection" -"\x00\x00\x00\x09publickey" -"\x00\x00\x00\x08password" -"\x00\x00\x00\x14keyboard-interactive" -# rsa_asn1_magic -"\x00\x30\x21\x30\x09\x06\x05\x2b\x0e\x03\x02\x1a\x05\x00\x04\x14" diff --git a/fuzzer-preauth_nomaths.dict b/fuzzer-preauth_nomaths.dict deleted file mode 100644 index b1d15d9..0000000 --- a/fuzzer-preauth_nomaths.dict +++ /dev/null @@ -1,33 +0,0 @@ -"aes128-ctr" -"aes256-ctr" -"aes128-cbc" -"aes256-cbc" -"3des-ctr" -"3des-cbc" -"hmac-sha1-96" -"hmac-sha1" -"hmac-sha2-256" -"zlib@openssh.com" -"zlib" -"none" -"\x00\x00\x00\x13ecdsa-sha2-nistp256" -"\x00\x00\x00\x13ecdsa-sha2-nistp384" -"\x00\x00\x00\x13ecdsa-sha2-nistp521" -"\x00\x00\x00\x07ssh-rsa" -"\x00\x00\x00\x07ssh-dss" -"curve25519-sha256" -"curve25519-sha256@libssh.org" -"ecdh-sha2-nistp521" -"ecdh-sha2-nistp384" -"ecdh-sha2-nistp256" -"diffie-hellman-group14-sha256" -"diffie-hellman-group14-sha1" -"diffie-hellman-group1-sha1" -"kexguess2@matt.ucc.asn.au" -"\x00\x00\x00\x0cssh-userauth" -"\x00\x00\x00\x0essh-connection" -"\x00\x00\x00\x09publickey" -"\x00\x00\x00\x08password" -"\x00\x00\x00\x14keyboard-interactive" -# rsa_asn1_magic -"\x00\x30\x21\x30\x09\x06\x05\x2b\x0e\x03\x02\x1a\x05\x00\x04\x14" diff --git a/fuzzer-pubkey.dict b/fuzzer-pubkey.dict deleted file mode 100644 index 734629c..0000000 --- a/fuzzer-pubkey.dict +++ /dev/null @@ -1,12 +0,0 @@ -"\x00\x00\x00\x07ssh-rsa" -"\x00\x00\x00\x07ssh-dss" -"\x00\x00\x00\x13ecdsa-sha2-nistp256" -"\x00\x00\x00\x13ecdsa-sha2-nistp384" -"\x00\x00\x00\x13ecdsa-sha2-nistp521" -"no-port-forwarding" -"no-port-forwarding" -"no-agent-forwarding" -"no-X11-forwarding" -"no-pty" -"command=\"" -"#" From d740dc548924f2faf0934e5f9a4b83d2b5d6902d Mon Sep 17 00:00:00 2001 From: Matt Johnston Date: Fri, 9 Mar 2018 23:16:37 +0800 Subject: [PATCH 30/30] Fix leaks in kex fuzzers --- fuzzer-kexdh.c | 6 +++++- fuzzer-kexecdh.c | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/fuzzer-kexdh.c b/fuzzer-kexdh.c index f7abea2..224ff58 100644 --- a/fuzzer-kexdh.c +++ b/fuzzer-kexdh.c @@ -57,10 +57,14 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS); kexdh_comb_key(dh_param, &dh_e, svr_opts.hostkey); - /* kexhashbuf is freed in kexdh_comb_key */ + mp_clear(ses.dh_K); m_free(ses.dh_K); mp_clear(&dh_e); + buf_free(ses.hash); + buf_free(ses.session_id); + /* kexhashbuf is freed in kexdh_comb_key */ + m_malloc_free_epoch(1, 0); } else { m_malloc_free_epoch(1, 1); diff --git a/fuzzer-kexecdh.c b/fuzzer-kexecdh.c index 693aecb..c3a450a 100644 --- a/fuzzer-kexecdh.c +++ b/fuzzer-kexecdh.c @@ -63,10 +63,14 @@ int LLVMFuzzerTestOneInput(const uint8_t *Data, size_t Size) { ses.kexhashbuf = buf_new(KEXHASHBUF_MAX_INTS); kexecdh_comb_key(ecdh_param, ecdh_qs, svr_opts.hostkey); - /* kexhashbuf is freed in kexdh_comb_key */ + mp_clear(ses.dh_K); m_free(ses.dh_K); buf_free(ecdh_qs); + buf_free(ses.hash); + buf_free(ses.session_id); + /* kexhashbuf is freed in kexdh_comb_key */ + m_malloc_free_epoch(1, 0); } else { m_malloc_free_epoch(1, 1);