On Wed, 2017-03-01 at 08:07 +0100, poma wrote:
From 28b7713cda1deba1b54bd9e52b0d62716e356b66 Mon Sep 17 00:00:00 2001 From: poma <poma gmail com> Date: Wed, 1 Mar 2017 07:05:40 +0100 Subject: [PATCH] nm-pptp-service: Grant proto GRE by firewalld. With recent kernels, the Poptop - The PPTP Server for Linux (pptpd) requires explicit load of nf_conntrack_pptp kernel module to achieve the operating state of the service itself. However this is not the case with the PPTP Client (pptp) on a Linux based platform. What is needed is to apply directly, rule within the firewalld, to grant proto gre, to achieve the operating state of the client itself. Ref. https://bugzilla.redhat.com/show_bug.cgi?id=1187328 https://bugzilla.redhat.com/show_bug.cgi?id=1214643
Hi poma, the patch does two things. I think there should be two patches for it. 1) drop loading the kernel module "nf_conntrack_pptp". The patch basically reverts https://git.gnome.org/browse/network-manager-pptp/commit/?id=695d4f2f3d1003e18be6f97bbb103e44f75d3c2b but it's not explained why that is correct beyond "this is not the case with...". It should be explained better whats wrong with 695d4f2f and how that affects the two bugs that were closed by it. Will the issue reapar, or was there a different issue in the first place? 2) call to firewallcmd. firewalld is commonly only available on Fedora/RHEL, thus patch would cause a warning on Debian systems... You would at least need to check whether such a binary file exists and only call it if necessary. Thomas
--- src/nm-pptp-service.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/nm-pptp-service.c b/src/nm-pptp-service.c index 1710fd9..6a66386 100644 --- a/src/nm-pptp-service.c +++ b/src/nm-pptp-service.c @@ -1113,7 +1113,7 @@ main (int argc, char *argv[]) GMainLoop *main_loop; gboolean persist = FALSE; GOptionContext *opt_ctx = NULL; - char *conntrack_module[] = { "/sbin/modprobe", "nf_conntrack_pptp", NULL }; + char *firewalld_grant_proto_gre[] = { "/bin/firewall-cmd", "--direct", "--add-rule", "ipv4", "filter", "INPUT", "0", "-p", "gre", "-j", "ACCEPT", NULL }; GError *error = NULL; gs_free char *bus_name_free = NULL; const char *bus_name; @@ -1178,13 +1178,17 @@ main (int argc, char *argv[]) if (!persist) g_signal_connect (plugin, "quit", G_CALLBACK (quit_mainloop), main_loop); - /* Newer kernels require nf_conntrack_pptp kernel module so that PPTP - * worked correctly. Load the module now. Ignore errors, the module - * might not exist (older kernels). + /* With recent kernels, the Poptop - The PPTP Server for Linux (pptpd) requires + * explicit load of nf_conntrack_pptp kernel module to achieve the operating state of the service itself. + * However this is not the case with the PPTP Client (pptp) on a Linux. + * What is needed is to apply directly, rule within the firewalld, to grant proto gre, + * to achieve the operating state of the client itself. + * Apply the rule now. Ignore errors, the rule may already be enabled. * https://bugzilla.redhat.com/show_bug.cgi?id=1187328 + * https://bugzilla.redhat.com/show_bug.cgi?id=1214643 */ - if (!g_spawn_sync (NULL, conntrack_module, NULL, 0, NULL, NULL, NULL, NULL, NULL, &error)) { - _LOGW ("modprobing nf_conntrack_pptp failed: %s", error->message); + if (!g_spawn_sync (NULL, firewalld_grant_proto_gre, NULL, 0, NULL, NULL, NULL, NULL, NULL, &error)) { + _LOGW ("granting proto gre by firewalld failed: %s", error->message); g_error_free (error); }
Attachment:
signature.asc
Description: This is a digitally signed message part