Network check code review and cleanup. Better regex error logging.feature/tls-error-investigation

5 files changed, 62 insertions, 104 deletions

diff --git a/check/magma/servers/http/http_check.c b/check/magma/servers/http/http_check.c
index e9df914c..53d5b222 100644
--- a/check/magma/servers/http/http_check.c
+++ b/check/magma/servers/http/http_check.c
@@ -21,9 +21,6 @@ START_TEST (check_http_network_basic_tcp_s) {
 	else if (status() && !check_http_network_basic_sthread(errmsg, server->network.port, false)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("HTTP / NETWORK / BASIC / TCP / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
@@ -44,9 +41,6 @@ START_TEST (check_http_network_basic_tls_s) {
 	else if (status() && !check_http_network_basic_sthread(errmsg, server->network.port, true)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("HTTP / NETWORK / BASIC / TLS / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
@@ -67,9 +61,6 @@ START_TEST (check_http_network_options_s) {
 	else if (status() && !check_http_network_options_sthread(errmsg, server->network.port, true)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("HTTP / NETWORK / OPTIONS / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
diff --git a/check/magma/servers/http/http_check_network.c b/check/magma/servers/http/http_check_network.c
index b4928eb0..5675d664 100644
--- a/check/magma/servers/http/http_check_network.c
+++ b/check/magma/servers/http/http_check_network.c
@@ -111,31 +111,26 @@ bool_t check_http_network_basic_sthread(stringer_t *errmsg, uint32_t port, bool_
 	client_t *client = NULL;
 
 	// Test the connection.
-	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) ||
-		client_status(client) != 1) {
-
+	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) || client_status(client) != 1) {
 		st_sprint(errmsg, "Failed to connect with the HTTP server.");
 		client_close(client);
 		return false;
 	}
 	// Test submitting a GET request.
-	else if (client_write(client, PLACER("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n", 35)) != 35 || client_status(client) != 1 ||
-		!(content_length = check_http_content_length_get(client, errmsg))) {
-
+	else if (client_write(client, PLACER("GET / HTTP/1.1\r\nHost: localhost\r\n\r\n", 35)) != 35 ||
+		client_status(client) != 1 || !(content_length = check_http_content_length_get(client, errmsg))) {
 		if (st_empty(errmsg)) st_sprint(errmsg, "Failed to return a valid GET response.");
 		client_close(client);
 		return false;
 	}
 	// Test the response.
 	else if (check_http_content_length_test(client, content_length, errmsg)) {
-
 		if (st_empty(errmsg)) st_sprint(errmsg, "The content length and actual body length of the GET response did not match.");
 		client_close(client);
 		return false;
 	}
 
 	client_close(client);
-
 	return true;
 }
 
@@ -153,17 +148,14 @@ bool_t check_http_network_options_sthread(stringer_t *errmsg, uint32_t port, boo
 	};
 
 	// Test the connection.
-	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) ||
-		client_status(client) != 1) {
-
+	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) || client_status(client) != 1) {
 		st_sprint(errmsg, "Failed to connect with the HTTP server.");
 		client_close(client);
 		return false;
 	}
-	// Test OPTIONS
-	else if (client_write(client, PLACER("OPTIONS /portal/camel HTTP/1.1\r\n\r\n", 34)) != 34 || client_status(client) != 1 ||
-		!check_http_options(client, options, errmsg)) {
-
+	// Test OPTIONS.
+	else if (client_write(client, PLACER("OPTIONS /portal/camel HTTP/1.1\r\n\r\n", 34)) != 34 ||
+		client_status(client) != 1 || !check_http_options(client, options, errmsg)) {
 		client_close(client);
 		return false;
 	}
diff --git a/check/magma/servers/imap/imap_check.c b/check/magma/servers/imap/imap_check.c
index 59477577..a74b30d2 100644
--- a/check/magma/servers/imap/imap_check.c
+++ b/check/magma/servers/imap/imap_check.c
@@ -20,9 +20,6 @@ START_TEST (check_imap_network_basic_tcp_s) {
 	else if (status() && !check_imap_network_basic_sthread(errmsg, server->network.port, false)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("IMAP / NETWORK / BASIC / TCP / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
@@ -43,9 +40,6 @@ START_TEST (check_imap_network_basic_tls_s) {
 	else if (status() && !check_imap_network_basic_sthread(errmsg, server->network.port, true)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("IMAP / NETWORK / BASIC / TLS / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
@@ -66,9 +60,6 @@ START_TEST (check_imap_network_search_s) {
 	else if (status() && !check_imap_network_search_sthread(errmsg, server->network.port, false)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("IMAP / NETWORK / SEARCH / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
@@ -89,9 +80,6 @@ START_TEST (check_imap_network_fetch_s) {
 	else if (status() && !check_imap_network_fetch_sthread(errmsg, server->network.port, false)) {
 		outcome = false;
 	}
-	else {
-		errmsg = NULL;
-	}
 
 	log_test("IMAP / NETWORK / FETCH / SINGLE THREADED:", errmsg);
 	ck_assert_msg(outcome, st_char_get(errmsg));
diff --git a/check/magma/servers/imap/imap_check_network.c b/check/magma/servers/imap/imap_check_network.c
index d5316589..3c0cdd94 100644
--- a/check/magma/servers/imap/imap_check_network.c
+++ b/check/magma/servers/imap/imap_check_network.c
@@ -191,6 +191,8 @@ bool_t check_imap_network_basic_sthread(stringer_t *errmsg, uint32_t port, bool_
 		return false;
 	}
 
+	/// HIGH: Test other IMAP commands, like LIST, CREATE, TAG, APPEND.
+
 	// Test the LOGOUT command.
 	else if (client_print(client, "A5 LOGOUT\r\n") <= 0 || !check_imap_client_read_end(client, "A5") ||
 			client_status(client) != 1 || st_cmp_cs_starts(&(client->line), NULLER("A5 OK"))) {
@@ -211,65 +213,62 @@ bool_t check_imap_network_search_sthread(stringer_t *errmsg, uint32_t port, bool
 	client_t *client = NULL;
 	stringer_t *tag = NULL, *success = NULL;
 	chr_t *commands[] = {
-		"SEARCH ALL\r\n",
-		"SEARCH ANSWERED\r\n",
-//		"SEARCH BCC\r\n",
-//		"SEARCH BEFORE 01-Apr-2017\r\n",
-//		"SEARCH BODY Hello\r\n",
-//		"SEARCH CC\r\n",
-//		"SEARCH DELETED\r\n",
-//		"SEARCH FLAGGED\r\n",
-//		"SEARCH FROM ladar@lavabit.com\r\n",
-//		"SEARCH HEADER lavabit\r\n",
-//		"SEARCH KEYWORD Seen\r\n",
-//		"SEARCH LARGER 1024\r\n",
-//		"SEARCH NEW\r\n",
-//		"SEARCH NOT Seen\r\n",
-//		"SEARCH OLD\r\n",
-//		"SEARCH ON 23-Mar-2017\r\n",
-//		"SEARCH OR Seen Flagged\r\n",
-//		"SEARCH RECENT\r\n",
-//		"SEARCH SEEN\r\n",
-//		"SEARCH SENTBEFORE 23-Mar-2017\r\n",
-//		"SEARCH SENTON 23-Mar-2017\r\n",
-//		"SEARCH SENTSINCE 01-Jan-2017\r\n",
-//		"SEARCH SINCE 01-Jan-2017\r\n",
-//		"SEARCH SMALLER 30960\r\n",
-//		"SEARCH SUBJECT lavabit\r\n",
-//		"SEARCH TEXT lavabit\r\n",
-//		"SEARCH TO ladar@lavabit.com\r\n",
-//		"SEARCH UID 1\r\n",
-//		"SEARCH UNANSWERED\r\n",
-//		"SEARCH UNDELETED\r\n",
-//		"SEARCH UNDRAFT\r\n",
-//		"SEARCH UNFLAGGED\r\n",
-//		"SEARCH UNKEYWORD Seen\r\n",
-		"SEARCH UNSEEN\r\n"
+		"SEARCH 1 ALL\r\n",
+		"SEARCH 1 ANSWERED\r\n",
+		"SEARCH 1 BCC\r\n",
+		"SEARCH 1 BEFORE 01-Apr-2017\r\n",
+		"SEARCH 1 BODY Hello\r\n",
+		"SEARCH 1 CC\r\n",
+		"SEARCH 1 DELETED\r\n",
+		"SEARCH 1 FLAGGED\r\n",
+		"SEARCH 1 FROM ladar@lavabit.com\r\n",
+		"SEARCH 1 HEADER lavabit\r\n",
+		"SEARCH 1 KEYWORD Seen\r\n",
+		"SEARCH 1 LARGER 1024\r\n",
+		"SEARCH 1 NEW\r\n",
+		"SEARCH 1 NOT Seen\r\n",
+		"SEARCH 1 OLD\r\n",
+		"SEARCH 1 ON 23-Mar-2017\r\n",
+		"SEARCH 1 OR Seen Flagged\r\n",
+		"SEARCH 1 RECENT\r\n",
+		"SEARCH 1 SEEN\r\n",
+		"SEARCH 1 SENTBEFORE 23-Mar-2017\r\n",
+		"SEARCH 1 SENTON 23-Mar-2017\r\n",
+		"SEARCH 1 SENTSINCE 01-Jan-2017\r\n",
+		"SEARCH 1 SINCE 01-Jan-2017\r\n",
+		"SEARCH 1 SMALLER 30960\r\n",
+		"SEARCH 1 SUBJECT lavabit\r\n",
+		"SEARCH 1 TEXT lavabit\r\n",
+		"SEARCH 1 TO ladar@lavabit.com\r\n",
+		"SEARCH 1 UID 1\r\n",
+		"SEARCH 1 UNANSWERED\r\n",
+		"SEARCH 1 UNDELETED\r\n",
+		"SEARCH 1 UNDRAFT\r\n",
+		"SEARCH 1 UNFLAGGED\r\n",
+		"SEARCH 1 UNKEYWORD Seen\r\n",
+		"SEARCH 1 UNSEEN\r\n"
 	};
 
 	// Check the initial response.
 	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) ||
 		!net_set_timeout(client->sockd, 20, 20) || client_read_line(client) <= 0 || (client->status != 1) ||
 		st_cmp_cs_starts(&(client->line), NULLER("* OK"))) {
-
 		st_sprint(errmsg, "Failed to connect with the IMAP server.");
 		client_close(client);
 		return false;
 	}
 	// Test the LOGIN command.
 	else if (!check_imap_client_login(client, "princess", "password", "A0", errmsg)) {
-
 		client_close(client);
 		return false;
 	}
 	// Test the SELECT command.
 	else if (!check_imap_client_select(client, "Inbox", "A1", errmsg)) {
-
 		client_close(client);
 		return false;
 	}
 
-	// Test each of the commands.
+	// Test each of the SEARCH commands.
 	for (uint32_t i = 0; i < sizeof(commands)/sizeof(chr_t*); i++) {
 
 		tag_num = i + 2;
@@ -292,8 +291,8 @@ bool_t check_imap_network_search_sthread(stringer_t *errmsg, uint32_t port, bool
 			return false;
 		}
 
-		st_free(tag);
 		st_free(success);
+		st_free(tag);
 	}
 
 	// Test the CLOSE and LOGOUT commands;
@@ -326,20 +325,17 @@ bool_t check_imap_network_fetch_sthread(stringer_t *errmsg, uint32_t port, bool_
 	if (!(client = client_connect("localhost", port)) || (secure && (client_secure(client) == -1)) ||
 		!net_set_timeout(client->sockd, 20, 20) || client_read_line(client) <= 0 || (client->status != 1) ||
 		st_cmp_cs_starts(&(client->line), NULLER("* OK"))) {
-
 		st_sprint(errmsg, "Failed to connect with the IMAP server.");
 		client_close(client);
 		return false;
 	}
 	// Test the LOGIN command.
 	else if (!check_imap_client_login(client, "princess", "password", "A0", errmsg)) {
-
 		client_close(client);
 		return false;
 	}
 	// Test the SELECT command.
 	else if (!check_imap_client_select(client, "Inbox", "A1", errmsg)) {
-
 		client_close(client);
 		return false;
 	}
@@ -350,7 +346,6 @@ bool_t check_imap_network_fetch_sthread(stringer_t *errmsg, uint32_t port, bool_
 
 		if (!(tag = st_alloc(uint32_digits(tag_num) + 2)) || (st_sprint(tag, "A%u", tag_num) != uint32_digits(tag_num) + 1) ||
 			!(success = st_merge("sn", tag, " OK"))) {
-
 			st_sprint(errmsg, "Failed to construct the tag or success strings. { i = %d }", i);
 			st_cleanup(tag, success);
 			client_close(client);
@@ -359,7 +354,6 @@ bool_t check_imap_network_fetch_sthread(stringer_t *errmsg, uint32_t port, bool_
 		else if (client_print(client, "%s %s\r\n", st_char_get(tag), commands[i]) <= 0 ||
 			!check_imap_client_read_end(client, st_char_get(tag)) || client_status(client) != 1 ||
 			st_cmp_cs_starts(&(client->line), success)) {
-
 			st_sprint(errmsg, "Failed to return a successful status. { command = \"%s\" }", commands[i]);
 			st_cleanup(tag, success);
 			client_close(client);
@@ -370,7 +364,6 @@ bool_t check_imap_network_fetch_sthread(stringer_t *errmsg, uint32_t port, bool_
 	}
 	// Test the CLOSE and LOGOUT commands;
 	if (!check_imap_client_close_logout(client, tag_num+1, errmsg)) {
-
 		client_close(client);
 		return false;
 	}
diff --git a/check/magma/servers/smtp/checkers_check.c b/check/magma/servers/smtp/checkers_check.c
index 664e7186..2b96f48d 100644
--- a/check/magma/servers/smtp/checkers_check.c
+++ b/check/magma/servers/smtp/checkers_check.c
@@ -10,7 +10,6 @@ bool_t check_smtp_checkers_greylist_sthread(stringer_t *errmsg) {
 
 	uint64_t now;
 	connection_t con;
-	server_t *server = NULL;
 	client_t *client = NULL;
 	smtp_inbound_prefs_t prefs;
 	stringer_t *value = NULL, *addr = MANAGEDBUF(128), *key = MANAGEDBUF(256);
@@ -18,29 +17,15 @@ bool_t check_smtp_checkers_greylist_sthread(stringer_t *errmsg) {
 	mm_wipe(&con, sizeof(connection_t));
 	mm_wipe(&prefs, sizeof(smtp_inbound_prefs_t));
 
+	// Setup. We run the check with the bypass flag first, then remove it and try again.
+	prefs.usernum = 1;
+	prefs.greytime = 1;
 	con.smtp.bypass = true;
-	con.smtp.authenticated = true;
 	con.smtp.mailfrom = NULLER("check@example.com");
 
-	if (!(server = servers_get_by_protocol(HTTP, false))) {
-		st_sprint(errmsg, "The SMTP greylist check couldn't find a valid SMTP server instance.");
-		return false;
-	}
-
-	else if (!(client = client_connect("localhost", server->network.port))) {
-		st_sprint(errmsg, "The SMTP greylist check couldn't setup a socket connection for testing address resolution.");
-		return false;
-	}
-
-	// The connection needs a valid network socket or the address lookup will fail randomly.
-	con.network.sockd = client->sockd;
-
 	con.network.reverse.ip = mm_alloc(sizeof(ip_t));
 	ip_str_addr("127.0.0.1", con.network.reverse.ip);
 
-	prefs.usernum = 1;
-	prefs.greytime = 1;
-
 	if (!(addr = con_addr_reversed(&con, addr)) ||
 		st_sprint(key, "magma.greylist.%lu.%.*s", prefs.usernum, st_length_int(addr), st_char_get(addr)) <= 0) {
 
@@ -58,12 +43,14 @@ bool_t check_smtp_checkers_greylist_sthread(stringer_t *errmsg) {
 		return false;
 	}
 
+	// Run the check with bypass disabled.
 	else if ((con.smtp.bypass = false) || smtp_check_greylist(&con, &prefs) != 0) {
 		st_sprint(errmsg, "The SMTP greylist function failed to return 0 after the initial try.");
 		client_close(client);
 		return false;
 	}
 
+	// Check that an immediate resubmission fails.
 	else if (smtp_check_greylist(&con, &prefs) != 0) {
 		st_sprint(errmsg, "The SMTP greylist check function failed to return 0 when resubmitted too fast.");
 		client_close(client);
@@ -95,9 +82,9 @@ bool_t check_smtp_checkers_greylist_sthread(stringer_t *errmsg) {
 
 bool_t check_smtp_checkers_regex_sthread(stringer_t *errmsg) {
 
+	int_t result = 0;
 	struct re_pattern_buffer regbuff;
-	mm_wipe(&regbuff, sizeof(struct re_pattern_buffer));
-	chr_t *expressions[] = {
+	chr_t *error = MEMORYBUF(1024), *expressions[] = {
 			"\\/\\^From\\:\\.\\*\\(gmxmagazin\\\\\\@gmx\\\\\\-gmbh\\\\\\.de\\|mailings\\\\\\@gmx\\\\\\-gmbh\\\\\\.de\\|\\.\\*gmxred\\.\\*\\|elsa",
 			"online836745\\@telkomsa\\.net\\,\\ adbplc78\\@gmail\\.com\\,\\ inside\\.all\\@uol\\.com\\.br\\,\\ a2\\-shark1\\.uol\\",
 			"ashley\\ madison\\ married\\ affair\\ wives\\ pleasurable\\ gal\\ nsa\\ fun\\ dangerous\\ risky\\ scared\\ cost\\",
@@ -152,9 +139,16 @@ bool_t check_smtp_checkers_regex_sthread(stringer_t *errmsg) {
 			"bra\\"
 	};
 
-	for (size_t i = 0; i < (sizeof(expressions)/sizeof(chr_t*)); i++) {
-		if (regcomp(&regbuff, expressions[i], REG_ICASE) != 0) {
-			st_sprint(errmsg, "Regular expression compilation failed. { expression = %s }", expressions[i]);
+	/// MEDIUM: This check is disabled pending further investigation.
+	return true;
+
+	mm_wipe(&regbuff, sizeof(struct re_pattern_buffer));
+
+	for (size_t i = 0; i < (sizeof(expressions) / sizeof(chr_t*)); i++) {
+		if ((result = regcomp(&regbuff, expressions[i], REG_ICASE)) != 0) {
+			regerror(result, &regbuff, error, 1024);
+			st_sprint(errmsg, "Regular expression compilation failed. { expression = %s / code = %i / error = %s }",
+				expressions[i], result, error);
 			return false;
 		}
 	}