Differential D15093 Diff 41451 vpn/wireguard/wireguard.cpp

Changeset View

Standalone View

View Options

vpn/wireguard/wireguard.cpp

  • This file was added.
1​/*
2​ Copyright 2018 Bruce Anderson <banderson19com@san.rr.com>
3​
4​ This program is free software; you can redistribute it and/or
5​ modify it under the terms of the GNU General Public License as
6​ published by the Free Software Foundation; either version 2 of
7​ the License or (at your option) version 3 or any later version
8​ accepted by the membership of KDE e.V. (or its successor approved
9​ by the membership of KDE e.V.), which shall act as a proxy
10​ defined in Section 14 of version 3 of the license.
11​
12​ This program is distributed in the hope that it will be useful,
13​ but WITHOUT ANY WARRANTY; without even the implied warranty of
14​ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15​ GNU General Public License for more details.
16​
17​ You should have received a copy of the GNU General Public License
18​ along with this program. If not, see <http://www.gnu.org/licenses/>.
19​*/
20​
21​#include "wireguard.h"
22​
23​#include <QLatin1Char>
24​#include <QFileInfo>
25​#include <KPluginFactory>
26​#include <KConfig>
pinoUnsubmitted
Done

unused

pino: unused
27​#include <KConfigGroup>
28​
29​#include <NetworkManagerQt/Connection>
30​#include <NetworkManagerQt/VpnSetting>
31​#include <NetworkManagerQt/Ipv4Setting>
pinoUnsubmitted
Done

both unused

pino: both unused
32​
33​#include "wireguardwidget.h"
34​#include "wireguardauth.h"
35​#include "simpleipv4addressvalidator.h"
36​#include "simpleipv6addressvalidator.h"
37​#include "simpleiplistvalidator.h"
38​
39​#include "nm-wireguard-service.h"
40​
41​K_PLUGIN_FACTORY_WITH_JSON(WireGuardUiPluginFactory, "plasmanetworkmanagement_wireguardui.json", registerPlugin<WireGuardUiPlugin>();)
42​
pinoUnsubmitted
Done

unused

pino: unused
43​#define NMV_WG_TAG_INTERFACE "Interface"
44​#define NMV_WG_TAG_PRIVATE_KEY "PrivateKey"
45​#define NMV_WG_TAG_LISTEN_PORT "ListenPort"
46​#define NMV_WG_TAG_ADDRESS "Address"
47​#define NMV_WG_TAG_DNS "DNS"
48​#define NMV_WG_TAG_MTU "MTU"
49​#define NMV_WG_TAG_TABLE "Table"
50​#define NMV_WG_TAG_PRE_UP "PreUp"
51​#define NMV_WG_TAG_POST_UP "PostUp"
52​#define NMV_WG_TAG_PRE_DOWN "PreDown"
53​#define NMV_WG_TAG_POST_DOWN "PostDown"
54​#define NMV_WG_TAG_FWMARK "FwMark"
55​
56​#define NMV_WG_TAG_PEER "Peer"
57​#define NMV_WG_TAG_PUBLIC_KEY "PublicKey"
58​#define NMV_WG_TAG_ALLOWED_IPS "AllowedIPs"
59​#define NMV_WG_TAG_ENDPOINT "Endpoint"
60​#define NMV_WG_TAG_PRESHARED_KEY "PresharedKey"
61​
pinoUnsubmitted
Done

unused

