Skip to content

Commit

Permalink
Network config: Support OpenVPN with no user cert
Browse files Browse the repository at this point in the history 
Bug: 839195
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I20514f612345a16629e5a1370277885b6b2e3ad3
Reviewed-on: https://chromium-review.googlesource.com/1066610
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Ben Chan <benchan@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560657}
  • Loading branch information
@stevenjb
stevenjb authored and Commit Bot committed May 22, 2018
1 parent b7716f4 commit a02de1a
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 42 deletions.
6 changes: 6 additions & 0 deletions chrome/app/settings_strings.grdp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,9 @@
<message name="IDS_SETTINGS_INTERNET_NETWORK_CA_DO_NOT_CHECK" desc="Settings > Internet > Network config: Label for selecting that no Certificate Authority should be used for an EAP network.">
Do not check
</message>
<message name="IDS_SETTINGS_INTERNET_NETWORK_NO_USER_CERT" desc="Settings > Internet > Network config: Label for selecting that no User Certificate should be used when connected to an OpenVPN network.">
No user certificate
</message>
<message name="IDS_SETTINGS_INTERNET_NETWORK_CERTIFICATE_NAME" desc="Settings > Internet > Network config: Formatting string for network certificate names.">
<ph name="ISSUED_BY">$1<ex>Google Inc</ex></ph> [<ph name="ISSUED_TO">$2<ex>John Doe</ex></ph>]
</message>
Expand Down Expand Up @@ -1907,6 +1910,9 @@
<message name="IDS_NETWORK_ERROR_POLICY_CONTROLLED" desc="Network error message shown when attempting to remove or illegally modify a policy controlled network">
Network is policy controlled
</message>
<message name="IDS_NETWORK_ERROR_PASSPHRASE_REQUIRED" desc="Network error message shown when a passphrase is required to connect to a network but was not supplied.">
Passphrase required
</message>
<message name="IDS_NETWORK_ERROR_UNKNOWN" desc="Network error message shown for an unknown or unexpected network configuration error">
Error configuring network
</message>
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/ui/webui/chromeos/network_element_localized_strings_provider.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/chromeos/net/shill_error.h"
#include "chrome/grit/generated_resources.h"
#include "chromeos/login/login_state.h"
#include "chromeos/network/network_connection_handler.h"
#include "components/login/localized_values_builder.h"
#include "content/public/browser/web_ui_data_source.h"
#include "third_party/cros_system_api/dbus/service_constants.h"
Expand Down Expand Up @@ -262,6 +263,7 @@ void AddConfigLocalizedStrings(content::WebUIDataSource* html_source) {
} localized_strings[] = {
{"networkCAUseDefault", IDS_SETTINGS_INTERNET_NETWORK_CA_USE_DEFAULT},
{"networkCADoNotCheck", IDS_SETTINGS_INTERNET_NETWORK_CA_DO_NOT_CHECK},
{"networkNoUserCert", IDS_SETTINGS_INTERNET_NETWORK_NO_USER_CERT},
{"networkCertificateName",
IDS_SETTINGS_INTERNET_NETWORK_CERTIFICATE_NAME},
{"networkCertificateNameHardwareBacked",
Expand Down Expand Up @@ -297,6 +299,8 @@ void AddErrorLocalizedStrings(content::WebUIDataSource* html_source) {
IDS_NETWORK_ERROR_CANNOT_CHANGE_SHARED_CONFIG},
{"Error.PolicyControlled", IDS_NETWORK_ERROR_POLICY_CONTROLLED},
{"networkErrorNoUserCertificate", IDS_NETWORK_ERROR_NO_USER_CERT},
{NetworkConnectionHandler::kErrorPassphraseRequired,
IDS_NETWORK_ERROR_PASSPHRASE_REQUIRED},
{"networkErrorUnknown", IDS_NETWORK_ERROR_UNKNOWN},
// TODO(stevenjb): Move this id to settings_strings.grdp:
{"networkErrorNotHardwareBacked",
Expand Down
4 changes: 0 additions & 4 deletions chromeos/network/network_connection_handler_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ std::string VPNCheckCredentials(
std::string username;
provider_properties.GetStringWithoutPathExpansion(
shill::kOpenVPNUserProperty, &username);
if (username.empty()) {
NET_LOG(ERROR) << "OpenVPN: No username for: " << service_path;
return NetworkConnectionHandler::kErrorConfigurationRequired;
}
bool passphrase_required = false;
provider_properties.GetBooleanWithoutPathExpansion(
shill::kPassphraseRequiredProperty, &passphrase_required);
Expand Down
105 changes: 67 additions & 38 deletions ui/webui/resources/cr_components/chromeos/network/network_config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var VPNConfigType = {
/** @const */ var DEFAULT_HASH = 'default';
/** @const */ var DO_NOT_CHECK_HASH = 'do-not-check';
/** @const */ var NO_CERTS_HASH = 'no-certs';
/** @const */ var NO_USER_CERT_HASH = 'no-user-cert';

Polymer({
is: 'network-config',
Expand Down Expand Up @@ -445,36 +446,39 @@ Polymer({
/** @private */
onCertificateListsChanged_: function() {
this.networkingPrivate.getCertificateLists(function(certificateLists) {
var caCerts = [this.getDefaultCert_(
this.i18n('networkCAUseDefault'), DEFAULT_HASH)];
caCerts = caCerts.concat(certificateLists.serverCaCertificates);
var isOpenVpn = this.type == CrOnc.Type.VPN &&
this.get('VPN.Type', this.configProperties_) ==
CrOnc.VPNType.OPEN_VPN;

var caCerts = certificateLists.serverCaCertificates.slice();
if (!isOpenVpn) {
// 'Default' is the same as 'Do not check' except it sets
// eap.UseSystemCAs (which does not apply to OpenVPN).
caCerts.unshift(this.getDefaultCert_(
this.i18n('networkCAUseDefault'), DEFAULT_HASH));
}
caCerts.push(this.getDefaultCert_(
this.i18n('networkCADoNotCheck'), DO_NOT_CHECK_HASH));
this.set('serverCaCerts_', caCerts);
if (this.selectedServerCaHash_ && !caCerts.find((cert) => {
return cert.hash == this.selectedServerCaHash_;
})) {
this.selectedServerCaHash_ = undefined;
}

var userCerts = certificateLists.userCertificates.slice();
// Only hardware backed user certs are supported.
userCerts.forEach(function(cert) {
if (!cert.hardwareBacked)
cert.hash = ''; // Clear the hash to invalidate the certificate.
});
if (isOpenVpn) {
// OpenVPN allows but does not require a user certificate.
userCerts.unshift(this.getDefaultCert_(
this.i18n('networkNoUserCert'), NO_USER_CERT_HASH));
}
if (!userCerts.length) {
userCerts = [this.getDefaultCert_(
this.i18n('networkCertificateNoneInstalled'), NO_CERTS_HASH)];
} else {
// Only hardware backed user certs are supported.
userCerts.forEach(function(cert) {
if (!cert.hardwareBacked)
cert.hash = ''; // Clear the hash to invalidate the certificate.
});
}
this.set('userCerts_', userCerts);
if (this.selectedUserCertHash_ && !userCerts.find((cert) => {
return cert.hash == this.selectedUserCertHash_;
})) {
this.selectedUserCertHash_ = undefined;
}

this.updateSelectedCerts_();
this.updateCertError_();
}.bind(this));
},
Expand Down Expand Up @@ -900,6 +904,7 @@ Polymer({
break;
}
this.updateCertError_();
this.onCertificateListsChanged_();
},

/** @private */
Expand Down Expand Up @@ -980,8 +985,6 @@ Polymer({
if (serverCa)
this.selectedServerCaHash_ = serverCa.hash;
}
if (!this.selectedServerCaHash_ && this.serverCaCerts_[0])
this.selectedServerCaHash_ = this.serverCaCerts_[0].hash;

if (certId) {
var userCert = this.userCerts_.find(function(cert) {
Expand All @@ -990,17 +993,42 @@ Polymer({
if (userCert)
this.selectedUserCertHash_ = userCert.hash;
}
if (!this.selectedUserCertHash_) {
// Find either the first valid entry or the 'no-certs' entry.
var selectUserCert = this.userCerts_.find(function(cert) {
return !!cert.hash;
});
if (selectUserCert)
this.selectedUserCertHash_ = selectUserCert.hash;
}
this.updateSelectedCerts_();
this.updateIsConfigured_();
},

/**
* @param {!Array<!chrome.networkingPrivate.Certificate>} certs
* @param {string|undefined} hash
* @private
* @return {!chrome.networkingPrivate.Certificate|undefined}
*/
findCert_: function(certs, hash) {
if (!hash)
return undefined;
return certs.find((cert) => {
return cert.hash == hash;
});
},

/**
* Called when the certificate list or a selected certificate changes.
* Ensures that each selected certificate exists in its list, or selects the
* correct default value.
* @private
*/
updateSelectedCerts_: function() {
if (!this.findCert_(this.serverCaCerts_, this.selectedServerCaHash_))
this.selectedServerCaHash_ = undefined;
if (!this.selectedServerCaHash_ && this.serverCaCerts_[0])
this.selectedServerCaHash_ = this.serverCaCerts_[0].hash;

if (!this.findCert_(this.userCerts_, this.selectedUserCertHash_))
this.selectedUserCertHash_ = undefined;
if (!this.selectedUserCertHash_ && this.userCerts_[0])
this.selectedUserCertHash_ = this.userCerts_[0].hash;
},

/**
* @return {boolean}
* @private
Expand Down Expand Up @@ -1161,8 +1189,10 @@ Polymer({
return !!this.get('L2TP.Username', vpn) &&
this.selectedUserCertHashIsValid_();
case VPNConfigType.OPEN_VPN:
return !!this.get('OpenVPN.Username', vpn) &&
this.selectedUserCertHashIsValid_();
// OpenVPN should require username + password OR a user cert. However,
// there may be servers with different requirements so err on the side
// of permissiveness.
return true;
}
return false;
},
Expand Down Expand Up @@ -1195,9 +1225,7 @@ Polymer({
var caHash = this.selectedServerCaHash_ || '';
if (!caHash || caHash == DO_NOT_CHECK_HASH || caHash == DEFAULT_HASH)
return [];
var serverCa = this.serverCaCerts_.find(function(cert) {
return cert.hash == caHash;
});
var serverCa = this.findCert_(this.serverCaCerts_, caHash);
return serverCa && serverCa.pem ? [serverCa.pem] : [];
},

Expand All @@ -1206,11 +1234,12 @@ Polymer({
* @private
*/
getUserCertPkcs11Id_: function() {
if (!this.selectedUserCertHashIsValid_())
var userCertHash = this.selectedUserCertHash_ || '';
if (!this.selectedUserCertHashIsValid_() ||
userCertHash == NO_USER_CERT_HASH) {
return '';
var userCert = this.userCerts_.find((cert) => {
return cert.hash == this.selectedUserCertHash_;
});
}
var userCert = this.findCert_(this.userCerts_, userCertHash);
return (userCert && userCert.PKCS11Id) || '';
},

Expand Down

0 comments on commit a02de1a

Please sign in to comment.