Avoid strdup() and simplify passsite search

Also, improve code, tests, and documentation

src/opts.c

@@ -1138,7 +1138,7 @@ passsite_str(passsite_t *passsite)
 	return ps;
 }
 
-char *
+static char *
 passsite_sites_str(passsite_site_t *site)
 {
 	char *s = NULL;
@@ -1165,7 +1165,7 @@ passsite_sites_str(passsite_site_t *site)
 	return s;
 }
 
-char *
+static char *
 passsite_ips_str(passsite_ip_t *ip)
 {
 	char *s = NULL;
@@ -1200,7 +1200,7 @@ passsite_ips_str(passsite_ip_t *ip)
 }
 
 #ifndef WITHOUT_USERAUTH
-char *
+static char *
 passsite_users_str(passsite_user_t *user)
 {
 	char *s = NULL;
@@ -1239,7 +1239,7 @@ passsite_users_str(passsite_user_t *user)
 	return s;
 }
 
-char *
+static char *
 passsite_keywords_str(passsite_keyword_t *keyword)
 {
 	char *s = NULL;
@@ -1273,7 +1273,7 @@ passsite_keywords_str(passsite_keyword_t *keyword)
 	return s;
 }
 
-char *
+static char *
 passsite_userkeywords_str(passsite_user_t *user)
 {
 	char *s = NULL;
@@ -1308,7 +1308,7 @@ passsite_userkeywords_str(passsite_user_t *user)
 }
 #endif /* !WITHOUT_USERAUTH */
 
-char *
+static char *
 passsite_filter_str(passsite_filter_t *pf)
 {
 	char *pfs = NULL;
@@ -1370,7 +1370,7 @@ passsite_filter_str(passsite_filter_t *pf)
 }
 
 #ifndef WITHOUT_USERAUTH
-char *
+static char *
 users_str(userlist_t *u)
 {
 	char *us = NULL;
@@ -2283,7 +2283,7 @@ opts_set_passsite(opts_t *opts, char *value, int line_num)
 #endif /* DEBUG_OPTS */
 }
 