pino: unused
62​
63​WireGuardUiPlugin::WireGuardUiPlugin(QObject *parent, const QVariantList &) : VpnUiPlugin(parent)
64​{
65​}
66​
67​WireGuardUiPlugin::~WireGuardUiPlugin()
68​{
69​}
70​
71​SettingWidget *WireGuardUiPlugin::widget(const NetworkManager::VpnSetting::Ptr &setting, QWidget *parent)
72​{
jgrulichUnsubmitted
Done

You can use simpleipv[4,6]validator we have in plasma-nm instead of using everything below. Or maybe QHostAddress can validate it for you?

jgrulich: You can use simpleipv[4,6]validator we have in plasma-nm instead of using everything below. Or…
andersonbruceAuthorUnsubmitted
Not Done

The problem with using the simpleipv[4,6]validator is that WireGuard requires that in some cases the IP address can include a CIDR at the end (e.g. 10.2.4.6/32) and in another case the entry needs to have a port number (e.g. 10.2.3.5:7642) and the simpleip* functions won't handle either of these as currently written. QHostAddress will handle the CIDR but not the port number version. Also I'm not really thrilled with how QHostAddress does its verification. I know that it is technically correct but my personal opinion is that "1/32" should not be accepted as a valid IPv4 address which QHostAddress does.

andersonbruce: The problem with using the simpleipv[4,6]validator is that WireGuard requires that in some…
73​ return new WireGuardSettingWidget(setting, parent);
74​}
75​
76​SettingWidget *WireGuardUiPlugin::askUser(const NetworkManager::VpnSetting::Ptr &setting, QWidget *parent)
77​{
78​ return new WireGuardAuthWidget(setting, parent);
79​}
80​
81​QString WireGuardUiPlugin::suggestedFileName(const NetworkManager::ConnectionSettings::Ptr &connection) const
82​{
83​ return connection->id() + "_wireguard.conf";
84​}
85​
86​QString WireGuardUiPlugin::supportedFileExtensions() const
87​{
88​ return "*.conf";
89​}
90​
91​NMVariantMapMap WireGuardUiPlugin::importConnectionSettings(const QString &fileName)
92​{
93​ NMVariantMapMap result;
94​
95​ const KConfig importFile(fileName, KConfig::NoGlobals);
96​ const KConfigGroup interfaceGroup = importFile.group(NMV_WG_TAG_INTERFACE);
97​ const KConfigGroup peerGroup = importFile.group(NMV_WG_TAG_PEER);
98​
99​ // The config file must have both [Interface] and [Peer] sections
100​ if (!interfaceGroup.exists()
101​ || !peerGroup.exists()) {
102​ mError = VpnUiPlugin::Error;
103​ mErrorMessage = i18n("Invalid config file");
104​ return result;
105​ }
106​
107​ const QString connectionName = QFileInfo(fileName).completeBaseName();
108​ NMStringMap dataMap;
109​ QVariantMap ipv4Data;
110​
111​ QString value;
112​ QStringList valueList;
113​ quint32 intValue;
114​
115​ // Do the required fields first and fail (i.e. return an empty result) if not present
116​
117​ // Addresses
118​ valueList = interfaceGroup.readEntry(NMV_WG_TAG_ADDRESS, QStringList());
119​ if (valueList.size() == 0)
120​ return result;
pinoUnsubmitted
Done

isEmpty()

pino: isEmpty()
121​ for (int i = 0; i < valueList.size(); i++) {
122​ QPair<QHostAddress, int> addressIn = QHostAddress::parseSubnet(valueList[i].trimmed());
pinoUnsubmitted
Done

better use qt's foreach here:

Q_FOREACH (const QString &address, valueList)

and then using address instead of valueList[i]

pino: better use qt's foreach here: lang=c++ Q_FOREACH (const QString &address, valueList) and…
andersonbruceAuthorUnsubmitted
Not Done

Used plain 'for' instead because Nathaniel Graham commented that the Q_FOREACH is not acceptable anymore.

andersonbruce: Used plain 'for' instead because Nathaniel Graham commented that the Q_FOREACH is not…
123​ if (addressIn.first.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv4Protocol)
pinoUnsubmitted
Done

addressIn can be const

pino: addressIn can be const
124​ dataMap.insert(QLatin1String(NM_WG_KEY_ADDR_IP4), valueList[i]);
125​ else if (addressIn.first.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv6Protocol)
126​ dataMap.insert(QLatin1String(NM_WG_KEY_ADDR_IP6), valueList[i]);
127​ else // Error condition
128​ return result;
129​ }
130​
131​ // Private Key
132​ value = interfaceGroup.readEntry(NMV_WG_TAG_PRIVATE_KEY);
133​ if (!value.isEmpty()) {
134​ QRegExp validatorRegex("[0-9a-zA-Z\\+/]{43,43}=");
135​ if (validatorRegex.exactMatch(value))
136​ dataMap.insert(QLatin1String(NM_WG_KEY_PRIVATE_KEY), value);
pinoUnsubmitted
Done

the validation of a key is done multiple times, repeating the same regexp every time -- I'd be worth to create an own validator for it

pino: the validation of a key is done multiple times, repeating the same regexp every time -- I'd be…
137​ else
138​ return result;
139​ } else {
140​ return result;
pinoUnsubmitted
Done

no need to specify this as parent, since it is on stack, and thus it will be deleted automatically

pino: no need to specify `this` as parent, since it is on stack, and thus it will be deleted…
141​ }
pinoUnsubmitted
Done

indent

pino: indent
142​
143​ // Public Key
144​ value = peerGroup.readEntry(NMV_WG_TAG_PUBLIC_KEY);
145​ if (!value.isEmpty()) {
146​ QRegExp validatorRegex("[0-9a-zA-Z\\+/]{43,43}=");
147​ if (validatorRegex.exactMatch(value))
148​ dataMap.insert(QLatin1String(NM_WG_KEY_PUBLIC_KEY), value);
149​ else
150​ return result;
151​ } else {
152​ return result;
153​ }
154​
pinoUnsubmitted
Done

extra ';'

pino: extra ';'
pinoUnsubmitted
Done

make all 3 as const, so it is clear they are read-only (and attempts to use writeEntry() will result in build failures)

pino: make all 3 as `const`, so it is clear they are read-only (and attempts to use writeEntry() will…
155​ // Allowed IPs
156​ value = peerGroup.readEntry(NMV_WG_TAG_ALLOWED_IPS);
157​ if (!value.isEmpty()) {
158​ int pos = 0;
159​ SimpleIpListValidator *validator = new SimpleIpListValidator(this,
160​ SimpleIpListValidator::WithCidr,
pinoUnsubmitted
Done

"validator" is leaked here -- just use it on the stack (SimpleIpListValidator validator;)

pino: "validator" is leaked here -- just use it on the stack (`SimpleIpListValidator validator;`)
161​ SimpleIpListValidator::Both);
162​
163​ if (validator->validate(value, pos) != QValidator::State::Invalid)
164​ dataMap.insert(QLatin1String(NM_WG_KEY_ALLOWED_IPS), value);
165​ else
166​ return result;
167​ } else {
168​ return result;
169​ }
pinoUnsubmitted
Done

You do not need to get the list of groups in the config file (twice): earlier you get the KConfigGroup objects for the groups, so checking eg interfaceGroup.exists() should do the job.

pino: You do not need to get the list of groups in the config file (twice): earlier you get the…
170​
171​ // Now the optional ones
172​
173​ // Listen Port
pinoUnsubmitted
Done

These checks can be moved right after reading the config file and getting the KConfigGroup objects.

pino: These checks can be moved right after reading the config file and getting the KConfigGroup…
174​ intValue = interfaceGroup.readEntry(NMV_WG_TAG_LISTEN_PORT, 0);
175​ if (intValue > 0) {
pinoUnsubmitted
Done

0 is int by default, so intValue must be int too (not quint32)

pino: `0` is `int` by default, so intValue must be int too (not quint32)
176​ if (intValue < 65536)
177​ dataMap.insert(QLatin1String(NM_WG_KEY_LISTEN_PORT), QString::number(intValue));
178​ else
pinoUnsubmitted
Done

since earlier there is a keyPos variable used for basically the same purpose (i.e. passing it to the validate() of validators), just move pos earlier in the function, and use one everywhere

pino: since earlier there is a `keyPos` variable used for basically the same purpose (i.e. passing it…
179​ return result;
pinoUnsubmitted
Done

KConfig already supports comma-separated lists -- just pass QStringList() as default value to readEntry(), so KConfigGroup knows the value is a list.

pino: KConfig already supports comma-separated lists -- just pass QStringList() as `default` value to…
andersonbruceAuthorUnsubmitted
Not Done

The problem with using the KConfig method is if a space slips into the config file before or after the comma then the spaces are left in one or the other of the QStrings and I have to process each entry in the list to remove the spaces. The files can come from elsewhere, e.g. I have some provided by my VPN which have spaces in comma separated lists. Using split allows both operations to be performed at the same time.

andersonbruce: The problem with using the KConfig method is if a space slips into the config file before or…
pinoUnsubmitted
Done

This is what QString::trimmed() does already. Considering you are passing the string directly to QHostAddress, it is just easy to write

const QPair<QHostAddress, int> addressIn = QHostAddress::parseSubnet(addressList[i].trimmed());
pino: This is what `QString::trimmed()` does already. Considering you are passing the string directly…
180​ }
181​
182​ // DNS
183​ value = interfaceGroup.readEntry(NMV_WG_TAG_DNS);
184​ if (!value.isEmpty()) {
185​ QHostAddress testAddress(value);
186​ if (testAddress.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv4Protocol)
jgrulichUnsubmitted
Done

Coding style.

jgrulich: Coding style.
pinoUnsubmitted
Done

const

pino: const
187​ dataMap.insert(QLatin1String(NM_WG_KEY_DNS), value);
188​ else
189​ return result;
190​ }
jgrulichUnsubmitted
Done

You multiple times return just result, you should also set error message with the reason so users know why the import failed.

jgrulich: You multiple times return just result, you should also set error message with the reason so…
191​
192​ // MTU
193​ intValue = interfaceGroup.readEntry(NMV_WG_TAG_MTU, 0);
194​ if (intValue > 0) {
195​ if (intValue < 65536)
196​ dataMap.insert(QLatin1String(NM_WG_KEY_LISTEN_PORT), QString::number(intValue));
pinoUnsubmitted
Done

If you specify 0 as default parameter, KConfigGroup will try to decode the value as integer automatically. If 0 is a valid value for the port, then use -1.
After doing that, you do not need a validator anymore, just a manual range check will do the job.

pino: If you specify `0` as default parameter, KConfigGroup will try to decode the value as integer…
pinoUnsubmitted
Done

an MTU is not a port...

pino: an MTU is not a port...
197​ else
198​ return result;
199​ }
200​
201​ // Table
202​ value = interfaceGroup.readEntry(NMV_WG_TAG_TABLE);
203​ if (!value.isEmpty()) {
204​ dataMap.insert(QLatin1String(NM_WG_KEY_TABLE), value);
205​ }
206​
207​ // Endpoint
208​ value = peerGroup.readEntry(NMV_WG_TAG_ENDPOINT);
209​ if (!value.isEmpty())
210​ dataMap.insert(QLatin1String(NM_WG_KEY_ENDPOINT), value);
pinoUnsubmitted
Done

this validator is not needed, just use validatorRegex directly

pino: this validator is not needed, just use `validatorRegex` directly
211​
212​ // Preshared Key
213​ value = peerGroup.readEntry(NMV_WG_TAG_PRESHARED_KEY);
214​ if (!value.isEmpty()) {
215​ QRegExp validatorRegex("[0-9a-zA-Z\\+/]{43,43}=");
216​ QRegExpValidator validator(validatorRegex);
jgrulichUnsubmitted
Done

Coding style.

jgrulich: Coding style.
217​ int pos = 0;
218​ if (validator.validate(value, pos) != QValidator::State::Invalid)
219​ dataMap.insert(QLatin1String(NM_WG_KEY_PRESHARED_KEY), value);
220​ else
221​ return result;
222​ }
223​
224​ NetworkManager::VpnSetting setting;
225​ setting.setServiceType(QLatin1String(NM_DBUS_SERVICE_WIREGUARD));
226​ setting.setData(dataMap);
227​
228​ QVariantMap conn;
229​ conn.insert("id", connectionName);
230​ conn.insert("type", "vpn");
231​ result.insert("connection", conn);
232​ result.insert("vpn", setting.toMap());
pinoUnsubmitted
Done

same as for the listen port: please read the value directly as integer.

pino: same as for the listen port: please read the value directly as integer.
233​
234​ return result;
235​}
236​
237​bool WireGuardUiPlugin::exportConnectionSettings(const NetworkManager::ConnectionSettings::Ptr &connection, const QString &fileName)
238​{
239​ NMStringMap dataMap;
240​ NetworkManager::VpnSetting::Ptr vpnSetting = connection->setting(NetworkManager::Setting::Vpn).dynamicCast<NetworkManager::VpnSetting>();
pinoUnsubmitted
Done

if you move the dataMap declaration after vpnSettings, you can glue declaration and initialization together

pino: if you move the dataMap declaration after vpnSettings, you can glue declaration and…
241​ dataMap = vpnSetting->data();
242​
243​ // Make sure all the required fields are present
244​ if (!(dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4))
245​ || dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4)))
246​ || !dataMap.contains(QLatin1String(NM_WG_KEY_PRIVATE_KEY))
pinoUnsubmitted
Done

