Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[th/wireguard-pt3 (v2)] #295

Merged
merged 7 commits into from Feb 22, 2019
Merged

[th/wireguard-pt3 (v2)] #295

merged 7 commits into from Feb 22, 2019

Conversation

thom311
Copy link
Member

@thom311 thom311 commented Feb 9, 2019

This resurrects the work from #281 and is based on #293.

I think this is ready to merged. A few things are still missing. They are documented in TODO comments in src/devices/nm-device-wireguard.c. But these things should not block this branch. And I don't even think they would block a 1.16.0 release (with this branch in).

Please test and review. Thanks

@lkundrak lkundrak force-pushed the th/wireguard-pt3 branch 8 times, most recently from c697299 to 896b171 Compare February 11, 2019 14:12
src/devices/nm-device.c Outdated Show resolved Hide resolved
shared/nm-utils/nm-macros-internal.h Outdated Show resolved Hide resolved
shared/nm-utils/nm-macros-internal.h Outdated Show resolved Hide resolved
Copy link
Contributor

@bengal bengal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

libnm-core/nm-setting-wireguard.c Outdated Show resolved Hide resolved
libnm-core/nm-setting-wireguard.c Outdated Show resolved Hide resolved
@thom311
Copy link
Member Author

thom311 commented Feb 17, 2019

Change in binary size when building with

    $ ./contrib/fedora/rpm/build_clean.sh -g -w crypto_gnutls \
                 -W debug -w iwd -W test -w libnm_glib

on x64_86, Fedora 29

commit 9a4cd1e (before) vs 489a6ab (after):

[before] 3360928 /usr/sbin/NetworkManager
          +55504 (+ 1.65 %)
[after]  3416432 /usr/sbin/NetworkManager

[before] 1781456 /usr/lib64/libnm.so.0.1.0
         + 37584 (+ 2.11%)
[after]  1819040 /usr/lib64/libnm.so.0.1.0

[before]  896664 /usr/bin/nmcli
          + 8256 (+ 0.92%)
[after]   904920 /usr/bin/nmcli

@lkundrak lkundrak force-pushed the th/wireguard-pt3 branch 5 times, most recently from ef55e09 to c674633 Compare February 22, 2019 09:48
Copy link
Contributor

@fgiudici fgiudici left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Branch LGTM

libnm-core/nm-setting-wireguard.h Outdated Show resolved Hide resolved
libnm-core/nm-keyfile-utils.h Show resolved Hide resolved
src/devices/nm-device-wireguard.c Outdated Show resolved Hide resolved
NEWS Outdated Show resolved Hide resolved
For now only add the core settings, no peers' data.

To support peers and the allowed-ips of the peers is more complicated
and will be done later. It's more complicated because these are nested
lists (allowed-ips) inside a list (peers). That is quite unusual and to
conveniently support that in D-Bus API, in keyfile format, in libnm,
and nmcli, is a effort.
Also, it's further complicated by the fact that each peer has a secret (the
preshared-key). Thus we probably need secret flags for each peer, which
is a novelty as well (until now we require a fixed set of secrets per
profile that is well known).
…rd profile

Use the script to test how GObject introspection with libnm's WireGuard
support works.

Also, since support for WireGuard peers is not yet implemented in nmcli
(or other clients), this script is rather useful.
Configuring peers (and allowed-ips of the peers) is not
yet supported.
That is slightly complex, because we need to (DNS) resolve the endpoints,
and we also have to retry periodically. For example, initially we may be
unable to resolve an endpoint, but later we may be.

What is also interesting is that during assume and reapply, we may not
have all information in the profile. Most notably, the private keys will
be missing. We need to cope with that and not reconfigure keys. However,
we still need to resolve names and update the endpoints.
@thom311
Copy link
Member Author

thom311 commented Feb 22, 2019

Thanks @fgiudici. Addresses your comments and repushed.

@lkundrak
Copy link
Member

LGTM

@thom311 thom311 merged commit 6d5aa85 into master Feb 22, 2019
thom311 added a commit that referenced this pull request Feb 22, 2019
@thom311
Copy link
Member Author

thom311 commented Feb 22, 2019

Thanks!! merged.

@thom311 thom311 deleted the th/wireguard-pt3 branch February 22, 2019 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants