Hi,
Please split this patch into three, documentation, implementation and
testing.
On Mon, 2016-06-13 at 16:48 +0530, Atul Anand wrote:
Pacrunner now scan stored domains to match the host of URL.
In this way the most appropriate proxy config is selected
to answer the proxy query.
---
doc/manager-api.txt | 3 +-
src/manager.c | 7 +-
src/pacrunner.h | 2 +
src/proxy.c | 207 +++++++++++++++++++++++++++++++++++++++--
unit/suite/manual_basic.test | 2 +
unit/suite/manual_exclude.test | 2 +
unit/suite/pac_basic.test | 2 +
unit/suite/pac_direct.test | 2 +
unit/suite/proxy_domain.test | 37 ++++++++
unit/suite/stub.test | 3 +
unit/test-pacrunner.c | 86 ++++++++++++++++-
11 files changed, 336 insertions(+), 17 deletions(-)
create mode 100644 unit/suite/proxy_domain.test
diff --git a/doc/manager-api.txt b/doc/manager-api.txt
index ab2f6b9..622fffa 100644
--- a/doc/manager-api.txt
+++ b/doc/manager-api.txt
@@ -62,7 +62,8 @@ Methods object CreateProxyConfiguration(dict settings)
array{string} Domains [optional]
- Domain names for which the URL is valid.
+ Domain names and IP ranges for which this proxy
+ configuration shall be valid.
array{string} Nameservers [optional]
The above should be the first patch updating documentation. IP range
formats should also be documented so that one does not have to guess
what the string should look like.
diff --git a/src/manager.c b/src/manager.c
index 1676466..5a8b4fd 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -35,7 +35,6 @@ struct proxy_config {
DBusConnection *conn;
guint watch;
- char **domains;
char **nameservers;
struct pacrunner_proxy *proxy;
@@ -58,7 +57,6 @@ static void destroy_config(gpointer data)
if (config->watch > 0)
g_dbus_remove_watch(config->conn, config->watch);
- g_strfreev(config->domains);
g_strfreev(config->nameservers);
g_free(config->sender);
@@ -224,12 +222,13 @@ static DBusMessage *create_proxy_config(DBusConnection *conn,
goto done;
}
- config->domains = domains;
config->nameservers = nameservers;
- domains = NULL;
nameservers = NULL;
+ if (pacrunner_proxy_set_domains(config->proxy, domains) < 0)
+ pacrunner_error("Failed to set proxy domains");
+
if (g_str_equal(method, "direct")) {
if (pacrunner_proxy_set_direct(config->proxy) < 0)
pacrunner_error("Failed to set direct proxy");
diff --git a/src/pacrunner.h b/src/pacrunner.h
index 6731d7c..db534cf 100644
--- a/src/pacrunner.h
+++ b/src/pacrunner.h
@@ -63,6 +63,8 @@ void pacrunner_proxy_unref(struct pacrunner_proxy *proxy);
const char *pacrunner_proxy_get_interface(struct pacrunner_proxy *proxy);
const char *pacrunner_proxy_get_script(struct pacrunner_proxy *proxy);
+int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy,
+ char **domains);
int pacrunner_proxy_set_direct(struct pacrunner_proxy *proxy);
int pacrunner_proxy_set_manual(struct pacrunner_proxy *proxy,
char **servers, char **excludes);
diff --git a/src/proxy.c b/src/proxy.c
index 8bb03af..2aeae0b 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -23,6 +23,9 @@
#include
#endif
+#include
+#include
+#include
#include
#include
@@ -37,6 +40,17 @@ struct pacrunner_proxy {
char *script;
GList **servers;
GList **excludes;
+ GList *domains;
+};
+
+struct proxy_domain {
+ char *domain;
+ int proto;
+ union {
+ struct in_addr ip4;
+ struct in6_addr ip6;
+ } addr;
+ int mask;
};
static GList *proxy_list = NULL;
@@ -77,6 +91,14 @@ struct pacrunner_proxy *pacrunner_proxy_ref(struct pacrunner_proxy
*proxy)
return proxy;
}
+static void proxy_domain_destroy(struct proxy_domain *domain)
+{
+ g_return_if_fail(domain != NULL);
+
+ g_free(domain->domain);
+ g_free(domain);
+}
+
static void reset_proxy(struct pacrunner_proxy *proxy)
{
DBG("proxy %p", proxy);
@@ -92,6 +114,12 @@ static void reset_proxy(struct pacrunner_proxy *proxy)
__pacrunner_manual_destroy_excludes(proxy->excludes);
proxy->excludes = NULL;
+
+ if (proxy->domains) {
+ g_list_free_full(proxy->domains,
+ (GDestroyNotify) proxy_domain_destroy);
Please don't cast here, if needed do necessary casts in
proxy_domain_destroy() instead.
+ }
+ proxy->domains = NULL;
}
void pacrunner_proxy_unref(struct pacrunner_proxy *proxy)
@@ -130,6 +158,69 @@ const char *pacrunner_proxy_get_script(struct pacrunner_proxy
*proxy)
return proxy->script;
}
+int pacrunner_proxy_set_domains(struct pacrunner_proxy *proxy, char **domains)
+{
+ int len;
+ char *slash, **domain;
+ char ip[INET6_ADDRSTRLEN + 1];
+
+ DBG("proxy %p domains %p", proxy, domains);
+
+ if (!proxy)
+ return -EINVAL;
+
+ if (!domains)
+ return -EINVAL;
+
+ for (domain = (char **)domains; *domain; domain++) {
The cast to (char **) does not seem necessary here?
+ struct proxy_domain *data;
+
+ data = g_malloc0(sizeof(struct proxy_domain));
+ g_return_val_if_fail(data != NULL, -EINVAL);
+
+ slash = strchr(*domain, '/');
+ if (!slash) {
+ data->domain = g_strdup(*domain);
+ data->proto = 0;
+
+ proxy->domains = g_list_append(proxy->domains, data);
+ continue;
+ }
+
+ len = slash - *domain;
+ if (len > INET6_ADDRSTRLEN)
+ return -EINVAL;
+
+ strncpy(ip, *domain, len);
+ ip[len] = '\0';
+
+ if (inet_pton(AF_INET, ip, &(data->addr.ip4)) == 1) {
+ data->domain = NULL;
+ data->proto = 4;
+
+ errno = 0;
+ data->mask = strtol(slash + 1, NULL, 10);
Hmm, it would look a bit cleaner if the return value for strtol() were
examined for INT_MIN and 32 with errno returned in that case.
+ if (errno || data->mask > 32)
+ return -EINVAL;
If this return is taken, 'data' leaks memory? It's probably easier with
a goto error; to the very end of this function?
+
+ proxy->domains = g_list_append(proxy->domains, data);
+ } else if (inet_pton(AF_INET6, ip, &(data->addr.ip6)) == 1) {
+ data->domain = NULL;
+ data->proto = 6;
+
+ errno = 0;
Same as above.
+ data->mask = strtol(slash + 1, NULL, 10);
+ if (errno || data->mask > 128)
+ return -EINVAL;
+
+ proxy->domains = g_list_append(proxy->domains, data);
+ } else
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int set_method(struct pacrunner_proxy *proxy,
enum pacrunner_proxy_method method)
{
@@ -324,10 +415,60 @@ int pacrunner_proxy_disable(struct pacrunner_proxy *proxy)
return 0;
}
+static int compare_legacy_ip_in_net(struct in_addr *host,
+ struct proxy_domain *match)
+{
+ if (ntohl(host->s_addr ^ match->addr.ip4.s_addr) >> (32 - match->mask))
+ return -1;
+
+ return 0;
+}
+
+static int compare_ipv6_in_net(struct in6_addr *host,
+ struct proxy_domain *match)
+{
+ int i, shift;
+
+ for (i = 0; i < (match->mask)/8; i++) {
+ if (host->s6_addr[i] != match->addr.ip6.s6_addr[i])
+ return -1;
+ }
+
+ if ((match->mask) % 8) {
+ /**
+ * If mask bits are not multiple of 8 , 1-7 bits are left
+ * to be compared.
+ */
+ shift = 8 - (match->mask - (i*8));
+
+ if ((host->s6_addr[i] >> shift) !=
+ (match->addr.ip6.s6_addr[i] >> shift))
+ return -1;
+ }
+
+ return 0;
+}
+
+static int compare_host_in_domain(const char *host, struct proxy_domain *match)
+{
+ if (g_str_has_suffix(host, match->domain)) {
g_str_has_suffix() appears starting from glib 2.2. Currently only 2.16
is required, so either do a string comparison for the last bytes or
update glib requirement to >= 2.2. It is possible some embedded
developers might still be upset with the latter, though.
+ size_t hlen = strlen(host);
+ size_t dlen = strlen(match->domain);
+
+ if (hlen == dlen || host[hlen - dlen - 1] == '.')
Can the following happen: hlen("foo"), dlen("myverylongdomain") ->
host[3 - 16 - 1] == host[-14] which is indexed outside the array?
+ return 0;
+ }
+
+ return -1;
+}
+
char *pacrunner_proxy_lookup(const char *url, const char *host)
{
- GList *list;
- struct pacrunner_proxy *selected_proxy = NULL;
+ GList *l, *list;
+ struct in_addr ip4_addr;
+ struct in6_addr ip6_addr;
+ struct pacrunner_proxy *selected_proxy = NULL, *default_proxy = NULL;
+ int protocol = 0;
DBG("url %s host %s", url, host);
@@ -340,17 +481,67 @@ char *pacrunner_proxy_lookup(const char *url, const char *host)
return NULL;
}
+ if (inet_pton(AF_INET, host, &ip4_addr) == 1) {
+ protocol = 4;
+ } else if (inet_pton(AF_INET6, host, &ip6_addr) == 1) {
+ protocol = 6;
+ } else if (host[0] == '[') {
proxy_set_domains() above seems not to be able to handle an IPv6
address written as [2002:dead:beef::1]. Should the host and the domains
allow for the same formatting? AF_INET and AF_INET6 may look nicer to
some eyes (but that may introduce unnecessary glitches in the code, so
it can stay as is).
+ char ip[INET6_ADDRSTRLEN + 1];
+ int len = strlen(host);
+
+ if (len < INET6_ADDRSTRLEN + 2 && host[len - 1] == ']') {
+ strncpy(ip, host + 1, len - 2);
+ ip[len - 2] = '\0';
+
+ if (inet_pton(AF_INET6, ip, &ip6_addr) == 1)
+ protocol = 6;
+ }
+ }
+
for (list = g_list_first(proxy_list); list; list = g_list_next(list)) {
struct pacrunner_proxy *proxy = list->data;
- if (proxy->method == PACRUNNER_PROXY_METHOD_MANUAL ||
- proxy->method == PACRUNNER_PROXY_METHOD_AUTO) {
- selected_proxy = proxy;
- break;
- } else if (proxy->method == PACRUNNER_PROXY_METHOD_DIRECT)
- selected_proxy = proxy;
+ if (!proxy->domains) {
+ if (!default_proxy)
+ default_proxy = proxy;
+ continue;
+ }
+
+ for (l = g_list_first(proxy->domains); l; l = g_list_next(l)) {
+ struct proxy_domain *data = l->data;
+
+ if (data->proto != protocol)
+ continue;
+
+ switch (protocol) {
+ case 4:
+ if (compare_legacy_ip_in_net(&ip4_addr,
+ data) == 0) {
+ selected_proxy = proxy;
+ goto found;
+ }
+ break;
+ case 6:
+ if (compare_ipv6_in_net(&ip6_addr,
+ data) == 0) {
+ selected_proxy = proxy;
+ goto found;
+ }
+ break;
+ default:
+ if (compare_host_in_domain(host, data) == 0) {
+ selected_proxy = proxy;
+ goto found;
+ }
+ break;
+ }
+ }
}
+ if (!selected_proxy)
+ selected_proxy = default_proxy;
+
+found:
pthread_mutex_unlock(&proxy_mutex);
if (!selected_proxy)
The above part should be patch #2, with the below patch #3. Good to see
there are test cases added for this!
diff --git a/unit/suite/manual_basic.test
b/unit/suite/manual_basic.test
index a5ec3a1..4406d9c 100644
--- a/unit/suite/manual_basic.test
+++ b/unit/suite/manual_basic.test
@@ -10,6 +10,8 @@
socks4://sockproxy.internal.com
[excludes]
+[domains]
+
[config]
VALID
diff --git a/unit/suite/manual_exclude.test b/unit/suite/manual_exclude.test
index c155743..211ae16 100644
--- a/unit/suite/manual_exclude.test
+++ b/unit/suite/manual_exclude.test
@@ -15,6 +15,8 @@ ftp://
*net
tri*
+[domains]
+
[config]
VALID
diff --git a/unit/suite/pac_basic.test b/unit/suite/pac_basic.test
index 58af200..c63757e 100644
--- a/unit/suite/pac_basic.test
+++ b/unit/suite/pac_basic.test
@@ -17,6 +17,8 @@ function FindProxyForURL(url, host)
[excludes]
+[domains]
+
[config]
VALID
diff --git a/unit/suite/pac_direct.test b/unit/suite/pac_direct.test
index 3164872..b820abc 100644
--- a/unit/suite/pac_direct.test
+++ b/unit/suite/pac_direct.test
@@ -11,6 +11,8 @@ function FindProxyForURL(url, host)
[excludes]
+[domains]
+
[config]
VALID
diff --git a/unit/suite/proxy_domain.test b/unit/suite/proxy_domain.test
new file mode 100644
index 0000000..8c2c5e4
--- /dev/null
+++ b/unit/suite/proxy_domain.test
@@ -0,0 +1,37 @@
+[title]
+Proxy Domain lookup
+
+[pac]
+
+[servers]
+http://proxy.suite.com
+
+[excludes]
+
+[domains]
+suite.com
+test.suite.com
+172.132.231.6/24
+
+[config]
+VALID
+
+[tests]
+http://foo.suite.com
foo.suite.com
+PROXY
proxy.suite.com
+http://172.132.231.101/search=?true 172.132.231.101
+PROXY
proxy.suite.com
+http://111.121.131.141/page1 111.121.131.141
+DIRECT
+http://notintel.com
notintel.com
+DIRECT
+http://intel.com
intel.com
+PROXY
proxy2.com; PROXY
secproxy2.com
+https://bar.domain2.com
bar.domain2.com
+PROXY
secproxy2.com; PROXY
proxy2.com
+http://192.168.4.4/index.html 192.168.4.4
+PROXY
proxy2.com; PROXY
secproxy2.com
+socks4://baz.domain3.com/xyz
baz.domain3.com
+SOCKS4
sockproxy3.com; PROXY
proxy3.com
+http://[fe80:96db:12ce::43ef]/ip6.mp4 [fe80:96db:12ce::43ef]
+PROXY
proxy3.com; SOCKS4
sockproxy3.com
diff --git a/unit/suite/stub.test b/unit/suite/stub.test
index 12a0426..cde0aeb 100644
--- a/unit/suite/stub.test
+++ b/unit/suite/stub.test
@@ -11,6 +11,9 @@ Stub suite file
[excludes]
# If so, optional exlusion rules can be written here
+[domains]
+# List of domains are here
+
[config]
# Result of the configuration: VALID or INVALID
diff --git a/unit/test-pacrunner.c b/unit/test-pacrunner.c
index f234a35..0c4ac69 100644
--- a/unit/test-pacrunner.c
+++ b/unit/test-pacrunner.c
@@ -42,9 +42,10 @@ enum test_suite_part {
SUITE_PAC = 1,
SUITE_SERVERS = 2,
SUITE_EXCLUDES = 3,
- SUITE_CONFIG = 4,
- SUITE_TESTS = 5,
- SUITE_NOTHING = 6,
+ SUITE_DOMAINS = 4,
+ SUITE_CONFIG = 5,
+ SUITE_TESTS = 6,
+ SUITE_NOTHING = 7,
};
enum cu_test_mode {
@@ -58,6 +59,7 @@ struct pacrunner_test_suite {
gchar *pac;
gchar **servers;
gchar **excludes;
+ gchar **domains;
bool config_result;
@@ -67,7 +69,7 @@ struct pacrunner_test_suite {
static struct pacrunner_test_suite *test_suite;
static bool verbose = false;
-static struct pacrunner_proxy *proxy;
+static struct pacrunner_proxy *proxy, *proxy2 = NULL, *proxy3 = NULL;
static bool test_config;
static void free_pacrunner_test_suite(struct pacrunner_test_suite *suite)
@@ -79,6 +81,7 @@ static void free_pacrunner_test_suite(struct pacrunner_test_suite
*suite)
g_free(suite->pac);
g_strfreev(suite->servers);
g_strfreev(suite->excludes);
+ g_strfreev(suite->domains);
g_strfreev(suite->tests);
g_free(suite);
@@ -142,6 +145,13 @@ static void print_test_suite(struct pacrunner_test_suite *suite)
} else
printf("(none)\n");
+ printf("\nDomains:\n");
+ if (suite->domains) {
+ for (line = suite->domains; *line; line++)
+ printf("%s\n", *line);
+ } else
+ printf("(none)\n");
+
printf("\nConfig result: %s\n",
suite->config_result ? "Valid" : "Invalid");
@@ -240,6 +250,15 @@ static struct pacrunner_test_suite *read_test_suite(const char
*path)
suite->excludes = array;
break;
+ case SUITE_DOMAINS:
+ array = _g_strappendv(suite->domains, *line);
+ if (!array)
+ goto error;
+
+ g_free(suite->domains);
+ suite->domains = array;
+
+ break;
case SUITE_CONFIG:
if (strncmp(*line, "VALID", 5) == 0)
suite->config_result = true;
@@ -272,6 +291,8 @@ static struct pacrunner_test_suite *read_test_suite(const char
*path)
part = SUITE_SERVERS;
else if (strncmp(*line, "[excludes]", 10) == 0)
part = SUITE_EXCLUDES;
+ else if (strncmp(*line, "[domains]", 9) == 0)
+ part = SUITE_DOMAINS;
else if (strncmp(*line, "[config]", 8) == 0)
part = SUITE_CONFIG;
else if (strncmp(*line, "[tests]", 7) == 0)
@@ -338,6 +359,54 @@ static void test_manual_config(void)
CU_ASSERT_TRUE(test_suite->config_result == test_config);
}
+static void test_proxy_domain(void)
+{
+ int val = 0;
+
+ if (pacrunner_proxy_set_domains(proxy, test_suite->domains) != 0)
+ val = -1;
+
+ proxy2 = pacrunner_proxy_create("eth1");
+ if (proxy2) {
+ char *servers[] = {
+ "http://proxy2.com",
+ "https://secproxy2.com",
+ NULL};
+ char *domains[] = {
+ "intel.com",
+ "domain2.com",
+ "192.168.4.0/16",
+ NULL};
+
+ if (pacrunner_proxy_set_manual(proxy2, servers, NULL) != 0)
+ val = -1;
+
+ if (pacrunner_proxy_set_domains(proxy2, domains) != 0)
+ val = -1;
+ }
+
+ proxy3 = pacrunner_proxy_create("wl0");
+ if (proxy3) {
+ char *servers[] = {
+ "http://proxy3.com",
+ "socks4://sockproxy3.com",
+ NULL};
+ char *domains[] = {
+ "redhat.com",
+ "domain3.com",
+ "fe80:96db::/32",
+ NULL};
+
+ if (pacrunner_proxy_set_manual(proxy3, servers, NULL) != 0)
+ val = -1;
+
+ if (pacrunner_proxy_set_domains(proxy3, domains) != 0)
+ val = -1;
+ }
+
+ CU_ASSERT_TRUE(val == 0);
+}
+
static void test_proxy_requests(void)
{
gchar **test_strings;
@@ -430,12 +499,21 @@ static void run_test_suite(const char *test_file_path, enum
cu_test_mode mode)
CU_add_test(cu_suite, "Manual config test",
test_manual_config);
+ if (test_suite->domains)
+ CU_add_test(cu_suite, "Proxy domain test",
+ test_proxy_domain);
+
if (test_suite->config_result && test_suite->tests)
CU_add_test(cu_suite, "Proxy requests test",
test_proxy_requests);
test_config = false;
+ if (test_suite->domains) {
+ pacrunner_proxy_unref(proxy2);
+ pacrunner_proxy_unref(proxy3);
+ }
+
switch (mode) {
case CU_MODE_BASIC:
CU_basic_set_mode(CU_BRM_VERBOSE);
Cheers,
Patrik