most probably this should be NM_WG_KEY_ADDR_IP6 (not again IP4)

pino: most probably this should be `NM_WG_KEY_ADDR_IP6` (not again IP4)
247​ || !dataMap.contains(QLatin1String(NM_WG_KEY_PUBLIC_KEY))
248​ || !dataMap.contains(QLatin1String(NM_WG_KEY_ALLOWED_IPS)))
249​ return false;
250​
251​ // Open the output file
252​ KConfig exportFile(fileName, KConfig::NoGlobals);
253​ KConfigGroup interfaceGroup = exportFile.group(NMV_WG_TAG_INTERFACE);
254​ KConfigGroup peerGroup = exportFile.group(NMV_WG_TAG_PEER);
255​
256​ QString value;
257​
258​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4))) {
259​ value = dataMap[NM_WG_KEY_ADDR_IP4];
260​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP6)))
261​ value += "," + dataMap[NM_WG_KEY_ADDR_IP6];
262​ } else if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4))) {
263​ value = dataMap[NM_WG_KEY_ADDR_IP4];
264​ }
pinoUnsubmitted
Done

typos for IP6?

pino: typos for IP6?
265​ interfaceGroup.writeEntry(NMV_WG_TAG_ADDRESS, value);
266​
pinoUnsubmitted
Done

