mbox series

[Openvpn-devel,RFC,0/4] add netlink support for Linux

Message ID 20180401131615.12567-1-a@unstable.cc
Headers show
Series add netlink support for Linux | expand

Message

Antonio Quartulli April 1, 2018, 3:16 a.m. UTC
Hi all,

this patchset introduces native netlink support for the Linux platform.

At the moment openvpn operates on the tun interface and on the routing
table by directly invoking the "ip" command (or ifconfig/route if
nettools is selected at compile time).

With this patchset, openvpn would not need to fork new processes to
run the "ip" binary any longer, but would directly talk to the kernel
by means of the netlink interface.
This means simpler/cleaner code and, possibly, faster execution.

Another important advantage of this change is that the openvpn
process will be in charge of directly working with the kernel, thus
it can be granted special capabilities so that interfaces/routes
operations can be carried out even when running as non-root.

Christian Hesse is working on a follow-up patch to properly allow the
above.

This patchset also offers a first step towards a refactoring
of the tun.c and route.c code.


The idea moving forward is to drop nettools support once this patchset
is merged, but to retain support for ip and the --ifconfig/route-noexec
options.


Feedback of any type is of course welcome.


This patch is posted as RFC because, as agreed during the last
Hackathon, it will be considered for merging only when unit-tests
will also be available. On to pof that, several aspects (like
allowing iproute2 to be still used) have to be properly implemented.


This code can also be found on GitHub (based on latest master) at:
https://github.com/ordex/openvpn/tree/sitnl


Regards,


Antonio Quartulli (4):
  configure: add LINUX conditional variable
  introduce sitnl: Simplified Interface To NetLink
  tun.c: use sitnl to handle tun configuration on Linux
  route.c: use sitnl to handle route configuration on Linux

 configure.ac            |    2 +
 src/openvpn/Makefile.am |    3 +
 src/openvpn/errlevel.h  |    1 +
 src/openvpn/route.c     |  364 +++------------
 src/openvpn/sitnl.c     | 1195 +++++++++++++++++++++++++++++++++++++++++++++++
 src/openvpn/sitnl.h     |  217 +++++++++
 src/openvpn/tun.c       |  199 +++-----
 7 files changed, 1547 insertions(+), 434 deletions(-)
 create mode 100644 src/openvpn/sitnl.c
 create mode 100644 src/openvpn/sitnl.h

Comments

Antonio Quartulli April 5, 2018, 9:43 p.m. UTC | #1
Hi all,

in case anybody cares, I have updated my patchset on GitHub[1] (I didn't
want to create more noise on the mailing list since it is still RFC).


This new version is quite different as I implemented a major
architectural change: instead of creating a standalone sitnl module, I
introduced a generic "networking API" (this is a concept that has been
discussed with Gert and other devs in the past, but that never came into
being).

This "networking API" is the glue code between tun.c/route.c and the
Linux specific code.

Two new files, namely networking_sitnl.c and networking_ip.c, provides
two implementations for this API: one uses the new sitnl code (netlink)
and one uses iproute2.

This new architecture could be re-used in the future to move other
platforms specific code (i.e. for drawin, openbsd, etc..) to their own
files and hide all the details behind the new API.


If you try to compile my branch, openvpn will use sitnl by default
unless you specify --enable-iproute2 at configure time.


Comments are review are welcome!


Cheers,

[1] https://github.com/ordex/openvpn/tree/sitnl
Christian Hesse April 10, 2018, 9:15 p.m. UTC | #2
Antonio Quartulli <a@unstable.cc> on Fri, 2018/04/06 15:43:
> Two new files, namely networking_sitnl.c and networking_ip.c, provides
> two implementations for this API: one uses the new sitnl code (netlink)
> and one uses iproute2.

This complicates the situation for my followup code: Running the process with
unprivileged user works with netlink interface only. If we want to support
netlink and iproute2 we end up with creating the files from templates (or
carry static files in at least two versions).

This kicks into the discussion we had about supporting newer systemd features
selectively... Shipping different static files for distributions and/or
systemd versions duplicates the number of files.
Antonio Quartulli April 10, 2018, 9:43 p.m. UTC | #3
Hi Christian,

On 11/04/18 15:15, Christian Hesse wrote:
> Antonio Quartulli <a@unstable.cc> on Fri, 2018/04/06 15:43:
>> Two new files, namely networking_sitnl.c and networking_ip.c, provides
>> two implementations for this API: one uses the new sitnl code (netlink)
>> and one uses iproute2.
> 
> This complicates the situation for my followup code: Running the process with
> unprivileged user works with netlink interface only. If we want to support
> netlink and iproute2 we end up with creating the files from templates (or
> carry static files in at least two versions).

