diff options
author | pneubeck@chromium.org <pneubeck@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-09 15:10:56 +0000 |
---|---|---|
committer | pneubeck@chromium.org <pneubeck@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-09 15:10:56 +0000 |
commit | 19d2aafef84472a3b62886a12da38e37c12cc7f2 (patch) | |
tree | 77488e920e7fec6cf62c3bd205dcc70cbb3ba5d7 /chromeos/network | |
parent | 18a9737c61c2ab50cf3c37c557d80faaec4fa28f (diff) | |
download | chromium_src-19d2aafef84472a3b62886a12da38e37c12cc7f2.zip chromium_src-19d2aafef84472a3b62886a12da38e37c12cc7f2.tar.gz chromium_src-19d2aafef84472a3b62886a12da38e37c12cc7f2.tar.bz2 |
Remove most of NetworkUIData.
As client certificate information is now retrieved from policy, most of NetworkUIData can be removed except ONCSource to indicate whether a network was managed after the policy got removed.
BUG=391292
R=stevenjb@chromium.org
Review URL: https://codereview.chromium.org/370623002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@282038 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chromeos/network')
-rw-r--r-- | chromeos/network/certificate_pattern.cc | 49 | ||||
-rw-r--r-- | chromeos/network/certificate_pattern.h | 14 | ||||
-rw-r--r-- | chromeos/network/client_cert_resolver.cc | 84 | ||||
-rw-r--r-- | chromeos/network/client_cert_util.cc | 83 | ||||
-rw-r--r-- | chromeos/network/client_cert_util.h | 23 | ||||
-rw-r--r-- | chromeos/network/network_connection_handler.cc | 37 | ||||
-rw-r--r-- | chromeos/network/network_connection_handler.h | 8 | ||||
-rw-r--r-- | chromeos/network/network_connection_handler_unittest.cc | 121 | ||||
-rw-r--r-- | chromeos/network/network_ui_data.cc | 112 | ||||
-rw-r--r-- | chromeos/network/network_ui_data.h | 50 | ||||
-rw-r--r-- | chromeos/network/network_ui_data_unittest.cc | 84 | ||||
-rw-r--r-- | chromeos/network/policy_util.cc | 6 |
12 files changed, 233 insertions, 438 deletions
diff --git a/chromeos/network/certificate_pattern.cc b/chromeos/network/certificate_pattern.cc index 777a33d..7d22088 100644 --- a/chromeos/network/certificate_pattern.cc +++ b/chromeos/network/certificate_pattern.cc @@ -28,17 +28,6 @@ bool GetAsListOfStrings(const base::Value& value, return true; } -base::ListValue* CreateListFromStrings( - const std::vector<std::string>& strings) { - base::ListValue* new_list = new base::ListValue; - for (std::vector<std::string>::const_iterator iter = strings.begin(); - iter != strings.end(); - ++iter) { - new_list->AppendString(*iter); - } - return new_list; -} - } // namespace //////////////////////////////////////////////////////////////////////////////// @@ -72,21 +61,6 @@ void IssuerSubjectPattern::Clear() { organizational_unit_.clear(); } -scoped_ptr<base::DictionaryValue> IssuerSubjectPattern::CreateONCDictionary() - const { - scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); - if (!common_name_.empty()) - dict->SetString(onc::client_cert::kCommonName, common_name_); - if (!locality_.empty()) - dict->SetString(onc::client_cert::kLocality, locality_); - if (!organization_.empty()) - dict->SetString(onc::client_cert::kOrganization, organization_); - if (!organizational_unit_.empty()) - dict->SetString(onc::client_cert::kOrganizationalUnit, - organizational_unit_); - return dict.Pass(); -} - void IssuerSubjectPattern::ReadFromONCDictionary( const base::DictionaryValue& dict) { Clear(); @@ -120,29 +94,6 @@ void CertificatePattern::Clear() { enrollment_uri_list_.clear(); } -scoped_ptr<base::DictionaryValue> CertificatePattern::CreateONCDictionary() - const { - scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue); - - if (!issuer_ca_pems_.empty()) { - dict->SetWithoutPathExpansion(onc::client_cert::kIssuerCAPEMs, - CreateListFromStrings(issuer_ca_pems_)); - } - - if (!issuer_.Empty()) - dict->SetWithoutPathExpansion(onc::client_cert::kIssuer, - issuer_.CreateONCDictionary().release()); - - if (!subject_.Empty()) - dict->SetWithoutPathExpansion(onc::client_cert::kSubject, - subject_.CreateONCDictionary().release()); - - if (!enrollment_uri_list_.empty()) - dict->SetWithoutPathExpansion(onc::client_cert::kEnrollmentURI, - CreateListFromStrings(enrollment_uri_list_)); - return dict.Pass(); -} - bool CertificatePattern::ReadFromONCDictionary( const base::DictionaryValue& dict) { Clear(); diff --git a/chromeos/network/certificate_pattern.h b/chromeos/network/certificate_pattern.h index 5a57910..9db5446 100644 --- a/chromeos/network/certificate_pattern.h +++ b/chromeos/network/certificate_pattern.h @@ -31,7 +31,7 @@ class CHROMEOS_EXPORT IssuerSubjectPattern { // Returns true if all fields in the pattern are empty. bool Empty() const; - // Clears out all values in this pattern (so Empty returns true). + // Clears out all values in this pattern. void Clear(); void set_common_name(const std::string& name) { common_name_ = name; } @@ -56,9 +56,6 @@ class CHROMEOS_EXPORT IssuerSubjectPattern { return organizational_unit_; } - // Creates a new dictionary containing the data in this pattern. - scoped_ptr<base::DictionaryValue> CreateONCDictionary() const; - // Replaces the content of this object with the values of |dictionary|. // |dictionary| should be a valid ONC IssuerSubjectPattern dictionary. void ReadFromONCDictionary(const base::DictionaryValue& dictionary); @@ -81,9 +78,6 @@ class CHROMEOS_EXPORT CertificatePattern { // all certs). Ignores enrollment_uri_; bool Empty() const; - // Clears out all the values in this pattern (so Empty returns true). - void Clear(); - void set_issuer(const IssuerSubjectPattern& issuer) { issuer_ = issuer; } void set_subject(const IssuerSubjectPattern& subject) { subject_ = subject; } void set_enrollment_uri_list(const std::vector<std::string>& uri_list) { @@ -103,15 +97,15 @@ class CHROMEOS_EXPORT CertificatePattern { return enrollment_uri_list_; } - // Creates a new dictionary containing the data in the certificate pattern. - scoped_ptr<base::DictionaryValue> CreateONCDictionary() const; - // Replaces the content of this object with the values of |dictionary|. // |dictionary| should be a valid ONC CertificatePattern dictionary. Returns // whether all required fields were present. bool ReadFromONCDictionary(const base::DictionaryValue& dictionary); private: + // Clears out all the values in this pattern. + void Clear(); + std::vector<std::string> issuer_ca_pems_; IssuerSubjectPattern issuer_; IssuerSubjectPattern subject_; diff --git a/chromeos/network/client_cert_resolver.cc b/chromeos/network/client_cert_resolver.cc index c4a03fa9..d16f033 100644 --- a/chromeos/network/client_cert_resolver.cc +++ b/chromeos/network/client_cert_resolver.cc @@ -25,7 +25,6 @@ #include "chromeos/network/client_cert_util.h" #include "chromeos/network/managed_network_configuration_handler.h" #include "chromeos/network/network_state.h" -#include "chromeos/network/network_ui_data.h" #include "chromeos/tpm_token_loader.h" #include "components/onc/onc_constants.h" #include "dbus/object_path.h" @@ -46,6 +45,7 @@ struct ClientCertResolver::NetworkAndMatchingCert { std::string service_path; client_cert::ConfigType cert_config_type; + // The id of the matching certificate or empty if no certificate was found. std::string pkcs11_id; }; @@ -91,14 +91,12 @@ bool CompareCertExpiration(const CertAndIssuer& a, // |client_cert_pattern|. struct NetworkAndCertPattern { NetworkAndCertPattern(const std::string& network_path, - client_cert::ConfigType config_type, - const CertificatePattern& pattern) + const client_cert::ClientCertConfig& client_cert_config) : service_path(network_path), - cert_config_type(config_type), - client_cert_pattern(pattern) {} + cert_config(client_cert_config) {} + std::string service_path; - client_cert::ConfigType cert_config_type; - CertificatePattern client_cert_pattern; + client_cert::ClientCertConfig cert_config; }; // A unary predicate that returns true if the given CertAndIssuer matches the @@ -108,7 +106,7 @@ struct MatchCertWithPattern { : net_and_pattern(pattern) {} bool operator()(const CertAndIssuer& cert_and_issuer) { - const CertificatePattern& pattern = net_and_pattern.client_cert_pattern; + const CertificatePattern& pattern = net_and_pattern.cert_config.pattern; if (!pattern.issuer().Empty() && !client_cert::CertPrincipalMatches(pattern.issuer(), cert_and_issuer.cert->issuer())) { @@ -193,57 +191,10 @@ void FindCertificateMatches(const net::CertificateList& certs, } } matches->push_back(ClientCertResolver::NetworkAndMatchingCert( - it->service_path, it->cert_config_type, pkcs11_id)); + it->service_path, it->cert_config.location, pkcs11_id)); } } -// Determines the type of the CertificatePattern configuration, i.e. is it a -// pattern within an EAP, IPsec or OpenVPN configuration. -client_cert::ConfigType OncToClientCertConfigurationType( - const base::DictionaryValue& network_config) { - using namespace ::onc; - - const base::DictionaryValue* wifi = NULL; - network_config.GetDictionaryWithoutPathExpansion(network_config::kWiFi, - &wifi); - if (wifi) { - const base::DictionaryValue* eap = NULL; - wifi->GetDictionaryWithoutPathExpansion(wifi::kEAP, &eap); - if (!eap) - return client_cert::CONFIG_TYPE_NONE; - return client_cert::CONFIG_TYPE_EAP; - } - - const base::DictionaryValue* vpn = NULL; - network_config.GetDictionaryWithoutPathExpansion(network_config::kVPN, &vpn); - if (vpn) { - const base::DictionaryValue* openvpn = NULL; - vpn->GetDictionaryWithoutPathExpansion(vpn::kOpenVPN, &openvpn); - if (openvpn) - return client_cert::CONFIG_TYPE_OPENVPN; - - const base::DictionaryValue* ipsec = NULL; - vpn->GetDictionaryWithoutPathExpansion(vpn::kIPsec, &ipsec); - if (ipsec) - return client_cert::CONFIG_TYPE_IPSEC; - - return client_cert::CONFIG_TYPE_NONE; - } - - const base::DictionaryValue* ethernet = NULL; - network_config.GetDictionaryWithoutPathExpansion(network_config::kEthernet, - ðernet); - if (ethernet) { - const base::DictionaryValue* eap = NULL; - ethernet->GetDictionaryWithoutPathExpansion(wifi::kEAP, &eap); - if (eap) - return client_cert::CONFIG_TYPE_EAP; - return client_cert::CONFIG_TYPE_NONE; - } - - return client_cert::CONFIG_TYPE_NONE; -} - void LogError(const std::string& service_path, const std::string& dbus_error_name, const std::string& dbus_error_message) { @@ -405,26 +356,15 @@ void ClientCertResolver::ResolveNetworks( } VLOG(2) << "Inspecting network " << network->path(); - // TODO(pneubeck): Access the ClientCertPattern without relying on - // NetworkUIData, which also parses other things that we don't need here. - // The ONCSource is irrelevant here. - scoped_ptr<NetworkUIData> ui_data( - NetworkUIData::CreateFromONC(onc::ONC_SOURCE_NONE, *policy)); + client_cert::ClientCertConfig cert_config; + OncToClientCertConfig(*policy, &cert_config); // Skip networks that don't have a ClientCertPattern. - if (ui_data->certificate_type() != CLIENT_CERT_TYPE_PATTERN) - continue; - - client_cert::ConfigType config_type = - OncToClientCertConfigurationType(*policy); - if (config_type == client_cert::CONFIG_TYPE_NONE) { - LOG(ERROR) << "UIData contains a CertificatePattern, but the policy " - << "doesn't. Network: " << network->path(); + if (cert_config.client_cert_type != ::onc::client_cert::kPattern) continue; - } - networks_with_pattern->push_back(NetworkAndCertPattern( - network->path(), config_type, ui_data->certificate_pattern())); + networks_with_pattern->push_back( + NetworkAndCertPattern(network->path(), cert_config)); } if (networks_with_pattern->empty()) return; diff --git a/chromeos/network/client_cert_util.cc b/chromeos/network/client_cert_util.cc index 536d578..57eb4fd 100644 --- a/chromeos/network/client_cert_util.cc +++ b/chromeos/network/client_cert_util.cc @@ -14,6 +14,7 @@ #include "base/values.h" #include "chromeos/network/certificate_pattern.h" #include "chromeos/network/network_event_log.h" +#include "components/onc/onc_constants.h" #include "net/base/net_errors.h" #include "net/cert/cert_database.h" #include "net/cert/nss_cert_database.h" @@ -101,6 +102,24 @@ std::string GetStringFromDictionary(const base::DictionaryValue& dict, return s; } +void GetClientCertTypeAndPattern( + const base::DictionaryValue& dict_with_client_cert, + ClientCertConfig* cert_config) { + using namespace ::onc::client_cert; + dict_with_client_cert.GetStringWithoutPathExpansion( + kClientCertType, &cert_config->client_cert_type); + + if (cert_config->client_cert_type == kPattern) { + const base::DictionaryValue* pattern = NULL; + dict_with_client_cert.GetDictionaryWithoutPathExpansion(kClientCertPattern, + &pattern); + if (pattern) { + bool success = cert_config->pattern.ReadFromONCDictionary(*pattern); + DCHECK(success); + } + } +} + } // namespace // Returns true only if any fields set in this pattern match exactly with @@ -199,7 +218,7 @@ scoped_refptr<net::X509Certificate> GetCertificateMatch( return latest; } -void SetShillProperties(const client_cert::ConfigType cert_config_type, +void SetShillProperties(const ConfigType cert_config_type, const std::string& tpm_slot, const std::string& tpm_pin, const std::string* pkcs11_id, @@ -258,7 +277,67 @@ void SetShillProperties(const client_cert::ConfigType cert_config_type, properties->SetStringWithoutPathExpansion(tpm_pin_property, tpm_pin); } -bool IsCertificateConfigured(const client_cert::ConfigType cert_config_type, +ClientCertConfig::ClientCertConfig() + : location(CONFIG_TYPE_NONE), + client_cert_type(onc::client_cert::kClientCertTypeNone) { +} + +void OncToClientCertConfig(const base::DictionaryValue& network_config, + ClientCertConfig* cert_config) { + using namespace ::onc; + + *cert_config = ClientCertConfig(); + + const base::DictionaryValue* dict_with_client_cert = NULL; + + const base::DictionaryValue* wifi = NULL; + network_config.GetDictionaryWithoutPathExpansion(network_config::kWiFi, + &wifi); + if (wifi) { + const base::DictionaryValue* eap = NULL; + wifi->GetDictionaryWithoutPathExpansion(wifi::kEAP, &eap); + if (!eap) + return; + + dict_with_client_cert = eap; + cert_config->location = CONFIG_TYPE_EAP; + } + + const base::DictionaryValue* vpn = NULL; + network_config.GetDictionaryWithoutPathExpansion(network_config::kVPN, &vpn); + if (vpn) { + const base::DictionaryValue* openvpn = NULL; + vpn->GetDictionaryWithoutPathExpansion(vpn::kOpenVPN, &openvpn); + const base::DictionaryValue* ipsec = NULL; + vpn->GetDictionaryWithoutPathExpansion(vpn::kIPsec, &ipsec); + if (openvpn) { + dict_with_client_cert = openvpn; + cert_config->location = CONFIG_TYPE_OPENVPN; + } else if (ipsec) { + dict_with_client_cert = ipsec; + cert_config->location = CONFIG_TYPE_IPSEC; + } else { + return; + } + } + + const base::DictionaryValue* ethernet = NULL; + network_config.GetDictionaryWithoutPathExpansion(network_config::kEthernet, + ðernet); + if (ethernet) { + const base::DictionaryValue* eap = NULL; + ethernet->GetDictionaryWithoutPathExpansion(wifi::kEAP, &eap); + if (!eap) + return; + dict_with_client_cert = eap; + cert_config->location = CONFIG_TYPE_EAP; + } + + if (dict_with_client_cert) + GetClientCertTypeAndPattern(*dict_with_client_cert, cert_config); +} + +bool IsCertificateConfigured(const ConfigType cert_config_type, const base::DictionaryValue& service_properties) { // VPN certificate properties are read from the Provider dictionary. const base::DictionaryValue* provider_properties = NULL; diff --git a/chromeos/network/client_cert_util.h b/chromeos/network/client_cert_util.h index 5b6839d..3a1a392 100644 --- a/chromeos/network/client_cert_util.h +++ b/chromeos/network/client_cert_util.h @@ -10,6 +10,7 @@ #include "base/memory/ref_counted.h" #include "chromeos/chromeos_export.h" +#include "chromeos/network/certificate_pattern.h" namespace base { class DictionaryValue; @@ -23,7 +24,6 @@ typedef std::vector<scoped_refptr<X509Certificate> > CertificateList; namespace chromeos { -class CertificatePattern; class IssuerSubjectPattern; namespace client_cert { @@ -35,6 +35,21 @@ enum ConfigType { CONFIG_TYPE_EAP }; +struct CHROMEOS_EXPORT ClientCertConfig { + ClientCertConfig(); + + // Independent of whether the client cert (pattern or reference) is + // configured, the location determines whether this network configuration + // supports client certs and what kind of configuration it requires. + ConfigType location; + + // One of the ClientCertTypes defined in ONC: kNone, kRef, or kPattern. + std::string client_cert_type; + + // If |client_cert_type| equals kPattern, this contains the pattern. + CertificatePattern pattern; +}; + // Returns true only if any fields set in this pattern match exactly with // similar fields in the principal. If organization_ or organizational_unit_ // are set, then at least one of the organizations or units in the principal @@ -61,6 +76,12 @@ void SetShillProperties(const ConfigType cert_config_type, bool IsCertificateConfigured(const client_cert::ConfigType cert_config_type, const base::DictionaryValue& service_properties); +// Determines the type of the CertificatePattern configuration, i.e. is it a +// pattern within an EAP, IPsec or OpenVPN configuration. +CHROMEOS_EXPORT void OncToClientCertConfig( + const base::DictionaryValue& network_config, + ClientCertConfig* cert_config); + } // namespace client_cert } // namespace chromeos diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc index ea61851..33473e8 100644 --- a/chromeos/network/network_connection_handler.cc +++ b/chromeos/network/network_connection_handler.cc @@ -13,6 +13,7 @@ #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/shill_manager_client.h" #include "chromeos/dbus/shill_service_client.h" +#include "chromeos/network/certificate_pattern.h" #include "chromeos/network/client_cert_util.h" #include "chromeos/network/managed_network_configuration_handler.h" #include "chromeos/network/network_configuration_handler.h" @@ -21,7 +22,6 @@ #include "chromeos/network/network_profile_handler.h" #include "chromeos/network/network_state.h" #include "chromeos/network/network_state_handler.h" -#include "chromeos/network/network_ui_data.h" #include "chromeos/network/shill_property_util.h" #include "chromeos/tpm_token_loader.h" #include "dbus/object_path.h" @@ -419,8 +419,17 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect( } } - scoped_ptr<NetworkUIData> ui_data = - shill_property_util::GetUIDataFromProperties(service_properties); + std::string guid; + service_properties.GetStringWithoutPathExpansion(shill::kGuidProperty, &guid); + std::string profile; + service_properties.GetStringWithoutPathExpansion(shill::kProfileProperty, + &profile); + const base::DictionaryValue* user_policy = + managed_configuration_handler_->FindPolicyByGuidAndProfile(guid, profile); + + client_cert::ClientCertConfig cert_config_from_policy; + if (user_policy) + client_cert::OncToClientCertConfig(*user_policy, &cert_config_from_policy); client_cert::ConfigType client_cert_type = client_cert::CONFIG_TYPE_NONE; if (type == shill::kTypeVPN) { @@ -436,8 +445,10 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect( // to deduce the authentication type based on the // kL2tpIpsecClientCertIdProperty here (and also in VPNConfigView). if (!vpn_client_cert_id.empty() || - (ui_data && ui_data->certificate_type() != CLIENT_CERT_TYPE_NONE)) + cert_config_from_policy.client_cert_type != + onc::client_cert::kClientCertTypeNone) { client_cert_type = client_cert::CONFIG_TYPE_IPSEC; + } } } else if (type == shill::kTypeWifi && security == shill::kSecurity8021x) { client_cert_type = client_cert::CONFIG_TYPE_EAP; @@ -466,11 +477,12 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect( // non-empty string. std::string pkcs11_id; - // Check certificate properties in kUIDataProperty if configured. - // Note: Wifi/VPNConfigView set these properties explicitly, in which case - // only the TPM must be configured. - if (ui_data && ui_data->certificate_type() == CLIENT_CERT_TYPE_PATTERN) { - pkcs11_id = CertificateIsConfigured(ui_data.get()); + // Check certificate properties from policy. + // Note: Wifi/VPNConfigView set the KeyID and CertID properties directly, + // in which case only the TPM must be configured. + if (cert_config_from_policy.client_cert_type == + onc::client_cert::kPattern) { + pkcs11_id = CertificateIsConfigured(cert_config_from_policy.pattern); // Ensure the certificate is available and configured. if (!cert_loader_->IsHardwareBacked() || pkcs11_id.empty()) { ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired); @@ -734,13 +746,12 @@ void NetworkConnectionHandler::CheckAllPendingRequests() { } std::string NetworkConnectionHandler::CertificateIsConfigured( - NetworkUIData* ui_data) { - if (ui_data->certificate_pattern().Empty()) + const CertificatePattern& pattern) { + if (pattern.Empty()) return std::string(); // Find the matching certificate. scoped_refptr<net::X509Certificate> matching_cert = - client_cert::GetCertificateMatch(ui_data->certificate_pattern(), - cert_loader_->cert_list()); + client_cert::GetCertificateMatch(pattern, cert_loader_->cert_list()); if (!matching_cert.get()) return std::string(); return CertLoader::GetPkcs11IdForCert(*matching_cert.get()); diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h index 77db56f..cc38dc7b 100644 --- a/chromeos/network/network_connection_handler.h +++ b/chromeos/network/network_connection_handler.h @@ -24,8 +24,8 @@ namespace chromeos { +class CertificatePattern; class NetworkState; -class NetworkUIData; // The NetworkConnectionHandler class is used to manage network connection // requests. This is the only class that should make Shill Connect calls. @@ -194,9 +194,9 @@ class CHROMEOS_EXPORT NetworkConnectionHandler void CheckPendingRequest(const std::string service_path); void CheckAllPendingRequests(); - // Returns the PKCS#11 ID of a cert matching the certificate pattern in - // |ui_data|. Returns empty string otherwise. - std::string CertificateIsConfigured(NetworkUIData* ui_data); + // Returns the PKCS#11 ID of a cert matching the certificate pattern. Returns + // empty string otherwise. + std::string CertificateIsConfigured(const CertificatePattern& pattern); void ErrorCallbackForPendingRequest(const std::string& service_path, const std::string& error_name); diff --git a/chromeos/network/network_connection_handler_unittest.cc b/chromeos/network/network_connection_handler_unittest.cc index 257a0f4..a615694 100644 --- a/chromeos/network/network_connection_handler_unittest.cc +++ b/chromeos/network/network_connection_handler_unittest.cc @@ -91,7 +91,9 @@ class NetworkConnectionHandlerTest : public testing::Test { test_manager_client_->AddTechnology(shill::kTypeCellular, true /* enabled */); dbus_manager->GetShillProfileClient()->GetTestInterface()->AddProfile( - "profile_path", std::string() /* shared profile */); + "shared_profile_path", std::string() /* shared profile */); + dbus_manager->GetShillProfileClient()->GetTestInterface()->AddProfile( + "user_profile_path", user_.username_hash()); base::RunLoop().RunUntilIdle(); LoginState::Initialize(); @@ -220,36 +222,32 @@ class NetworkConnectionHandlerTest : public testing::Test { ASSERT_EQ(1U, loaded_certs->size()); } - void SetupPolicy() { - const char* kNetworkConfigs = - "[ { \"GUID\": \"wifi1\"," - " \"Name\": \"wifi1\"," - " \"Type\": \"WiFi\"," - " \"WiFi\": {" - " \"Security\": \"WPA-PSK\"," - " \"SSID\": \"wifi1\"," - " \"Passphrase\": \"passphrase\"" - " }" - "} ]"; - + void SetupPolicy(const std::string& network_configs_json, + const base::DictionaryValue& global_config, + bool user_policy) { std::string error; scoped_ptr<base::Value> network_configs_value( - base::JSONReader::ReadAndReturnError( - kNetworkConfigs, base::JSON_ALLOW_TRAILING_COMMAS, NULL, &error)); + base::JSONReader::ReadAndReturnError(network_configs_json, + base::JSON_ALLOW_TRAILING_COMMAS, + NULL, + &error)); ASSERT_TRUE(network_configs_value) << error; base::ListValue* network_configs = NULL; ASSERT_TRUE(network_configs_value->GetAsList(&network_configs)); - base::DictionaryValue global_config; - global_config.SetBooleanWithoutPathExpansion( - ::onc::global_network_config::kAllowOnlyPolicyNetworksToAutoconnect, - true); - - managed_config_handler_->SetPolicy(::onc::ONC_SOURCE_USER_POLICY, - "", // userhash - *network_configs, - global_config); + if (user_policy) { + managed_config_handler_->SetPolicy( + ::onc::ONC_SOURCE_USER_POLICY, + user_.username_hash(), + *network_configs, + global_config); + } else { + managed_config_handler_->SetPolicy(::onc::ONC_SOURCE_DEVICE_POLICY, + std::string(), // no username hash + *network_configs, + global_config); + } base::RunLoop().RunUntilIdle(); } @@ -322,23 +320,34 @@ TEST_F(NetworkConnectionHandlerTest, NetworkConnectionHandlerConnectFailure) { namespace { -const char* kConfigRequiresCertificateTemplate = - "{ \"GUID\": \"wifi4\", \"Type\": \"wifi\", \"Connectable\": false," - " \"Security\": \"802_1x\"," - " \"UIData\": \"{" - " \\\"certificate_type\\\": \\\"pattern\\\"," - " \\\"certificate_pattern\\\": {" - " \\\"Subject\\\": {\\\"CommonName\\\": \\\"%s\\\" }" - " } }\" }"; +const char* kPolicyWithCertPatternTemplate = + "[ { \"GUID\": \"wifi4\"," + " \"Name\": \"wifi4\"," + " \"Type\": \"WiFi\"," + " \"WiFi\": {" + " \"Security\": \"WPA-EAP\"," + " \"SSID\": \"wifi_ssid\"," + " \"EAP\": {" + " \"Outer\": \"EAP-TLS\"," + " \"ClientCertType\": \"Pattern\"," + " \"ClientCertPattern\": {" + " \"Subject\": {" + " \"CommonName\" : \"%s\"" + " }" + " }" + " }" + " }" + "} ]"; } // namespace // Handle certificates. TEST_F(NetworkConnectionHandlerTest, ConnectCertificateMissing) { StartCertLoader(); + SetupPolicy(base::StringPrintf(kPolicyWithCertPatternTemplate, "unknown"), + base::DictionaryValue(), // no global config + true); // load as user policy - EXPECT_TRUE(Configure( - base::StringPrintf(kConfigRequiresCertificateTemplate, "unknown"))); Connect("wifi4"); EXPECT_EQ(NetworkConnectionHandler::kErrorCertificateRequired, GetResultAndReset()); @@ -352,9 +361,10 @@ TEST_F(NetworkConnectionHandlerTest, ConnectWithCertificateSuccess) { test_nssdb_.get(), &certs); - EXPECT_TRUE(Configure( - base::StringPrintf(kConfigRequiresCertificateTemplate, - certs[0]->subject().common_name.c_str()))); + SetupPolicy(base::StringPrintf(kPolicyWithCertPatternTemplate, + certs[0]->subject().common_name.c_str()), + base::DictionaryValue(), // no global config + true); // load as user policy Connect("wifi4"); EXPECT_EQ(kSuccessResult, GetResultAndReset()); @@ -367,9 +377,10 @@ TEST_F(NetworkConnectionHandlerTest, test_nssdb_.get(), &certs); - EXPECT_TRUE(Configure( - base::StringPrintf(kConfigRequiresCertificateTemplate, - certs[0]->subject().common_name.c_str()))); + SetupPolicy(base::StringPrintf(kPolicyWithCertPatternTemplate, + certs[0]->subject().common_name.c_str()), + base::DictionaryValue(), // no global config + true); // load as user policy Connect("wifi4"); @@ -412,6 +423,17 @@ const char* kConfigManagedSharedConnectable = "{ \"GUID\": \"wifi1\", \"Type\": \"wifi\", \"State\": \"idle\", " " \"Connectable\": true }"; +const char* kPolicy = + "[ { \"GUID\": \"wifi1\"," + " \"Name\": \"wifi1\"," + " \"Type\": \"WiFi\"," + " \"WiFi\": {" + " \"Security\": \"WPA-PSK\"," + " \"SSID\": \"wifi1\"," + " \"Passphrase\": \"passphrase\"" + " }" + "} ]"; + } // namespace TEST_F(NetworkConnectionHandlerTest, ReconnectOnLoginEarlyPolicyLoading) { @@ -426,8 +448,14 @@ TEST_F(NetworkConnectionHandlerTest, ReconnectOnLoginEarlyPolicyLoading) { EXPECT_EQ(shill::kStateIdle, GetServiceStringProperty("wifi1", shill::kStateProperty)); - // Policy application should disconnect from the shared and unmanaged network. - SetupPolicy(); + // Applying the policy which restricts autoconnect should disconnect from the + // shared, unmanaged network. + base::DictionaryValue global_config; + global_config.SetBooleanWithoutPathExpansion( + ::onc::global_network_config::kAllowOnlyPolicyNetworksToAutoconnect, + true); + + SetupPolicy(kPolicy, global_config, false /* load as device policy */); EXPECT_EQ(shill::kStateIdle, GetServiceStringProperty("wifi0", shill::kStateProperty)); EXPECT_EQ(shill::kStateIdle, @@ -455,7 +483,14 @@ TEST_F(NetworkConnectionHandlerTest, ReconnectOnLoginLatePolicyLoading) { EXPECT_EQ(shill::kStateIdle, GetServiceStringProperty("wifi1", shill::kStateProperty)); - SetupPolicy(); + // Applying the policy which restricts autoconnect should disconnect from the + // shared, unmanaged network. + base::DictionaryValue global_config; + global_config.SetBooleanWithoutPathExpansion( + ::onc::global_network_config::kAllowOnlyPolicyNetworksToAutoconnect, + true); + + SetupPolicy(kPolicy, global_config, false /* load as device policy */); EXPECT_EQ(shill::kStateIdle, GetServiceStringProperty("wifi0", shill::kStateProperty)); EXPECT_EQ(shill::kStateOnline, diff --git a/chromeos/network/network_ui_data.cc b/chromeos/network/network_ui_data.cc index 27b4beb..2a59271 100644 --- a/chromeos/network/network_ui_data.cc +++ b/chromeos/network/network_ui_data.cc @@ -6,14 +6,12 @@ #include "base/logging.h" #include "base/values.h" -#include "chromeos/network/onc/onc_signature.h" +#include "components/onc/onc_constants.h" namespace chromeos { // Top-level UI data dictionary keys. const char NetworkUIData::kKeyONCSource[] = "onc_source"; -const char NetworkUIData::kKeyCertificatePattern[] = "certificate_pattern"; -const char NetworkUIData::kKeyCertificateType[] = "certificate_type"; const char NetworkUIData::kKeyUserSettings[] = "user_settings"; const char NetworkUIData::kONCSourceUserImport[] = "user_import"; const char NetworkUIData::kONCSourceDevicePolicy[] = "device_policy"; @@ -33,12 +31,6 @@ const StringEnumEntry< ::onc::ONCSource> kONCSourceTable[] = { { NetworkUIData::kONCSourceUserPolicy, ::onc::ONC_SOURCE_USER_POLICY } }; -const StringEnumEntry<ClientCertType> kClientCertTable[] = { - { "none", CLIENT_CERT_TYPE_NONE }, - { "pattern", CLIENT_CERT_TYPE_PATTERN }, - { "ref", CLIENT_CERT_TYPE_REF } -}; - // Converts |enum_value| to the corresponding string according to |table|. If no // enum value of the table matches (which can only occur if incorrect casting // was used to obtain |enum_value|), returns an empty string instead. @@ -67,9 +59,7 @@ Enum StringToEnum(const StringEnumEntry<Enum>(& table)[N], } // namespace -NetworkUIData::NetworkUIData() - : onc_source_(::onc::ONC_SOURCE_NONE), - certificate_type_(CLIENT_CERT_TYPE_NONE) { +NetworkUIData::NetworkUIData() : onc_source_(::onc::ONC_SOURCE_NONE) { } NetworkUIData::NetworkUIData(const NetworkUIData& other) { @@ -77,12 +67,9 @@ NetworkUIData::NetworkUIData(const NetworkUIData& other) { } NetworkUIData& NetworkUIData::operator=(const NetworkUIData& other) { - certificate_pattern_ = other.certificate_pattern_; onc_source_ = other.onc_source_; - certificate_type_ = other.certificate_type_; if (other.user_settings_) user_settings_.reset(other.user_settings_->DeepCopy()); - policy_guid_ = other.policy_guid_; return *this; } @@ -92,21 +79,6 @@ NetworkUIData::NetworkUIData(const base::DictionaryValue& dict) { onc_source_ = StringToEnum(kONCSourceTable, source, ::onc::ONC_SOURCE_NONE); std::string type_string; - dict.GetString(kKeyCertificateType, &type_string); - certificate_type_ = - StringToEnum(kClientCertTable, type_string, CLIENT_CERT_TYPE_NONE); - - if (certificate_type_ == CLIENT_CERT_TYPE_PATTERN) { - const base::DictionaryValue* cert_dict = NULL; - dict.GetDictionary(kKeyCertificatePattern, &cert_dict); - if (cert_dict) - certificate_pattern_.ReadFromONCDictionary(*cert_dict); - if (certificate_pattern_.Empty()) { - // This case may occur if UIData from an older CrOS version is read. - LOG(WARNING) << "Couldn't parse a valid certificate pattern."; - certificate_type_ = CLIENT_CERT_TYPE_NONE; - } - } const base::DictionaryValue* user_settings = NULL; if (dict.GetDictionary(kKeyUserSettings, &user_settings)) @@ -131,93 +103,17 @@ void NetworkUIData::FillDictionary(base::DictionaryValue* dict) const { if (!source_string.empty()) dict->SetString(kKeyONCSource, source_string); - if (certificate_type_ != CLIENT_CERT_TYPE_NONE) { - std::string type_string = EnumToString(kClientCertTable, certificate_type_); - dict->SetString(kKeyCertificateType, type_string); - - if (certificate_type_ == CLIENT_CERT_TYPE_PATTERN && - !certificate_pattern_.Empty()) { - dict->Set(kKeyCertificatePattern, - certificate_pattern_.CreateONCDictionary().release()); - } - } if (user_settings_) dict->SetWithoutPathExpansion(kKeyUserSettings, user_settings_->DeepCopy()); } -namespace { - -void GetAndTranslateClientCertType(const base::DictionaryValue& onc_object, - NetworkUIData* ui_data) { - using namespace ::onc::client_cert; - - std::string client_cert_type; - if (!onc_object.GetStringWithoutPathExpansion(kClientCertType, - &client_cert_type)) { - return; - } - - ClientCertType type; - if (client_cert_type == kClientCertTypeNone) { - type = CLIENT_CERT_TYPE_NONE; - } else if (client_cert_type == kRef) { - type = CLIENT_CERT_TYPE_REF; - } else if (client_cert_type == kPattern) { - type = CLIENT_CERT_TYPE_PATTERN; - } else { - type = CLIENT_CERT_TYPE_NONE; - NOTREACHED(); - } - - ui_data->set_certificate_type(type); -} - -void TranslateCertificatePattern(const base::DictionaryValue& onc_object, - NetworkUIData* ui_data) { - CertificatePattern pattern; - bool success = pattern.ReadFromONCDictionary(onc_object); - DCHECK(success); - ui_data->set_certificate_pattern(pattern); -} - -void TranslateONCHierarchy(const onc::OncValueSignature& signature, - const base::DictionaryValue& onc_object, - NetworkUIData* ui_data) { - if (&signature == &onc::kCertificatePatternSignature) { - TranslateCertificatePattern(onc_object, ui_data); - } else if (&signature == &onc::kEAPSignature || - &signature == &onc::kIPsecSignature || - &signature == &onc::kOpenVPNSignature) { - GetAndTranslateClientCertType(onc_object, ui_data); - } - - // Recurse into nested objects. - for (base::DictionaryValue::Iterator it(onc_object); !it.IsAtEnd(); - it.Advance()) { - const base::DictionaryValue* inner_object; - if (!it.value().GetAsDictionary(&inner_object)) - continue; - - const onc::OncFieldSignature* field_signature = - GetFieldSignature(signature, it.key()); - - TranslateONCHierarchy(*field_signature->value_signature, *inner_object, - ui_data); - } -} - -} // namespace - // static scoped_ptr<NetworkUIData> NetworkUIData::CreateFromONC( - ::onc::ONCSource onc_source, - const base::DictionaryValue& onc_network) { + ::onc::ONCSource onc_source) { scoped_ptr<NetworkUIData> ui_data(new NetworkUIData()); - TranslateONCHierarchy(onc::kNetworkConfigurationSignature, onc_network, - ui_data.get()); - ui_data->set_onc_source(onc_source); + ui_data->onc_source_ = onc_source; return ui_data.Pass(); } diff --git a/chromeos/network/network_ui_data.h b/chromeos/network/network_ui_data.h index 42658c4..66cb236 100644 --- a/chromeos/network/network_ui_data.h +++ b/chromeos/network/network_ui_data.h @@ -10,7 +10,6 @@ #include "base/basictypes.h" #include "base/memory/scoped_ptr.h" #include "chromeos/chromeos_export.h" -#include "chromeos/network/certificate_pattern.h" #include "components/onc/onc_constants.h" namespace base { @@ -19,12 +18,6 @@ class DictionaryValue; namespace chromeos { -enum ClientCertType { - CLIENT_CERT_TYPE_NONE = 0, - CLIENT_CERT_TYPE_REF = 1, - CLIENT_CERT_TYPE_PATTERN = 2 -}; - // Helper for accessing and setting values in the network's UI data dictionary. // Accessing values is done via static members that take the network as an // argument. In order to fill a UI data dictionary, construct an instance, set @@ -32,7 +25,6 @@ enum ClientCertType { // |network|: // // NetworkUIData ui_data; -// ui_data.set_onc_source(onc::ONC_SOURCE_USER_IMPORT); // ui_data.FillDictionary(network->ui_data()); class CHROMEOS_EXPORT NetworkUIData { public: @@ -42,35 +34,12 @@ class CHROMEOS_EXPORT NetworkUIData { explicit NetworkUIData(const base::DictionaryValue& dict); ~NetworkUIData(); - void set_onc_source(::onc::ONCSource onc_source) { onc_source_ = onc_source; } ::onc::ONCSource onc_source() const { return onc_source_; } - void set_certificate_pattern(const CertificatePattern& pattern) { - certificate_pattern_ = pattern; - } - const CertificatePattern& certificate_pattern() const { - return certificate_pattern_; - } - void set_certificate_type(ClientCertType type) { - certificate_type_ = type; - } - ClientCertType certificate_type() const { - return certificate_type_; - } - bool is_managed() const { - return onc_source_ == ::onc::ONC_SOURCE_DEVICE_POLICY || - onc_source_ == ::onc::ONC_SOURCE_USER_POLICY; - } const base::DictionaryValue* user_settings() const { return user_settings_.get(); } void set_user_settings(scoped_ptr<base::DictionaryValue> dict); - const std::string& policy_guid() const { - return policy_guid_; - } - void set_policy_guid(const std::string& guid) { - policy_guid_ = guid; - } // Returns |onc_source_| as a string, one of kONCSource*. std::string GetONCSourceAsString() const; @@ -79,23 +48,13 @@ class CHROMEOS_EXPORT NetworkUIData { // keys appropriate for Network::ui_data() as defined below (kKeyXXX). void FillDictionary(base::DictionaryValue* dict) const; - // Creates a NetworkUIData object from |onc_network|, which has to be a valid - // ONC NetworkConfiguration dictionary. - // This function is used to create the "UIData" field of the Shill - // configuration. - static scoped_ptr<NetworkUIData> CreateFromONC( - ::onc::ONCSource onc_source, - const base::DictionaryValue& onc_network); + // Creates a NetworkUIData object from |onc_source|. This function is used to + // create the "UIData" property of the Shill configuration. + static scoped_ptr<NetworkUIData> CreateFromONC(::onc::ONCSource onc_source); // Key for storing source of the ONC network. static const char kKeyONCSource[]; - // Key for storing the certificate pattern. - static const char kKeyCertificatePattern[]; - - // Key for storing the certificate type. - static const char kKeyCertificateType[]; - // Key for storing the user settings. static const char kKeyUserSettings[]; @@ -105,11 +64,8 @@ class CHROMEOS_EXPORT NetworkUIData { static const char kONCSourceUserPolicy[]; private: - CertificatePattern certificate_pattern_; ::onc::ONCSource onc_source_; - ClientCertType certificate_type_; scoped_ptr<base::DictionaryValue> user_settings_; - std::string policy_guid_; }; } // namespace chromeos diff --git a/chromeos/network/network_ui_data_unittest.cc b/chromeos/network/network_ui_data_unittest.cc index ff07b0c..0617643 100644 --- a/chromeos/network/network_ui_data_unittest.cc +++ b/chromeos/network/network_ui_data_unittest.cc @@ -5,7 +5,6 @@ #include "chromeos/network/network_ui_data.h" #include "base/values.h" -#include "chromeos/network/onc/onc_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace chromeos { @@ -17,101 +16,18 @@ TEST(NetworkUIDataTest, ONCSource) { { NetworkUIData ui_data(ui_data_dict); EXPECT_EQ(::onc::ONC_SOURCE_USER_IMPORT, ui_data.onc_source()); - EXPECT_FALSE(ui_data.is_managed()); } ui_data_dict.SetString(NetworkUIData::kKeyONCSource, "device_policy"); { NetworkUIData ui_data(ui_data_dict); EXPECT_EQ(::onc::ONC_SOURCE_DEVICE_POLICY, ui_data.onc_source()); - EXPECT_TRUE(ui_data.is_managed()); } ui_data_dict.SetString(NetworkUIData::kKeyONCSource, "user_policy"); { NetworkUIData ui_data(ui_data_dict); EXPECT_EQ(::onc::ONC_SOURCE_USER_POLICY, ui_data.onc_source()); - EXPECT_TRUE(ui_data.is_managed()); } } -TEST(NetworkUIDataTest, CertificateType) { - { - base::DictionaryValue ui_data_dict; - ui_data_dict.SetString(NetworkUIData::kKeyCertificateType, "none"); - NetworkUIData ui_data(ui_data_dict); - EXPECT_EQ(CLIENT_CERT_TYPE_NONE, ui_data.certificate_type()); - } - { - base::DictionaryValue ui_data_dict; - ui_data_dict.SetString(NetworkUIData::kKeyCertificateType, "ref"); - NetworkUIData ui_data(ui_data_dict); - EXPECT_EQ(CLIENT_CERT_TYPE_REF, ui_data.certificate_type()); - } - { - // for type pattern we need to have some kind of pattern - std::string organization("Little If Any, Inc."); - base::DictionaryValue ui_data_dict; - base::DictionaryValue* pattern_dict = new base::DictionaryValue; - base::DictionaryValue* issuer_dict = new base::DictionaryValue; - issuer_dict->SetString("Organization", organization); - pattern_dict->Set("Issuer", issuer_dict); - ui_data_dict.Set("certificate_pattern", pattern_dict); - ui_data_dict.SetString(NetworkUIData::kKeyCertificateType, "pattern"); - NetworkUIData ui_data(ui_data_dict); - EXPECT_EQ(CLIENT_CERT_TYPE_PATTERN, ui_data.certificate_type()); - } -} - -TEST(NetworkUIDataTest, CertificatePattern) { - std::string organization("Little If Any, Inc."); - base::DictionaryValue ui_data_dict; - base::DictionaryValue* pattern_dict = new base::DictionaryValue; - base::DictionaryValue* issuer_dict = new base::DictionaryValue; - issuer_dict->SetString("Organization", organization); - pattern_dict->Set("Issuer", issuer_dict); - ui_data_dict.SetString("certificate_type", "pattern"); - ui_data_dict.Set("certificate_pattern", pattern_dict); - NetworkUIData ui_data(ui_data_dict); - EXPECT_FALSE(ui_data.certificate_pattern().Empty()); - EXPECT_EQ(organization, - ui_data.certificate_pattern().issuer().organization()); -} - -class CreateUIDataTest - : public ::testing::TestWithParam<std::pair<std::string, std::string> > { -}; - -TEST_P(CreateUIDataTest, CreateUIDataFromONC) { - namespace test_utils = onc::test_utils; - scoped_ptr<base::DictionaryValue> onc_network = - test_utils::ReadTestDictionary(GetParam().first); - - scoped_ptr<base::DictionaryValue> expected_uidata = - test_utils::ReadTestDictionary(GetParam().second); - - scoped_ptr<NetworkUIData> actual_uidata = - NetworkUIData::CreateFromONC( - ::onc::ONC_SOURCE_USER_POLICY, *onc_network); - EXPECT_TRUE(actual_uidata != NULL); - - base::DictionaryValue actual_uidata_dict; - actual_uidata->FillDictionary(&actual_uidata_dict); - EXPECT_TRUE(test_utils::Equals(&actual_uidata_dict, expected_uidata.get())); -} - -INSTANTIATE_TEST_CASE_P( - CreateUIDataTest, - CreateUIDataTest, - ::testing::Values( - std::make_pair("wifi_clientcert_with_cert_pems.onc", - "uidata_for_wifi_clientcert.json"), - std::make_pair("valid_wifi_clientref.onc", - "uidata_for_wifi_clientref.json"), - std::make_pair("valid_wifi_psk.onc", - "uidata_for_wifi_psk.json"), - std::make_pair("openvpn_clientcert_with_cert_pems.onc", - "uidata_for_openvpn_clientcert.json"), - std::make_pair("l2tpipsec_clientcert_with_cert_pems.onc", - "uidata_for_l2tpipsec_clientcert.json"))); - } // namespace chromeos diff --git a/chromeos/network/policy_util.cc b/chromeos/network/policy_util.cc index 0a7526c..55fb320 100644 --- a/chromeos/network/policy_util.cc +++ b/chromeos/network/policy_util.cc @@ -178,11 +178,7 @@ scoped_ptr<base::DictionaryValue> CreateShillConfiguration( shill_dictionary->SetStringWithoutPathExpansion(shill::kProfileProperty, profile.path); - scoped_ptr<NetworkUIData> ui_data; - if (policy) - ui_data = NetworkUIData::CreateFromONC(onc_source, *policy); - else - ui_data.reset(new NetworkUIData()); + scoped_ptr<NetworkUIData> ui_data(NetworkUIData::CreateFromONC(onc_source)); if (settings) { // Shill doesn't know that sensitive data is contained in the UIData |