Discussion:
[RFC 1/6] proxy-discoveryd: Basic core added
(too old to reply)
Tomasz Bursztyka
2015-04-10 12:17:38 UTC
Permalink
This currently does nothing besides providing the basic skeleton for the
daemon to be developped.

proxy-discoveryd is a daemon that will manage the network proxy
configuration per-network interface. It will provide those through DBus,
and thus it will be possible for any application to know what proxies to
use, if any, when relevant.
---
.gitignore | 1 +
Makefile.am | 26 +++++++++++
configure.ac | 11 +++++
src/proxy-discovery/proxy-discoveryd-manager.c | 65 ++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.c | 65 ++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.h | 38 +++++++++++++++
units/.gitignore | 1 +
units/systemd-proxy-discoveryd.service.in | 18 +++++++
8 files changed, 225 insertions(+)
create mode 100644 src/proxy-discovery/proxy-discoveryd-manager.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.h
create mode 100644 units/systemd-proxy-discoveryd.service.in

diff --git a/.gitignore b/.gitignore
index 875ada5..f479d37 100644
--- a/.gitignore
+++ b/.gitignore
@@ -108,6 +108,7 @@
/systemd-notify
/systemd-nspawn
/systemd-path
+/systemd-proxy-discoveryd
/systemd-pull
/systemd-quotacheck
/systemd-random-seed
diff --git a/Makefile.am b/Makefile.am
index 0a57389..3f4e4d3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5889,6 +5889,32 @@ EXTRA_DIST += \
endif