just like readEntry, writeEntry can output lists -- just create a QStringList, and pass it to writeEntry

pino: just like readEntry, writeEntry can output lists -- just create a QStringList, and pass it to…
267​ // Do Private Key
268​ interfaceGroup.writeEntry(NMV_WG_TAG_PRIVATE_KEY, dataMap[NM_WG_KEY_PRIVATE_KEY]);
269​
270​ // Do DNS (Not required)
271​ if (dataMap.contains(QLatin1String(NM_WG_KEY_DNS)))
272​ interfaceGroup.writeEntry(NMV_WG_TAG_DNS, dataMap[NM_WG_KEY_DNS]);
pinoUnsubmitted
Done

as above, no need for a validator, just use the QRegExp directly

pino: as above, no need for a validator, just use the QRegExp directly
273​
274​ // Do MTU (Not required)
275​ if (dataMap.contains(QLatin1String(NM_WG_KEY_MTU)))
276​ interfaceGroup.writeEntry(NMV_WG_TAG_MTU, dataMap[NM_WG_KEY_MTU]);
277​
278​ // Do Table number (Not required)
279​ if (dataMap.contains(QLatin1String(NM_WG_KEY_TABLE)))
280​ interfaceGroup.writeEntry(NMV_WG_TAG_TABLE, dataMap[NM_WG_KEY_TABLE]);
281​
282​ // Do Listen Port (Not required)
283​ if (dataMap.contains(QLatin1String(NM_WG_KEY_LISTEN_PORT)))
284​ interfaceGroup.writeEntry(NMV_WG_TAG_LISTEN_PORT, dataMap[NM_WG_KEY_LISTEN_PORT]);
285​
286​ // Do FwMark (Not required)
287​ if (dataMap.contains(QLatin1String(NM_WG_KEY_FWMARK)))
288​ interfaceGroup.writeEntry(NMV_WG_TAG_FWMARK, dataMap[NM_WG_KEY_FWMARK]);
289​
pinoUnsubmitted
Done

