From 7459925bd01698151645f4a3aa72d45294c005b4 Mon Sep 17 00:00:00 2001 From: Olof hagsand Date: Tue, 29 Dec 2020 18:29:06 +0100 Subject: [PATCH] Limited fuzz by AFL committed, see [fuzz/README.md](fuzz/README.md) for details --- CHANGELOG.md | 5 ++ apps/cli/Makefile.in | 10 ++- fuzz/README.md | 12 ++- fuzz/backend/README.md | 2 + fuzz/cli/README.md | 45 ++++++++++++ fuzz/cli/input/1.cli | 1 + fuzz/cli/input/2.cli | 1 + fuzz/cli/input/3.cli | 1 + fuzz/cli/runfuzz.sh | 47 ++++++++++++ include/clixon_custom.h | 11 +++ lib/src/clixon_proto.c | 134 ++++++++++++++++++++++++++++++++++ lib/src/clixon_proto_client.c | 1 + util/Makefile.in | 9 ++- 13 files changed, 276 insertions(+), 3 deletions(-) create mode 100644 fuzz/cli/README.md create mode 100644 fuzz/cli/input/1.cli create mode 100644 fuzz/cli/input/2.cli create mode 100644 fuzz/cli/input/3.cli create mode 100755 fuzz/cli/runfuzz.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ddb988..dafd1c16 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,11 @@ Users may have to change how they access the system * CLIspec dbxml API: Ability to specify deletion of _any_ vs _specific_ entry. * In a cli_del() call, the cvv arg list either exactly matches the api-format-path in which case _any_ deletion is specified, otherwise, if there is an extra element in the cvv list, that is used for a specific delete. +### Minor changes + +* Limited fuzz by AFL committed, + * see [fuzz/README.md](fuzz/README.md) for details + ### Corrected Bugs * [Presence container configs not displayed in 'show config set' #164 ](https://github.com/clicon/clixon/issues/164) diff --git a/apps/cli/Makefile.in b/apps/cli/Makefile.in index cc2bed8a..8d15e120 100644 --- a/apps/cli/Makefile.in +++ b/apps/cli/Makefile.in @@ -46,7 +46,11 @@ else endif SH_SUFFIX = @SH_SUFFIX@ INSTALLFLAGS = @INSTALLFLAGS@ -LDFLAGS = @LDFLAGS@ +ifeq ($(LINKAGE),dynamic) + LDFLAGS = @LDFLAGS@ +else + LDFLAGS = @LDFLAGS@ -rdynamic -L. +endif prefix = @prefix@ datarootdir = @datarootdir@ @@ -160,7 +164,11 @@ test: test.c $(LIBOBJ) $(MYLIB) $(CC) $(INCLUDES) $(LDFLAGS) $< $(LIBOBJ) -L. $(MYLIB) $(LIBS) -o $@ $(APPL): $(APPOBJ) $(MYLIB) $(LIBDEPS) +ifeq ($(LINKAGE),dynamic) $(CC) $(LDFLAGS) $(APPOBJ) -L. $(MYLIB) $(LIBS) -o $@ +else + $(CC) $(LDFLAGS) $(APPOBJ) -L. $(LIBOBJ) $(LIBS) -o $@ +endif $(MYLIBDYNAMIC) : $(LIBOBJ) $(LIBDEPS) ifeq ($(HOST_VENDOR),apple) diff --git a/fuzz/README.md b/fuzz/README.md index 18deafde..0d71bacc 100644 --- a/fuzz/README.md +++ b/fuzz/README.md @@ -1,3 +1,13 @@ # Fuzzing with AFL -This is experimental +Clixon can be fuzzed with [american fuzzy lop](https://github.com/google/AFL/releases) but not without pain. + +So far the backend and cli can be fuzzed. + +Some issues are as follows: +- Static linking. Fuzzing requires static linking. You can statically link clixon using: `LINKAGE=static ./configure` but that does not work with Clixon plugins (at least yet). Therefore fuzzing has been made with no plugins using the hello example only. +- Multiple processes. Only the backend can run stand-alone, cli/netconf/restconf requires a backend. When you fuzz eg clixon_cli, the backend must be running and it will be slow due to IPC. Possibly one could link them together and run as a monolith by making a threaded image. +- Internal protocol 1: The internal protocol uses XML but deviates from netconf by using a (binary) header where the length is encoded, instead of ']]>]]>' as a terminating string. AFL does not like that. By setting CLIXON_PROTO_PLAIN the internal protocol uses pure netconf (with some limitations). +- Internal protocol 2: The internal protocol uses TCP unix sockets while AFL requires stdio. One can use a package called "preeny" to translate stdio into sockets. But it is slow. + +Restconf also has the extra problem of running TSL sockets. \ No newline at end of file diff --git a/fuzz/backend/README.md b/fuzz/backend/README.md index 793e7e0c..9631d243 100644 --- a/fuzz/backend/README.md +++ b/fuzz/backend/README.md @@ -4,6 +4,8 @@ This dir contains code for fuzzing clixon backend. It requires the preeny package to change sockets to stdio. +Plugins do not work + ## Prereqs Preeny has a "desocketizing" module necessary to map stdio to the internal sockets that the backend uses. Install preeny example: diff --git a/fuzz/cli/README.md b/fuzz/cli/README.md new file mode 100644 index 00000000..c6d6998d --- /dev/null +++ b/fuzz/cli/README.md @@ -0,0 +1,45 @@ +# Clixon fuzzing + +This dir contains code for fuzzing clixon cli. + +Note: cli plugins do not work. + +## Prereqs + +See [AFL docs](https://afl-1.readthedocs.io/en/latest) for installing afl. +On ubuntu this may be enough: +``` + sudo apt install afl +``` + +You may have to change cpu frequency: +``` + cd /sys/devices/system/cpu + echo performance | tee cpu?/cpufreq/scaling_governor +``` + +And possibly change core behaviour: +``` + echo core >/proc/sys/kernel/core_pattern +``` + +## Build + +Build clixon statically with the afl-clang compiler: +``` + CC=/usr/bin/afl-clang-fast LINKAGE=static ./configure + make clean + cd apps/cli + make clixon_cli + sudo make install +``` + +## Run tests + +Start the backend and Use the script `runfuzz.sh` to run one test with a cli spec and an input string, eg: +``` + ./runfuzz.sh /usr/local/etc/hello.xml "set table parameter a value 23" +``` + +After (or during) the test, investigate results in the output dir. + diff --git a/fuzz/cli/input/1.cli b/fuzz/cli/input/1.cli new file mode 100644 index 00000000..5ccc80fe --- /dev/null +++ b/fuzz/cli/input/1.cli @@ -0,0 +1 @@ +set hello world diff --git a/fuzz/cli/input/2.cli b/fuzz/cli/input/2.cli new file mode 100644 index 00000000..7d16809c --- /dev/null +++ b/fuzz/cli/input/2.cli @@ -0,0 +1 @@ +show configuration diff --git a/fuzz/cli/input/3.cli b/fuzz/cli/input/3.cli new file mode 100644 index 00000000..49a0751d --- /dev/null +++ b/fuzz/cli/input/3.cli @@ -0,0 +1 @@ +validate diff --git a/fuzz/cli/runfuzz.sh b/fuzz/cli/runfuzz.sh new file mode 100755 index 00000000..f9a41d56 --- /dev/null +++ b/fuzz/cli/runfuzz.sh @@ -0,0 +1,47 @@ +#!/usr/bin/env bash +# Run a fuzzing test using american fuzzy lop +set -eux + +if [ $# -ne 0 ]; then + echo "usage: $0\n" + exit 255 +fi + +APPNAME=example +cfg=conf.xml + +cat < $cfg + + $cfg + *:* + /usr/local/share/clixon + clixon-hello + hello + /usr/local/var/hello.sock + /usr/local/lib/hello/clispec + /usr/local/var/$APPNAME/$APPNAME.pidfile + /usr/local/var/$APPNAME + init + false + +EOF + +#cfg=/usr/local/etc/hello.xml # XXX +# Kill previous +sudo clixon_backend -z -f $cfg -s init + +# Start backend +sudo clixon_backend -f $cfg -s init + +MEGS=500 # memory limit for child process (50 MB) + +# remove input and input dirs +#test ! -d input || rm -rf input +test ! -d output || rm -rf output + +# create if dirs dont exists +#test -d input || mkdir input +test -d output || mkdir output + +# Run script +afl-fuzz -i input -o output -m $MEGS -- clixon_cli -f $cfg diff --git a/include/clixon_custom.h b/include/clixon_custom.h index 3db197f5..5fd14be5 100644 --- a/include/clixon_custom.h +++ b/include/clixon_custom.h @@ -92,3 +92,14 @@ * clixon-4.4 */ #define STATE_ORDERED_BY_SYSTEM + +/*! Make internal XML protocol use plain strings instead of binary header + * Experimental + * This is only for testing, a specific usecase is fuzzing as described in fuzz/backend + * Note session-ids are not handled properly (a bunch of other things too) + * This could be mitigated by sending the session-id as an attribute + * But doing this one should probably revise/remove the code around clicon_msg_encode/decode + * which currently is somewhat hardwired. I.e., it may be difficult to have both variants as ifdef:s + * and you may consider replacing the old code altogether. + */ +#undef CLIXON_PROTO_PLAIN diff --git a/lib/src/clixon_proto.c b/lib/src/clixon_proto.c index 3ebf3a5c..379e227c 100644 --- a/lib/src/clixon_proto.c +++ b/lib/src/clixon_proto.c @@ -67,6 +67,9 @@ /* clicon */ #include "clixon_err.h" #include "clixon_queue.h" +#ifdef CLIXON_PROTO_PLAIN +#include "clixon_event.h" +#endif #include "clixon_hash.h" #include "clixon_handle.h" #include "clixon_log.h" @@ -334,6 +337,20 @@ clicon_msg_send(int s, __FUNCTION__, ntohl(msg->op_len)); if (clicon_debug_get() > 2) msg_dump(msg); +#ifdef CLIXON_PROTO_PLAIN + { + cbuf *cb = NULL; + if ((cb = cbuf_new()) == NULL) + goto done; + cprintf(cb, "%s]]>]]>", (char*)&msg->op_body); + if (atomicio((ssize_t (*)(int, void *, size_t))write, + s, cbuf_get(cb), cbuf_len(cb)+1) < 0){ + clicon_err(OE_CFG, errno, "atomicio"); + clicon_log(LOG_WARNING, "%s: write: %s", __FUNCTION__, strerror(errno)); + goto done; + } + } +#else if (atomicio((ssize_t (*)(int, void *, size_t))write, s, msg, ntohl(msg->op_len)) < 0){ clicon_err(OE_CFG, errno, "atomicio"); @@ -341,11 +358,122 @@ clicon_msg_send(int s, strerror(errno), ntohs(msg->op_len), msg->op_body); goto done; } +#endif /* CLIXON_PROTO_PLAIN */ retval = 0; done: return retval; } +#ifdef CLIXON_PROTO_PLAIN +/*! Receive a message using plain ascii + * @see netconf_input_cb() + */ +static int +clicon_msg_rcv1(int s, + cbuf **cb1, + int *eof) +{ + int retval = -1; + unsigned char buf[BUFSIZ]; + int i; + int len; + cbuf *cb=NULL; + int xml_state = 0; + int poll; + + clicon_debug(1, "%s", __FUNCTION__); + if ((cb = cbuf_new()) == NULL){ + clicon_err(OE_XML, errno, "cbuf_new"); + return retval; + } + memset(buf, 0, sizeof(buf)); + while (1){ + if ((len = read(s, buf, sizeof(buf))) < 0){ + if (errno == ECONNRESET) + len = 0; /* emulate EOF */ + else{ + clicon_log(LOG_ERR, "%s: read: %s", __FUNCTION__, strerror(errno)); + goto done; + } + } /* read */ + if (len == 0){ /* EOF */ + // cc_closed++; + close(s); + goto ok; + } + for (i=0; i]]>", + buf[i], + &xml_state)) { + /* OK, we have an xml string from a client */ + /* Remove trailer */ + *(((char*)cbuf_get(cb)) + cbuf_len(cb) - strlen("]]>]]>")) = '\0'; + *cb1 = cb; + clicon_debug(2, "%s", cbuf_get(cb)); + cb = NULL; + goto ok; + } + } + /* poll==1 if more, poll==0 if none */ + if ((poll = clixon_event_poll(s)) < 0) + goto done; + if (poll == 0) + break; /* No data to read */ + } /* while */ + ok: + retval = 0; + done: + clicon_debug(1, "%s done", __FUNCTION__); + if (cb) + cbuf_free(cb); + // if (cc_closed) + // retval = -1; + return retval; +} + +/*! Receive a message using plain ascii + * @note message is copied once too many + * @note session-id is made up + */ +static int +clicon_msg_rcv_plain(int s, + struct clicon_msg **msg0, + int *eof) +{ + int retval = -1; + cbuf *cb = NULL; + size_t sz; + static int ii = 0; + struct clicon_msg *msg = NULL; + + if (clicon_msg_rcv1(s, &cb, eof) < 0) + goto done; + if (cb == NULL){ + clicon_err(OE_CFG, EFAULT, "unrecognized input"); + *eof = 1; + goto ok; + } + sz = sizeof(struct clicon_msg) + cbuf_len(cb) + 1; + if ((msg = (struct clicon_msg *)malloc(sz)) == NULL){ + clicon_err(OE_CFG, errno, "malloc"); + goto done; + } + memset(msg, 0, sz); + msg->op_len = htonl(sz); + msg->op_id = htonl(ii++); /* XXX seesion-ids are randomized */ + strcpy((char*)&msg->op_body, cbuf_get(cb)); /* XXX message data copied */ + cbuf_free(cb); + *msg0 = msg; + ok: + retval = 0; + done: + return retval; +} +#endif /* CLIXON_PROTO_PLAIN */ + /*! Receive a CLICON message * * XXX: timeout? and signals? @@ -373,6 +501,12 @@ clicon_msg_rcv(int s, sigfn_t oldhandler; uint32_t mlen; +#ifdef CLIXON_PROTO_PLAIN /* for testing, eg fuzzing */ + if (clicon_msg_rcv_plain(s, msg, eof) < 0) + goto done; + return 0; +#endif + *eof = 0; if (0) set_signal(SIGINT, atomicio_sig_handler, &oldhandler); diff --git a/lib/src/clixon_proto_client.c b/lib/src/clixon_proto_client.c index d5a079f6..35d3bf5c 100644 --- a/lib/src/clixon_proto_client.c +++ b/lib/src/clixon_proto_client.c @@ -168,6 +168,7 @@ clicon_rpc_msg(clicon_handle h, * @param[out] session_id Session id * @retval 0 OK and session_id set * @retval -1 Error + * @note This function may send a synchronous(blocking) HELLO request to the backend as a side-effect */ static int session_id_check(clicon_handle h, diff --git a/util/Makefile.in b/util/Makefile.in index f9a9da5f..49901516 100644 --- a/util/Makefile.in +++ b/util/Makefile.in @@ -66,7 +66,14 @@ CPPFLAGS = @CPPFLAGS@ INCLUDES = -I. @INCLUDES@ -I$(top_srcdir)/lib -I$(top_srcdir)/include -CLIXON_LIB = libclixon$(SH_SUFFIX).$(CLIXON_MAJOR).$(CLIXON_MINOR) +ifeq ($(LINKAGE),dynamic) + CLIXON_LIB = libclixon$(SH_SUFFIX).$(CLIXON_MAJOR).$(CLIXON_MINOR) +else + CLIXON_LIB = libclixon.a +endif + +# For dependency. A little strange that we rely on it being built in the src dir +# even though it may exist in $(libdir). But the new version may not have been installed yet. LIBDEPS = $(top_srcdir)/lib/src/$(CLIXON_LIB) # Utilities, unit testings. Not installed.