Keeping support for iproute2 is part of our agreement during the
discussion at the last hackathon. Some of the reasons might be
summarized in the hackathon page on the wiki. Therefore, we need to find
a way to deal with that.

> 
> This kicks into the discussion we had about supporting newer systemd features
> selectively... Shipping different static files for distributions and/or
> systemd versions duplicates the number of files.
> 

I am not into systemd, therefore I am not able to comment on the
strategy we need to adopt.

However, what I imagine is that each distribution, when deciding what
library to use (sitnl vs iproute2), will also decide which of the
provided unit files to ship (if we have multiple precompiled files).
Or our Makefile should generate the right ones based on the
--enable-iproute2 switch (maybe this is what you meant with templates?).


Cheers,
Gert Doering April 10, 2018, 9:48 p.m. UTC | #4
Hi,

On Wed, Apr 11, 2018 at 03:43:11PM +0800, Antonio Quartulli wrote:
> However, what I imagine is that each distribution, when deciding what
> library to use (sitnl vs iproute2), will also decide which of the
> provided unit files to ship (if we have multiple precompiled files).

This is how I envisioned how the alternatives would look like - if
you compile yourself, it's your own responsibility, but for the distro 
maintainers, they need to ensure that different pieces match.

> Or our Makefile should generate the right ones based on the
> --enable-iproute2 switch (maybe this is what you meant with templates?).

That would be an interesting idea :-) - not sure it's worth the extra
complications, though.

gert
David Sommerseth April 11, 2018, 1:50 a.m. UTC | #5
On 11/04/18 09:43, Antonio Quartulli wrote:
> 
>> This kicks into the discussion we had about supporting newer systemd features
>> selectively... Shipping different static files for distributions and/or
>> systemd versions duplicates the number of files.
>
> I am not into systemd, therefore I am not able to comment on the
> strategy we need to adopt.
> 
> However, what I imagine is that each distribution, when deciding what
> library to use (sitnl vs iproute2), will also decide which of the
> provided unit files to ship (if we have multiple precompiled files).
> Or our Makefile should generate the right ones based on the
> --enable-iproute2 switch (maybe this is what you meant with templates?).

Systemd is developing quite fast, and is consistently improving on the
security side - with more and more interesting lock-down features, most which
can be automated if enabled correctly in the unit files.  But newer features
may not work so well on older systemd releases.

So this is actually a two-fold challenge

- How to figure out which features systemd supports?  We can here presume the
  host building the package runs the systemd version OpenVPN needs to
  integrate against.

- How to output/generate unit files which are consistent with the available
  features?

We will most likely need some kind of template solution to achieve this.  The
template approach used with Makefile.am/Makefile.in is too limited for our
need - that's essentially just a wrapped in sed, which replaces defined
variables with something else.

There's plenty of other alternatives as well.  But that can easily mean
increasing the build time dependencies.  I'm not convinced that is the right
approach for this need.

I've been pondering on what would be the best approach ... using plain bash
with friends (awk, sed, etc), using Python (lots of template engine
alternatives, some are built-in) or even possibilities with XML+XSLT.  All of
these required tools for either approach are mostly available by default on
most Liux distributions.  Since systemd is Linux only, that's the base
restriction.  All of these alternatives have some pros, but more cons.

But in the end, I believe that currently it is probably better to have a
simple shell script doing the generation.

A unit file typically consists of three sections (Unit, Service and Install).
Now, the Service section is the one which will be mostly modified.  In the
Unit section, only the description is slightly modified between server and
client variants.

Such a generator script typically need to have some kind of "feature matrix"
which enlists which features we're interested in using in the currently
available systemd version on the system.  Then pass this "detected features"
to a function which creates the [Service] section on-the-fly and dumps
everything to stdout.  The [Unit] section to use would be determined by the
role (client or server), which could be an argument to the script.  This role
would need to also be used when creating the [Service] section too.


Thoughts?
Antonio Quartulli April 11, 2018, 2:06 a.m. UTC | #6
On 11/04/18 19:50, David Sommerseth wrote:
> But in the end, I believe that currently it is probably better to have a
> simple shell script doing the generation.
> 

+1

Unless we have to create something quite complex (not the case here)
that needs further extensions in the future (probably not the case too)
we should keep this simple and use a plain bash script.

my 2 cents.

Cheers,