# ------------------------------------------------------------------------------
+if ENABLE_PROXY_DISCOVERYD
+
+rootlibexec_PROGRAMS += \
+ systemd-proxy-discoveryd
+
+systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/proxy-discoveryd.c \
+ src/proxy-discovery/proxy-discoveryd.h \
+ src/proxy-discovery/proxy-discoveryd-manager.c
+
+systemd_proxy_discoveryd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-shared.la
+
+nodist_systemunit_DATA += \
+ units/systemd-proxy-discoveryd.service
+
+EXTRA_DIST += \
+ units/systemd-proxy-discoveryd.service.in
+
+CLEANFILES += \
+ units/systemd-proxy-discoveryd.service
+
+endif
+
+# ------------------------------------------------------------------------------
if ENABLE_LOGIND
systemd_logind_SOURCES = \
src/login/logind.c \
diff --git a/configure.ac b/configure.ac
index 960b15d..3db3230 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1139,6 +1139,16 @@ AS_IF([test "x$enable_networkd" != "xno"], [
AM_CONDITIONAL(ENABLE_NETWORKD, [test "x$have_networkd" = "xyes"])

# ------------------------------------------------------------------------------
+have_proxy_discoveryd=no
+AC_ARG_ENABLE(proxy-discoveryd, AS_HELP_STRING([--disable-proxy-discoveryd], [disable proxy-discoveryd]))
+AS_IF([test "x$enable_proxy_discoveryd" != "xno"], [
+ AC_DEFINE(ENABLE_PROXY_DISCOVERYD, 1, [Define if proxy-discoveryd support is to be enabled])
+ have_proxy_discoveryd=yes
+ M4_DEFINES="$M4_DEFINES -DENABLE_PROXY_DISCOVERYD"
+])
+AM_CONDITIONAL(ENABLE_PROXY_DISCOVERYD, [test "x$have_proxy_discoveryd" = "xyes"])
+
+# ------------------------------------------------------------------------------
have_efi=no
AC_ARG_ENABLE(efi, AS_HELP_STRING([--disable-efi], [disable EFI support]))
if test "x$enable_efi" != "xno"; then
@@ -1560,6 +1570,7 @@ AC_MSG_RESULT([
localed: ${have_localed}
networkd: ${have_networkd}
resolved: ${have_resolved}
+ proxy-discoveryd: ${have_proxy_discoveryd}
default DNS servers: ${DNS_SERVERS}
coredump: ${have_coredump}
polkit: ${have_polkit}
diff --git a/src/proxy-discovery/proxy-discoveryd-manager.c b/src/proxy-discovery/proxy-discoveryd-manager.c
new file mode 100644
index 0000000..3aaec68
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-manager.c
@@ -0,0 +1,65 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "proxy-discoveryd.h"
+
+int manager_new(Manager **ret) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int r;
+
+ assert(ret);
+
+ m = new0(Manager, 1);
+ if (!m)
+ return -ENOMEM;
+
+ r = sd_event_default(&m->event);
+ if (r < 0)
+ return r;
+
+ r = sd_event_set_watchdog(m->event, true);
+ if (r < 0)
+ return r;
+
+ r = sd_event_add_signal(m->event, NULL, SIGTERM, NULL, NULL);
+ if (r < 0)
+ return r;
+
+ r = sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
+ if (r < 0)
+ return r;
+
+ *ret = m;
+ m = NULL;
+
+ return 0;
+}
+
+Manager *manager_free(Manager *m) {
+ if (!m)
+ return NULL;
+
+ sd_event_unref(m->event);
+
+ free(m);
+
+ return NULL;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.c b/src/proxy-discovery/proxy-discoveryd.c
new file mode 100644
index 0000000..f023312
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd.c
@@ -0,0 +1,65 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "sd-event.h"
+#include "sd-daemon.h"
+
+#include "proxy-discoveryd.h"
+
+int main(int argc, char *argv[]) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int r;
+
+ log_set_target(LOG_TARGET_AUTO);
+ log_parse_environment();
+ log_open();
+
+ if (argc != 1) {
+ log_error("This program takes no arguments.");
+ r = -EINVAL;
+ goto out;
+ }
+
+ r = manager_new(&m);
+ if (r < 0) {
+ log_error_errno(r, "Could not create manager: %m");
+ goto out;
+ }
+
+ sd_notify(false,
+ "READY=1\n"
+ "STATUS=Processing requests...");
+
+ r = sd_event_loop(m->event);
+ if (r < 0) {
+ log_error_errno(r, "Event loop failed: %m");
+ goto out;
+ }
+
+ sd_event_get_exit_code(m->event, &r);
+
+out:
+ sd_notify(false,
+ "STOPPING=1\n"
+ "STATUS=Shutting down...");
+
+ return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
new file mode 100644
index 0000000..125abc0
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -0,0 +1,38 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "sd-event.h"
+
+#include "util.h"
+
+typedef struct Manager Manager;
+
+struct Manager {
+ sd_event *event;
+};
+
+int manager_new(Manager **ret);
+Manager *manager_free(Manager *m);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)
diff --git a/units/.gitignore b/units/.gitignore
index ad469c1..63ae1af 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -51,6 +51,7 @@
/systemd-networkd.service
/systemd-***@.service
/systemd-poweroff.service
+/systemd-proxy-discoveryd.service
/systemd-quotacheck.service
/systemd-random-seed.service
/systemd-reboot.service
diff --git a/units/systemd-proxy-discoveryd.service.in b/units/systemd-proxy-discoveryd.service.in
new file mode 100644
index 0000000..7ac02e0
--- /dev/null
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+ExecStart=@rootlibexecdir@/systemd-proxy-discoveryd
+StandardOutput=null
--
2.0.5
Tomasz Bursztyka
2015-04-10 12:17:39 UTC
Permalink
The config file will be in /etc/systemd/proxy/<filename>.conf

It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).

Only one PAC based proxy configuration will be loaded at a time.
---
Makefile.am | 6 +-
src/proxy-discovery/.gitignore | 1 +
src/proxy-discovery/proxy-discoveryd-manager.c | 46 +++++++++++++
.../proxy-discoveryd-proxy-gperf.gperf | 17 +++++
src/proxy-discovery/proxy-discoveryd-proxy.c | 77 ++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.c | 6 ++
src/proxy-discovery/proxy-discoveryd.h | 27 ++++++++
7 files changed, 179 insertions(+), 1 deletion(-)
create mode 100644 src/proxy-discovery/.gitignore
create mode 100644 src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf
create mode 100644 src/proxy-discovery/proxy-discoveryd-proxy.c

diff --git a/Makefile.am b/Makefile.am
index 3f4e4d3..6e839b9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5897,19 +5897,23 @@ rootlibexec_PROGRAMS += \
systemd_proxy_discoveryd_SOURCES = \
src/proxy-discovery/proxy-discoveryd.c \
src/proxy-discovery/proxy-discoveryd.h \
- src/proxy-discovery/proxy-discoveryd-manager.c
+ src/proxy-discovery/proxy-discoveryd-manager.c \
+ src/proxy-discovery/proxy-discoveryd-proxy.c \
+ src/proxy-discovery/proxy-discoveryd-proxy-gperf.c

systemd_proxy_discoveryd_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la

nodist_systemunit_DATA += \
+ src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf \
units/systemd-proxy-discoveryd.service

EXTRA_DIST += \
units/systemd-proxy-discoveryd.service.in

CLEANFILES += \
+ src/proxy-discovery/proxy-discoveryd-proxy-gperf.c \
units/systemd-proxy-discoveryd.service

endif
diff --git a/src/proxy-discovery/.gitignore b/src/proxy-discovery/.gitignore
new file mode 100644
index 0000000..5a67f7f
--- /dev/null
+++ b/src/proxy-discovery/.gitignore
@@ -0,0 +1 @@
+/proxy-discoveryd-proxy-gperf.c
diff --git a/src/proxy-discovery/proxy-discoveryd-manager.c b/src/proxy-discovery/proxy-discoveryd-manager.c
index 3aaec68..62fcc23 100644
--- a/src/proxy-discovery/proxy-discoveryd-manager.c
+++ b/src/proxy-discovery/proxy-discoveryd-manager.c
@@ -19,8 +19,14 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/

+#include "conf-parser.h"
+#include "conf-files.h"
+#include "strv.h"
+
#include "proxy-discoveryd.h"

+static const char *proxy_discoveryd_dir = "/etc/systemd/proxy/";
+
int manager_new(Manager **ret) {
_cleanup_manager_free_ Manager *m = NULL;
int r;
@@ -47,6 +53,8 @@ int manager_new(Manager **ret) {
if (r < 0)
return r;

+ LIST_HEAD_INIT(m->default_proxies);
+
*ret = m;
m = NULL;

@@ -54,12 +62,50 @@ int manager_new(Manager **ret) {
}

Manager *manager_free(Manager *m) {
+ Proxy *proxy;
+
if (!m)
return NULL;

sd_event_unref(m->event);

+ while((proxy = m->default_proxies))
+ proxy_free(proxy);
+
free(m);

return NULL;
}
+
+int manager_load_config(Manager *m) {
+ _cleanup_strv_free_ char **files = NULL;
+ Proxy *proxy;
+ char **file;
+ int r;
+
+ assert(m);
+
+ while ((proxy = m->default_proxies))
+ proxy_free(proxy);
+
+ r = conf_files_list(&files, ".proxy", NULL, proxy_discoveryd_dir, NULL);
+ if (r < 0)
+ return log_error_errno(r, "Failed to enumerate proxy files: %m");
+
+ STRV_FOREACH_BACKWARDS(file, files) {
+ r = proxy_load(m, *file, &proxy);
+ if (r < 0)
+ log_error_errno(r, "Failed to load file %s: %m", *file);
+
+ if (!proxy)
+ continue;
+
+ LIST_PREPEND(proxy_next, m->default_proxies, proxy);
+
+ /* Only one PAC file */
+ if (proxy->pac_path)
+ break;
+ }
+
+ return 0;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf b/src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf
new file mode 100644
index 0000000..0d42b3c
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf
@@ -0,0 +1,17 @@
+%{
+#include <stddef.h>
+#include "conf-parser.h"
+#include "proxy-discoveryd.h"
+%}
+struct ConfigPerfItem;
+%null_strings
+%language=ANSI-C
+%define slot-name section_and_lvalue
+%define hash-function-name proxy_discoveryd_proxy_gperf_hash
+%define lookup-function-name proxy_discoveryd_proxy_gperf_lookup
+%readonly-tables
+%omit-struct-type
+%struct-type
+%includes
+%%
+Proxy.PAC, config_parse_path, 0, offsetof(Proxy, pac_path)
diff --git a/src/proxy-discovery/proxy-discoveryd-proxy.c b/src/proxy-discovery/proxy-discoveryd-proxy.c
new file mode 100644
index 0000000..d18395b
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-proxy.c
@@ -0,0 +1,77 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "proxy-discoveryd.h"
+
+#include "conf-parser.h"
+
+void proxy_free(Proxy *proxy) {
+ if (!proxy)
+ return;
+
+ if (proxy->manager)
+ LIST_REMOVE(proxy_next, proxy->manager->default_proxies, proxy);
+
+ free(proxy);
+}
+
+int proxy_load(Manager *manager, const char *filename, Proxy **ret) {
+ _cleanup_proxy_free_ Proxy *proxy = NULL;
+ _cleanup_fclose_ FILE *file = NULL;
+ int r;
+
+ assert(manager);
+ assert(filename);
+ assert(ret);
+
+ *ret = NULL;
+
+ file = fopen(filename, "re");
+ if (!file) {
+ if (errno == ENOENT)
+ return 0;
+ else
+ return -errno;
+ }
+
+ if (null_or_empty_fd(fileno(file))) {
+ log_debug("Skipping empty file: %s", filename);
+ return 0;
+ }
+
+ proxy = new0(Proxy, 1);
+ if (!proxy)
+ return log_oom();
+
+ proxy->manager = manager;
+
+ r = config_parse(NULL, filename, file, "Proxy\0",
+ config_item_perf_lookup,
+ proxy_discoveryd_proxy_gperf_lookup,
+ false, false, true, proxy);
+ if (r < 0)
+ return r;
+
+ *ret = proxy;
+ proxy = NULL;
+
+ return 0;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.c b/src/proxy-discovery/proxy-discoveryd.c
index f023312..b4789d1 100644
--- a/src/proxy-discovery/proxy-discoveryd.c
+++ b/src/proxy-discovery/proxy-discoveryd.c
@@ -44,6 +44,12 @@ int main(int argc, char *argv[]) {
goto out;
}

+ r = manager_load_config(m);
+ if (r < 0) {
+ log_error_errno(r, "Could not load configuration files: %m");
+ goto out;
+ }
+
sd_notify(false,
"READY=1\n"
"STATUS=Processing requests...");
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
index 125abc0..f51fca5 100644
--- a/src/proxy-discovery/proxy-discoveryd.h
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -24,15 +24,42 @@
#include "sd-event.h"

#include "util.h"
+#include "list.h"

typedef struct Manager Manager;
+typedef struct Proxy Proxy;

struct Manager {
sd_event *event;
+
+ LIST_HEAD(Proxy, default_proxies);
+};
+
+struct Proxy {
+ Manager *manager;
+
+ char *pac_path;
+
+ LIST_FIELDS(Proxy, proxy_next);
};

+/* Manager */
+
int manager_new(Manager **ret);
Manager *manager_free(Manager *m);

+int manager_load_config(Manager *m);
+
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
#define _cleanup_manager_free_ _cleanup_(manager_freep)
+
+/* Proxy */
+
+void proxy_free(Proxy *proxy);
+
+int proxy_load(Manager *manager, const char *filename, Proxy **ret);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Proxy*, proxy_free);
+#define _cleanup_proxy_free_ _cleanup_(proxy_freep)
+
+const struct ConfigPerfItem* proxy_discoveryd_proxy_gperf_lookup(const char *key, unsigned length);
--
2.0.5
Lennart Poettering
2015-04-10 15:43:27 UTC
Permalink
Post by Tomasz Bursztyka
The config file will be in /etc/systemd/proxy/<filename>.conf
It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).
Only one PAC based proxy configuration will be loaded at a time.
(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-10 22:22:00 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
The config file will be in /etc/systemd/proxy/<filename>.conf
It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).
Only one PAC based proxy configuration will be loaded at a time.
(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).
why would you have a global PAC file. I think they should be all per interface and nothing else.

Regards

Marcel
Marcel Holtmann
2015-04-12 20:03:45 UTC
Permalink
Hi Lennart,
Post by Marcel Holtmann
Post by Lennart Poettering
Post by Tomasz Bursztyka
The config file will be in /etc/systemd/proxy/<filename>.conf
It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).
Only one PAC based proxy configuration will be loaded at a time.
(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).
why would you have a global PAC file. I think they should be all per
interface and nothing else.
Well, maybe not a global PAC file, but probably an explicitly
configurable global HTTP proxy, if people want that... I mean, it is a
pretty common setting to have I figure, and the daemon should
proibably cover both PAC and straightforward proxy config...
yes that makes sense. So what we have done in PACrunner was that instead of a PAC file you could just give it the HTTP proxy address. And that would also work per interface. When then libproxy or someone did FindProxyForURL, the configured HTTP proxy URL was returned.

Of course in these situations, no PAC files are executed, but the D-Bus API for talking to systemd-proxydiscoverd to get the proxy to use can be still used.

I still wonder if it is a good idea to have a global proxy there. I would assume you just configure the proxy per interface in your network configuration file. This should be treated similar to DNS configuration. You want this per interface.

Regards

Marcel
Tomasz Bursztyka
2015-04-13 10:18:56 UTC
Permalink
Hi Marcel and Lennart,
Post by Marcel Holtmann
Post by Marcel Holtmann
Post by Lennart Poettering
Post by Tomasz Bursztyka
The config file will be in /etc/systemd/proxy/<filename>.conf
It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).
Only one PAC based proxy configuration will be loaded at a time.
(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).
why would you have a global PAC file. I think they should be all per
interface and nothing else.
Well, maybe not a global PAC file, but probably an explicitly
configurable global HTTP proxy, if people want that... I mean, it is a
pretty common setting to have I figure, and the daemon should
proibably cover both PAC and straightforward proxy config...
yes that makes sense. So what we have done in PACrunner was that instead of a PAC file you could just give it the HTTP proxy address. And that would also work per interface. When then libproxy or someone did FindProxyForURL, the configured HTTP proxy URL was returned.
Of course in these situations, no PAC files are executed, but the D-Bus API for talking to systemd-proxydiscoverd to get the proxy to use can be still used.
I still wonder if it is a good idea to have a global proxy there. I would assume you just configure the proxy per interface in your network configuration file. This should be treated similar to DNS configuration. You want this per interface.
The main point of proxy/<file>.conf files will be indeed to get manually
set proxies.
Keywords such as: Protocol, Host, Port, etc...
We used to set everything through URIs in pacrunner, but it's finally
easier to set stuff one by one: less room for bugs and errors.

Tom came with the idea we could do it expressive enough so even complex
proxy setups could be done,
thus such PAC keyword could be removed eventually.
Then only dynamic context due to dhcp/wpad would generate a PAC based
proxy configuration.

Tomasz
Lennart Poettering
2015-04-13 14:23:02 UTC
Permalink
Post by Marcel Holtmann
Hi Lennart,
Post by Marcel Holtmann
Post by Lennart Poettering
Post by Tomasz Bursztyka
The config file will be in /etc/systemd/proxy/<filename>.conf
It currently only load "Proxy" parts, with the key PAC. Rest is ignored.
The PAC keyword is a path to a .pac file (a specific js script for proxy
configuration).
Only one PAC based proxy configuration will be loaded at a time.
(Just a side note: I figure in the long run we should probably track
PAC data per-interface (plus maybe one global setting), so that
clients can query this specifically for an interface, and so that we
can search PAC data over the right network. But I figure for now this
doesn't matter too much.).
why would you have a global PAC file. I think they should be all per
interface and nothing else.
Well, maybe not a global PAC file, but probably an explicitly
configurable global HTTP proxy, if people want that... I mean, it is a
pretty common setting to have I figure, and the daemon should
proibably cover both PAC and straightforward proxy config...
yes that makes sense. So what we have done in PACrunner was that
instead of a PAC file you could just give it the HTTP proxy
address. And that would also work per interface. When then libproxy
or someone did FindProxyForURL, the configured HTTP proxy URL was
returned.
Yupp, definitely makes sense. networkd should have a setting
HttpProxy= or so that proxy-discoveryd should pick up.

Oh, and there's one more thing I want: I figure for the bigger
webbrowsers which have their own JS engine, it might make sense to
export the discovered per-interface PAC files directly, so that they
can opt for making use of our WPAD logic, but can run the JS stuff on
their own. And this would then probably also mean synthesizing a PAC
file from that networkd information.
Post by Marcel Holtmann
I still wonder if it is a good idea to have a global proxy there. I
would assume you just configure the proxy per interface in your
network configuration file. This should be treated similar to DNS
configuration. You want this per interface.
Well, we do have per-interface DNS as well as global DNS. We also have
per-interface NTP as well as global NTP. I am tempted to say that this
shouldn't be any different...

Lennart
--
Lennart Poettering, Red Hat
Tomasz Bursztyka
2015-04-10 12:17:43 UTC
Permalink
Add the proxy-discoveryd related tasks.
---
TODO | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/TODO b/TODO
index 7be62c9..4331ac7 100644
--- a/TODO
+++ b/TODO
@@ -882,6 +882,16 @@ Features:
- some servers don't do rapid commit without a filled in IA_NA, verify
this behavior

+* proxy-discoveryd:
+ - add the dedicated PAC functions (see Mozilla's nsProxyAutoConfig.js), some may be ported as native functions
+ - add the possibility to create a proxy configuration per-interface via DBus
+ - add the possibility to request a proxy for an interface via DBus
+ - add support for manual based proxy configuration (vs PAC based)
+ - follow the interfaces (their index and IPs) through netlink:
+ - remove the ioctl calls and use the netlink gathered informations instead
+ - the default interface gets the lead for the default proxy configuration to be ran instead of the system-wide one
+ - support IPv6
+
External:

* dbus:
--
2.0.5
Tom Gundersen
2015-04-10 13:26:57 UTC
Permalink
On Fri, Apr 10, 2015 at 2:17 PM, Tomasz Bursztyka
Post by Tomasz Bursztyka
Add the proxy-discoveryd related tasks.
---
TODO | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/TODO b/TODO
index 7be62c9..4331ac7 100644
--- a/TODO
+++ b/TODO
- some servers don't do rapid commit without a filled in IA_NA, verify
this behavior
+ - add the dedicated PAC functions (see Mozilla's nsProxyAutoConfig.js), some may be ported as native functions
+ - add the possibility to create a proxy configuration per-interface via DBus
How about specifying information per-link in .network files and use
sd-network to extract this into proxy-discovyrd? I.e., like timesyncd
and resolved currently work.
Post by Tomasz Bursztyka
+ - add the possibility to request a proxy for an interface via DBus
+ - add support for manual based proxy configuration (vs PAC based)
+ - remove the ioctl calls and use the netlink gathered informations instead
+ - the default interface gets the lead for the default proxy configuration to be ran instead of the system-wide one
+ - support IPv6
This we should probably have from the beginning, any reason you kept
it IPv4 only for now (apart from keeping it simple as a POC)?
Post by Tomasz Bursztyka
+
--
2.0.5
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Tomasz Bursztyka
2015-04-10 12:17:42 UTC
Permalink
It currently exposes a simple method for requesting a proxy from an url
and its host.
---
Makefile.am | 1 +
.../org.freedesktop.proxydiscovery1.conf | 46 +++++++++++++++++++++
.../org.freedesktop.proxydiscovery1.service | 12 ++++++
src/proxy-discovery/proxy-discoveryd-bus.c | 48 ++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd-manager.c | 45 ++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.c | 6 +++
src/proxy-discovery/proxy-discoveryd.h | 4 ++
units/org.freedesktop.proxydiscovery1.busname | 15 +++++++
8 files changed, 177 insertions(+)
create mode 100644 src/proxy-discovery/org.freedesktop.proxydiscovery1.conf
create mode 100644 src/proxy-discovery/org.freedesktop.proxydiscovery1.service
create mode 100644 src/proxy-discovery/proxy-discoveryd-bus.c
create mode 100644 units/org.freedesktop.proxydiscovery1.busname

diff --git a/Makefile.am b/Makefile.am
index 25ea0dd..385c92c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5899,6 +5899,7 @@ systemd_proxy_discoveryd_SOURCES = \
src/proxy-discovery/duktape.c \
src/proxy-discovery/proxy-discoveryd.c \
src/proxy-discovery/proxy-discoveryd.h \
+ src/proxy-discovery/proxy-discoveryd-bus.c \
src/proxy-discovery/proxy-discoveryd-manager.c \
src/proxy-discovery/proxy-discoveryd-pac.c \
src/proxy-discovery/proxy-discoveryd-proxy.c \
diff --git a/src/proxy-discovery/org.freedesktop.proxydiscovery1.conf b/src/proxy-discovery/org.freedesktop.proxydiscovery1.conf
new file mode 100644
index 0000000..110c114
--- /dev/null
+++ b/src/proxy-discovery/org.freedesktop.proxydiscovery1.conf
@@ -0,0 +1,46 @@
+<?xml version="1.0"?> <!--*-nxml-*-->
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+
+<!--
+ This file is part of systemd.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+-->
+
+<busconfig>
+
+ <policy user="root">
+ <allow own="org.freedesktop.proxydiscovery1"/>
+ <allow send_destination="org.freedesktop.proxydiscovery1"/>
+ <allow receive_sender="org.freedesktop.proxydiscovery1"/>
+ </policy>
+
+ <policy context="default">
+ <deny send_destination="org.freedesktop.proxydiscovery1"/>
+
+ <allow send_destination="org.freedesktop.proxydiscovery1"
+ send_interface="org.freedesktop.DBus.Introspectable"/>
+
+ <allow send_destination="org.freedesktop.proxydiscovery1"
+ send_interface="org.freedesktop.DBus.Peer"/>
+
+ <allow send_destination="org.freedesktop.proxydiscovery1"
+ send_interface="org.freedesktop.DBus.Properties"
+ send_member="Get"/>
+
+ <allow send_destination="org.freedesktop.proxydiscovery1"
+ send_interface="org.freedesktop.DBus.Properties"
+ send_member="GetAll"/>
+
+ <allow send_destination="org.freedesktop.proxydiscovery1"
+ send_interface="org.freedesktop.proxydiscovery1.Manager"
+ send_member="FindProxy"/>
+
+ <allow receive_sender="org.freedesktop.proxydiscovery1"/>
+ </policy>
+
+</busconfig>
diff --git a/src/proxy-discovery/org.freedesktop.proxydiscovery1.service b/src/proxy-discovery/org.freedesktop.proxydiscovery1.service
new file mode 100644
index 0000000..d438381
--- /dev/null
+++ b/src/proxy-discovery/org.freedesktop.proxydiscovery1.service
@@ -0,0 +1,12 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[D-BUS Service]
+Name=org.freedesktop.proxydiscovery1
+Exec=/bin/false
+User=root
+SystemdService=dbus-org.freedesktop.proxydiscovery1.service
diff --git a/src/proxy-discovery/proxy-discoveryd-bus.c b/src/proxy-discovery/proxy-discoveryd-bus.c
new file mode 100644
index 0000000..b917f38
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-bus.c
@@ -0,0 +1,48 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "bus-util.h"
+
+#include "proxy-discoveryd.h"
+
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ r = proxy_execute(m->default_proxies, message);
+ if (r < 0)
+ sd_bus_reply_method_return(message, "s", p);
+
+ return 1;
+}
+
+const sd_bus_vtable manager_vtable[] = {
+ SD_BUS_VTABLE_START(0),
+
+ SD_BUS_METHOD("FindProxy", "ss", "s", method_find_proxy, SD_BUS_VTABLE_UNPRIVILEGED),
+
+ SD_BUS_VTABLE_END
+};
diff --git a/src/proxy-discovery/proxy-discoveryd-manager.c b/src/proxy-discovery/proxy-discoveryd-manager.c
index 62fcc23..e5aef38 100644
--- a/src/proxy-discovery/proxy-discoveryd-manager.c
+++ b/src/proxy-discovery/proxy-discoveryd-manager.c
@@ -19,6 +19,8 @@
along with systemd; If not, see <http://www.gnu.org/licenses/>.
***/

+#include "bus-util.h"
+#include "bus-error.h"
#include "conf-parser.h"
#include "conf-files.h"
#include "strv.h"
@@ -53,6 +55,10 @@ int manager_new(Manager **ret) {
if (r < 0)
return r;

+ r = sd_bus_default_system(&m->bus);
+ if (r < 0 && r != -ENOENT) /* TODO: drop when we can rely on kdbus */
+ return r;
+
LIST_HEAD_INIT(m->default_proxies);

*ret = m;
@@ -68,6 +74,7 @@ Manager *manager_free(Manager *m) {
return NULL;

sd_event_unref(m->event);
+ sd_bus_unref(m->bus);

while((proxy = m->default_proxies))
proxy_free(proxy);
@@ -77,6 +84,44 @@ Manager *manager_free(Manager *m) {
return NULL;
}

+int manager_bus_listen(Manager *m) {
+ _cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+ int r;
+
+ assert(m);
+ assert(m->event);
+
+ if (!m->bus) /* TODO: drop when we can rely on kdbus */
+ return 0;
+
+ r = sd_bus_add_object_vtable(m->bus, NULL,
+ "/org/freedesktop/proxydiscovery1",
+ "org.freedesktop.proxydiscovery1.Manager",
+ manager_vtable, m);
+ if (r < 0)
+ return log_error_errno(r, "Failed to add manager object vtable: %m");
+
+ r = sd_bus_call_method(m->bus,
+ "org.freedesktop.systemd1",
+ "/org/freedesktop/systemd1",
+ "org.freedesktop.systemd1.Manager",
+ "Subscribe", &error, NULL, NULL);
+ if (r < 0) {
+ log_error("Failed to enable subscription: %s", bus_error_message(&error, r));
+ return r;
+ }
+
+ r = sd_bus_request_name(m->bus, "org.freedesktop.proxydiscovery1", 0);
+ if (r < 0)
+ return log_error_errno(r, "Failed to register name: %m");
+
+ r = sd_bus_attach_event(m->bus, m->event, 0);
+ if (r < 0)
+ return log_error_errno(r, "Failed to attach bus to event loop: %m");
+
+ return 0;
+}
+
int manager_load_config(Manager *m) {
_cleanup_strv_free_ char **files = NULL;
Proxy *proxy;
diff --git a/src/proxy-discovery/proxy-discoveryd.c b/src/proxy-discovery/proxy-discoveryd.c
index b4789d1..10af22f 100644
--- a/src/proxy-discovery/proxy-discoveryd.c
+++ b/src/proxy-discovery/proxy-discoveryd.c
@@ -44,6 +44,12 @@ int main(int argc, char *argv[]) {
goto out;
}

+ r = manager_bus_listen(m);
+ if (r < 0) {
+ log_error_errno(r, "Could not listed to system bus: %m");
+ goto out;
+ }
+
r = manager_load_config(m);
if (r < 0) {
log_error_errno(r, "Could not load configuration files: %m");
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
index d8d9482..c97afad 100644
--- a/src/proxy-discovery/proxy-discoveryd.h
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -32,6 +32,7 @@ typedef struct Proxy Proxy;

struct Manager {
sd_event *event;
+ sd_bus *bus;

LIST_HEAD(Proxy, default_proxies);
};
@@ -52,8 +53,11 @@ struct Proxy {
int manager_new(Manager **ret);
Manager *manager_free(Manager *m);

+int manager_bus_listen(Manager *m);
int manager_load_config(Manager *m);

+extern const sd_bus_vtable manager_vtable[];
+
DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
#define _cleanup_manager_free_ _cleanup_(manager_freep)

diff --git a/units/org.freedesktop.proxydiscovery1.busname b/units/org.freedesktop.proxydiscovery1.busname
new file mode 100644
index 0000000..c6cfc8a
--- /dev/null
+++ b/units/org.freedesktop.proxydiscovery1.busname
@@ -0,0 +1,15 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy Discovery Service Bus Name
+#Documentation=man:systemd-proxy-discoveryd.service(8)
+#Documentation=http://www.freedesktop.org/wiki/Software/systemd/proxy-discoveryd
+
+[BusName]
+Service=systemd-proxy-discoveryd.service
+AllowWorld=talk
--
2.0.5
Lennart Poettering
2015-04-10 16:02:33 UTC
Permalink
Post by Tomasz Bursztyka
+
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
Please don't mix variable declarations and function invocations in one
line (also see CODING_STYLE). Also, missing OOM check...
Post by Tomasz Bursztyka
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ r = proxy_execute(m->default_proxies, message);
+ if (r < 0)
+ sd_bus_reply_method_return(message, "s", p);
Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?

Also, why allocate "DIRECT" with strdup() at all?

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-10 22:33:32 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
Please don't mix variable declarations and function invocations in one
line (also see CODING_STYLE). Also, missing OOM check...
Post by Tomasz Bursztyka
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ r = proxy_execute(m->default_proxies, message);
+ if (r < 0)
+ sd_bus_reply_method_return(message, "s", p);
Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?
Also, why allocate "DIRECT" with strdup() at all?
there are no errors. Either you get a proxy directive or you return DIRECT to indicate no proxy. What would you do in an error case anyway. The backup is always assume no proxy.

Regards

Marcel
Marcel Holtmann
2015-04-12 19:53:10 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
Please don't mix variable declarations and function invocations in one
line (also see CODING_STYLE). Also, missing OOM check...
Post by Tomasz Bursztyka
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ r = proxy_execute(m->default_proxies, message);
+ if (r < 0)
+ sd_bus_reply_method_return(message, "s", p);
Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?
Also, why allocate "DIRECT" with strdup() at all?
There are no errors. Either you get a proxy directive or you return
DIRECT to indicate no proxy. What would you do in an error case
anyway. The backup is always assume no proxy.
Well, so far it has been our coding styles to propagate errors up if
they cause the invoked operations to fail, and allow the higher up
code deal with them and possibly eat them up. I mean, the HTTP client
can eat the error up and downgrade to DIRECT on its own, there's no
need to do this from our side already.
of course it can, but it also does not help you at all. You are not making anything better here.

I think that one clean interface to this needs to be a API compatible libproxy. Similar to what we did in PACrunner since that allows existing application to just use systemd-proxydiscoverd.

Regards

Marcel
Lennart Poettering
2015-04-12 19:59:35 UTC
Permalink
Post by Marcel Holtmann
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message, void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
Please don't mix variable declarations and function invocations in one
line (also see CODING_STYLE). Also, missing OOM check...
Post by Tomasz Bursztyka
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ r = proxy_execute(m->default_proxies, message);
+ if (r < 0)
+ sd_bus_reply_method_return(message, "s", p);
Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?
Also, why allocate "DIRECT" with strdup() at all?
There are no errors. Either you get a proxy directive or you return
DIRECT to indicate no proxy. What would you do in an error case
anyway. The backup is always assume no proxy.
Well, so far it has been our coding styles to propagate errors up if
they cause the invoked operations to fail, and allow the higher up
code deal with them and possibly eat them up. I mean, the HTTP client
can eat the error up and downgrade to DIRECT on its own, there's no
need to do this from our side already.
of course it can, but it also does not help you at all. You are not making anything better here.
I think that one clean interface to this needs to be a API
compatible libproxy. Similar to what we did in PACrunner since that
allows existing application to just use systemd-proxydiscoverd.
Well, clients have to deal with errors anyway, since they are talking
to this via dbus. I mean, the service might simply be missing on the
system, and the client code should then downgrade to DIRECT
anyway... Hence, the right client side behaviour is to eat up *all*
error conditions, regardless if they stem from the dbus code, the
socket calls used by dbus or ultimately from the proxy discovery
daemon...

Lennart
--
Lennart Poettering, Red Hat
Tomasz Bursztyka
2015-04-13 11:45:29 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Marcel Holtmann
Post by Lennart Poettering
Hmm, is this right? Shouldn't we return the error code to the client
instead of eating up and returning "DIRECT"?
Also, why allocate "DIRECT" with strdup() at all?
There are no errors. Either you get a proxy directive or you return
DIRECT to indicate no proxy. What would you do in an error case
anyway. The backup is always assume no proxy.
Well, so far it has been our coding styles to propagate errors up if
they cause the invoked operations to fail, and allow the higher up
code deal with them and possibly eat them up. I mean, the HTTP client
can eat the error up and downgrade to DIRECT on its own, there's no
need to do this from our side already.
of course it can, but it also does not help you at all. You are not making anything better here.
I think that one clean interface to this needs to be a API
compatible libproxy. Similar to what we did in PACrunner since that
allows existing application to just use systemd-proxydiscoverd.
Well, clients have to deal with errors anyway, since they are talking
to this via dbus. I mean, the service might simply be missing on the
system, and the client code should then downgrade to DIRECT
anyway... Hence, the right client side behaviour is to eat up *all*
error conditions, regardless if they stem from the dbus code, the
socket calls used by dbus or ultimately from the proxy discovery
daemon...
Either way is fine to me, the end result will be the same anyway: the client
cannot do anything but trying to connect directly. It will need to be
clearly
told in the api documentation.

Tomasz
Tomasz Bursztyka
2015-04-10 12:17:40 UTC
Permalink
The proxy PAC file - basically a javascript file - will be executed through
duktape js engine (duktape.org). For this to work, one will need to embed
duktape.h/c files in src/proxy-discovery/.

The current support is sub-optimal as it might generate context switches
via ioctls or netlink calls. This will be fixed later on.
---
Makefile.am | 7 ++
src/proxy-discovery/proxy-discoveryd-pac.c | 185 +++++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.h | 13 ++
3 files changed, 205 insertions(+)
create mode 100644 src/proxy-discovery/proxy-discoveryd-pac.c

diff --git a/Makefile.am b/Makefile.am
index 6e839b9..25ea0dd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd

systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
src/proxy-discovery/proxy-discoveryd.c \
src/proxy-discovery/proxy-discoveryd.h \
src/proxy-discovery/proxy-discoveryd-manager.c \
+ src/proxy-discovery/proxy-discoveryd-pac.c \
src/proxy-discovery/proxy-discoveryd-proxy.c \
src/proxy-discovery/proxy-discoveryd-proxy-gperf.c

+systemd_proxy_discoveryd_CFLAGS = \
+ $(AM_CFLAGS) \
+ -fno-fast-math
+
systemd_proxy_discoveryd_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la
diff --git a/src/proxy-discovery/proxy-discoveryd-pac.c b/src/proxy-discovery/proxy-discoveryd-pac.c
new file mode 100644
index 0000000..5c779bb
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-pac.c
@@ -0,0 +1,185 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "duktape.h"
+
+#include "proxy-discoveryd.h"
+#include "local-addresses.h"
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <net/if.h>
+
+struct PAC {
+ duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
+ struct ifreq ifr = {};
+ int sk;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0)
+ return -1;
+
+ ifr.ifr_ifindex = ifindex;
+
+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
+ close(sk);
+ return -1;
+ }
+
+ address->in = ((struct sockaddr_in *) &ifr.ifr_addr)->sin_addr;
+
+ close(sk);
+
+ return 0;
+}
+
+static int pac_dns_resolve(duk_context *ctx) {
+ _cleanup_free_ char *address = NULL;
+ struct addrinfo hints = {};
+ struct addrinfo *res, *addr;
+ const char *hostname;
+ int r;
+
+ hostname = duk_require_string(ctx, 0);
+
+ hints.ai_family = AF_INET;
+
+ r = getaddrinfo(hostname, NULL, &hints, &res);
+ if (r != 0)
+ return 0;
+
+ for (addr = res; addr; addr = addr->ai_next) {
+ union in_addr_union a;
+
+ if (addr->ai_family != AF_INET ||
+ addr->ai_addrlen != sizeof(struct sockaddr_in))
+ continue;
+
+ a.in = ((struct sockaddr_in *) addr->ai_addr)->sin_addr;
+ in_addr_to_string(AF_INET, &a, &address);
+
+ break;
+ }
+
+ freeaddrinfo(res);
+
+ duk_push_string(ctx, address);
+
+ return 1;
+}
+
+static int pac_my_ip_address(duk_context *ctx) {
+ _cleanup_free_ struct local_address *gateways = NULL;
+ _cleanup_free_ char *address = NULL;
+ union in_addr_union addr;
+ int r;
+
+ r = local_gateways(NULL, 0, AF_INET, &gateways);
+ if (r <= 0)
+ return 0;
+
+ r = get_addresses_from_interface(gateways->ifindex, &addr);
+ if (r < 0)
+ return 0;
+
+ in_addr_to_string(AF_INET, &addr, &address);
+
+ duk_push_string(ctx, address);
+
+ return 1;
+}
+
+static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+ duk_context *ctx;
+
+ ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
+ if (!ctx)
+ return -ENOMEM;
+
+ duk_push_global_object(ctx);
+ duk_push_c_function(ctx, pac_dns_resolve, 1);
+ duk_put_prop_string(ctx, -2, "dnsResolve");
+ duk_push_c_function(ctx, pac_my_ip_address, 0);
+ duk_put_prop_string(ctx, -2, "myIpAddress");
+
+ duk_push_pointer(ctx, user_data);
+ duk_put_prop_string(ctx, -2, "_user_data_");
+
+ duk_pop(ctx);
+
+ if (duk_peval_file(ctx, pac_file) != 0) {
+ duk_destroy_heap(ctx);
+ return -EINVAL;
+ }
+
+ pac->ctx = ctx;
+
+ return 0;
+}
+
+int pac_load(Proxy *proxy, struct PAC **ret) {
+ _cleanup_pac_free_ struct PAC *pac = NULL;
+ int r;
+
+ assert(proxy);
+ assert(ret);
+
+ pac = new0(struct PAC, 1);
+ if (!pac)
+ return -ENOMEM;
+
+ r = create_context(pac, proxy->pac_path, proxy);
+ if (r < 0)
+ return r;
+
+ *ret = pac;
+ pac = NULL;
+
+ return 0;
+}
+
+void pac_free(struct PAC *pac) {
+ if (pac->ctx)
+ duk_destroy_heap(pac->ctx);
+
+ free(pac);
+}
+
+int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret) {
+ duk_push_global_object(pac->ctx);
+ duk_get_prop_string(pac->ctx, -1, "FindProxyForURL");
+ duk_push_string(pac->ctx, url);
+ duk_push_string(pac->ctx, host);
+
+ if (duk_pcall(pac->ctx, 2) != DUK_EXEC_SUCCESS)
+ return -1;
+
+ *ret = strdup(duk_to_string(pac->ctx, -1));
+
+ duk_pop(pac->ctx);
+ duk_pop(pac->ctx);
+
+ return 0;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
index f51fca5..17c5378 100644
--- a/src/proxy-discovery/proxy-discoveryd.h
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -35,10 +35,13 @@ struct Manager {
LIST_HEAD(Proxy, default_proxies);
};

+struct PAC;
+
struct Proxy {
Manager *manager;

char *pac_path;
+ struct PAC *pac;

LIST_FIELDS(Proxy, proxy_next);
};
@@ -63,3 +66,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Proxy*, proxy_free);
#define _cleanup_proxy_free_ _cleanup_(proxy_freep)

const struct ConfigPerfItem* proxy_discoveryd_proxy_gperf_lookup(const char *key, unsigned length);
+
+/* PAC */
+
+int pac_load(Proxy *proxy, struct PAC **ret);
+void pac_free(struct PAC *pac);
+
+int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct PAC*, pac_free);
+#define _cleanup_pac_free_ _cleanup_(pac_freep)
--
2.0.5
Tom Gundersen
2015-04-10 13:20:18 UTC
Permalink
On Fri, Apr 10, 2015 at 2:17 PM, Tomasz Bursztyka
Post by Tomasz Bursztyka
The proxy PAC file - basically a javascript file - will be executed through
duktape js engine (duktape.org). For this to work, one will need to embed
duktape.h/c files in src/proxy-discovery/.
The current support is sub-optimal as it might generate context switches
via ioctls or netlink calls. This will be fixed later on.
---
Makefile.am | 7 ++
src/proxy-discovery/proxy-discoveryd-pac.c | 185 +++++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.h | 13 ++
3 files changed, 205 insertions(+)
create mode 100644 src/proxy-discovery/proxy-discoveryd-pac.c
diff --git a/Makefile.am b/Makefile.am
index 6e839b9..25ea0dd 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
Post by Tomasz Bursztyka
src/proxy-discovery/proxy-discoveryd.c \
src/proxy-discovery/proxy-discoveryd.h \
src/proxy-discovery/proxy-discoveryd-manager.c \
+ src/proxy-discovery/proxy-discoveryd-pac.c \
src/proxy-discovery/proxy-discoveryd-proxy.c \
src/proxy-discovery/proxy-discoveryd-proxy-gperf.c
+systemd_proxy_discoveryd_CFLAGS = \
+ $(AM_CFLAGS) \
+ -fno-fast-math
Hm, what's this all about?
Post by Tomasz Bursztyka
+
systemd_proxy_discoveryd_LDADD = \
libsystemd-internal.la \
libsystemd-shared.la
diff --git a/src/proxy-discovery/proxy-discoveryd-pac.c b/src/proxy-discovery/proxy-discoveryd-pac.c
new file mode 100644
index 0000000..5c779bb
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-pac.c
@@ -0,0 +1,185 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "duktape.h"
+
+#include "proxy-discoveryd.h"
+#include "local-addresses.h"
+
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <netdb.h>
+#include <net/if.h>
+
+struct PAC {
+ duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
+ struct ifreq ifr = {};
+ int sk;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0)
+ return -1;
Never return 'made up' error values. In this case return -errno, for
internal code (where 'r' is used instead of errno) just return r.
Post by Tomasz Bursztyka
+
+ ifr.ifr_ifindex = ifindex;
+
+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
+ close(sk);
+ return -1;
Same as above, return -errno. Also split this up into two:

r = ioctl(sk, SIOCGIFNAME, &ifr);
if (r < 0)
return -errno;

etc.

If you also use

_cleanup_close_ int sk = -1;

you don't have to worry about closing the socket.
Post by Tomasz Bursztyka
+ }
+
+ address->in = ((struct sockaddr_in *) &ifr.ifr_addr)->sin_addr;
+
+ close(sk);
+
+ return 0;
+}
The above function is fine as part of a prototype, but for the final
version we should use rtnl like everywhere else, no?
Post by Tomasz Bursztyka
+static int pac_dns_resolve(duk_context *ctx) {
+ _cleanup_free_ char *address = NULL;
+ struct addrinfo hints = {};
+ struct addrinfo *res, *addr;
+ const char *hostname;
+ int r;
+
+ hostname = duk_require_string(ctx, 0);
+
+ hints.ai_family = AF_INET;
+
+ r = getaddrinfo(hostname, NULL, &hints, &res);
+ if (r != 0)
+ return 0;
+
+ for (addr = res; addr; addr = addr->ai_next) {
+ union in_addr_union a;
+
+ if (addr->ai_family != AF_INET ||
+ addr->ai_addrlen != sizeof(struct sockaddr_in))
+ continue;
+
+ a.in = ((struct sockaddr_in *) addr->ai_addr)->sin_addr;
+ in_addr_to_string(AF_INET, &a, &address);
+
+ break;
+ }
+
+ freeaddrinfo(res);
+
+ duk_push_string(ctx, address);
+
+ return 1;
+}
+
+static int pac_my_ip_address(duk_context *ctx) {
+ _cleanup_free_ struct local_address *gateways = NULL;
+ _cleanup_free_ char *address = NULL;
+ union in_addr_union addr;
+ int r;
+
+ r = local_gateways(NULL, 0, AF_INET, &gateways);
+ if (r <= 0)
+ return 0;
+
+ r = get_addresses_from_interface(gateways->ifindex, &addr);
+ if (r < 0)
+ return 0;
+
+ in_addr_to_string(AF_INET, &addr, &address);
+
+ duk_push_string(ctx, address);
+
+ return 1;
+}
+
+static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+ duk_context *ctx;
+
+ ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
+ if (!ctx)
+ return -ENOMEM;
+
+ duk_push_global_object(ctx);
+ duk_push_c_function(ctx, pac_dns_resolve, 1);
+ duk_put_prop_string(ctx, -2, "dnsResolve");
+ duk_push_c_function(ctx, pac_my_ip_address, 0);
+ duk_put_prop_string(ctx, -2, "myIpAddress");
+
+ duk_push_pointer(ctx, user_data);
+ duk_put_prop_string(ctx, -2, "_user_data_");
+
+ duk_pop(ctx);
+
+ if (duk_peval_file(ctx, pac_file) != 0) {
+ duk_destroy_heap(ctx);
+ return -EINVAL;
+ }
+
+ pac->ctx = ctx;
+
+ return 0;
+}
+
+int pac_load(Proxy *proxy, struct PAC **ret) {
+ _cleanup_pac_free_ struct PAC *pac = NULL;
+ int r;
+
+ assert(proxy);
+ assert(ret);
+
+ pac = new0(struct PAC, 1);
+ if (!pac)
+ return -ENOMEM;
+
+ r = create_context(pac, proxy->pac_path, proxy);
+ if (r < 0)
+ return r;
+
+ *ret = pac;
+ pac = NULL;
+
+ return 0;
+}
+
+void pac_free(struct PAC *pac) {
+ if (pac->ctx)
+ duk_destroy_heap(pac->ctx);
+
+ free(pac);
+}
+
+int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret) {
+ duk_push_global_object(pac->ctx);
+ duk_get_prop_string(pac->ctx, -1, "FindProxyForURL");
+ duk_push_string(pac->ctx, url);
+ duk_push_string(pac->ctx, host);
+
+ if (duk_pcall(pac->ctx, 2) != DUK_EXEC_SUCCESS)
+ return -1;
Return real error value, not -1.
Post by Tomasz Bursztyka
+
+ *ret = strdup(duk_to_string(pac->ctx, -1));
+
+ duk_pop(pac->ctx);
+ duk_pop(pac->ctx);
+
+ return 0;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
index f51fca5..17c5378 100644
--- a/src/proxy-discovery/proxy-discoveryd.h
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -35,10 +35,13 @@ struct Manager {
LIST_HEAD(Proxy, default_proxies);
};
+struct PAC;
+
struct Proxy {
Manager *manager;
char *pac_path;
+ struct PAC *pac;
LIST_FIELDS(Proxy, proxy_next);
};
@@ -63,3 +66,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Proxy*, proxy_free);
#define _cleanup_proxy_free_ _cleanup_(proxy_freep)
const struct ConfigPerfItem* proxy_discoveryd_proxy_gperf_lookup(const char *key, unsigned length);
+
+/* PAC */
+
+int pac_load(Proxy *proxy, struct PAC **ret);
+void pac_free(struct PAC *pac);
+
+int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct PAC*, pac_free);
+#define _cleanup_pac_free_ _cleanup_(pac_freep)
--
2.0.5
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Tomasz Bursztyka
2015-04-13 10:24:57 UTC
Permalink
Hi Tom,
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will be
time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
Post by Tom Gundersen
Post by Tomasz Bursztyka
src/proxy-discovery/proxy-discoveryd.c \
src/proxy-discovery/proxy-discoveryd.h \
src/proxy-discovery/proxy-discoveryd-manager.c \
+ src/proxy-discovery/proxy-discoveryd-pac.c \
src/proxy-discovery/proxy-discoveryd-proxy.c \
src/proxy-discovery/proxy-discoveryd-proxy-gperf.c
+systemd_proxy_discoveryd_CFLAGS = \
+ $(AM_CFLAGS) \
+ -fno-fast-math
Hm, what's this all about?
duktape refuses to compile with fast-math directive. It can be forced
however.
I just avoided it for the PoC.
Post by Tom Gundersen
Post by Tomasz Bursztyka
+ }
+
+ address->in = ((struct sockaddr_in *) &ifr.ifr_addr)->sin_addr;
+
+ close(sk);
+
+ return 0;
+}
The above function is fine as part of a prototype, but for the final
version we should use rtnl like everywhere else, no?
Yep, I only did that for a working PoC. But rtnl based logic should be
the way to go.