-passsite_site_t *
+static passsite_site_t *
 opts_find_site(passsite_site_t *list, char *s)
 {
 	while (list) {

src/opts.h

@@ -314,7 +314,6 @@ void opts_set_ciphers(opts_t *, const char *, const char *) NONNULL(1,2,3);
 void opts_set_ciphersuites(opts_t *, const char *, const char *) NONNULL(1,2,3);
 void opts_set_passsite(opts_t *, char *, int);
 
-passsite_site_t *opts_find_site(passsite_site_t *, char *) NONNULL(2);
 passsite_ip_t *opts_find_ip(passsite_ip_t *, char *) NONNULL(2);
 #ifndef WITHOUT_USERAUTH
 passsite_keyword_t *opts_find_keyword(passsite_keyword_t *, char *) NONNULL(2);

src/protossl.c

@@ -591,25 +591,20 @@ protossl_srccert_create(pxy_conn_ctx_t *ctx)
 }
 
 static int NONNULL(1,2)
-protossl_pass_site(pxy_conn_ctx_t *ctx, char *site)
+protossl_passsite(pxy_conn_ctx_t *ctx, const char *site)
 {
 	log_finest_va("ENTER, %s, %s, %s", site, STRORDASH(ctx->sslctx->sni), STRORDASH(ctx->sslctx->ssl_names));
 
-	// Avoid multithreading issues by duping the site arg as a local var
-	// site == ctx->spec->opts->passsites->site, which is used by all threads
-	// @todo Check if multithreaded read access causes any issues
-	char *_site = strdup(site);
-	if (!_site) {
-		ctx->enomem = 1;
-		return -1;
-	}
-
-	int rv = 0;
-
 	// site has surrounding slashes: "/example.com/"
 	// site is never empty or just "//", @see opts_set_passsite(),
 	// so no need to check if the length of site > 0 or 2
-	size_t len = strlen(_site);
+	size_t len = strlen(site);
+
+	// Avoid multithreading issues by copying the site arg to a local var
+	// site == ctx->spec->opts->passsites->site, which may be being used by other threads
+	// @todo Check if multithreaded read access causes any issues
+	char _site[len + 1];
+	memcpy(_site, site, sizeof _site);
 
 	// Skip the first slash
 	char *s = _site + 1;
@@ -618,13 +613,14 @@ protossl_pass_site(pxy_conn_ctx_t *ctx, char *site)
 		_site[len - 2] = '\0';
 		if (ctx->sslctx->sni && strstr(ctx->sslctx->sni, s)) {
 			log_finest_va("Match substring in sni: %s, %s", ctx->sslctx->sni, s);
-			rv = 1;
-		} else if (ctx->sslctx->ssl_names && strstr(ctx->sslctx->ssl_names, s)) {
+			return 1;
+		}
+		if (ctx->sslctx->ssl_names && strstr(ctx->sslctx->ssl_names, s)) {
 			log_finest_va("Match substring in common names: %s, %s", ctx->sslctx->ssl_names, s);
-			rv = 1;
+			return 1;
 		}
 		// The end of substring search
-		goto out;
+		return 0;
 	}
 	// The start of exact search
 
@@ -635,20 +631,18 @@ protossl_pass_site(pxy_conn_ctx_t *ctx, char *site)
 	// SNI: "example.com"
 	if (ctx->sslctx->sni && !strcmp(ctx->sslctx->sni, s)) {
 		log_finest_va("Match exact with sni: %s", ctx->sslctx->sni);
-		rv = 1;
-		goto out;
+		return 1;
 	}
 
 	// @attention Make sure ssl_names is not null
 	if (!ctx->sslctx->ssl_names) {
-		goto out;
+		return 0;
 	}
 
 	// Single common name: "example.com"
 	if (!strcmp(ctx->sslctx->ssl_names, s)) {
 		log_finest_va("Match exact with single common name: %s", ctx->sslctx->ssl_names);
-		rv = 1;
-		goto out;
+		return 1;
 	}
 
 	// Restore the slash at the end
@@ -657,15 +651,13 @@ protossl_pass_site(pxy_conn_ctx_t *ctx, char *site)
 	// First common name: "example.com/"
 	if (strstr(ctx->sslctx->ssl_names, s) == ctx->sslctx->ssl_names) {
 		log_finest_va("Match exact with the first common name: %s, %s", ctx->sslctx->ssl_names, s);
-		rv = 1;
-		goto out;
+		return 1;
 	}
 
 	// Middle common name: "/example.com/"
 	if (strstr(ctx->sslctx->ssl_names, _site)) {
 		log_finest_va("Match exact with a middle common name: %s, %s", ctx->sslctx->ssl_names, _site);
-		rv = 1;
-		goto out;
+		return 1;
 	}
 
 	// Replace the last slash with null
@@ -674,20 +666,16 @@ protossl_pass_site(pxy_conn_ctx_t *ctx, char *site)
 	// Last common name: "/example.com"
 	if (strstr(ctx->sslctx->ssl_names, _site) == ctx->sslctx->ssl_names + strlen(ctx->sslctx->ssl_names) - strlen(_site)) {
 		log_finest_va("Match exact with the last common name: %s, %s", ctx->sslctx->ssl_names, _site);
-		rv = 1;
+		return 1;
 	}
-out:
-	free(_site);
-	return rv;
+	return 0;
 }
 
 static int
 protossl_apply_passsite(pxy_conn_ctx_t *ctx, passsite_site_t *list)
 {
-	int rv = 0;
 	while (list) {
-		rv = protossl_pass_site(ctx, list->site);
-		if (rv == 1) {
+		if (protossl_passsite(ctx, list->site)) {
 			// Do not print the surrounding slashes
 			log_err_level_printf(LOG_WARNING, "Found passsite: %.*s for %s:%s, %s:%s, "
 #ifndef WITHOUT_USERAUTH
@@ -701,14 +689,11 @@ protossl_apply_passsite(pxy_conn_ctx_t *ctx, passsite_site_t *list)
 #endif /* !WITHOUT_USERAUTH */
 				STRORDASH(ctx->sslctx->sni), STRORDASH(ctx->sslctx->ssl_names));
 			ctx->sslctx->passsite = 1;
-			break;
-		} else if (rv == -1) {
-			// enomem
-			break;
+			return 1;
 		}
 		list = list->next;
 	}
-	return rv;
+	return 0;
 }
 
 /*
@@ -764,18 +749,18 @@ protossl_srcssl_create(pxy_conn_ctx_t *ctx, SSL *origssl)
 				if (ctx->desc) {
 					log_finest_va("Searching user keyword: %s, %s", ctx->user, ctx->desc);
 					passsite_keyword_t *keyword = opts_find_keyword(user->keyword, ctx->desc);
-					if (keyword && (protossl_apply_passsite(ctx, keyword->site) != 0)) {
+					if (keyword && protossl_apply_passsite(ctx, keyword->site)) {
 						goto out;
 					}
 				}
-				if (protossl_apply_passsite(ctx, user->site) != 0) {
+				if (protossl_apply_passsite(ctx, user->site)) {
 					goto out;
 				}
 			}
 			if (ctx->desc) {
 				log_finest_va("Searching keyword: %s", ctx->desc);
 				passsite_keyword_t *keyword = opts_find_keyword(pf->keyword, ctx->desc);
-				if (keyword && (protossl_apply_passsite(ctx, keyword->site) != 0)) {
+				if (keyword && protossl_apply_passsite(ctx, keyword->site)) {
 					goto out;
 				}
 			}
@@ -784,12 +769,12 @@ protossl_srcssl_create(pxy_conn_ctx_t *ctx, SSL *origssl)
 		if (ctx->srchost_str) {
 			log_finest_va("Searching ip: %s", ctx->srchost_str);
 			passsite_ip_t *ip = opts_find_ip(pf->ip, ctx->srchost_str);
-			if (ip && (protossl_apply_passsite(ctx, ip->site) != 0)) {
+			if (ip && protossl_apply_passsite(ctx, ip->site)) {
 				goto out;
 			}
 		}
 		log_finest("Searching all");
-		if (pf->all && (protossl_apply_passsite(ctx, pf->all) != 0)) {
+		if (pf->all && protossl_apply_passsite(ctx, pf->all)) {
 			goto out;
 		}
 #ifndef WITHOUT_USERAUTH

src/sslproxy.conf.5

@@ -108,13 +108,13 @@ Default: drop
 Passthrough site: site[*] [(clientaddr|user|*) [description keyword]]. If 
 the site matches SNI or common names in the SSL certificate, the connection is 
 passed through the proxy. Per site filters can be defined using client IP 
-addresses, users, and description keywords. '*' matches all users. User auth 
-should be enabled for user and description keyword filtering to work. 
-Case is ignored while matching description keywords. Multiple sites are 
-allowed, one on each line. PassSite rules can search for exact or substring 
-matches. Append an asterisk to the site field to search for substring match. 
-Note that the substring search is not a regex or wildcard search, and that the 
-asterisk at the end is removed before search.
+addresses, users, and description keywords. '*' matches all client IP 
+addresses or users. User auth should be enabled for user and description 
+keyword filtering to work. Case is ignored while matching description 
+keywords. Multiple sites are allowed, one on each line. PassSite rules can 
+search for exact or substring matches. Append an asterisk to the site field to 
+search for substring match. Note that the substring search is not a regex or 
+wildcard search, and that the asterisk at the end is removed before search.
 .TP
 \fBDHGroupParams STRING\fR
 Use DH group params from pemfile. Equivalent to -g command line option.

tests/check/opts.t.c

@@ -734,41 +734,52 @@ START_TEST(opts_set_passsite_05)
 	free(s);
 	fail_unless(!opts->passsites->next, "next set");
 
-	s = strdup("example.com 192.168.0.1");
+	s = strdup("example.com *");
 	opts_set_passsite(opts, s, 1);
 	free(s);
 	fail_unless(opts->passsites->next, "next not set");
 	fail_unless(!opts->passsites->next->next, "next->next set");
 
+	s = strdup("example.com 192.168.0.1");
+	opts_set_passsite(opts, s, 2);
+	free(s);
+	fail_unless(opts->passsites->next, "next not set");
+	fail_unless(opts->passsites->next->next, "next->next not set");
+	fail_unless(!opts->passsites->next->next->next, "next->next->next set");
+
 #ifndef WITHOUT_USERAUTH
 	opts->user_auth = 1;
 	// Use root user, opts_set_passsite() calls sys_isuser() to validate the user
 	s = strdup("example.com root");
-	opts_set_passsite(opts, s, 2);
+	opts_set_passsite(opts, s, 3);
 	free(s);
 	fail_unless(opts->passsites->next, "next not set");
 	fail_unless(opts->passsites->next->next, "next->next not set");
-	fail_unless(!opts->passsites->next->next->next, "next->next->next set");
+	fail_unless(opts->passsites->next->next->next, "next->next->next not set");
+	fail_unless(!opts->passsites->next->next->next->next, "next->next->next->next set");
 
 	s = strdup("*.google.com * android");
-	opts_set_passsite(opts, s, 3);
+	opts_set_passsite(opts, s, 4);
 	free(s);
-	opts->user_auth = 0;
 #endif /* !WITHOUT_USERAUTH */
 	ps = passsite_str(opts->passsites);
 	fail_unless(opts->passsites->next, "next not set");
-#ifndef WITHOUT_USERAUTH
 	fail_unless(opts->passsites->next->next, "next->next not set");
+#ifndef WITHOUT_USERAUTH
 	fail_unless(opts->passsites->next->next->next, "next->next->next not set");
-	fail_unless(!opts->passsites->next->next->next->next, "next->next->next->next set");
+	fail_unless(opts->passsites->next->next->next->next, "next->next->next->next not set");
+	fail_unless(!opts->passsites->next->next->next->next->next, "next->next->next->next->next set");
 	fail_unless(!strcmp(ps, "passsite 0: site=/*.google.com/,ip=,user=,keyword=android,all=1\n"
 			"passsite 1: site=/example.com/,ip=,user=root,keyword=,all=0\n"
 			"passsite 2: site=/example.com/,ip=192.168.0.1,user=,keyword=,all=0\n"
-			"passsite 3: site=/example.com/,ip=,user=,keyword=,all=1"),
+			"passsite 3: site=/example.com/,ip=,user=,keyword=,all=1\n"
+			"passsite 4: site=/example.com/,ip=,user=,keyword=,all=1"),
 			"failed parsing multiple passites");
 #else /* WITHOUT_USERAUTH */
+	fail_unless(!opts->passsites->next->next->next, "next->next->next set");
 	fail_unless(!strcmp(ps, "passsite 0: site=/example.com/,ip=192.168.0.1,all=0\n"
-			"passsite 1: site=/example.com/,ip=,all=1"),
+			"passsite 1: site=/example.com/,ip=,all=1\n"
+			"passsite 2: site=/example.com/,ip=,all=1"),
 			"failed parsing multiple passites");
 #endif /* WITHOUT_USERAUTH */
 	free(ps);