modules, tests: Improve environment compatibility and portability.

* Check that snake oil cert exists before starting test, and
  if it doesn't, abort early. (Debian users can run
  'apt intall ssl-cert' if this cert pair is missing.)
* net_telnet: Fix bounds check for TELOPT_TSPEED prior to memmove.
* Get rid of warning on STDERR when running MySQL-dependent tests.
* mod_auth_mysql: Use char buffer instead of single char for gender,
  so that we can pass NULL directly.
* mod_mysql: Fix indeterminate variadic function behavior.

Author: InterLinked1

bbs/crypt.c

@@ -284,7 +284,6 @@ int bbs_password_verify(const char *password, const char *salt, const char *hash
 	return res;
 }
 
-#define BCRYPT_FULL_HASH_LEN 60
 int bbs_password_verify_bcrypt(const char *password, const char *combined)
 {
 	int res;

include/crypt.h

@@ -13,6 +13,8 @@
  *
  */
 
+#define BCRYPT_FULL_HASH_LEN 60
+
 /*!
  * \brief Generate a cryptographically secure random string containing only alphanumeric characters
  * \param[out] buf

include/mod_mysql.h

@@ -38,8 +38,6 @@ int sql_prepare(MYSQL_STMT *stmt, const char *fmt, const char *query);
 /*! \brief Automatically adjust the format string based on whether any arguments are NULL */
 int sql_fmt_autonull(char *fmt, ...);
 
-int sql_bind_param_single(va_list ap, int i, const char *cur, MYSQL_BIND bind[], unsigned long int lengths[], int bind_ints[], long long bind_longs[], char *bind_strings[], MYSQL_TIME bind_dates[], my_bool bind_null[]);
-
 #define sql_prep_bind_exec(stmt, query, fmt, ...) __sql_prep_bind_exec(stmt, query, __FILE__, __LINE__, __func__, fmt, ## __VA_ARGS__)
 
 int __sql_prep_bind_exec(MYSQL_STMT *stmt, const char *query, const char *file, int line, const char *func, const char *fmt, ...);

io/io_tls.c

@@ -690,7 +690,7 @@ static int ssl_load_config(int reload)
 
 	if (!cfg) {
 		if (!reload) {
-			bbs_warning("SSL/TLS will be unavailable since tls.conf is missing\n");
+			bbs_warning("SSL/TLS server will be unavailable since tls.conf is missing\n");
 		}
 		return -1; /* Impossible to do TLS server stuff if we don't know what the server key/cert are */
 	}

modules/mod_auth_mysql.c

@@ -291,14 +291,13 @@ static int invalid_birthday(struct tm *tm)
 }
 
 static int make_user(const char *username, const char *password, const char *fullname, const char *email, const char *phone,
-	const char *address, const char *city, const char *state, const char *zip, const char *dob, char gender)
+	const char *address, const char *city, const char *state, const char *zip, const char *dob, const char *gender)
 {
-	char pw_hash[61];
+	char pw_hash[BCRYPT_FULL_HASH_LEN + 1];
 	MYSQL *mysql = NULL;
 	MYSQL_STMT *stmt;
 	int res = -1;
 	char sql[184];
-	char genderbuf[2] = { gender, '\0' }; /* We can't pass a char directly into sql_prep_bind_exec, we must pass a char* */
 	struct tm birthday;
 	char types[16] = "sssssssssts";
 
@@ -323,8 +322,8 @@ static int make_user(const char *username, const char *password, const char *ful
 	}
 
 	/* Bind parameters and execute */
-	sql_fmt_autonull(types, username, pw_hash, fullname, email, phone, address, city, state, zip, dob ? &birthday : NULL, genderbuf);
-	if (sql_prep_bind_exec(stmt, sql, types, username, pw_hash, fullname, email, phone, address, city, state, zip, dob ? &birthday : NULL, genderbuf)) {
+	sql_fmt_autonull(types, username, pw_hash, fullname, email, phone, address, city, state, zip, dob ? &birthday : NULL, gender);
+	if (sql_prep_bind_exec(stmt, sql, types, username, pw_hash, fullname, email, phone, address, city, state, zip, dob ? &birthday : NULL, gender)) {
 		goto cleanup;
 	}
 	res = 0;
@@ -341,9 +340,8 @@ static int user_register(struct bbs_node *node)
 {
 	/* bcrypt caps password lengths at 72, so that's where that came from */
 	char fullname[64], username[64], password[72], password2[72];
-	char email[64], phone[16] = "", address[64] = "", city[64], state[32], zip[10] = "", dob[11] = "";
+	char email[64], phone[16] = "", address[64] = "", city[64], state[32], zip[10] = "", dob[11] = "", gender[2] = "";
 	char how_heard[256] = "";
-	int gender = 0;
 	int res;
 #define MAX_REG_ATTEMPTS 6
 	int tries = MAX_REG_ATTEMPTS;
@@ -458,12 +456,13 @@ static int user_register(struct bbs_node *node)
 		bbs_node_unbuffer(node); /* We need to be unbuffered for tread */
 		if (register_gender) {
 			for (; tries > 0; tries--) { /* Retries here count less than retries of the main loop */
+				int c;
 				NEG_RETURN(bbs_node_writef(node, "%-*s", REG_QLEN, REG_FMT "\rGender (MFX): ")); /* Erase existing line in case we're retrying */
-				gender = bbs_node_tread(node, MIN_MS(1));
-				NONPOS_RETURN(gender);
-				gender = (char) tolower(gender);
-				if (gender == 'm' || gender == 'f' || gender == 'x') {
-					NEG_RETURN(bbs_node_writef(node, "%c\n", gender)); /* Print response + newline */
+				c = bbs_node_tread(node, MIN_MS(1));
+				NONPOS_RETURN(c);
+				gender[0] = (char) tolower(c);
+				if (gender[0] == 'm' || gender[0] == 'f' || gender[0] == 'x') {
+					NEG_RETURN(bbs_node_writef(node, "%s\n", gender)); /* Print response + newline */
 					break; /* Got a valid response */
 				}
 				/* Invalid, try again */
@@ -497,8 +496,9 @@ static int user_register(struct bbs_node *node)
 	bbs_auth("New registration attempt for user %s from IP %s\n", username, node->ip);
 
 	/* How heard is logged but not passed to make_user */
-	bbs_debug(1, "New registration attempt: name = %s, username = %s, email = %s, phone = %s, address = %s, city = %s, state = %s, zip = %s, dob = %s, gender = %c, how heard = %s\n",
-		fullname, username, email, S_IF(phone), S_IF(address), city, state, S_IF(zip), S_IF(dob), gender ? gender : ' ', S_IF(how_heard));
+	bbs_debug(1, "New registration attempt: "
+		"name = '%s', username = '%s', email = '%s', phone = '%s', address = '%s', city = '%s', state = '%s', zip = '%s', dob = '%s', gender = '%s', how heard = '%s'\n",
+		fullname, username, email, S_IF(phone), S_IF(address), city, state, S_IF(zip), S_IF(dob), S_IF(gender), S_IF(how_heard));
 
 #define NULL_IFEMPTY(s) (!*s ? NULL : s)
 
@@ -538,7 +538,7 @@ static int user_register(struct bbs_node *node)
 	}
 
 	/* Actually create the user */
-	res = make_user(username, password, fullname, email, NULL_IFEMPTY(phone), NULL_IFEMPTY(address), city, state, NULL_IFEMPTY(zip), NULL_IFEMPTY(dob), (char) gender);
+	res = make_user(username, password, fullname, email, NULL_IFEMPTY(phone), NULL_IFEMPTY(address), city, state, NULL_IFEMPTY(zip), NULL_IFEMPTY(dob), NULL_IFEMPTY(gender));
 
 	if (res) {
 		NEG_RETURN(bbs_node_writef(node, "%s%s%s\n", COLOR(COLOR_FAILURE), "Your registration was rejected.", COLOR_RESET));
@@ -547,6 +547,7 @@ static int user_register(struct bbs_node *node)
 	}
 	/* If user registration actually succeeded, then this function call will succeed. If not, it won't. */
 	res = bbs_authenticate(node, username, password);
+	bbs_memzero(password, sizeof(password)); /* No longer need the password */
 	if (res) {
 		/* Something went wrong */
 		NEG_RETURN(bbs_node_writef(node, "%s%s%s\n", COLOR(COLOR_FAILURE), "An error occured in processing your registration.\n", COLOR_RESET));

modules/mod_mysql.c

@@ -208,7 +208,7 @@ int sql_fmt_autonull(char *fmt, ...)
 	return 0;
 }
 
-int sql_bind_param_single(va_list ap, int i, const char *cur, MYSQL_BIND bind[], unsigned long int lengths[], int bind_ints[], long long bind_longs[], char *bind_strings[], MYSQL_TIME bind_dates[], my_bool bind_null[])
+static int sql_bind_param_single(va_list *ap_ptr, int i, const char *cur, MYSQL_BIND bind[], unsigned long int lengths[], int bind_ints[], long long bind_longs[], char *bind_strings[], MYSQL_TIME bind_dates[], my_bool bind_null[])
 {
 	struct tm *tm;
 	char format_char = (char) tolower(*cur);
@@ -219,39 +219,35 @@ int sql_bind_param_single(va_list ap, int i, const char *cur, MYSQL_BIND bind[],
 		bbs_assert(0); /* We'd crash anyways below. */
 	}
 
-#if 0
-	bbs_debug(10, "Executing fmt char: %c\n", format_char);
-#endif
-
 	/* Ref: https://dev.mysql.com/doc/c-api/5.7/en/c-api-prepared-statement-data-structures.html */
 	/* See for C <-> MySQL/MariaDB types: https://dev.mysql.com/doc/c-api/5.7/en/c-api-prepared-statement-type-codes.html */
 	switch (format_char) {
 	case 'i': /* Integer */
-		bind_ints[i] = va_arg(ap, int);
+		bind_ints[i] = va_arg(*ap_ptr, int);
 		/* This is a number type, so there is no need to specify buffer_length */
 		bind[i].buffer_type = MYSQL_TYPE_LONG; /* Yes, this is correct */
 		bind[i].buffer = (char *) &bind_ints[i];
 		bind[i].is_null = &bind_null[i];
 		bind[i].length = 0;
 		break;
 	case 'l': /* Long int */
-		bind_longs[i] = va_arg(ap, long long);
+		bind_longs[i] = va_arg(*ap_ptr, long long);
 		/* This is a number type, so there is no need to specify buffer_length */
 		bind[i].buffer_type = MYSQL_TYPE_LONGLONG;
 		bind[i].buffer = (char *) &bind_longs[i];
 		bind[i].is_null = &bind_null[i];
 		bind[i].length = 0;
 		break;
 	case 'd': /* Double */
-		bind_ints[i] = va_arg(ap, int);
+		bind_ints[i] = va_arg(*ap_ptr, int);
 		/* This is a number type, so there is no need to specify buffer_length */
 		bind[i].buffer_type = MYSQL_TYPE_LONG;
 		bind[i].buffer = (char *) &bind_ints[i];
 		bind[i].is_null = &bind_null[i];
 		bind[i].length = 0;
 		break;
 	case 's': /* String */
-		bind_strings[i] = va_arg(ap, char *);
+		bind_strings[i] = va_arg(*ap_ptr, char *);
 		lengths[i] = strlen(S_IF(bind_strings[i]));
 		if (!bind_strings[i] && !bind_null[i]) {
 			bbs_warning("String at index %d is NULL, but not specified? (Format char: %c)\n", i, *cur);
@@ -263,7 +259,7 @@ int sql_bind_param_single(va_list ap, int i, const char *cur, MYSQL_BIND bind[],
 		bind[i].length = &lengths[i]; /* For strings, we actually do need the length. We'll be able to find it in the array. */
 		break;
 	case 't': /* Date */
-		tm = va_arg(ap, struct tm *);
+		tm = va_arg(*ap_ptr, struct tm *);
 		if (!bind_null[i]) {
 			bind_dates[i].year = (unsigned int) TM_YEAR(tm->tm_year);
 			bind_dates[i].month = (unsigned int) TM_MONTH(tm->tm_mon);
@@ -313,7 +309,10 @@ int __sql_prep_bind_exec(MYSQL_STMT *stmt, const char *query, const char *file,
 
 	va_start(ap, fmt);
 	for (i = 0; i < num_args; i++, cur++) { /* Bind the parameters themselves for this round */
-		if (sql_bind_param_single(ap, (int) i, cur, bind, lengths, bind_ints, bind_longs, bind_strings, bind_dates, bind_null)) {
+		/* On some platforms, we can technically pass ap by value,
+		 * but its value upon returning is technically indeterminate (we're supposed to call va_end after that).
+		 * Since we want to iterate through, we need to explicitly pass the pointer instead. */
+		if (sql_bind_param_single(&ap, (int) i, cur, bind, lengths, bind_ints, bind_longs, bind_strings, bind_dates, bind_null)) {
 			va_end(ap);
 			return -1;
 		}

nets/net_telnet.c

@@ -770,7 +770,7 @@ static int __telnet_process_command(struct bbs_node *node, struct telnet_setting
 			}
 			break;
 		case TELOPT_TSPEED:
-			if (buf[3] == TELQUAL_IS && res >= 3) {
+			if (buf[3] == TELQUAL_IS && res >= 6) {
 				bbs_debug(3, "Terminal speed is %.*s\n", (int) res - 6, buf + 4); /* First 4 bytes are command, and last two are IAC SE */
 				if (res - 6 < (int) len - 1) {
 					memmove(buf, buf + 4, (size_t) res - 6);

tests/test.c

@@ -591,6 +591,7 @@ static int mysql_spawn(void)
 		"--datadir=" MYSQL_DATA_DIR,
 		"--pid-file=" MYSQL_PID_FILE,
 		"--general_log_file=" MYSQL_LOG_FILE,
+		"--expire-logs-days=0", /* Prevents warning about needing --log-bin to make --expire-log-days or --bin-log-expire-log-seconds work */
 		"--log-error=" MYSQL_ERROR_LOG,
 		"--socket=" MYSQL_SOCKET,
 		"--port=" XSTR(MYSQL_PORT),
@@ -606,8 +607,14 @@ static int mysql_spawn(void)
 	TEST_MKDIR(MYSQL_DATA_DIR);
 
 	/* The mysql user exist from installing MySQL as a service */
+	errno = 0;
 	pwd = getpwnam("mysql"); /* Not thread-safe, but we don't need that */
 	if (!pwd) {
+		if (errno == 0) {
+			/* errno can be 0 for certain errors, this happens when the username was not found */
+			bbs_error("User '%s' does not exist, can't run MySQL-dependent test\n", "mysql");
+			return -1;
+		}
 		bbs_error("getpwnam failed: %s\n", strerror(errno));
 		return -1;
 	}
@@ -622,7 +629,7 @@ static int mysql_spawn(void)
 	}
 
 	/* First, initialize the temporary DB: */
-	if (system("mysql_install_db --skip-test-db --user=mysql --ldata=" MYSQL_DATA_DIR " > /dev/null")) { /* Yuck... but why reinvent the wheel? */
+	if (system("mysql_install_db --expire-logs-days=0 --skip-test-db --user=mysql --ldata=" MYSQL_DATA_DIR " > /dev/null")) { /* Yuck... but why reinvent the wheel? */
 		bbs_error("Failed to initialize database\n");
 		return -1;
 	}
@@ -1058,7 +1065,7 @@ static int run_test(const char *filename, int multiple)
 		res = -1;
 	} else {
 		struct timeval start, end;
-		int64_t sec_dif, usec_dif, tot_dif;
+		int64_t sec_dif, usec_dif, tot_dif = 0;
 		int core_before = 0;
 		pid_t childpid = -1;
 		if (eaccess(TEST_ROOT_DIR, R_OK) && mkdir(TEST_ROOT_DIR, 0700)) {
@@ -1131,7 +1138,7 @@ static int run_test(const char *filename, int multiple)
 		if (multiple && !test_autorun) {
 			bbs_debug(2, "Skipping test %s\n", testmod->name);
 			total_fail--;
-			goto cleanup;
+			goto done;
 		}
 		if (option_use_mysql && !mysql_child && mysql_spawn()) {
 			res = -1;
@@ -1214,16 +1221,17 @@ static int run_test(const char *filename, int multiple)
 			bbs_debug(1, "%d soft assertion%s failed\n", soft_assertions_failed, ESS(soft_assertions_failed));
 			res = -1;
 		}
+cleanup:
 		if (res) {
-			fprintf(stderr, "== Test %sFAILED%s: %5lums %-20s %s\n", COLOR(COLOR_FAILURE), COLOR_RESET, tot_dif, testmod->name, testmod->description);
+			fprintf(stderr, "== Test %sFAILED%s: %5lums %-25s %s\n", COLOR(COLOR_FAILURE), COLOR_RESET, tot_dif, testmod->name, testmod->description);
 		} else {
-			fprintf(stderr, "== Test %sPASSED%s: %5lums %-20s %s\n", COLOR(COLOR_SUCCESS), COLOR_RESET, tot_dif, testmod->name, testmod->description);
+			fprintf(stderr, "== Test %sPASSED%s: %5lums %-25s %s\n", COLOR(COLOR_SUCCESS), COLOR_RESET, tot_dif, testmod->name, testmod->description);
 			total_pass++;
 			total_fail--; /* We didn't actually fail so undo that bit */
 		}
 	}
 
-cleanup:
+done:
 	dlclose(lib);
 	if (testmod) {
 		bbs_warning("Test module still registered?\n");

tests/test.h

@@ -75,6 +75,7 @@ struct test_module *TEST_MODULE_SELF_SYM(void);
 #define TEST_ADD_CONFIG_INTO_DIR(filename, dir) TEST_COPY_CONFIG(TEST_CONFIGS_SRC_DIR "/" filename, dir)
 #define TEST_ADD_CONFIG(filename) TEST_ADD_CONFIG_NAME(filename, filename)
 #define TEST_ADD_SUBCONFIG(subdir, filename) TEST_ADD_CONFIG_NAME(subdir "/" filename, filename)
+#define TEST_REQUIRE_FILE(file) if (eaccess(file, R_OK)) { bbs_error("Aborting test, can't access required file '%s': %s\n", file, strerror(errno)); return -1; }
 
 #define TEST_HOSTNAME "bbs.example.com"
 

tests/test_ftps.c

@@ -31,8 +31,12 @@ static int pre(void)
 	test_preload_module("io_tls.so");
 	test_load_module("net_ftp.so");
 
-	TEST_ADD_CONFIG("transfers.conf");
+	/* Not all platforms have it and we don't create it.
+	 * On Debian, can run 'apt-get install ssl-cert' if this cert pair is missing. */
+	TEST_REQUIRE_FILE("/etc/ssl/private/ssl-cert-snakeoil.key");
+
 	TEST_ADD_CONFIG("tls.conf");
+	TEST_ADD_CONFIG("transfers.conf");
 	TEST_ADD_SUBCONFIG("tls", "net_ftp.conf");
 
 	TEST_RESET_MKDIR(TEST_TRANSFER_DIR);