Skip to content

Commit

Permalink
[CrOS] Support IPv6 overlay for WireGuard
Browse files Browse the repository at this point in the history
Currently only IPv4 overlay can be configured for a WireGuard service in
UI. This patch extends the support to IPv6.

Changes:
- Use a separate field for local addresses instead of the field in
  StaticIPConfig, so that we can support an array of IP addresses
  instead of a single one. Shill-ONC translation and mojo interface are
  updated according to it.
- Change the input validation for the "Client IP address" field and
  "Allowed IPs" field so that users can input IPv6 address.

Note that this patch does not change any user-visible element in the
UI. Only the validation rules are changed.

  out/Default/chromeos_unittests`
  xvfb-run -s "-screen 0 1024x768x24" ./out/Default/browser_tests \
  --gtest_filter=NetworkComponentsNetworkConfigTest`
  created and modified.

Bug: b:262662603
Test: unit test: `autoninja -C out/Default chromeos_unittests;
Test: broswer unit test: `autoninja -C out/Default browser_tests;
Test: tast run $DUT network.VPNUI*
Test: manually verified that dual-stack WireGuard service can be
Change-Id: I29b9568ed5ce884991f0ac18d35f26f14ad2a025
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4225076
Commit-Queue: Jie Jiang <jiejiang@chromium.org>
Reviewed-by: Gordon Seto <gordonseto@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1106073}
  • Loading branch information
Jie Jiang authored and Chromium LUCI CQ committed Feb 16, 2023
1 parent 434fa55 commit a2ff85d
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 36 deletions.
2 changes: 1 addition & 1 deletion ash/webui/common/resources/network/network_config.html
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
<network-config-input label="[[i18n('OncVPN-WireGuard-IPAddress')]]"
id="wireguard-ip-input"
value="{{ipAddressInput_}}"
property="[[managedProperties_.staticIpConfig.ipAddress]]">
property="[[managedProperties_.typeProperties.vpn.wireguard.ipAddresses]]">
</network-config-input>
<network-config-input label="[[i18n('OncVPN-WireGuard-DNS')]]"
value="{{nameServersInput_}}"
Expand Down
80 changes: 67 additions & 13 deletions ash/webui/common/resources/network/network_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import './network_config_toggle.js';
import './network_password_input.js';
import './network_shared.css.js';