I'll apply the return code comments.

Tomasz
Zbigniew Jędrzejewski-Szmek
2015-04-13 13:09:28 UTC
Permalink
Post by Tomasz Bursztyka
Hi Tom,
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will
be time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.

Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.

Zbyszek
Marcel Holtmann
2015-04-13 15:43:32 UTC
Permalink
Hi Zbyszek,
Post by Zbigniew Jędrzejewski-Szmek
Post by Tomasz Bursztyka
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will
be time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.
Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.
that is not how this actually works with duktape. The release versions are building a single duktape.[ch] that you can only find in the releases. Otherwise you have to stitch that manually. It is pretty much designed to be included into other project as a copy. That is why nobody has actually packages this so far.

So while I get that including copies of other libraries is generally bad for security update reasons, but this one would qualify as an exception. It is just that we have to monitor duktape upstream releases and with that also update the duktape version in systemd.

It is something that is not the greatest solution, but something that is feasible. Keep in mind that embedding your JS engine right into your application allows for optimizations that you really want. We have build PACrunner against V8 and MozJS and these are two really painful experiences. For anybody wanting to build a small system, dealing with these two upstream packages is a nightmare. It sort of works in a large distribution like Fedora, but anything small it is not an option.

Regards

Marcel
Zbigniew Jędrzejewski-Szmek
2015-04-13 16:41:25 UTC
Permalink
Post by Marcel Holtmann
Hi Zbyszek,
Post by Zbigniew Jędrzejewski-Szmek
Post by Tomasz Bursztyka
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will
be time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.
Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.
that is not how this actually works with duktape. The release versions are building a single duktape.[ch] that you can only find in the releases. Otherwise you have to stitch that manually. It is pretty much designed to be included into other project as a copy. That is why nobody has actually packages this so far.
So while I get that including copies of other libraries is generally bad for security update reasons, but this one would qualify as an exception. It is just that we have to monitor duktape upstream releases and with that also update the duktape version in systemd.
It is something that is not the greatest solution, but something that is feasible. Keep in mind that embedding your JS engine right into your application allows for optimizations that you really want. We have build PACrunner against V8 and MozJS and these are two really painful experiences. For anybody wanting to build a small system, dealing with these two upstream packages is a nightmare. It sort of works in a large distribution like Fedora, but anything small it is not an option.
I know that embedding is the way that upstream suggests. But it is a
bad fit for the way that systemd is distributed. Our primary consumers
are distributions, and the first thing most distributions will do is
to rip out the embedded copy in order to use a shared one. Something
as big as a javascript engine is bound to have security updates and by
embedding it systemd we would be forced to a) follow upstream changes,
b) release systemd updates based on duktape releases. Not something that
we want to do or would do very well.

