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

(WIP) networking: Add support for OpenVPN #19363

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/networkmanager/dialogs-common.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { BondDialog, getGhostSettings as getBondGhostSettings } from './bond.jsx
import { BridgeDialog, getGhostSettings as getBridgeGhostSettings } from './bridge.jsx';
import { BridgePortDialog } from './bridgeport.jsx';
import { IpSettingsDialog } from './ip-settings.jsx';
import { OpenVPNDialog, getOpenVPNGhostSettings } from './openvpn.jsx';
import { TeamDialog, getGhostSettings as getTeamGhostSettings } from './team.jsx';
import { TeamPortDialog } from './teamport.jsx';
import { VlanDialog, getGhostSettings as getVlanGhostSettings } from './vlan.jsx';
Expand Down Expand Up @@ -203,6 +204,7 @@ export const NetworkAction = ({ buttonText, iface, connectionSettings, type }) =
if (type == 'team') settings = getTeamGhostSettings({ newIfaceName });
if (type == 'bridge') settings = getBridgeGhostSettings({ newIfaceName });
if (type == 'wg') settings = getWireGuardGhostSettings({ newIfaceName });
if (type == 'openvpn') settings = getOpenVPNGhostSettings({ newIfaceName });
}

const properties = { connection: con, dev, settings };
Expand Down Expand Up @@ -234,6 +236,8 @@ export const NetworkAction = ({ buttonText, iface, connectionSettings, type }) =
dlg = <BridgeDialog {...properties} />;
else if (type == 'wg')
dlg = <WireGuardDialog {...properties} />;
else if (type == 'openvpn')
dlg = <OpenVPNDialog {...properties} />;
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved
else if (type == 'mtu')
dlg = <MtuDialog {...properties} />;
else if (type == 'mac')
Expand Down
26 changes: 24 additions & 2 deletions pkg/networkmanager/interfaces.js
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,13 @@ export function NetworkManagerModel() {
};
}

// As NM support many vpn plugins, single out openvpn with the service name o.fd.NM.openvpn
if (settings.vpn && settings.vpn['service-type'].v === "org.freedesktop.NetworkManager.openvpn") {
result.openvpn = {
data: settings.vpn.data.v
};
}

return result;
}

Expand Down Expand Up @@ -733,8 +740,12 @@ export function NetworkManagerModel() {
}
};
}));
} else {
} else
delete result.wireguard;

if (settings.vpn) {
set("vpn", "service-type", "s", settings.vpn['service-type']);
set("vpn", "data", "a{ss}", settings.vpn.data);
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved
}

return result;
Expand Down Expand Up @@ -1005,7 +1016,8 @@ export function NetworkManagerModel() {
props: {
Connection: { conv: conv_Object(type_Connection) },
Ip4Config: { conv: conv_Object(type_Ipv4Config) },
Ip6Config: { conv: conv_Object(type_Ipv6Config) }
Ip6Config: { conv: conv_Object(type_Ipv6Config) },
Vpn: { def: false }
// See below for "Group"
},

Expand Down Expand Up @@ -1306,6 +1318,16 @@ export function NetworkManagerModel() {
type_Settings);
};

self.list_active_connections = function() {
const result = [];
for (const path in objects) {
const obj = objects[path];
if (priv(obj).type === type_ActiveConnection)
result.push(obj);
}
return result;
};

/* Initialization.
*/