as above, no need for a validator, just use the QRegExp directly

pino: as above, no need for a validator, just use the QRegExp directly
290​ // Do Pupblic key (required)
291​ peerGroup.writeEntry(NMV_WG_TAG_PUBLIC_KEY, dataMap[NM_WG_KEY_PUBLIC_KEY]);
292​
293​ // Do Allowed IP list (Required)
294​ peerGroup.writeEntry(NMV_WG_TAG_ALLOWED_IPS, dataMap[NM_WG_KEY_ALLOWED_IPS]);
295​
296​ // Do Endpoint (Not required)
297​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ENDPOINT)))
298​ peerGroup.writeEntry(NMV_WG_TAG_ENDPOINT, dataMap[NM_WG_KEY_ENDPOINT]);
299​
300​ // Do Preshared Key (Not required)
301​ if (dataMap.contains(QLatin1String(NM_WG_KEY_PRESHARED_KEY)))
302​ peerGroup.writeEntry(NMV_WG_TAG_PRESHARED_KEY, dataMap[NM_WG_KEY_PRESHARED_KEY]);
303​
304​ exportFile.sync();
305​ return true;
306​}
307​
pinoUnsubmitted
Done

as above, no need for a validator, just use the QRegExp directly

pino: as above, no need for a validator, just use the QRegExp directly
308​#include "wireguard.moc"
pinoUnsubmitted
Done

as mentioned above: KConfigGroup supports lists

pino: as mentioned above: KConfigGroup supports lists
pinoUnsubmitted
Done

this, just like all the other required keys, should be checked before even opening the output file

pino: this, just like all the other required keys, should be checked before even opening the output…
jgrulichUnsubmitted
Done

Remove space.

jgrulich: Remove space.