Differential D15093 Diff 42118 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​#include "wireguardkeyvalidator.h"
39​
40​#include "nm-wireguard-service.h"
41​
42​K_PLUGIN_FACTORY_WITH_JSON(WireGuardUiPluginFactory, "plasmanetworkmanagement_wireguardui.json", registerPlugin<WireGuardUiPlugin>();)
pinoUnsubmitted
Done

unused

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

unused

pino: unused
62​
63​
64​WireGuardUiPlugin::WireGuardUiPlugin(QObject *parent, const QVariantList &) : VpnUiPlugin(parent)
65​{
66​}
67​
68​WireGuardUiPlugin::~WireGuardUiPlugin()
69​{
70​}
71​
72​SettingWidget *WireGuardUiPlugin::widget(const NetworkManager::VpnSetting::Ptr &setting, QWidget *parent)
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​{
74​ return new WireGuardSettingWidget(setting, parent);
75​}
76​
77​SettingWidget *WireGuardUiPlugin::askUser(const NetworkManager::VpnSetting::Ptr &setting, QWidget *parent)
78​{
79​ return new WireGuardAuthWidget(setting, parent);
80​}
81​
82​QString WireGuardUiPlugin::suggestedFileName(const NetworkManager::ConnectionSettings::Ptr &connection) const
83​{
84​ return connection->id() + "_wireguard.conf";
85​}
86​
87​QString WireGuardUiPlugin::supportedFileExtensions() const
88​{
89​ return "*.conf";
90​}
91​
92​NMVariantMapMap WireGuardUiPlugin::importConnectionSettings(const QString &fileName)
93​{
94​ NMVariantMapMap result;
95​
96​ const KConfig importFile(fileName, KConfig::NoGlobals);
97​ const KConfigGroup interfaceGroup = importFile.group(NMV_WG_TAG_INTERFACE);
98​ const KConfigGroup peerGroup = importFile.group(NMV_WG_TAG_PEER);
99​
100​ // The config file must have both [Interface] and [Peer] sections
101​ if (!interfaceGroup.exists()
102​ || !peerGroup.exists()) {
103​ mError = VpnUiPlugin::Error;
104​ mErrorMessage = i18n("Config file needs both [Peer] and [Interface]");
105​ return result;
106​ }
107​
108​ const QString connectionName = QFileInfo(fileName).completeBaseName();
109​ NMStringMap dataMap;
110​ QVariantMap ipv4Data;
111​
112​ QString value;
113​ QStringList valueList;
114​ int intValue;
115​
116​ // Do the required fields first and fail (i.e. return an empty result) if not present
117​
118​ // Addresses
119​ valueList = interfaceGroup.readEntry(NMV_WG_TAG_ADDRESS, QStringList());
120​ if (valueList.isEmpty()) {
pinoUnsubmitted
Done

isEmpty()

pino: isEmpty()
121​ mError = VpnUiPlugin::Error;
122​ mErrorMessage = i18n("No address in config file");
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​ return result;
pinoUnsubmitted
Done

addressIn can be const

pino: addressIn can be const
124​ }
125​
126​ for (const QString &address :valueList) {
127​ const QPair<QHostAddress, int> addressIn = QHostAddress::parseSubnet(address.trimmed());
128​ if (addressIn.first.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv4Protocol) {
129​ dataMap.insert(QLatin1String(NM_WG_KEY_ADDR_IP4), address);
130​ } else if (addressIn.first.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv6Protocol) {
131​ dataMap.insert(QLatin1String(NM_WG_KEY_ADDR_IP6), address);
132​ } else { // Error condition
133​ mError = VpnUiPlugin::Error;
134​ mErrorMessage = i18n("No valid address in config file");
135​ return result;
136​ }
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​ }
138​
139​ WireGuardKeyValidator keyValidator(nullptr);
140​ int pos = 0;
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​ // Private Key
143​ value = interfaceGroup.readEntry(NMV_WG_TAG_PRIVATE_KEY);
144​ if (!value.isEmpty()) {
145​ if (keyValidator.validate(value, pos) == QValidator::State::Acceptable) {
146​ dataMap.insert(QLatin1String(NM_WG_KEY_PRIVATE_KEY), value);
147​ } else {
148​ mError = VpnUiPlugin::Error;
149​ mErrorMessage = i18n("No valid Private Key in config file");
150​ return result;
151​ }
152​ } else {
153​ mError = VpnUiPlugin::Error;
154​ mErrorMessage = i18n("No Private Key in config file");
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​ return result;
156​ }
157​
158​ // Public Key
159​ value = peerGroup.readEntry(NMV_WG_TAG_PUBLIC_KEY);
160​ if (!value.isEmpty()) {
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​ if (keyValidator.validate(value, pos) == QValidator::State::Acceptable) {
162​ dataMap.insert(QLatin1String(NM_WG_KEY_PUBLIC_KEY), value);
163​ } else {
164​ mError = VpnUiPlugin::Error;
165​ mErrorMessage = i18n("No valid Public Key in config file");
166​ return result;
167​ }
168​ } else {
169​ mError = VpnUiPlugin::Error;
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​ mErrorMessage = i18n("No Public Key in config file");
171​ return result;
172​ }
173​
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​ // Allowed IPs
175​ value = peerGroup.readEntry(NMV_WG_TAG_ALLOWED_IPS);
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 (!value.isEmpty()) {
177​ SimpleIpListValidator validator(this,
178​ SimpleIpListValidator::WithCidr,
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​ SimpleIpListValidator::Both);
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​ if (validator.validate(value, pos) != QValidator::State::Invalid) {
182​ dataMap.insert(QLatin1String(NM_WG_KEY_ALLOWED_IPS), value);
183​ } else {
184​ mError = VpnUiPlugin::Error;
185​ mErrorMessage = i18n("Invalid Allowed IP in config file");
186​ return result;
jgrulichUnsubmitted
Done

Coding style.

jgrulich: Coding style.
pinoUnsubmitted
Done

const

pino: const
187​ }
188​ } else {
189​ mError = VpnUiPlugin::Error;
190​ mErrorMessage = i18n("No Allowed IPs in config file");
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​ return result;
192​ }
193​
194​ // Now the optional ones
195​
196​ // Listen Port
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​ intValue = interfaceGroup.readEntry(NMV_WG_TAG_LISTEN_PORT, 0);
198​ if (intValue > 0) {
199​ if (intValue < 65536) {
200​ dataMap.insert(QLatin1String(NM_WG_KEY_LISTEN_PORT), QString::number(intValue));
201​ } else {
202​ mError = VpnUiPlugin::Error;
203​ mErrorMessage = i18n("Invalid Listen Port in config file");
204​ return result;
205​ }
206​ }
207​
208​ // DNS
209​ value = interfaceGroup.readEntry(NMV_WG_TAG_DNS);
210​ if (!value.isEmpty()) {
pinoUnsubmitted
Done

this validator is not needed, just use validatorRegex directly

pino: this validator is not needed, just use `validatorRegex` directly
211​ const QHostAddress testAddress(value);
212​ if (testAddress.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv4Protocol
213​ || testAddress.protocol() == QAbstractSocket::NetworkLayerProtocol::IPv6Protocol) {
214​ dataMap.insert(QLatin1String(NM_WG_KEY_DNS), value);
215​ } else {
216​ mError = VpnUiPlugin::Error;
jgrulichUnsubmitted
Done

Coding style.

jgrulich: Coding style.
217​ mErrorMessage = i18n("Invalid DNS in config file");
218​ return result;
219​ }
220​ }
221​
222​ // MTU
223​ intValue = interfaceGroup.readEntry(NMV_WG_TAG_MTU, 0);
224​ if (intValue > 0)
225​ dataMap.insert(QLatin1String(NM_WG_KEY_LISTEN_PORT), QString::number(intValue));
226​
227​ // Table
228​ value = interfaceGroup.readEntry(NMV_WG_TAG_TABLE);
229​ if (!value.isEmpty()) {
230​ dataMap.insert(QLatin1String(NM_WG_KEY_TABLE), value);
231​ }
232​
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​ // Endpoint
234​ value = peerGroup.readEntry(NMV_WG_TAG_ENDPOINT);
235​ if (!value.isEmpty())
236​ dataMap.insert(QLatin1String(NM_WG_KEY_ENDPOINT), value);
237​
238​ // Preshared Key
239​ value = peerGroup.readEntry(NMV_WG_TAG_PRESHARED_KEY);
240​ if (!value.isEmpty()) {
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​ if (keyValidator.validate(value, pos) == QValidator::State::Acceptable) {
242​ dataMap.insert(QLatin1String(NM_WG_KEY_PRESHARED_KEY), value);
243​ } else {
244​ mError = VpnUiPlugin::Error;
245​ mErrorMessage = i18n("Invalid Preshared Key in config file");
246​ return result;
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​ }
248​ }
249​
250​ NetworkManager::VpnSetting setting;
251​ setting.setServiceType(QLatin1String(NM_DBUS_SERVICE_WIREGUARD));
252​ setting.setData(dataMap);
253​
254​ QVariantMap conn;
255​ conn.insert("id", connectionName);
256​ conn.insert("type", "vpn");
257​ result.insert("connection", conn);
258​ result.insert("vpn", setting.toMap());
259​
260​ return result;
261​}
262​
263​bool WireGuardUiPlugin::exportConnectionSettings(const NetworkManager::ConnectionSettings::Ptr &connection, const QString &fileName)
264​{
pinoUnsubmitted
Done

typos for IP6?

pino: typos for IP6?
265​ NetworkManager::VpnSetting::Ptr vpnSetting = connection->setting(NetworkManager::Setting::Vpn).dynamicCast<NetworkManager::VpnSetting>();
266​ NMStringMap dataMap = vpnSetting->data();
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​
268​ // Make sure all the required fields are present
269​ if (!(dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4))
270​ || dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP6)))
271​ || !dataMap.contains(QLatin1String(NM_WG_KEY_PRIVATE_KEY))
272​ || !dataMap.contains(QLatin1String(NM_WG_KEY_PUBLIC_KEY))
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​ || !dataMap.contains(QLatin1String(NM_WG_KEY_ALLOWED_IPS)))
274​ return false;
275​
276​ // Open the output file
277​ KConfig exportFile(fileName, KConfig::NoGlobals);
278​ KConfigGroup interfaceGroup = exportFile.group(NMV_WG_TAG_INTERFACE);
279​ KConfigGroup peerGroup = exportFile.group(NMV_WG_TAG_PEER);
280​
281​ QStringList outputList;
282​
283​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP4)))
284​ outputList.append(dataMap[NM_WG_KEY_ADDR_IP4]);
285​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ADDR_IP6)))
286​ outputList.append(dataMap[NM_WG_KEY_ADDR_IP6]);
287​ interfaceGroup.writeEntry(NMV_WG_TAG_ADDRESS, outputList);
288​
289​ // Do Private Key
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​ interfaceGroup.writeEntry(NMV_WG_TAG_PRIVATE_KEY, dataMap[NM_WG_KEY_PRIVATE_KEY]);
291​
292​ // Do DNS (Not required)
293​ if (dataMap.contains(QLatin1String(NM_WG_KEY_DNS)))
294​ interfaceGroup.writeEntry(NMV_WG_TAG_DNS, dataMap[NM_WG_KEY_DNS]);
295​
296​ // Do MTU (Not required)
297​ if (dataMap.contains(QLatin1String(NM_WG_KEY_MTU)))
298​ interfaceGroup.writeEntry(NMV_WG_TAG_MTU, dataMap[NM_WG_KEY_MTU]);
299​
300​ // Do Table number (Not required)
301​ if (dataMap.contains(QLatin1String(NM_WG_KEY_TABLE)))
302​ interfaceGroup.writeEntry(NMV_WG_TAG_TABLE, dataMap[NM_WG_KEY_TABLE]);
303​
304​ // Do Listen Port (Not required)
305​ if (dataMap.contains(QLatin1String(NM_WG_KEY_LISTEN_PORT)))
306​ interfaceGroup.writeEntry(NMV_WG_TAG_LISTEN_PORT, dataMap[NM_WG_KEY_LISTEN_PORT]);
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​ // Do FwMark (Not required)
309​ if (dataMap.contains(QLatin1String(NM_WG_KEY_FWMARK)))
310​ interfaceGroup.writeEntry(NMV_WG_TAG_FWMARK, dataMap[NM_WG_KEY_FWMARK]);
311​
312​ // Do Pupblic key (required)
313​ peerGroup.writeEntry(NMV_WG_TAG_PUBLIC_KEY, dataMap[NM_WG_KEY_PUBLIC_KEY]);
314​
315​ // Do Allowed IP list (Required)
316​ peerGroup.writeEntry(NMV_WG_TAG_ALLOWED_IPS, dataMap[NM_WG_KEY_ALLOWED_IPS]);
317​
318​ // Do Endpoint (Not required)
319​ if (dataMap.contains(QLatin1String(NM_WG_KEY_ENDPOINT)))
jgrulichUnsubmitted
Done

Remove space.

jgrulich: Remove space.
320​ peerGroup.writeEntry(NMV_WG_TAG_ENDPOINT, dataMap[NM_WG_KEY_ENDPOINT]);
321​
322​ // Do Preshared Key (Not required)
323​ if (dataMap.contains(QLatin1String(NM_WG_KEY_PRESHARED_KEY)))
324​ peerGroup.writeEntry(NMV_WG_TAG_PRESHARED_KEY, dataMap[NM_WG_KEY_PRESHARED_KEY]);
325​
326​ exportFile.sync();
327​ return true;
328​}
329​
330​#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…