Expand Down
2 changes: 1 addition & 1 deletion pkg/networkmanager/ip-settings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ export const IpSettingsDialog = ({ topic, connection, dev, settings }) => {
aria-label={_("Select method")}
onChange={(_, val) => setMethod(val)}
value={method}>
{get_ip_method_choices(topic, dev.DeviceType).map(choice => <FormSelectOption value={choice.choice} label={choice.title} key={choice.choice} />)}
{get_ip_method_choices(topic, dev?.DeviceType).map(choice => <FormSelectOption value={choice.choice} label={choice.title} key={choice.choice} />)}
</FormSelect>
<Tooltip content={_("Add address")}>
<Button variant="secondary"
Expand Down
20 changes: 19 additions & 1 deletion pkg/networkmanager/network-interface.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export const NetworkInterfacePage = ({

const dev_name = iface.Name;
const dev = iface.Device;
const active_connection = model.list_active_connections().find(ac => ac.Connection === iface.MainConnection);
const isManaged = iface && (!dev || is_managed(dev));

let ghostSettings = null;
Expand Down Expand Up @@ -146,6 +147,9 @@ export const NetworkInterfacePage = ({
}

function disconnect() {
if (active_connection.Vpn)
active_connection.deactivate();

if (!dev) {
console.log("Trying to switch off without a device?");
return;
Expand Down Expand Up @@ -563,6 +567,19 @@ export const NetworkInterfacePage = ({
return renderSettingsRow(_("WireGuard"), rows, configure);
}

function renderOpenVPNSettingsRow() {
const rows = [];
const options = settings.openvpn;

if (!options) {
return null;
}

const configure = <NetworkAction type="openvpn" iface={iface} connectionSettings={settings} />;

return renderSettingsRow(_("OpenVPN"), rows, configure);
}

return [
render_group(),
renderAutoconnectRow(),
Expand All @@ -576,6 +593,7 @@ export const NetworkInterfacePage = ({
renderTeamSettingsRow(),
renderTeamPortSettingsRow(),
renderWireGuardSettingsRow(),
renderOpenVPNSettingsRow(),
];
}

Expand Down Expand Up @@ -679,7 +697,7 @@ export const NetworkInterfacePage = ({
tooltipId="interface-switch"
excuse={ _("Not permitted to configure network devices") }>
<Switch id="interface-switch"
isChecked={!!(dev && dev.ActiveConnection)}
isChecked={!!active_connection}
isDisabled={!iface || (dev && dev.State == 20) || !privileged}
onChange={(_event, enable) => enable ? connect() : disconnect()}
aria-label={_("Enable or disable the device")} />
Expand Down
1 change: 1 addition & 0 deletions pkg/networkmanager/network-main.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ export const NetworkPage = ({ privileged, operationInProgress, usage_monitor, pl
const actions = privileged && (
<>
<NetworkAction buttonText={_("Add VPN")} type='wg' />
<NetworkAction buttonText={_("Add OpenVPN")} type='openvpn' />
Comment on lines 141 to +142
Copy link
Member

Choose a reason for hiding this comment

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

This needs design. We deliberately called the initial button "VPN", not "Wireguard", as we anticipated that we're going to have more types. This somehow needs to get folded into "Add VPN".

<NetworkAction buttonText={_("Add bond")} type='bond' />
<NetworkAction buttonText={_("Add team")} type='team' />
<NetworkAction buttonText={_("Add bridge")} type='bridge' />
Expand Down
216 changes: 216 additions & 0 deletions pkg/networkmanager/openvpn.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
import React, { useContext, useEffect, useState } from 'react';
import { Name, NetworkModal, dialogSave } from "./dialogs-common";
import { FileUpload } from '@patternfly/react-core/dist/esm/components/FileUpload/index.js';
import { FormFieldGroup, FormFieldGroupExpandable, FormFieldGroupHeader, FormGroup } from '@patternfly/react-core/dist/esm/components/Form/index.js';
import { TextInput } from '@patternfly/react-core/dist/esm/components/TextInput/index.js';
import cockpit from 'cockpit';
import { ModelContext } from './model-context';
import { useDialogs } from 'dialogs.jsx';
import * as python from "python.js";

const _ = cockpit.gettext;

// TODO: clean it
async function ovpnToJSON(ovpn) {
const configFile = "/tmp/temp.ovpn"; // TODO: use a random name for temporary files
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend to not create that file name on the JS side, it's too much churn. Let the Python script create it using NamedTemporaryFile, process it, read it, and return the JSON on its stdout (as it already does).

const pythonScript =
`import configparser
import json
import subprocess
import os

with open("${configFile}", "w") as ovpn_file:
ovpn_file.write("""${ovpn}""")

subprocess.run(["nmcli", "con", "import", "--temporary", "type", "openvpn", "file", "${configFile}"], stdout=subprocess.DEVNULL)
Copy link
Member

Choose a reason for hiding this comment

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

Please try if you can do file - or file /dev/stdin and pipe the ${ovpn} data into the process; then you don't need the configFile temp file at all. Otherwise use a NamedTemporaryFile.


config_object = configparser.ConfigParser()
file = open("/run/NetworkManager/system-connections/temp.nmconnection","r")
Copy link
Member

Choose a reason for hiding this comment

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

Is that the defined API of nmcli con import?

Copy link
Member

Choose a reason for hiding this comment

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

Please use with to make sure the file gets closed in the end.

config_object.read_file(file)
output_dict=dict()
sections=config_object.sections()
for section in sections:
items=config_object.items(section)
output_dict[section]=dict(items)

json_string=json.dumps(output_dict)
print(json_string)
file.close()

# clean up the temporary file
os.remove("/tmp/temp.ovpn")
subprocess.run(["nmcli", "con", "del", "temp"], stdout=subprocess.DEVNULL)
`;
const json = await python.spawn(pythonScript, null, { error: 'message', superuser: 'try' });
return json;
}

export function OpenVPNDialog({ settings, connection, dev }) {
const Dialogs = useDialogs();
const idPrefix = "network-openvpn-settings";
const model = useContext(ModelContext);
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved

const [iface, setIface] = useState(settings.connection.interface_name);
const [configFileName, setConfigFileName] = useState("");
const [configVal, setConfigVal] = useState("");
const [caFileName, setCaFileName] = useState("");
const [caVal, setCaVal] = useState("");
const [certFileName, setCertFileName] = useState("");
const [certVal, setCertVal] = useState("");
const [keyFileName, setKeyFileName] = useState("");
const [keyVal, setKeyVal] = useState("");
const [dialogError, setDialogError] = useState("");
const [vpnSettings, setVpnSettings] = useState(settings.openvpn.data); // TODO: eventually there should a list of proper defaults instead of an empty object

useEffect(() => {
if (!configVal) return;

async function getConfigJSON() {
try {
const json = await ovpnToJSON(configVal);
const vpnObj = JSON.parse(json).vpn;
setVpnSettings(vpnObj);
} catch (e) {
setDialogError(e.message);
}
}
getConfigJSON();
}, [configFileName, configVal]);

useEffect(() => {
async function readKeys() {
try {
const [ca, cert, key] = await Promise.all([
cockpit.file(vpnSettings.ca, { superuser: 'try' }).read(),
cockpit.file(vpnSettings.cert, { superuser: 'try' }).read(),
cockpit.file(vpnSettings.key, { superuser: 'try' }).read(),
]);
setCaVal(ca);
setCertVal(cert);
setKeyVal(key);

setCaFileName(vpnSettings.ca.split("/").at(-1));
setCertFileName(vpnSettings.cert.split("/").at(-1));
setKeyFileName(vpnSettings.key.split("/").at(-1));
} catch (e) {
setDialogError(e.message);
}
}

readKeys();
}, [vpnSettings]);

async function onSubmit() {
const user = await cockpit.user();
const caPath = `${user.home}/.cert/${caFileName}`;
const userCertPath = `${user.home}/.cert/${certFileName}`;
const userKeyPath = `${user.home}/.cert/${keyFileName}`;

try {
// check if remote or certificates are empty
if (!vpnSettings.remote.trim())
throw new Error(_("Remote cannot be empty."));
if (!caVal?.trim())
throw new Error(_("CA certificate is empty."));
if (!certVal?.trim())
throw new Error(_("User certificate is empty."));
if (!keyVal?.trim())
throw new Error(_("User private key is empty."));

await cockpit.spawn(["mkdir", "-p", `${user.home}/.cert/nm-openvpn`]);
await Promise.all([cockpit.file(caPath).replace(caVal),
cockpit.file(userCertPath).replace(certVal),
cockpit.file(userKeyPath).replace(keyVal)
]);
} catch (e) {
setDialogError(e.message);
return;
}

function createSettingsObject() {
return {
...settings,
connection: {
...settings.connection,
type: 'vpn',
Comment on lines +130 to +135
Copy link
Contributor

Choose a reason for hiding this comment

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

These 6 added lines are not executed by any test. Details

},
vpn: {
data: {
...vpnSettings,
ca: caPath,
cert: userCertPath,
key: userKeyPath,
'connection-type': 'tls', // this is not an openvpn option, rather specific to NM
},
'service-type': 'org.freedesktop.NetworkManager.openvpn'
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved
}
};
}

dialogSave({
connection,
dev,
model,
settings: createSettingsObject(),
onClose: Dialogs.close,
setDialogError,
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved
});
}

return (
<NetworkModal
title={!connection ? _("Add OpenVPN") : _("Edit OpenVPN settings")}
isCreateDialog={!connection}
onSubmit={onSubmit}
dialogError={dialogError}
idPrefix={idPrefix}
subhoghoshX marked this conversation as resolved.
Show resolved Hide resolved
>
<Name idPrefix={idPrefix} iface={iface} setIface={setIface} />
<FormFieldGroup
header={
<FormFieldGroupHeader titleText={{ text: _("Import") }}
titleDescription={_("Upload a .ovpn file to automatically fill in the details in the next (Manual) section")}
/>
}
>
<FormGroup label={_("OpenVPN config")} id={idPrefix + '-config-group'}>
<FileUpload id={idPrefix + '-config'} filename={configFileName} onFileInputChange={(_, file) => setConfigFileName(file.name)} type='text' onDataChange={(_, val) => setConfigVal(val)} hideDefaultPreview onClearClick={() => { setConfigFileName(''); setConfigVal('') }} />
</FormGroup>
</FormFieldGroup>
<FormFieldGroup header={ <FormFieldGroupHeader titleText={{ text: _("Manual") }} /> }>
<FormGroup label={_("Remote")}>
<TextInput id={idPrefix + '-remote-input'} value={vpnSettings.remote} onChange={(_, val) => setVpnSettings(settings => ({ ...settings, remote: val }))} />
</FormGroup>
<FormGroup label={_("CA certificate")} id={idPrefix + '-ca-group'}>
<FileUpload id={idPrefix + '-ca'} filename={caFileName} onFileInputChange={(_, file) => setCaFileName(file.name)} type='text' onDataChange={(_, val) => setCaVal(val)} hideDefaultPreview onClearClick={() => { setCaFileName(''); setCaVal('') }} />
</FormGroup>
<FormGroup label={_("User certificate")} id={idPrefix + '-user-cert-group'}>
<FileUpload id={idPrefix + '-user-cert'} filename={certFileName} onFileInputChange={(_, file) => setCertFileName(file.name)} type='text' onDataChange={(_, val) => setCertVal(val)} hideDefaultPreview onClearClick={() => { setCertFileName(''); setCertVal('') }} />
</FormGroup>
<FormGroup label={_("User private key")} id={idPrefix + '-private-key-group'}>
<FileUpload id={idPrefix + '-user-key'} filename={keyFileName} onFileInputChange={(_, file) => setKeyFileName(file.name)} type='text' onDataChange={(_, val) => setKeyVal(val)} hideDefaultPreview onClearClick={() => { setKeyFileName(''); setKeyVal('') }} />
</FormGroup>
</FormFieldGroup>
<FormFieldGroupExpandable header={ <FormFieldGroupHeader titleText={{ text: _("Advanced options") }} /> }>
<FormGroup />
</FormFieldGroupExpandable>
</NetworkModal>
);
}

export function getOpenVPNGhostSettings({ newIfaceName }) {
return {
connection: {
id: `con-${newIfaceName}`,
interface_name: newIfaceName,
},
openvpn: {
data: {
remote: '',
ca: '',
cert: '',
key: '',
}
}
};
}
2 changes: 1 addition & 1 deletion test/run
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ PREPARE_OPTS=""
RUN_OPTS=""
ALL_TESTS="$(test/common/run-tests --test-dir test/verify -l)"

RE_NETWORKING='Networking|Bonding|TestBridge|WireGuard|Firewall|Team|IPA|AD'
RE_NETWORKING='Networking|Bonding|TestBridge|WireGuard|OpenVPN|Firewall|Team|IPA|AD'
RE_STORAGE='Storage'
RE_EXPENSIVE='HostSwitching|MultiMachine|Updates|Superuser|Kdump|Pages'

Expand Down
Loading
Loading