import {assert, assertNotReached} from '//resources/ash/common/assert.js';
import {I18nBehavior} from '//resources/ash/common/i18n_behavior.js';
import {loadTimeData} from '//resources/ash/common/load_time_data.m.js';
import {assert, assertNotReached} from '//resources/ash/common/assert.js';
import {flush, Polymer} from '//resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {CertificateType, ConfigProperties, CrosNetworkConfigRemote, EAPConfigProperties, GlobalPolicy, HiddenSsidMode, IPSecConfigProperties, L2TPConfigProperties, ManagedBoolean, ManagedEAPProperties, ManagedInt32, ManagedIPSecProperties, ManagedL2TPProperties, ManagedOpenVPNProperties, ManagedProperties, ManagedString, ManagedStringList, ManagedWireGuardProperties, NetworkCertificate, OpenVPNConfigProperties, SecurityType, StartConnectResult, SubjectAltName, VpnType, WireGuardConfigProperties} from 'chrome://resources/mojo/chromeos/services/network_config/public/mojom/cros_network_config.mojom-webui.js';
import {ConnectionStateType, IPConfigType, NetworkType, OncSource, PolicySource} from 'chrome://resources/mojo/chromeos/services/network_config/public/mojom/network_types.mojom-webui.js';
Expand Down Expand Up @@ -72,6 +72,33 @@ const WireGuardKeyConfigType = {

/** @type {string} */ const PLACEHOLDER_CREDENTIAL = '(credential)';

/**
* A light-weight regular expression for testing an IPv4 address string. Note
* that this is not a complete check and thus some invalid input can also be
* accepted.
* @type {RegExp}
* @private
*/
const IPV4_ADDR_REGEX = /^([0-9]+\.){3}[0-9]+$/i;

/**
* A light-weight regular expression for testing an IPv6 address string. Note
* that this is not a complete check and thus some invalid input can also be
* accepted.
* @type {RegExp}
* @private
*/
const IPV6_ADDR_REGEX = /^(\:?[0-9a-f]{0,4}){2,8}$/i;

/**
* A light-weight regular expression for testing an IP CIDR string (e.g.,
* 192.168.1.0/24). Both IPv4 and IPv6 are accepted. Note that this is not a
* complete check and thus some invalid input can also be accepted.
* @type {RegExp}
* @private
*/
const IP_CIDR_REGEX = /^[0-9a-f\.\:]+\/[0-9]+?$/i;

Polymer({
_template: getTemplate(),
is: 'network-config',
Expand Down Expand Up @@ -990,14 +1017,15 @@ Polymer({
*/
getWireGuardConfigProperties_(wireguard) {
const config = {
ipAddresses: this.getActiveStringList_(wireguard.ipAddresses) ?? [],
privateKey: OncMojo.getActiveString(wireguard.privateKey),
peers: [],
};
if (wireguard.peers && wireguard.peers.activeValue) {
for (const peer of wireguard.peers.activeValue) {
const peerCopied = Object.assign({}, peer);
if (this.hasGuid_()) {
// Shill does not return exact value for crendential fields, showing
// Shill does not return exact value for credential fields, showing
// a placeholder here.
peerCopied.presharedKey = PLACEHOLDER_CREDENTIAL;
}
Expand Down Expand Up @@ -1077,12 +1105,11 @@ Polymer({
break;
}
assert(vpn.wireguard);
assert(managedProperties.staticIpConfig);
configVpn.wireguard =
this.getWireGuardConfigProperties_(vpn.wireguard);
this.ipAddressInput_ = configVpn.wireguard.ipAddresses.join(',');
const staticIpConfig = managedProperties.staticIpConfig;
this.ipAddressInput_ = staticIpConfig.ipAddress.activeValue;
if (staticIpConfig.nameServers) {
if (staticIpConfig && staticIpConfig.nameServers) {
this.nameServersInput_ =
staticIpConfig.nameServers.activeValue.join(',');
}
Expand Down Expand Up @@ -1948,18 +1975,46 @@ Polymer({
!!input.match(/^[a-z0-9+/=]+$/i);
},

/**
* Checks if the input ipAddresses is a comma-delimited string which contains
* IP addresses (v4, v6, or both).
* @param {string|undefined} ipAddresses
* @return {boolean}
* @private
*/
isValidWireGuardIpAddresses_(ipAddresses) {
if (!ipAddresses) {
return false;
}
// Currently shill only supports at most 1 IPv4 + 1 IPv6 address.
let v4Count = 0;
let v6Count = 0;
for (const ipAddress of ipAddresses.split(',')) {
if (ipAddress.match(IPV4_ADDR_REGEX)) {
v4Count++;
} else if (ipAddress.match(IPV6_ADDR_REGEX)) {
v6Count++;
} else {
return false;
}
}
if (v4Count > 1 || v6Count > 1) {
return false;
}
return v4Count + v6Count > 0;
},

/**
* @param {WireGuardConfigProperties|null|undefined} wireguard
* @param {string|undefined} ipAddress
* @param {string|undefined} ipAddresses
* @return {boolean}
* @private
*/
isWireGuardConfigurationValid_(wireguard, ipAddress) {
isWireGuardConfigurationValid_(wireguard, ipAddresses) {
if (!wireguard) {
return false;
}
// ipAddress should be a valid IPv4 address
if (!ipAddress || !ipAddress.match(/^([0-9]+\.){3}[0-9]+$/i)) {
if (!this.isValidWireGuardIpAddresses_(ipAddresses)) {
return false;
}
if (this.isWireGuardUserPrivateKeyInputActive_ &&
Expand All @@ -1979,10 +2034,9 @@ Polymer({
if (!peer.endpoint || !peer.endpoint.match(/^[a-zA-Z0-9\-\.]+:[0-9]+$/i)) {
return false;
}
// allowedIps should be comma-seperated list of IP/cidr
// allowedIps should be comma-separated list of IP/cidr.
if (!peer.allowedIps ||
!peer.allowedIps.match(
/^([0-9\.]+(\/[0-9]+)?,)*[0-9\.]+(\/[0-9]+)?$/i)) {
!peer.allowedIps.split(',').every(s => s.match(IP_CIDR_REGEX))) {
return false;
}
return true;
Expand Down Expand Up @@ -2182,9 +2236,9 @@ Polymer({
assert(!!wireguard);
propertiesToSet.typeConfig.vpn.host = 'wireguard';
propertiesToSet.ipAddressConfigType = 'Static';
wireguard.ipAddresses = this.ipAddressInput_.split(',');
propertiesToSet.staticIpConfig = {
gateway: this.ipAddressInput_,
ipAddress: this.ipAddressInput_,
routingPrefix: 32,
type: IPConfigType.kIPv4,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1001,15 +1001,44 @@ suite('network-config', function() {
await flushAsync();
assertTrue(networkConfig.enableConnect);

networkConfig.set('ipAddressInput_', '10.10.0.1/32');
networkConfig.notifyPath(`configProperties_.ipAddressInput_`);
await flushAsync();
assertFalse(networkConfig.enableConnect);
const badInputsForIp = [
'10.10.0.1/32',
'10.10.0.1,bad ip',
'10.10.10.1,10.10.10.2',
'fd00::1,fd00::2',
];
for (const input of badInputsForIp) {
networkConfig.set('ipAddressInput_', input);
networkConfig.notifyPath(`configProperties_.ipAddressInput_`);
await flushAsync();
assertFalse(networkConfig.enableConnect);
}

networkConfig.set('ipAddressInput_', '10.10.0.1');
networkConfig.notifyPath(`configProperties_.ipAddressInput_`);
await flushAsync();
assertTrue(networkConfig.enableConnect);
const goodInputsForIp = ['10.10.0.1', 'fd00::1', '10.10.10.1,fd00::1'];
for (const input of goodInputsForIp) {
networkConfig.set('ipAddressInput_', input);
networkConfig.notifyPath(`configProperties_.ipAddressInput_`);
await flushAsync();
assertTrue(networkConfig.enableConnect);
}

const badInputsForAllowedIps = ['0.0.0.0', '::', '0.0.0.0,::/0'];
for (const input of badInputsForAllowedIps) {
peer.allowedIps = input;
networkConfig.notifyPath(
`configProperties_.typeConfig.vpn.wireguard.peers.0.endpoint`);
await flushAsync();
assertFalse(networkConfig.enableConnect);
}

const goodInputsForAllowedIps = ['0.0.0.0/0', '::/0', '0.0.0.0/0,::/0'];
for (const input of goodInputsForAllowedIps) {
peer.allowedIps = input;
networkConfig.notifyPath(
`configProperties_.typeConfig.vpn.wireguard.peers.0.endpoint`);
await flushAsync();
assertTrue(networkConfig.enableConnect);
}
});
});

Expand All @@ -1020,15 +1049,16 @@ suite('network-config', function() {
OncMojo.getDefaultManagedProperties(NetworkType.kVPN, 'someguid', '');
wg1.typeProperties.vpn.type = VpnType.kWireGuard;
wg1.typeProperties.vpn.wireguard = {
ipAddresses: {activeValue: ['10.10.0.1', 'fd00::1']},
peers: {
activeValue: [{
publicKey: 'KFhwdv4+jKpSXMW6xEUVtOe4Mo8l/xOvGmshmjiHx1Y=',
endpoint: '192.168.66.66:32000',
allowedIps: '0.0.0.0/0',
allowedIps: '0.0.0.0/0,::/0',
}],
},
};
wg1.staticIpConfig = {ipAddress: {activeValue: '10.10.0.1'}};
wg1.staticIpConfig = {nameServers: {activeValue: ['8.8.8.8', '8.8.4.4']}};
setNetworkConfig(wg1);
initNetworkConfig();
});
Expand All @@ -1042,11 +1072,12 @@ suite('network-config', function() {
const configProperties = networkConfig.get('configProperties_');
const peer = configProperties.typeConfig.vpn.wireguard.peers[0];
assertEquals('UseCurrent', networkConfig.wireguardKeyType_);
assertEquals('10.10.0.1', networkConfig.get('ipAddressInput_'));
assertEquals('10.10.0.1,fd00::1', networkConfig.get('ipAddressInput_'));
assertEquals(
'KFhwdv4+jKpSXMW6xEUVtOe4Mo8l/xOvGmshmjiHx1Y=', peer.publicKey);
assertEquals('192.168.66.66:32000', peer.endpoint);
assertEquals('0.0.0.0/0', peer.allowedIps);
assertEquals('0.0.0.0/0,::/0', peer.allowedIps);
assertEquals('8.8.8.8,8.8.4.4', networkConfig.get('nameServersInput_'));
});
});

Expand Down
13 changes: 9 additions & 4 deletions chromeos/ash/components/network/onc/onc_translation_tables.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ const FieldTranslationEntry openvpn_fields[] = {
{nullptr}};

const FieldTranslationEntry wireguard_fields[] = {
{::onc::wireguard::kIPAddresses, shill::kWireGuardIPAddress},
{::onc::wireguard::kPublicKey, shill::kWireGuardPublicKey},
{::onc::wireguard::kPrivateKey, shill::kWireGuardPrivateKey},
{::onc::wireguard::kPeers, shill::kWireGuardPeers},
Expand Down Expand Up @@ -490,8 +491,9 @@ const FieldTranslationEntry* GetFieldTranslationTable(
const chromeos::onc::OncValueSignature& onc_signature) {
for (const OncValueTranslationEntry* it = onc_value_translation_table;
it->onc_signature != nullptr; ++it) {
if (it->onc_signature == &onc_signature)
if (it->onc_signature == &onc_signature) {
return it->field_translation_table;
}
}
return nullptr;
}
Expand Down Expand Up @@ -541,8 +543,9 @@ bool GetShillPropertyName(const std::string& onc_field_name,
std::string* shill_property_name) {
for (const FieldTranslationEntry* it = table; it->onc_field_name != nullptr;
++it) {
if (it->onc_field_name != onc_field_name)
if (it->onc_field_name != onc_field_name) {
continue;
}
*shill_property_name = it->shill_property_name;
return true;
}
Expand All @@ -553,8 +556,9 @@ bool TranslateStringToShill(const StringTranslationEntry table[],
const std::string& onc_value,
std::string* shill_value) {
for (int i = 0; table[i].onc_value != nullptr; ++i) {
if (onc_value != table[i].onc_value)
if (onc_value != table[i].onc_value) {
continue;
}
*shill_value = table[i].shill_value;
return true;
}
Expand All @@ -568,8 +572,9 @@ bool TranslateStringToONC(const StringTranslationEntry table[],
const std::string& shill_value,
std::string* onc_value) {
for (int i = 0; table[i].shill_value != nullptr; ++i) {
if (shill_value != table[i].shill_value)
if (shill_value != table[i].shill_value) {
continue;
}
*onc_value = table[i].onc_value;
return true;
}
Expand Down
4 changes: 4 additions & 0 deletions chromeos/ash/services/network_config/cros_network_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1619,6 +1619,8 @@ mojom::ManagedWireGuardPropertiesPtr GetManagedWireGuardProperties(
NET_LOG(ERROR) << "Expected dictionary, found: " << *wg_dict;
return wg;
}
wg->ip_addresses =
GetManagedStringList(wg_dict, ::onc::wireguard::kIPAddresses);
wg->private_key = GetManagedString(wg_dict, ::onc::wireguard::kPrivateKey);
wg->public_key = GetManagedString(wg_dict, ::onc::wireguard::kPublicKey);
wg->peers = GetManagedWireGuardPeerList(wg_dict, ::onc::wireguard::kPeers);
Expand Down Expand Up @@ -2161,6 +2163,8 @@ absl::optional<base::Value::Dict> GetOncFromConfigProperties(
base::Value::Dict wireguard_dict;
SetString(::onc::wireguard::kPrivateKey, wireguard.private_key,
&wireguard_dict);
SetStringList(::onc::wireguard::kIPAddresses, wireguard.ip_addresses,
&wireguard_dict);

base::Value::List peer_list;
if (wireguard.peers) {
Expand Down
1 change: 1 addition & 0 deletions chromeos/components/onc/onc_signature.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const OncFieldSignature openvpn_fields[] = {

const OncFieldSignature wireguard_fields[] = {
{::onc::kRecommended, &kRecommendedSignature},
{::onc::wireguard::kIPAddresses, &kStringListSignature},
{::onc::wireguard::kPrivateKey, &kStringSignature},
{::onc::wireguard::kPublicKey, &kStringSignature},
{::onc::wireguard::kPeers, &kWireGuardPeerListSignature},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"Provider": {
"Host": "wireguard",
"Type": "wireguard",
"WireGuard.IPAddress": [ "1.2.3.4", "fd00::1" ],
"WireGuard.Peers": [
{
"AllowedIPs": "0.0.0.0/0",
Expand All @@ -16,7 +17,6 @@
},
"SaveCredentials": true,
"StaticIPConfig": {
"Address": "10.20.30.101",
"NameServers": [ "8.8.8.8", "8.8.4.4", "0.0.0.0", "0.0.0.0" ]
}
}
2 changes: 1 addition & 1 deletion chromeos/components/test/data/onc/shill_wireguard.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@
"Provider.Host": "wireguard",
"Provider.Type": "wireguard",
"StaticIPConfig": {
"Address": "10.20.30.101",
"NameServers": [ "8.8.8.8", "8.8.4.4", "0.0.0.0", "0.0.0.0" ]
},
"WireGuard.IPAddress": [ "1.2.3.4", "fd00::1" ],
"WireGuard.Peers": [
{
"AllowedIPs": "0.0.0.0/0",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
{ "GUID": "guid",
"Type": "VPN",
"Name": "MyWireGuard",
"IPAddressConfigType": "Static",
"IPAddressConfigType": "DHCP",
"NameServersConfigType": "Static",
"StaticIPConfig": {
"IPAddress": "10.20.30.101",
"NameServers": [ "8.8.8.8", "8.8.4.4", "0.0.0.0", "0.0.0.0" ],
"Type": "IPv4"
},
"VPN": {
"Type": "WireGuard",
"Host": "wireguard",
"WireGuard": {
"IPAddresses" : [ "1.2.3.4", "fd00::1" ],
"Peers": [
{
"AllowedIPs": "0.0.0.0/0",
Expand Down
2 changes: 1 addition & 1 deletion chromeos/components/test/data/onc/valid_wireguard.onc
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"Type": "WireGuard",
"Host": "wireguard",
"WireGuard": {
"IPAddresses": [ "1.2.3.4", "fd00::1" ],
"Peers": [
{
"AllowedIPs": "0.0.0.0/0",
Expand All @@ -20,7 +21,6 @@
},
"StaticIPConfig": {
"Type": "IPv4",
"IPAddress": "10.20.30.101",
"NameServers": [ "8.8.8.8", "8.8.4.4" ]
}
}

0 comments on commit a2ff85d

Please sign in to comment.