We really should try to base this on a javascript engine which
provides a shared library.

Zbyszek
Marcel Holtmann
2015-04-13 16:51:05 UTC
Permalink
Hi Zbyszek,
Post by Zbigniew Jędrzejewski-Szmek
Post by Marcel Holtmann
Post by Zbigniew Jędrzejewski-Szmek
Post by Tomasz Bursztyka
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will
be time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.
Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.
that is not how this actually works with duktape. The release versions are building a single duktape.[ch] that you can only find in the releases. Otherwise you have to stitch that manually. It is pretty much designed to be included into other project as a copy. That is why nobody has actually packages this so far.
So while I get that including copies of other libraries is generally bad for security update reasons, but this one would qualify as an exception. It is just that we have to monitor duktape upstream releases and with that also update the duktape version in systemd.
It is something that is not the greatest solution, but something that is feasible. Keep in mind that embedding your JS engine right into your application allows for optimizations that you really want. We have build PACrunner against V8 and MozJS and these are two really painful experiences. For anybody wanting to build a small system, dealing with these two upstream packages is a nightmare. It sort of works in a large distribution like Fedora, but anything small it is not an option.
I know that embedding is the way that upstream suggests. But it is a
bad fit for the way that systemd is distributed. Our primary consumers
are distributions, and the first thing most distributions will do is
to rip out the embedded copy in order to use a shared one. Something
as big as a javascript engine is bound to have security updates and by
embedding it systemd we would be forced to a) follow upstream changes,
b) release systemd updates based on duktape releases. Not something that
we want to do or would do very well.
We really should try to base this on a javascript engine which
provides a shared library.
so do you know of a small Javascript engine with a proper license then? And that also has no additional dependencies.

I was not kidding when I said the V8 and MozJS are a nightmare when you want to build a small system. So these two are pretty much out of the question for the PAC service.

Regards

Marcel
Zbigniew Jędrzejewski-Szmek
2015-04-13 17:10:54 UTC
Permalink
Post by Marcel Holtmann
Hi Zbyszek,
Post by Zbigniew Jędrzejewski-Szmek
Post by Marcel Holtmann
Post by Zbigniew Jędrzejewski-Szmek
Post by Tomasz Bursztyka
Post by Tom Gundersen
Post by Tomasz Bursztyka
--- a/Makefile.am
+++ b/Makefile.am
@@ -5895,12 +5895,19 @@ rootlibexec_PROGRAMS += \
systemd-proxy-discoveryd
systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/duktape.h \
+ src/proxy-discovery/duktape.c \
These files are not included in the patch, how do you intend to ship them?
I figured this could be included as a separate commit, when it will
be time to commit proxy-discoveryd someday.
Those 2 files represent more than 2 megabytes, so I did not want to
flood the ml.
duktape should not be embedded in systemd. It's a separate project,
which gets updates and has a life of its own.
Preferably, duktape would be packaged by distributions and we would
use it like any other external library. If that is not possible, we'll
have to look for some other option, but only then. For example,
git submodule is still better than embedding of a snapshot.
that is not how this actually works with duktape. The release versions are building a single duktape.[ch] that you can only find in the releases. Otherwise you have to stitch that manually. It is pretty much designed to be included into other project as a copy. That is why nobody has actually packages this so far.
So while I get that including copies of other libraries is generally bad for security update reasons, but this one would qualify as an exception. It is just that we have to monitor duktape upstream releases and with that also update the duktape version in systemd.
It is something that is not the greatest solution, but something that is feasible. Keep in mind that embedding your JS engine right into your application allows for optimizations that you really want. We have build PACrunner against V8 and MozJS and these are two really painful experiences. For anybody wanting to build a small system, dealing with these two upstream packages is a nightmare. It sort of works in a large distribution like Fedora, but anything small it is not an option.
I know that embedding is the way that upstream suggests. But it is a
bad fit for the way that systemd is distributed. Our primary consumers
are distributions, and the first thing most distributions will do is
to rip out the embedded copy in order to use a shared one. Something
as big as a javascript engine is bound to have security updates and by
embedding it systemd we would be forced to a) follow upstream changes,
b) release systemd updates based on duktape releases. Not something that
we want to do or would do very well.
We really should try to base this on a javascript engine which
provides a shared library.
so do you know of a small Javascript engine with a proper license then? And that also has no additional dependencies.
No, but I don't think this can be the guiding principle here. We
already have dependencies on a large number of things: gnutls,
docbook, unifont, libmicrohttpd, python, and a bunch of smaller
libraries. I know that another dependency might be difficult for
embedded systems, but a "small" library might actually be more work to
maintain the long run than bigger library which is easier to integrate.
Post by Marcel Holtmann
I was not kidding when I said the V8 and MozJS are a nightmare when you want to build a small system. So these two are pretty much out of the question for the PAC service.
I certainly can believe that ;)

Zbyszek
Lennart Poettering
2015-04-10 15:49:17 UTC
Permalink
Post by Tomasz Bursztyka
+struct PAC {
+ duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
+ struct ifreq ifr = {};
+ int sk;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0)
+ return -1;
No made up errors please! Return -errno or so.
Post by Tomasz Bursztyka
+
+ ifr.ifr_ifindex = ifindex;
+
+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
+ close(sk);
+ return -1;
Same here...

Also, please don't use the old ioctls for querying network
information. Use netlink through sd-rtnl. You can look at the
systemd-resolved sources, they do this already, and this code should
probably do it very similar from that.
Post by Tomasz Bursztyka
+static int pac_dns_resolve(duk_context *ctx) {
+ _cleanup_free_ char *address = NULL;
+ struct addrinfo hints = {};
+ struct addrinfo *res, *addr;
+ const char *hostname;
+ int r;
+
+ hostname = duk_require_string(ctx, 0);
+
+ hints.ai_family = AF_INET;
+
+ r = getaddrinfo(hostname, NULL, &hints, &res);
+ if (r != 0)
+ return 0;
Hm, synchronous getaddrinfo() is nasty... Please use sd-resolve for
this, which adds asynchronous getaddrinfo() for cases like this...
Post by Tomasz Bursztyka
+
+ for (addr = res; addr; addr = addr->ai_next) {
+ union in_addr_union a;
+
+ if (addr->ai_family != AF_INET ||
+ addr->ai_addrlen != sizeof(struct sockaddr_in))
+ continue;
+
+ a.in = ((struct sockaddr_in *) addr->ai_addr)->sin_addr;
+ in_addr_to_string(AF_INET, &a, &address);
This can actually fail need to check for OOM...
Post by Tomasz Bursztyka
+ r = local_gateways(NULL, 0, AF_INET, &gateways);
+ if (r <= 0)
+ return 0;
+
+ r = get_addresses_from_interface(gateways->ifindex, &addr);
+ if (r < 0)
+ return 0;
BTW; local_addresses() can do this for you...
Post by Tomasz Bursztyka
+ in_addr_to_string(AF_INET, &addr, &address);
Can fail, due to OOM, needs OOM check...
Post by Tomasz Bursztyka
+static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+ duk_context *ctx;
+
+ ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
+ if (!ctx)
+ return -ENOMEM;
+
+ duk_push_global_object(ctx);
+ duk_push_c_function(ctx, pac_dns_resolve, 1);
+ duk_put_prop_string(ctx, -2, "dnsResolve");
+ duk_push_c_function(ctx, pac_my_ip_address, 0);
+ duk_put_prop_string(ctx, -2, "myIpAddress");
+
+ duk_push_pointer(ctx, user_data);
+ duk_put_prop_string(ctx, -2, "_user_data_");
+
+ duk_pop(ctx);
+
+ if (duk_peval_file(ctx, pac_file) != 0) {
+ duk_destroy_heap(ctx);
+ return -EINVAL;
+ }
How is error handling done in duktape? The individual functions cannot
fail? And are any errors returned?
Post by Tomasz Bursztyka
+int pac_execute(struct PAC *pac, const char *url, const char *host, char **ret) {
+ duk_push_global_object(pac->ctx);
+ duk_get_prop_string(pac->ctx, -1, "FindProxyForURL");
+ duk_push_string(pac->ctx, url);
+ duk_push_string(pac->ctx, host);
+
+ if (duk_pcall(pac->ctx, 2) != DUK_EXEC_SUCCESS)
+ return -1;
+
+ *ret = strdup(duk_to_string(pac->ctx, -1));
Missing OOM check...

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-10 22:26:30 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+struct PAC {
+ duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
+ struct ifreq ifr = {};
+ int sk;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0)
+ return -1;
No made up errors please! Return -errno or so.
Post by Tomasz Bursztyka
+
+ ifr.ifr_ifindex = ifindex;
+
+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
+ close(sk);
+ return -1;
Same here...
Also, please don't use the old ioctls for querying network
information. Use netlink through sd-rtnl. You can look at the
systemd-resolved sources, they do this already, and this code should
probably do it very similar from that.
Post by Tomasz Bursztyka
+static int pac_dns_resolve(duk_context *ctx) {
+ _cleanup_free_ char *address = NULL;
+ struct addrinfo hints = {};
+ struct addrinfo *res, *addr;
+ const char *hostname;
+ int r;
+
+ hostname = duk_require_string(ctx, 0);
+
+ hints.ai_family = AF_INET;
+
+ r = getaddrinfo(hostname, NULL, &hints, &res);
+ if (r != 0)
+ return 0;
Hm, synchronous getaddrinfo() is nasty... Please use sd-resolve for
this, which adds asynchronous getaddrinfo() for cases like this...
you do realize that you want this synchronous. These are the magic dnsResolve and myIpAddress Javascript functions that are expected to be present when executing the PAC file. There are a few more, but they can be implemented in Javascript and don't need a C backend. These two actually need that. So you need to report the result.

Regards

Marcel
Lennart Poettering
2015-04-12 18:32:59 UTC
Permalink
Post by Marcel Holtmann
Post by Lennart Poettering
Post by Tomasz Bursztyka
+static int pac_dns_resolve(duk_context *ctx) {
+ _cleanup_free_ char *address = NULL;
+ struct addrinfo hints = {};
+ struct addrinfo *res, *addr;
+ const char *hostname;
+ int r;
+
+ hostname = duk_require_string(ctx, 0);
+
+ hints.ai_family = AF_INET;
+
+ r = getaddrinfo(hostname, NULL, &hints, &res);
+ if (r != 0)
+ return 0;
Hm, synchronous getaddrinfo() is nasty... Please use sd-resolve for
this, which adds asynchronous getaddrinfo() for cases like this...
you do realize that you want this synchronous. These are the magic
dnsResolve and myIpAddress Javascript functions that are expected to
be present when executing the PAC file. There are a few more, but
they can be implemented in Javascript and don't need a C
backend. These two actually need that. So you need to report the
result.
Ah I see, this function is only ever called from the PAC javascript
code? OK, in that case it should be synchronous indeed.

Lennart
--
Lennart Poettering, Red Hat
Tomasz Bursztyka
2015-04-13 11:30:44 UTC
Permalink
Post by Lennart Poettering
Post by Tomasz Bursztyka
+struct PAC {
+ duk_context *ctx;
+};
+
+static int get_addresses_from_interface(int ifindex, union in_addr_union *address) {
+ struct ifreq ifr = {};
+ int sk;
+
+ sk = socket(AF_INET, SOCK_DGRAM, 0);
+ if (sk < 0)
+ return -1;
No made up errors please! Return -errno or so.
Post by Tomasz Bursztyka
+
+ ifr.ifr_ifindex = ifindex;
+
+ if (ioctl(sk, SIOCGIFNAME, &ifr) < 0 || ioctl(sk, SIOCGIFADDR, &ifr) < 0) {
+ close(sk);
+ return -1;
Same here...
Also, please don't use the old ioctls for querying network
information. Use netlink through sd-rtnl. You can look at the
systemd-resolved sources, they do this already, and this code should
probably do it very similar from that.
Yes, it was just to get something working as a PoC. But nothing to be
pushed.
I started to work on an sd-rtnl part that will keep track of the
interfaces, their IPs and
which one it tight to the default route.
Post by Lennart Poettering
Post by Tomasz Bursztyka
+static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+ duk_context *ctx;
+
+ ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
+ if (!ctx)
+ return -ENOMEM;
+
+ duk_push_global_object(ctx);
+ duk_push_c_function(ctx, pac_dns_resolve, 1);
+ duk_put_prop_string(ctx, -2, "dnsResolve");
+ duk_push_c_function(ctx, pac_my_ip_address, 0);
+ duk_put_prop_string(ctx, -2, "myIpAddress");
+
+ duk_push_pointer(ctx, user_data);
+ duk_put_prop_string(ctx, -2, "_user_data_");
+
+ duk_pop(ctx);
+
+ if (duk_peval_file(ctx, pac_file) != 0) {
+ duk_destroy_heap(ctx);
+ return -EINVAL;
+ }
How is error handling done in duktape? The individual functions cannot
fail? And are any errors returned?
There are yes. Let's use them. I'll also hook the internal duktape
failure functions etc...

And I'll apply the other comments as well

Tomasz
Tomasz Bursztyka
2015-04-10 12:17:41 UTC
Permalink
This is sub-optimal as duktape JS engine does not provide any mean to
share the same context over different threads. Each one needs its own,
thus the need to load the PAC file everytime. This will need to be
investigated and fixed.
---
src/proxy-discovery/proxy-discoveryd-pac.c | 7 +---
src/proxy-discovery/proxy-discoveryd-proxy.c | 58 ++++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.h | 2 +
3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/src/proxy-discovery/proxy-discoveryd-pac.c b/src/proxy-discovery/proxy-discoveryd-pac.c
index 5c779bb..1999b78 100644
--- a/src/proxy-discovery/proxy-discoveryd-pac.c
+++ b/src/proxy-discovery/proxy-discoveryd-pac.c
@@ -111,7 +111,7 @@ static int pac_my_ip_address(duk_context *ctx) {
return 1;
}

-static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
+static int create_context(struct PAC *pac, char *pac_file) {
duk_context *ctx;

ctx = duk_create_heap(NULL, NULL, NULL, NULL, NULL);
@@ -124,9 +124,6 @@ static int create_context(struct PAC *pac, char *pac_file, void *user_data) {
duk_push_c_function(ctx, pac_my_ip_address, 0);
duk_put_prop_string(ctx, -2, "myIpAddress");

- duk_push_pointer(ctx, user_data);
- duk_put_prop_string(ctx, -2, "_user_data_");
-
duk_pop(ctx);

if (duk_peval_file(ctx, pac_file) != 0) {
@@ -150,7 +147,7 @@ int pac_load(Proxy *proxy, struct PAC **ret) {
if (!pac)
return -ENOMEM;

- r = create_context(pac, proxy->pac_path, proxy);
+ r = create_context(pac, proxy->pac_path);
if (r < 0)
return r;

diff --git a/src/proxy-discovery/proxy-discoveryd-proxy.c b/src/proxy-discovery/proxy-discoveryd-proxy.c
index d18395b..43d258f 100644
--- a/src/proxy-discovery/proxy-discoveryd-proxy.c
+++ b/src/proxy-discovery/proxy-discoveryd-proxy.c
@@ -22,6 +22,12 @@
#include "proxy-discoveryd.h"

#include "conf-parser.h"
+#include "async.h"
+
+struct proxy_parameters {
+ sd_bus_message *message;
+ Proxy *proxy;
+};

void proxy_free(Proxy *proxy) {
if (!proxy)
@@ -75,3 +81,55 @@ int proxy_load(Manager *manager, const char *filename, Proxy **ret) {

return 0;
}
+
+static void *run_thread(void *data) {
+ _cleanup_free_ struct proxy_parameters *params = NULL;
+ _cleanup_pac_free_ struct PAC *pac = NULL;
+ _cleanup_free_ char *result = NULL;
+ const char *url, *host;
+
+ params = data;
+
+ if (sd_bus_message_read(params->message, "ss", &url, &host) < 0)
+ goto answer;
+
+ if (pac_load(params->proxy, &pac) < 0)
+ goto answer;
+
+ pac_execute(pac, url, host, &result);
+
+answer:
+ if (!result)
+ result = strdup("DIRECT");
+
+ sd_bus_reply_method_return(params->message, "s", result);
+
+ params->message = sd_bus_message_unref(params->message);
+
+ return NULL;
+}
+
+int proxy_execute(Proxy *proxy, sd_bus_message *message) {
+ _cleanup_free_ struct proxy_parameters *params = NULL;
+ int r;
+
+ if (!proxy)
+ return -EINVAL;
+
+ params = new0(struct proxy_parameters, 1);
+ if (!params)
+ return log_oom();
+
+ params->message = sd_bus_message_ref(message);
+ params->proxy = proxy;
+
+ r = asynchronous_job(run_thread, params);
+ if (r < 0) {
+ params->message = sd_bus_message_unref(params->message);
+ return r;
+ }
+
+ params = NULL;
+
+ return 0;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
index 17c5378..d8d9482 100644
--- a/src/proxy-discovery/proxy-discoveryd.h
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -22,6 +22,7 @@
***/

#include "sd-event.h"
+#include "sd-bus.h"

#include "util.h"
#include "list.h"
@@ -61,6 +62,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
void proxy_free(Proxy *proxy);

int proxy_load(Manager *manager, const char *filename, Proxy **ret);
+int proxy_execute(Proxy *proxy, sd_bus_message *message);

DEFINE_TRIVIAL_CLEANUP_FUNC(Proxy*, proxy_free);
#define _cleanup_proxy_free_ _cleanup_(proxy_freep)
--
2.0.5
Lennart Poettering
2015-04-10 15:56:33 UTC
Permalink
Post by Tomasz Bursztyka
+static void *run_thread(void *data) {
+ _cleanup_free_ struct proxy_parameters *params = NULL;
+ _cleanup_pac_free_ struct PAC *pac = NULL;
+ _cleanup_free_ char *result = NULL;
+ const char *url, *host;
+
+ params = data;
+
+ if (sd_bus_message_read(params->message, "ss", &url, &host) < 0)
+ goto answer;
+
+ if (pac_load(params->proxy, &pac) < 0)
+ goto answer;
+
+ pac_execute(pac, url, host, &result);
+
+ if (!result)
+ result = strdup("DIRECT");
+
+ sd_bus_reply_method_return(params->message, "s", result);
sd-bus is not thread-safe. You cannot send messages from different
threads at the same time...

Why precisely do you need threads here? Because the PAC programs might
be slow?

WHat about unbounded loops in PAC programs? We need to protect from
that, which kinda suggests we muight want to run the PAC stuff out of
process so that we can neatly kill the program when it runs for too
long?
Post by Tomasz Bursztyka
+
+ params->message = sd_bus_message_unref(params->message);
+
+ return NULL;
+}
+
+int proxy_execute(Proxy *proxy, sd_bus_message *message) {
+ _cleanup_free_ struct proxy_parameters *params = NULL;
+ int r;
+
+ if (!proxy)
+ return -EINVAL;
+
+ params = new0(struct proxy_parameters, 1);
+ if (!params)
+ return log_oom();
Hmm, so we generally try to write functions so that they log all
errors on their own, or none (so that the caller can log instead). In
the upper error condition you fail with -EINVAL without logging, in
the lower error condition you fail with -ENOMEM but log. Please decide
whether the function should log all errors or none, and make the
caller handle this nicely. Otherwise we might easily get into cases
where errors are logged twice or none at all.

Usually for more "library-like" calls we leave the logging to the
caller, and for "main-program-like" calls we log in the calls
themselves, but the line between these two cases are blurry...

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-10 22:31:50 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+static void *run_thread(void *data) {
+ _cleanup_free_ struct proxy_parameters *params = NULL;
+ _cleanup_pac_free_ struct PAC *pac = NULL;
+ _cleanup_free_ char *result = NULL;
+ const char *url, *host;
+
+ params = data;
+
+ if (sd_bus_message_read(params->message, "ss", &url, &host) < 0)
+ goto answer;
+
+ if (pac_load(params->proxy, &pac) < 0)
+ goto answer;
+
+ pac_execute(pac, url, host, &result);
+
+ if (!result)
+ result = strdup("DIRECT");
+
+ sd_bus_reply_method_return(params->message, "s", result);
sd-bus is not thread-safe. You cannot send messages from different
threads at the same time...
Why precisely do you need threads here? Because the PAC programs might
be slow?
the Javascript execution is normally blocking and it can do whatever it wants. If it wants to draw a smiley face, while looking up the proxy, it can do that.

I think the PAC execution needs to be done in a thread, but not the D-Bus message handling.
Post by Lennart Poettering
WHat about unbounded loops in PAC programs? We need to protect from
that, which kinda suggests we muight want to run the PAC stuff out of
process so that we can neatly kill the program when it runs for too
long?
Remember that in theory for every HTTP request a browser makes it is suppose to ask the PAC file first. Meaning it will run this a lot. So spawning processes might be not really useful. Even if we start one PAC processing process for each interface, you still want threads since you might have multiple requests from different application. And if one can be answered from a cache, then you do not want to wait until the Javascript execution finished.

Regards

Marcel
Lennart Poettering
2015-04-12 18:38:34 UTC
Permalink
Post by Marcel Holtmann
Post by Lennart Poettering
sd-bus is not thread-safe. You cannot send messages from different
threads at the same time...
Why precisely do you need threads here? Because the PAC programs might
be slow?
the Javascript execution is normally blocking and it can do whatever
it wants. If it wants to draw a smiley face, while looking up the
proxy, it can do that.
I think the PAC execution needs to be done in a thread, but not the D-Bus message handling.
Post by Lennart Poettering
WHat about unbounded loops in PAC programs? We need to protect from
that, which kinda suggests we muight want to run the PAC stuff out of
process so that we can neatly kill the program when it runs for too
long?
Remember that in theory for every HTTP request a browser makes it is
suppose to ask the PAC file first. Meaning it will run this a
lot. So spawning processes might be not really useful. Even if we
start one PAC processing process for each interface, you still want
threads since you might have multiple requests from different
application. And if one can be answered from a cache, then you do
not want to wait until the Javascript execution finished.
Hmm, this really feels like we should still do this out-of-process
with a worker process logic. Reuse processes to keep the cost down,
and run up to a fixed number of them in parallel to allow multiple
sumultaneous requests. THis would have have the major added benefit
that we could lock the worker threads down with seccomp and whatnot,
which is certainly a good thing given that we'll execute foreign,
untrusted code we downloaded from the network with this. And we can
easily kill the execution with a time limit if we desir to do so...

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-12 19:58:28 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Marcel Holtmann
Post by Lennart Poettering
sd-bus is not thread-safe. You cannot send messages from different
threads at the same time...
Why precisely do you need threads here? Because the PAC programs might
be slow?
the Javascript execution is normally blocking and it can do whatever
it wants. If it wants to draw a smiley face, while looking up the
proxy, it can do that.
I think the PAC execution needs to be done in a thread, but not the
D-Bus message handling.
Post by Lennart Poettering
WHat about unbounded loops in PAC programs? We need to protect from
that, which kinda suggests we muight want to run the PAC stuff out of
process so that we can neatly kill the program when it runs for too
long?
Remember that in theory for every HTTP request a browser makes it is
suppose to ask the PAC file first. Meaning it will run this a
lot. So spawning processes might be not really useful. Even if we
start one PAC processing process for each interface, you still want
threads since you might have multiple requests from different
application. And if one can be answered from a cache, then you do
not want to wait until the Javascript execution finished.
Hmm, this really feels like we should still do this out-of-process
with a worker process logic. Reuse processes to keep the cost down,
and run up to a fixed number of them in parallel to allow multiple
sumultaneous requests. THis would have have the major added benefit
that we could lock the worker threads down with seccomp and whatnot,
which is certainly a good thing given that we'll execute foreign,
untrusted code we downloaded from the network with this. And we can
easily kill the execution with a time limit if we desir to do so...
I get the feeling that this needs to be measured and someone has to write some performance tool around this. So while you have only seen tiny PAC files, I am seen ours. And that one is massive. I bet that other corporation have similar large ones.

Regards

Marcel
Tomasz Bursztyka
2015-04-13 11:37:29 UTC
Permalink
Hi Lennart and Marcel,

I'll stick to the thread solution for now, moving the dbus handling in
the main process.
I forgot to check about this issue.

We could have a timeout in the pac runtime, so it would cancel the
duktape context
relevantly, so no need to track the threads from the main process and
cancel them the
hard way.

Tomasz
Lennart Poettering
2015-04-13 14:29:31 UTC
Permalink
Post by Tomasz Bursztyka
Hi Lennart and Marcel,
I'll stick to the thread solution for now, moving the dbus handling in the
main process.
I forgot to check about this issue.
I think the cancellation issue should not be ignored. Doing this as
thread is not really an option if we cannot cancel this properly after
a timeout.

I don't see why an out-of-process logic would be that hard... It could
be implemented in a pretty minimalistic way: create an
AF_UNIX/SOCK_SEQPACKET socket before forking, leave one end of it in
the daemon, and pass the other end to all the worker processes. Then,
when there's an URL to look up, just write it in one datagram into the
socket from the daemon side, and wait for one reply on it. Make all
worker threads read() from the other side (the same socket in all of
them), and the kernel will then deliver it to an idle worker thread
and all is good.

Lennart
--
Lennart Poettering, Red Hat
Tom Gundersen
2015-04-10 13:06:52 UTC
Permalink
On Fri, Apr 10, 2015 at 2:17 PM, Tomasz Bursztyka
Post by Tomasz Bursztyka
This currently does nothing besides providing the basic skeleton for the
daemon to be developped.
proxy-discoveryd is a daemon that will manage the network proxy
configuration per-network interface. It will provide those through DBus,
and thus it will be possible for any application to know what proxies to
use, if any, when relevant.
---
.gitignore | 1 +
Makefile.am | 26 +++++++++++
configure.ac | 11 +++++
src/proxy-discovery/proxy-discoveryd-manager.c | 65 ++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.c | 65 ++++++++++++++++++++++++++
src/proxy-discovery/proxy-discoveryd.h | 38 +++++++++++++++
units/.gitignore | 1 +
units/systemd-proxy-discoveryd.service.in | 18 +++++++
8 files changed, 225 insertions(+)
create mode 100644 src/proxy-discovery/proxy-discoveryd-manager.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.h
create mode 100644 units/systemd-proxy-discoveryd.service.in
diff --git a/.gitignore b/.gitignore
index 875ada5..f479d37 100644
--- a/.gitignore
+++ b/.gitignore
@@ -108,6 +108,7 @@
/systemd-notify
/systemd-nspawn
/systemd-path
+/systemd-proxy-discoveryd
/systemd-pull
/systemd-quotacheck
/systemd-random-seed
diff --git a/Makefile.am b/Makefile.am
index 0a57389..3f4e4d3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -5889,6 +5889,32 @@ EXTRA_DIST += \
endif
# ------------------------------------------------------------------------------
+if ENABLE_PROXY_DISCOVERYD
+
+rootlibexec_PROGRAMS += \
+ systemd-proxy-discoveryd
+
+systemd_proxy_discoveryd_SOURCES = \
+ src/proxy-discovery/proxy-discoveryd.c \
+ src/proxy-discovery/proxy-discoveryd.h \
+ src/proxy-discovery/proxy-discoveryd-manager.c
+
+systemd_proxy_discoveryd_LDADD = \
+ libsystemd-internal.la \
+ libsystemd-shared.la
+
+nodist_systemunit_DATA += \
+ units/systemd-proxy-discoveryd.service
+
+EXTRA_DIST += \
+ units/systemd-proxy-discoveryd.service.in
+
+CLEANFILES += \
+ units/systemd-proxy-discoveryd.service
+
+endif
+
+# ------------------------------------------------------------------------------
if ENABLE_LOGIND
systemd_logind_SOURCES = \
src/login/logind.c \
diff --git a/configure.ac b/configure.ac
index 960b15d..3db3230 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1139,6 +1139,16 @@ AS_IF([test "x$enable_networkd" != "xno"], [
AM_CONDITIONAL(ENABLE_NETWORKD, [test "x$have_networkd" = "xyes"])
# ------------------------------------------------------------------------------
+have_proxy_discoveryd=no
+AC_ARG_ENABLE(proxy-discoveryd, AS_HELP_STRING([--disable-proxy-discoveryd], [disable proxy-discoveryd]))
+AS_IF([test "x$enable_proxy_discoveryd" != "xno"], [
+ AC_DEFINE(ENABLE_PROXY_DISCOVERYD, 1, [Define if proxy-discoveryd support is to be enabled])
+ have_proxy_discoveryd=yes
+ M4_DEFINES="$M4_DEFINES -DENABLE_PROXY_DISCOVERYD"
+])
+AM_CONDITIONAL(ENABLE_PROXY_DISCOVERYD, [test "x$have_proxy_discoveryd" = "xyes"])
+
+# ------------------------------------------------------------------------------
have_efi=no
AC_ARG_ENABLE(efi, AS_HELP_STRING([--disable-efi], [disable EFI support]))
if test "x$enable_efi" != "xno"; then
@@ -1560,6 +1570,7 @@ AC_MSG_RESULT([
localed: ${have_localed}
networkd: ${have_networkd}
resolved: ${have_resolved}
+ proxy-discoveryd: ${have_proxy_discoveryd}
default DNS servers: ${DNS_SERVERS}
coredump: ${have_coredump}
polkit: ${have_polkit}
diff --git a/src/proxy-discovery/proxy-discoveryd-manager.c b/src/proxy-discovery/proxy-discoveryd-manager.c
new file mode 100644
index 0000000..3aaec68
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd-manager.c
@@ -0,0 +1,65 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "proxy-discoveryd.h"
+
+int manager_new(Manager **ret) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int r;
+
+ assert(ret);
+
+ m = new0(Manager, 1);
+ if (!m)
+ return -ENOMEM;
+
+ r = sd_event_default(&m->event);
+ if (r < 0)
+ return r;
+
+ r = sd_event_set_watchdog(m->event, true);
+ if (r < 0)
+ return r;
+
+ r = sd_event_add_signal(m->event, NULL, SIGTERM, NULL, NULL);
+ if (r < 0)
+ return r;
+
+ r = sd_event_add_signal(m->event, NULL, SIGINT, NULL, NULL);
+ if (r < 0)
+ return r;
+
+ *ret = m;
+ m = NULL;
+
+ return 0;
+}
+
+Manager *manager_free(Manager *m) {
+ if (!m)
+ return NULL;
+
+ sd_event_unref(m->event);
+
+ free(m);
+
+ return NULL;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.c b/src/proxy-discovery/proxy-discoveryd.c
new file mode 100644
index 0000000..f023312
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd.c
@@ -0,0 +1,65 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "sd-event.h"
+#include "sd-daemon.h"
+
+#include "proxy-discoveryd.h"
+
+int main(int argc, char *argv[]) {
+ _cleanup_manager_free_ Manager *m = NULL;
+ int r;
+
+ log_set_target(LOG_TARGET_AUTO);
+ log_parse_environment();
+ log_open();
+
+ if (argc != 1) {
+ log_error("This program takes no arguments.");
+ r = -EINVAL;
+ goto out;
+ }
+
+ r = manager_new(&m);
+ if (r < 0) {
+ log_error_errno(r, "Could not create manager: %m");
+ goto out;
+ }
+
+ sd_notify(false,
+ "READY=1\n"
+ "STATUS=Processing requests...");
+
+ r = sd_event_loop(m->event);
+ if (r < 0) {
+ log_error_errno(r, "Event loop failed: %m");
+ goto out;
+ }
+
+ sd_event_get_exit_code(m->event, &r);
+
+ sd_notify(false,
+ "STOPPING=1\n"
+ "STATUS=Shutting down...");
+
+ return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS;
+}
diff --git a/src/proxy-discovery/proxy-discoveryd.h b/src/proxy-discovery/proxy-discoveryd.h
new file mode 100644
index 0000000..125abc0
--- /dev/null
+++ b/src/proxy-discovery/proxy-discoveryd.h
@@ -0,0 +1,38 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+ This file is part of systemd.
+
+ Copyright (C) 2015 Intel Corporation. All rights reserved.
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "sd-event.h"
+
+#include "util.h"
+
+typedef struct Manager Manager;
+
+struct Manager {
+ sd_event *event;
+};
+
+int manager_new(Manager **ret);
+Manager *manager_free(Manager *m);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)
We generally try to avoid this define in internal code, and just use
_cleanup_(manager_freep) inline.
Post by Tomasz Bursztyka
diff --git a/units/.gitignore b/units/.gitignore
index ad469c1..63ae1af 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -51,6 +51,7 @@
/systemd-networkd.service
/systemd-poweroff.service
+/systemd-proxy-discoveryd.service
/systemd-quotacheck.service
/systemd-random-seed.service
/systemd-reboot.service
diff --git a/units/systemd-proxy-discoveryd.service.in b/units/systemd-proxy-discoveryd.service.in
new file mode 100644
index 0000000..7ac02e0
--- /dev/null
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+StandardOutput=null
Why this special handling of stdout? Is it the JS engine? Probably
should have a comment as it is a bit odd.
Post by Tomasz Bursztyka
--
2.0.5
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Tomasz Bursztyka
2015-04-13 10:05:47 UTC
Permalink
Hi Tom,
Post by Tom Gundersen
Post by Tomasz Bursztyka
+int manager_new(Manager **ret);
+Manager *manager_free(Manager *m);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)
We generally try to avoid this define in internal code, and just use
_cleanup_(manager_freep) inline.
Ok, I followed networkd but indeed other parts don't do that (machined
for instance).
Post by Tom Gundersen
Post by Tomasz Bursztyka
diff --git a/units/.gitignore b/units/.gitignore
index ad469c1..63ae1af 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -51,6 +51,7 @@
/systemd-networkd.service
/systemd-poweroff.service
+/systemd-proxy-discoveryd.service
/systemd-quotacheck.service
/systemd-random-seed.service
/systemd-reboot.service
diff --git a/units/systemd-proxy-discoveryd.service.in b/units/systemd-proxy-discoveryd.service.in
new file mode 100644
index 0000000..7ac02e0
--- /dev/null
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+StandardOutput=null
Why this special handling of stdout? Is it the JS engine? Probably
should have a comment as it is a bit odd.
I copy-pasted the .service file we had for pacrunner, this needs to be
tailored for proxy-discoveryd.

The stdout directing to /dev/null is to avoid the JS engine to print
anything.
Though for duktape specifically, we can hook the
debug/printing/warning/... functions, so we could
redirect messages to the logs.

Tomasz
Tom Gundersen
2015-04-13 10:54:36 UTC
Permalink
On Mon, Apr 13, 2015 at 12:05 PM, Tomasz Bursztyka
Post by Tom Gundersen
Post by Tomasz Bursztyka
+int manager_new(Manager **ret);
+Manager *manager_free(Manager *m);
+
+DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
+#define _cleanup_manager_free_ _cleanup_(manager_freep)
We generally try to avoid this define in internal code, and just use
_cleanup_(manager_freep) inline.
Ok, I followed networkd but indeed other parts don't do that (machined for
instance).
Do as I say, not as I do :P I didn't get the memo yet when I wrote the
initial networkd parts, I should fix this up to avoid confusion in the
future.
Post by Tom Gundersen
Post by Tomasz Bursztyka
diff --git a/units/.gitignore b/units/.gitignore
index ad469c1..63ae1af 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -51,6 +51,7 @@
/systemd-networkd.service
/systemd-poweroff.service
+/systemd-proxy-discoveryd.service
/systemd-quotacheck.service
/systemd-random-seed.service
/systemd-reboot.service
diff --git a/units/systemd-proxy-discoveryd.service.in
b/units/systemd-proxy-discoveryd.service.in
new file mode 100644
index 0000000..7ac02e0
--- /dev/null
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+StandardOutput=null
Why this special handling of stdout? Is it the JS engine? Probably
should have a comment as it is a bit odd.
I copy-pasted the .service file we had for pacrunner, this needs to be
tailored for proxy-discoveryd.
The stdout directing to /dev/null is to avoid the JS engine to print
anything.
Though for duktape specifically, we can hook the debug/printing/warning/...
functions, so we could
redirect messages to the logs.
That's what I thought. Would be nice to hook this up properly
internally I think. Either to just redirect to /dev/null there, or to
log correctly.

-t
Tom Gundersen
2015-04-10 13:30:22 UTC
Permalink
On Fri, Apr 10, 2015 at 2:17 PM, Tomasz Bursztyka
Hi,
Hi Tomasz!
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
Thanks for working on this, looks promising! I made some minor
comments on the individual patches.

Cheers,

Tom
Next features to come are a bit detailed in the TODO (last patch).
proxy-discoveryd: Basic core added
proxy-discoveryd: Add the basics for parsing the default configuration
proxy-discoveryd: Add PAC support through duktape js engine
proxy-discoveryd: Execute the PAC based proxy in a thread
proxy-discoveryd: Add the basic parts for handling DBus methods
update TODO
.gitignore | 1 +
Makefile.am | 38 +++++
TODO | 10 ++
configure.ac | 11 ++
src/proxy-discovery/.gitignore | 1 +
.../org.freedesktop.proxydiscovery1.conf | 46 ++++++
.../org.freedesktop.proxydiscovery1.service | 12 ++
src/proxy-discovery/proxy-discoveryd-bus.c | 48 ++++++
src/proxy-discovery/proxy-discoveryd-manager.c | 156 ++++++++++++++++++
src/proxy-discovery/proxy-discoveryd-pac.c | 182 +++++++++++++++++++++
.../proxy-discoveryd-proxy-gperf.gperf | 17 ++
src/proxy-discovery/proxy-discoveryd-proxy.c | 135 +++++++++++++++
src/proxy-discovery/proxy-discoveryd.c | 77 +++++++++
src/proxy-discovery/proxy-discoveryd.h | 84 ++++++++++
units/.gitignore | 1 +
units/org.freedesktop.proxydiscovery1.busname | 15 ++
units/systemd-proxy-discoveryd.service.in | 18 ++
17 files changed, 852 insertions(+)
create mode 100644 src/proxy-discovery/.gitignore
create mode 100644 src/proxy-discovery/org.freedesktop.proxydiscovery1.conf
create mode 100644 src/proxy-discovery/org.freedesktop.proxydiscovery1.service
create mode 100644 src/proxy-discovery/proxy-discoveryd-bus.c
create mode 100644 src/proxy-discovery/proxy-discoveryd-manager.c
create mode 100644 src/proxy-discovery/proxy-discoveryd-pac.c
create mode 100644 src/proxy-discovery/proxy-discoveryd-proxy-gperf.gperf
create mode 100644 src/proxy-discovery/proxy-discoveryd-proxy.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.c
create mode 100644 src/proxy-discovery/proxy-discoveryd.h
create mode 100644 units/org.freedesktop.proxydiscovery1.busname
create mode 100644 units/systemd-proxy-discoveryd.service.in
--
2.0.5
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Dan Williams
2015-04-10 14:22:10 UTC
Permalink
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
I haven't looked in great detail, but is there some way that other
services (like NetworkManager) which have information about proxies
discovered via DHCP, would get this information to the daemon? AFAIUI
Lennart in the past has wanted "higher level" stuff (like this daemon I
presume) to pull that information from "lower level" processes (like
NetworkManager or systemd-networkd). What are the provisions for that?

Dan
Lennart Poettering
2015-04-10 15:17:41 UTC
Permalink
Post by Dan Williams
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
I haven't looked in great detail, but is there some way that other
services (like NetworkManager) which have information about proxies
discovered via DHCP, would get this information to the daemon? AFAIUI
Lennart in the past has wanted "higher level" stuff (like this daemon I
presume) to pull that information from "lower level" processes (like
NetworkManager or systemd-networkd). What are the provisions for that?
Well, there are three cases where this matters:

1. systemd-timesyncd for NTP servers
2. systemd-resolved for DNS servers
3. and now systemd-proxy-discoveryd for HTTP proxy/PAC stuff

All of them should get some data from networkd, for example when the
user configures an NTP/DNS/proxy server in the network configuration,
or when this data is learnt via DHCP. In this case we should have
proper stacking, since the three daemons should pull the data out of
networkd on their own. The way this should work (and already does for
the case of timesyncd/resolved), is by watching files in /run, so that
we require no bus interaction to call this.

Now, what's left to do is open up the three daemons for foreign
networking daemons, such as NM. I don't think that using files in /run
is a good idea as a public interface, hence we'd prefer to use dbus
for those cases. Of course, this would mean that in that case NM would
push the data into our daemons, which I think is a bit of a layer
violation, but I am fine with, since it is outside of systemd's own
codebase at least.

So idea would basically be that we provide in all three daemons calls
like:

SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
SetAdditionalPAC(ias)

Or so, which could be used push additional NTP, DNS, or PAC data into
the respective daemon, and for the specified ifindex.

For the NTP case we haven't implemented this so far, since timesyncd
is actually an early-boot service, and dbsu1 is not available in
early-boot, and we didn't want to deal with the complexities this
creates. We can add this as soon as kdbus is reality.

For the DNS cases I haven't implemented this so far, because I didn't
find the time to yet.

For the PAC case this is not implemented because the code is very very
new, and not even merged yet.

I hope this clarifies the road map on this a bit.

Lennart
--
Lennart Poettering, Red Hat
Dan Williams
2015-04-10 17:06:46 UTC
Permalink
Post by Lennart Poettering
Post by Dan Williams
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
I haven't looked in great detail, but is there some way that other
services (like NetworkManager) which have information about proxies
discovered via DHCP, would get this information to the daemon? AFAIUI
Lennart in the past has wanted "higher level" stuff (like this daemon I
presume) to pull that information from "lower level" processes (like
NetworkManager or systemd-networkd). What are the provisions for that?
1. systemd-timesyncd for NTP servers
2. systemd-resolved for DNS servers
3. and now systemd-proxy-discoveryd for HTTP proxy/PAC stuff
All of them should get some data from networkd, for example when the
user configures an NTP/DNS/proxy server in the network configuration,
or when this data is learnt via DHCP. In this case we should have
proper stacking, since the three daemons should pull the data out of
networkd on their own. The way this should work (and already does for
the case of timesyncd/resolved), is by watching files in /run, so that
we require no bus interaction to call this.
Now, what's left to do is open up the three daemons for foreign
networking daemons, such as NM. I don't think that using files in /run
is a good idea as a public interface, hence we'd prefer to use dbus
for those cases. Of course, this would mean that in that case NM would
push the data into our daemons, which I think is a bit of a layer
violation, but I am fine with, since it is outside of systemd's own
codebase at least.
So what you're saying is that systemd project helpers will use the
"correct" layering, and pull from lower systemd components, but that
external users will push the information up to the systemd helpers,
technically an "incorrect" layering? I guess that works but that's
somewhat icky.
Post by Lennart Poettering
So idea would basically be that we provide in all three daemons calls
SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
I would strongly suggest using strings in the API for IP addresses, not
byte arrays. Also, remember that we want to push domains too, for split
DNS in the VPN or other cases. So it really should be something like:

SetAdditionalDNS(ia(sas)) (index, array[domain, array[server]])

what were the (uay) arguments you proposed?
Post by Lennart Poettering
SetAdditionalPAC(ias)
Or so, which could be used push additional NTP, DNS, or PAC data into
the respective daemon, and for the specified ifindex.
For the NTP case we haven't implemented this so far, since timesyncd
is actually an early-boot service, and dbsu1 is not available in
early-boot, and we didn't want to deal with the complexities this
creates. We can add this as soon as kdbus is reality.
NM would like to somehow convey NTP information that it receives via
DHCP too.
Post by Lennart Poettering
For the DNS cases I haven't implemented this so far, because I didn't
find the time to yet.
For the PAC case this is not implemented because the code is very very
new, and not even merged yet.
I hope this clarifies the road map on this a bit.
Yes, it does. Now how about the roadmap for libsystemd-network being a
standalone library? :)

Dan
Lennart Poettering
2015-04-10 17:19:35 UTC
Permalink
Post by Dan Williams
Post by Lennart Poettering
Now, what's left to do is open up the three daemons for foreign
networking daemons, such as NM. I don't think that using files in /run
is a good idea as a public interface, hence we'd prefer to use dbus
for those cases. Of course, this would mean that in that case NM would
push the data into our daemons, which I think is a bit of a layer
violation, but I am fine with, since it is outside of systemd's own
codebase at least.
So what you're saying is that systemd project helpers will use the
"correct" layering, and pull from lower systemd components, but that
external users will push the information up to the systemd helpers,
technically an "incorrect" layering? I guess that works but that's
somewhat icky.
Yes. But see it like this: I really like to keep things clean within
the systemd project. I am also sure that we need these calls to push
server data into the daemons sooner or later anyway, not just for the
NM usecase.

We tried to come up with a nice way to allow
resolved/timesyncd/... pull the data out of NM too, but defining
abstract bus ifaces or file formats in /run neither appeared to
attractive to us, the slightly ugly solution of allowing NM to push
the data into our daemons has the ebenfit of not requiring neither,
without having to add any other interface at all...
Post by Dan Williams
Post by Lennart Poettering
So idea would basically be that we provide in all three daemons calls
SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
I would strongly suggest using strings in the API for IP addresses, not
byte arrays.
Why so?
Post by Dan Williams
Also, remember that we want to push domains too, for split
SetAdditionalDNS(ia(sas)) (index, array[domain, array[server]])
what were the (uay) arguments you proposed?
resolved supprots split DNS, but it will not allow multiple DNS
servers with different domains on the same interface. Instead, you
bind a set of DNS servers and a set of domains to an interface, and
that's it, there's no further connection between servers and domains.

I fail to see the strong usecase for allowing per-server domains. I
mean in the VPn case you have one explicit interface for the VPN
connection, so this should be covered with the current design.

(uay) is supposed to be the address family (AF_INET or AF_INET6),
followed by the address bytes.
Post by Dan Williams
Post by Lennart Poettering
For the NTP case we haven't implemented this so far, since timesyncd
is actually an early-boot service, and dbsu1 is not available in
early-boot, and we didn't want to deal with the complexities this
creates. We can add this as soon as kdbus is reality.
NM would like to somehow convey NTP information that it receives via
DHCP too.
Use the same mechanism.
Post by Dan Williams
Yes, it does. Now how about the roadmap for libsystemd-network being a
standalone library? :)
Well, we still keep adding stuff...

Lennart
--
Lennart Poettering, Red Hat
Dan Williams
2015-04-10 19:05:57 UTC
Permalink
Post by Lennart Poettering
Post by Dan Williams
Post by Lennart Poettering
Now, what's left to do is open up the three daemons for foreign
networking daemons, such as NM. I don't think that using files in /run
is a good idea as a public interface, hence we'd prefer to use dbus
for those cases. Of course, this would mean that in that case NM would
push the data into our daemons, which I think is a bit of a layer
violation, but I am fine with, since it is outside of systemd's own
codebase at least.
So what you're saying is that systemd project helpers will use the
"correct" layering, and pull from lower systemd components, but that
external users will push the information up to the systemd helpers,
technically an "incorrect" layering? I guess that works but that's
somewhat icky.
Yes. But see it like this: I really like to keep things clean within
the systemd project. I am also sure that we need these calls to push
server data into the daemons sooner or later anyway, not just for the
NM usecase.
We tried to come up with a nice way to allow
resolved/timesyncd/... pull the data out of NM too, but defining
abstract bus ifaces or file formats in /run neither appeared to
attractive to us, the slightly ugly solution of allowing NM to push
the data into our daemons has the ebenfit of not requiring neither,
without having to add any other interface at all...
Post by Dan Williams
Post by Lennart Poettering
So idea would basically be that we provide in all three daemons calls
SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
I would strongly suggest using strings in the API for IP addresses, not
byte arrays.
Why so?
It's much easier to use for languages other than C, like Python or JS or
Ruby or dbus-send or even C in many cases. We originally used binary
addresses in the D-Bus interface for NetworkManager, and eventually
found that was the wrong choice for these reasons. It's also easier to
pull in and out of D-Bus, with dbus-glib or GIO. I'm not sure about
sd_bus though.
Post by Lennart Poettering
Post by Dan Williams
Also, remember that we want to push domains too, for split
SetAdditionalDNS(ia(sas)) (index, array[domain, array[server]])
what were the (uay) arguments you proposed?
resolved supprots split DNS, but it will not allow multiple DNS
servers with different domains on the same interface. Instead, you
bind a set of DNS servers and a set of domains to an interface, and
that's it, there's no further connection between servers and domains.
If I understand correctly, this means you cannot direct *.foobar.com to
one set of DNS servers,and *.baz.com to another set, assuming those are
bound to the same interface? Why not allow that?
Post by Lennart Poettering
I fail to see the strong usecase for allowing per-server domains. I
mean in the VPn case you have one explicit interface for the VPN
connection, so this should be covered with the current design.
IPSec doesn't give you an kernel netdev at all, it's just routing on an
existing interface. So for that interface, you'd have both the VPN
servers and upstream servers, all bound to the same interface, and no
ability to direct VPN domains to the VPN servers since they are all on
the same interface?

Dan
Post by Lennart Poettering
(uay) is supposed to be the address family (AF_INET or AF_INET6),
followed by the address bytes.
Post by Dan Williams
Post by Lennart Poettering
For the NTP case we haven't implemented this so far, since timesyncd
is actually an early-boot service, and dbsu1 is not available in
early-boot, and we didn't want to deal with the complexities this
creates. We can add this as soon as kdbus is reality.
NM would like to somehow convey NTP information that it receives via
DHCP too.
Use the same mechanism.
Post by Dan Williams
Yes, it does. Now how about the roadmap for libsystemd-network being a
standalone library? :)
Well, we still keep adding stuff...
Lennart
Lennart Poettering
2015-04-12 18:31:36 UTC
Permalink
Post by Dan Williams
Post by Lennart Poettering
Post by Dan Williams
Post by Lennart Poettering
So idea would basically be that we provide in all three daemons calls
SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
I would strongly suggest using strings in the API for IP addresses, not
byte arrays.
Why so?
It's much easier to use for languages other than C, like Python or JS or
Ruby or dbus-send or even C in many cases. We originally used binary
addresses in the D-Bus interface for NetworkManager, and eventually
found that was the wrong choice for these reasons. It's also easier to
pull in and out of D-Bus, with dbus-glib or GIO. I'm not sure about
sd_bus though.
I am very much convinced that passing normalized data through dbus is
the right thing. And that implies byte arrays for binary data such as
IP addresses... We have structured data in dbus, that's pretty much
the biggest benefit of dbus over raw sockets. We should use it and not
chicken out, because our bindings suck and encode everything in
formatted strings after all... If it's not easy to decode structured
data with your bus binding, then the answer is to fix the bus binding,
not to just revert to unstructured data.
Post by Dan Williams
Post by Lennart Poettering
Post by Dan Williams
Also, remember that we want to push domains too, for split
SetAdditionalDNS(ia(sas)) (index, array[domain, array[server]])
what were the (uay) arguments you proposed?
resolved supprots split DNS, but it will not allow multiple DNS
servers with different domains on the same interface. Instead, you
bind a set of DNS servers and a set of domains to an interface, and
that's it, there's no further connection between servers and domains.
If I understand correctly, this means you cannot direct *.foobar.com to
one set of DNS servers,and *.baz.com to another set, assuming those are
bound to the same interface? Why not allow that?
Well, resolved is not supposed to support every crazy set up people
can think of. I mean, if they want that they can certainly run their
own local DNS server. resolved is about figuring out the common
usecases and covering them nicely and automatically, and little else.
Post by Dan Williams
Post by Lennart Poettering
I fail to see the strong usecase for allowing per-server domains. I
mean in the VPn case you have one explicit interface for the VPN
connection, so this should be covered with the current design.
IPSec doesn't give you an kernel netdev at all, it's just routing on an
existing interface. So for that interface, you'd have both the VPN
servers and upstream servers, all bound to the same interface, and no
ability to direct VPN domains to the VPN servers since they are all on
the same interface?
IPSec used in that mode does not convey DNS server info, does it? In
that case it's supposed to be transparent, but with such domain server
config it wouldn't be transparent after all...

Lennart
--
Lennart Poettering, Red Hat
Dan Williams
2015-04-13 15:07:15 UTC
Permalink
Post by Lennart Poettering
Post by Dan Williams
Post by Lennart Poettering
Post by Dan Williams
Post by Lennart Poettering
So idea would basically be that we provide in all three daemons calls
SetAdditionalNTP(ias)
SetAdditionalDNS(ia(uay))
I would strongly suggest using strings in the API for IP addresses, not
byte arrays.
Why so?
It's much easier to use for languages other than C, like Python or JS or
Ruby or dbus-send or even C in many cases. We originally used binary
addresses in the D-Bus interface for NetworkManager, and eventually
found that was the wrong choice for these reasons. It's also easier to
pull in and out of D-Bus, with dbus-glib or GIO. I'm not sure about
sd_bus though.
I am very much convinced that passing normalized data through dbus is
the right thing. And that implies byte arrays for binary data such as
IP addresses... We have structured data in dbus, that's pretty much
the biggest benefit of dbus over raw sockets. We should use it and not
chicken out, because our bindings suck and encode everything in
formatted strings after all... If it's not easy to decode structured
data with your bus binding, then the answer is to fix the bus binding,
not to just revert to unstructured data.
Post by Dan Williams
Post by Lennart Poettering
Post by Dan Williams
Also, remember that we want to push domains too, for split
SetAdditionalDNS(ia(sas)) (index, array[domain, array[server]])
what were the (uay) arguments you proposed?
resolved supprots split DNS, but it will not allow multiple DNS
servers with different domains on the same interface. Instead, you
bind a set of DNS servers and a set of domains to an interface, and
that's it, there's no further connection between servers and domains.
If I understand correctly, this means you cannot direct *.foobar.com to
one set of DNS servers,and *.baz.com to another set, assuming those are
bound to the same interface? Why not allow that?
Well, resolved is not supposed to support every crazy set up people
can think of. I mean, if they want that they can certainly run their
own local DNS server. resolved is about figuring out the common
usecases and covering them nicely and automatically, and little else.
Post by Dan Williams
Post by Lennart Poettering
I fail to see the strong usecase for allowing per-server domains. I
mean in the VPn case you have one explicit interface for the VPN
connection, so this should be covered with the current design.
IPSec doesn't give you an kernel netdev at all, it's just routing on an
existing interface. So for that interface, you'd have both the VPN
servers and upstream servers, all bound to the same interface, and no
ability to direct VPN domains to the VPN servers since they are all on
the same interface?
IPSec used in that mode does not convey DNS server info, does it? In
that case it's supposed to be transparent, but with such domain server
config it wouldn't be transparent after all...
It certainly can convey DNS information. Try the Red Hat VPN servers
with libreswan for an example. Your eth0 will now have the original,
DHCP/static/etc provided LAN DNS servers, and secondary VPN-provided DNS
servers that can be tied to one or more domains that the VPN also
provided.

Dan
Lennart Poettering
2015-04-10 15:40:03 UTC
Permalink
Post by Tomasz Bursztyka
have_efi=no
AC_ARG_ENABLE(efi, AS_HELP_STRING([--disable-efi], [disable EFI support]))
if test "x$enable_efi" != "xno"; then
@@ -1560,6 +1570,7 @@ AC_MSG_RESULT([
localed: ${have_localed}
networkd: ${have_networkd}
resolved: ${have_resolved}
+ proxy-discoveryd: ${have_proxy_discoveryd}
weird indenting?
Post by Tomasz Bursztyka
+++ b/units/systemd-proxy-discoveryd.service.in
@@ -0,0 +1,18 @@
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
Hmm, should this really be an early-boot service? Can you explain?
Post by Tomasz Bursztyka
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
Which this dependency?
Post by Tomasz Bursztyka
+[Service]
+Restart=on-failure
+StandardOutput=null
Why this last line?

Lennart
--
Lennart Poettering, Red Hat
Tomasz Bursztyka
2015-04-13 10:05:41 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Tomasz Bursztyka
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
Hmm, should this really be an early-boot service? Can you explain?
That's a mistake indeed.
Post by Lennart Poettering
Post by Tomasz Bursztyka
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
Which this dependency?
Took that quickly from pacrunner.service.
It does not look relevant.
Post by Lennart Poettering
Post by Tomasz Bursztyka
+[Service]
+Restart=on-failure
+StandardOutput=null
Why this last line?
See my answer to Tom's comment.


Tomasz
Zbigniew Jędrzejewski-Szmek
2015-04-11 19:02:51 UTC
Permalink
Post by Tomasz Bursztyka
+# This file is part of systemd.
+#
+# systemd is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Lesser General Public License as published by
+# the Free Software Foundation; either version 2.1 of the License, or
+# (at your option) any later version.
+
+[Unit]
+Description=Proxy service
+DefaultDependencies=no
+Requires=dbus.socket
+After=dbus.socket
+Before=remote-fs.target
+
+[Service]
+Restart=on-failure
+StandardOutput=null
What privileges does this daemon require? I'd guess it can run as normal
user at least... Since this is supposed to be executing untrusted javascript
code, let's lock it down heavily from the start.

Zbyszek
Tomasz Bursztyka
2015-04-13 10:11:55 UTC
Permalink
Hi Zbigniew,
Post by Zbigniew Jędrzejewski-Szmek
Post by Tomasz Bursztyka
+
+[Service]
+Restart=on-failure
+StandardOutput=null
What privileges does this daemon require? I'd guess it can run as normal
user at least... Since this is supposed to be executing untrusted javascript
code, let's lock it down heavily from the start.
I agree. It only requires to get access to dbus and netlink, so nothing
specific to root.

And yes for the JS engine itself there should be more to be done: all JS
context
should be fully contained. PAC files can be anything which sounds scary.

Tomasz
Zbigniew Jędrzejewski-Szmek
2015-04-11 19:41:15 UTC
Permalink
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,

the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files? Are they widely used in corporate setting or something?
Is there no saner standard?

If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.

Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.

Zbyszek
Andrei Borzenkov
2015-04-12 06:38:17 UTC
Permalink
В Sat, 11 Apr 2015 19:41:15 +0000
Post by Zbigniew Jędrzejewski-Szmek
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files? Are they widely used in corporate setting or something?
I am aware of several large companies that use it, yes.
Post by Zbigniew Jędrzejewski-Szmek
Is there no saner standard?
I'm not aware of it. Alternative would be central service that answers
the same question.
Post by Zbigniew Jędrzejewski-Szmek
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Those PAC files I have seen used relatively dumb URL string comparison,
at most doing wildcard domain match. I.e. they do not even attempt to
resolve anything but just take literal URL as entered by user.
Post by Zbigniew Jędrzejewski-Szmek
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
Zbyszek
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Marcel Holtmann
2015-04-12 19:51:23 UTC
Permalink
Hi Lennart,
Post by Zbigniew Jędrzejewski-Szmek
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files?
Yes, it's kinda necessary. PAC is pretty widely used in corporate
setting. Windows does the WPAD stuff (the protocol to discover PAD) in
all its versions since a long time. In fact, it immediately issues the
wpad requests as first thing when you plug in an ethernet cable, in
addition to DHCP.
Post by Zbigniew Jędrzejewski-Szmek
Are they widely used in corporate setting or something?
Is there no saner standard?
Nope, not really, I fear.
Post by Zbigniew Jędrzejewski-Szmek
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
Well, it's Java script code. People can just add code like "for (;;);
", and we must be able to cancel the lookup then safely. I doubt
that's cleanly doable with either thread-based or event loop based
solutions. I am pretty sure a worker process logic is the way to
go. The worker process should get the PAC data when it is forked off,
and then read FindProxyForURL data from a pipe and output the result
on another, or something similar, and easily sandboxable...
are you sure that you are not overthinking this? I think that duktape actually allows just canceling the JS engine execution, no matter what operation it runs. So you could just cancel the JS context.

Regards

Marcel
Lennart Poettering
2015-04-12 19:55:29 UTC
Permalink
Post by Marcel Holtmann
Hi Lennart,
Post by Zbigniew Jędrzejewski-Szmek
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files?
Yes, it's kinda necessary. PAC is pretty widely used in corporate
setting. Windows does the WPAD stuff (the protocol to discover PAD) in
all its versions since a long time. In fact, it immediately issues the
wpad requests as first thing when you plug in an ethernet cable, in
addition to DHCP.
Post by Zbigniew Jędrzejewski-Szmek
Are they widely used in corporate setting or something?
Is there no saner standard?
Nope, not really, I fear.
Post by Zbigniew Jędrzejewski-Szmek
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
Well, it's Java script code. People can just add code like "for (;;);
", and we must be able to cancel the lookup then safely. I doubt
that's cleanly doable with either thread-based or event loop based
solutions. I am pretty sure a worker process logic is the way to
go. The worker process should get the PAC data when it is forked off,
and then read FindProxyForURL data from a pipe and output the result
on another, or something similar, and easily sandboxable...
are you sure that you are not overthinking this? I think that
duktape actually allows just canceling the JS engine execution, no
matter what operation it runs. So you could just cancel the JS
context.
How is this implemented in duktype? I mean, cancelling threads is
fricking awful. POSIX thread cancellation is awful, and I am pretty
sure NSS calls are incompatible with it anway. Which means you have to
check flags after each javascript instruction -- which however doesn't
really work too well either, since NSS calls will block as long as
they want, hence you couldn't use this to cancel those...

I really would prefer doing this out-of-process. Then we can kill the
stuff, regardless what it is doing, without interfering with anything
else. Heck, we can even let the kernel help us with the timeout thing
with RLIMIT_CPU...

Lennart
--
Lennart Poettering, Red Hat
Marcel Holtmann
2015-04-12 20:16:51 UTC
Permalink
Hi Lennart,
Post by Lennart Poettering
Post by Marcel Holtmann
Post by Zbigniew Jędrzejewski-Szmek
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files?
Yes, it's kinda necessary. PAC is pretty widely used in corporate
setting. Windows does the WPAD stuff (the protocol to discover PAD) in
all its versions since a long time. In fact, it immediately issues the
wpad requests as first thing when you plug in an ethernet cable, in
addition to DHCP.
Post by Zbigniew Jędrzejewski-Szmek
Are they widely used in corporate setting or something?
Is there no saner standard?
Nope, not really, I fear.
Post by Zbigniew Jędrzejewski-Szmek
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
Well, it's Java script code. People can just add code like "for (;;);
", and we must be able to cancel the lookup then safely. I doubt
that's cleanly doable with either thread-based or event loop based
solutions. I am pretty sure a worker process logic is the way to
go. The worker process should get the PAC data when it is forked off,
and then read FindProxyForURL data from a pipe and output the result
on another, or something similar, and easily sandboxable...
are you sure that you are not overthinking this? I think that
duktape actually allows just canceling the JS engine execution, no
matter what operation it runs. So you could just cancel the JS
context.
How is this implemented in duktype? I mean, cancelling threads is
fricking awful. POSIX thread cancellation is awful, and I am pretty
sure NSS calls are incompatible with it anway. Which means you have to
check flags after each javascript instruction -- which however doesn't
really work too well either, since NSS calls will block as long as
they want, hence you couldn't use this to cancel those...
I am not saying that we cancel the thread. I know that this is painful. I am saying that we just cancel the duktape context and its execution, then the thread would just yield all by itself. So I think the question is if a master thread could just tell the duktape context to quit. That is something we might want to figure out.
Post by Lennart Poettering
I really would prefer doing this out-of-process. Then we can kill the
stuff, regardless what it is doing, without interfering with anything
else. Heck, we can even let the kernel help us with the timeout thing
with RLIMIT_CPU...
We would need to define some IPC and managing the pool of these processes. Of course this is possible since almost any browser does it this way. It however sounds like a lot of effort and complexity if we can do the same with threads and just cancel the duktape execution instead.

Regards

Marcel
Marcel Holtmann
2015-04-12 19:49:04 UTC
Permalink
Hi Zbyszek,
Post by Zbigniew Jędrzejewski-Szmek
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files? Are they widely used in corporate setting or something?
Is there no saner standard?
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
PACrunner is an existing implementation of this concept. It uses threads and seems to work just fine. We bridged libproxy API compatible library that talks to the PACrunner over D-Bus.

I am confused why everybody worries about DNS here. Just use C library name resolving calls. Let it resolve it and be done with it. You are synchronous anyway since the name resolving happens as a Javascript function call. It just happens that this is mapping to actually system code internally.

Regards

Marcel
Lennart Poettering
2015-04-12 19:51:38 UTC
Permalink
Post by Marcel Holtmann
PACrunner is an existing implementation of this concept. It uses
threads and seems to work just fine. We bridged libproxy API
compatible library that talks to the PACrunner over D-Bus.
How does the abort-after-max-runtime logic work in PACrunner? How do
you abort the thread? Do you use POSIX thread cancellation (yuck!), or
do you check a cancelation flag after each javascript instruction? or
what do you do?

Lennart
--
Lennart Poettering, Red Hat
Dimitri John Ledkov
2015-04-14 05:55:13 UTC
Permalink
On 11 April 2015 at 13:41, Zbigniew Jędrzejewski-Szmek
Post by Zbigniew Jędrzejewski-Szmek
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
Hi,
the idea of having centralized proxy config is certainly nice. But the
PAC files make me shiver. So the first question: is it really necessary
to support PAC files? Are they widely used in corporate setting or something?
Is there no saner standard?
Yes.
Yes.
No.

I only wish for system-wide WPAD support and PAC auto-parsing.

The standard started by netscape or some such, hence interpreted
javascript, and nobody came up with something more declarative /
deterministic that catched on.
Post by Zbigniew Jędrzejewski-Szmek
If the PAC files must be interpreted, I think this is the hardest
part. FindProxyForURL is started for every request, potentially
hundreds of times per second and more. This means that starting a
process per invocation is out of the question, and even starting a
thread per invocation seems to be too much. But each call fall into an
infinite loop and hang. So the run time of FindProxyForURL should be
bounded. FindProxyForURL can also resolve names over the network,
which would best be done asynchronously.
Well pac file is generally cached, and is static it its contents
(possibly many complex clauses if this / if that) but it's ideal to
keep it around for subsequent queries.
Post by Zbigniew Jędrzejewski-Szmek
Things in systemd are usually implemented through poll loops, which
makes it easy to support thousands of concurrent "jobs". I'd think
that this would be the best option here too, with a number of "workers"
executing FindProxyForURL()s and stopping when name resolution is
requested and continuing when the name is resolved.
Zbyszek
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
--
Regards,

Dimitri.

https://clearlinux.org
Open Source Technology Center
Intel Corporation (UK) Ltd. - Co. Reg. #1134945 - Pipers Way, Swindon SN3 1RJ.
Cameron Norman
2015-04-12 21:12:24 UTC
Permalink
On Fri, Apr 10, 2015 at 5:17 AM, Tomasz Bursztyka
Hi,
[snip]
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
It seems that duktape is really not in a suitable shape to be packaged
in distributions (https://github.com/svaarala/duktape/issues/94). Do
you have any plans to get it into shape?

Also, and I am just curious, what is the specific reasoning duktape is
preferred? Smaller memory footprint?

Thanks,
--
Cameron Norman
Daurnimator
2015-04-13 00:30:51 UTC
Permalink
Post by Cameron Norman
On Fri, Apr 10, 2015 at 5:17 AM, Tomasz Bursztyka
Hi,
[snip]
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
It seems that duktape is really not in a suitable shape to be packaged
in distributions (https://github.com/svaarala/duktape/issues/94). Do
you have any plans to get it into shape?
Also, and I am just curious, what is the specific reasoning duktape is
preferred? Smaller memory footprint?
Thanks,
--
Cameron Norman
Have you looked into MuJS instead of duktape? http://mujs.com/
It has a C api similar to Lua, with all state encapsulated in an
opaque structure, that you interface with via a virtual stack.
Tomasz Bursztyka
2015-04-13 11:52:53 UTC
Permalink
Hi,
Post by Daurnimator
Have you looked into MuJS instead of duktape? http://mujs.com/
It has a C api similar to Lua, with all state encapsulated in an
opaque structure, that you interface with via a virtual stack.
It could be easily tested. I did so the PAC related code is contained in
a specific
place, sharing nothing specific but a somehow generic internal api.
I have personally no bold opinion at this time about which JS engine to use.

Tomasz
Marcel Holtmann
2015-04-13 14:50:05 UTC
Permalink
Hi Tomasz,
Post by Daurnimator
Have you looked into MuJS instead of duktape? http://mujs.com/
It has a C api similar to Lua, with all state encapsulated in an
opaque structure, that you interface with via a virtual stack.
It could be easily tested. I did so the PAC related code is contained in a specific
place, sharing nothing specific but a somehow generic internal api.
I have personally no bold opinion at this time about which JS engine to use.
interesting to test, but it clearly states that its license is AGPL which is clearly more restrictive than the permissive license of duktape.

Regards

Marcel
David Woodhouse
2015-07-16 12:45:35 UTC
Permalink
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
What overriding reason is there for doing this instead of just using
PacRunner? If it's just the JS engine, that's *already* a plugin for
PacRunner and it's easy enough to add new options. Hell *I* managed to
add v8 support at some point in the dim and distant past, and I don't
even admit to knowing any C++. Or JavaScript.

You seem to be reimplementing the part of the solution that was already
basically *working*, while there are other parts which desperately need
to be completed.

PacRunner works. There is a libproxy plugin which uses it¹ — and
alternatively, PacRunner even provides its own drop-in replacement for
libproxy, implementing the nice simple API without the complete horror
the libproxy turned in to.

So we have the dÊmon working, and we have a simple way for client
libraries and applications to use it.

The *only* thing that has so far held me back from proposing a
packaging guideline in my Linux distribution of choice which says
"applications SHALL use libproxy or query PacRunner by default" has
been the fact that NetworkManager does not pass on the proxy
information to PacRunner, so applications which query it won't yet get
the *right* results².

If you're going to look at this stuff, I wish you'd fix *that* (for
both NetworkManager and systemd-networkd) instead of polishing
something that already existed.

Then again, as long as it continues to work, I don't really care too
much. In some way, having it be part of systemd would at least give
more credence to the idea that applications SHOULD be using it by
default.
--
David Woodhouse Open Source Technology Centre
***@intel.com Intel Corporation

¹ https://code.google.com/p/libproxy/issues/detail?id=152² https://bugzilla.gnome.org/show_bug.cgi?id=701824 and https://mail.gnome.org/archives/networkmanager-list/2013-June/msg00093.html
Tomasz Bursztyka
2015-07-16 18:27:33 UTC
Permalink
Hi David,
Post by David Woodhouse
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
What overriding reason is there for doing this instead of just using
PacRunner? If it's just the JS engine, that's *already* a plugin for
PacRunner and it's easy enough to add new options. Hell *I* managed to
add v8 support at some point in the dim and distant past, and I don't
even admit to knowing any C++. Or JavaScript.
You seem to be reimplementing the part of the solution that was already
basically *working*, while there are other parts which desperately need
to be completed.
PacRunner works. There is a libproxy plugin which uses it¹ — and
alternatively, PacRunner even provides its own drop-in replacement for
libproxy, implementing the nice simple API without the complete horror
the libproxy turned in to.
So we have the dæmon working, and we have a simple way for client
libraries and applications to use it.
The *only* thing that has so far held me back from proposing a
packaging guideline in my Linux distribution of choice which says
"applications SHALL use libproxy or query PacRunner by default" has
been the fact that NetworkManager does not pass on the proxy
information to PacRunner, so applications which query it won't yet get
the *right* results².
If you're going to look at this stuff, I wish you'd fix *that* (for
both NetworkManager and systemd-networkd) instead of polishing
something that already existed.
Then again, as long as it continues to work, I don't really care too
much. In some way, having it be part of systemd would at least give
more credence to the idea that applications SHOULD be using it by
default.
Ok maybe that's was unclear in my mail, but systemd guys want such feature
in systemd. I know the work you did to integrate PACrunner properly.
And Marcel - who created PACrunner - was attending this hackfest as well.
So all parties involved know what are the reasons for that proposal.

Tomasz
David Woodhouse
2015-07-16 12:56:19 UTC
Permalink
Post by Tomasz Bursztyka
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message,
void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ if (r &lt; 0)
+ sd_bus_reply_method_return(message, "s", p);
+
+ return 1;
+}
That seems to be making no attempt to use the *correct* proxy
configuration according to the request.

In the case of things like split-tunnel VPNs, we want to handle it
basically the same way that we handle DNS.

Requests within the VPN's DNS domains, and the IP ranges which are
routed to the VPN, need to be resolved according to the VPN's proxy
configuration. And everything else needs to be resolved according to
the local proxy configuration.

NetworkManager already sets up dnsmasq to do precisely this for DNS.
--
David Woodhouse Open Source Technology Centre
***@intel.com Intel Corporation
Tomasz Bursztyka
2015-07-16 18:16:15 UTC
Permalink
Hi David,
Post by David Woodhouse
Post by Tomasz Bursztyka
+static int method_find_proxy(sd_bus *bus, sd_bus_message *message,
void *userdata, sd_bus_error *error) {
+ _cleanup_free_ char *p = strdup("DIRECT");
+ Manager *m = userdata;
+ int r;
+
+ assert(bus);
+ assert(message);
+ assert(m);
+
+ if (r &lt; 0)
+ sd_bus_reply_method_return(message, "s", p);
+
+ return 1;
+}
That seems to be making no attempt to use the *correct* proxy
configuration according to the request.
In the case of things like split-tunnel VPNs, we want to handle it
basically the same way that we handle DNS.
Requests within the VPN's DNS domains, and the IP ranges which are
routed to the VPN, need to be resolved according to the VPN's proxy
configuration. And everything else needs to be resolved according to
the local proxy configuration.
NetworkManager already sets up dnsmasq to do precisely this for DNS.
This is all known. This thread was meant to be just an RFC so only the very
basics of the beginning of a proposal.

Tomasz
David Woodhouse
2015-07-16 12:59:23 UTC
Permalink
Post by Tom Gundersen
Post by Tomasz Bursztyka
+ - support IPv6
This we should probably have from the beginning, any reason you kept
it IPv4 only for now (apart from keeping it simple as a POC)?
Wow, yeah. This far into the 21st century, you basically have to go out
of your *way* to misdesign things badly enough to make them work only
for Legacy IP and not IPv6. Do not simply note it in the TODO list.

You cannot retrofit sanity.
--
David Woodhouse Open Source Technology Centre
***@intel.com Intel Corporation
Tomasz Bursztyka
2015-07-16 18:18:22 UTC
Permalink
Hi David,
Post by David Woodhouse
Post by Tom Gundersen
Post by Tomasz Bursztyka
+ - support IPv6
This we should probably have from the beginning, any reason you kept
it IPv4 only for now (apart from keeping it simple as a POC)?
Wow, yeah. This far into the 21st century, you basically have to go out
of your *way* to misdesign things badly enough to make them work only
for Legacy IP and not IPv6. Do not simply note it in the TODO list.
You cannot retrofit sanity.
The answer was in Tom's question: this was just a bare POC.
This thread is 3 months old btw - and dead - where have you been?

Tomasz
Lucas De Marchi
2015-08-13 21:03:10 UTC
Permalink
Hi Tomasz,

On Fri, Apr 10, 2015 at 9:17 AM, Tomasz Bursztyka
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
Next features to come are a bit detailed in the TODO (last patch).
proxy-discoveryd: Basic core added
proxy-discoveryd: Add the basics for parsing the default configuration
proxy-discoveryd: Add PAC support through duktape js engine
proxy-discoveryd: Execute the PAC based proxy in a thread
proxy-discoveryd: Add the basic parts for handling DBus methods
update TODO
What happened to this patch set? Are you going to send a new version?

Lucas De Marchi
Tomasz Bursztyka
2015-08-14 06:43:33 UTC
Permalink
Hi Lucas,

Maybe, maybe not.

It was not said publicly in that thread, but some systemd guys are
nacking this
due to the JS engine it would provide.
Tom contacted me in private to tell me this is put on hold (though
himself, he really
want that service to get in, as Lennart and David). I guess the time
people get convinced
somehow.

I'll probably come back to that in the end of September, I am fully busy
with other
things right now. If there is really nothing to be done on systemd side,
then there
loss. We will improve PACrunner and that's it.

Br,

Tomasz
Post by Marcel Holtmann
Hi Tomasz,
On Fri, Apr 10, 2015 at 9:17 AM, Tomasz Bursztyka
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
Next features to come are a bit detailed in the TODO (last patch).
proxy-discoveryd: Basic core added
proxy-discoveryd: Add the basics for parsing the default configuration
proxy-discoveryd: Add PAC support through duktape js engine
proxy-discoveryd: Execute the PAC based proxy in a thread
proxy-discoveryd: Add the basic parts for handling DBus methods
update TODO
What happened to this patch set? Are you going to send a new version?
Lucas De Marchi
Tomasz Bursztyka
2015-08-14 06:45:07 UTC
Permalink
Too used to "reply to all". well...
Post by Tomasz Bursztyka
Hi Lucas,
Maybe, maybe not.
It was not said publicly in that thread, but some systemd guys are
nacking this
due to the JS engine it would provide.
Tom contacted me in private to tell me this is put on hold (though
himself, he really
want that service to get in, as Lennart and David). I guess the time
people get convinced
somehow.
I'll probably come back to that in the end of September, I am fully
busy with other
things right now. If there is really nothing to be done on systemd
side, then there
loss. We will improve PACrunner and that's it.
Br,
Tomasz
Post by Marcel Holtmann
Hi Tomasz,
On Fri, Apr 10, 2015 at 9:17 AM, Tomasz Bursztyka
Hi,
As it has been discussed in the systemd hackfest during the Linux Conference
Europe, one daemon could centralize the management of all network proxy
configurations. Idea is something user can do per-application (like in
firefox for instance) or broader (per-DM like in Gnome), user could do it
once and for all through such daemon and applications would then request it
to know whether or not a proxy has to be used and which one.
As a notice, this is nothing new. Such standalone daemon has been already
done by the past, pacrunner. systemd-proxy-discoveryd will more or less
implement the same ideas with improvements. It will get rid of big JS
engines like spidermonkey or v8 which are overkill for the tiny PAC files
to be executed on, for instance. From pacrunner experience, APIs will be
also improved.
This one is using - at least in this RFC - the duktape JS engine to run
the PAC files. Note it is not provided in this patchset. Latest version
1.2.x was used.
Next features to come are a bit detailed in the TODO (last patch).
proxy-discoveryd: Basic core added
proxy-discoveryd: Add the basics for parsing the default
configuration
proxy-discoveryd: Add PAC support through duktape js engine
proxy-discoveryd: Execute the PAC based proxy in a thread
proxy-discoveryd: Add the basic parts for handling DBus methods
update TODO
What happened to this patch set? Are you going to send a new version?
Lucas De Marchi
_______________________________________________
systemd-devel mailing list
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Loading...