Translate ONC ProxySettings <-> Shill ProxyConfig

chromeos/network/onc/onc_utils.cc

Index: chromeos/network/onc/onc_utils.cc
diff --git a/chromeos/network/onc/onc_utils.cc b/chromeos/network/onc/onc_utils.cc
index 2e4d384c4985fdde5ef2b0fb49f789fa373654f8..8d8a47614cdb93844a19dd007bb150f0e944ae01 100644
--- a/chromeos/network/onc/onc_utils.cc
+++ b/chromeos/network/onc/onc_utils.cc
@@ -17,11 +17,17 @@
 #include "chromeos/network/onc/onc_utils.h"
 #include "chromeos/network/onc/onc_validator.h"
 #include "components/device_event_log/device_event_log.h"
+#include "components/proxy_config/proxy_config_dictionary.h"
 #include "crypto/encryptor.h"
 #include "crypto/hmac.h"
 #include "crypto/symmetric_key.h"
+#include "net/base/host_port_pair.h"
 #include "net/cert/pem_tokenizer.h"
 #include "net/cert/x509_certificate.h"
+#include "net/proxy/proxy_bypass_rules.h"
+#include "net/proxy/proxy_config.h"
+#include "net/proxy/proxy_server.h"
+#include "url/url_constants.h"
 
 using namespace ::onc;
 
@@ -738,5 +744,216 @@ bool IsRecommendedValue(const base::DictionaryValue* onc,
           recommended_keys->end());
 }
 
+namespace {
+
+const char kSocksScheme[] = "socks";
+
+net::ProxyServer ConvertOncProxyLocationToHostPort(
+    net::ProxyServer::Scheme default_proxy_scheme,
+    const base::DictionaryValue& onc_proxy_location) {
+  std::string host;
+  onc_proxy_location.GetStringWithoutPathExpansion(::onc::proxy::kHost, &host);
+  // Parse |host| according to the format [<scheme>"://"]<server>[":"<port>].
+  net::ProxyServer proxy_server =
+      net::ProxyServer::FromURI(host, default_proxy_scheme);

[eroman, 2015/07/08 21:26:38]

Parsing might fail, in which case proxy_server will be invalid. Where does this
data come from?

[stevenjb, 2015/07/08 22:13:08]

Theoretically, ONC validation should ensure that these are valid values, but
even if the are not, if I read the code correctly, this would fail gracefully;
ProxyConfigDictionary::EncodeAndAppendProxyServer would do nothing and |spec| in
AppendProxyServerForScheme would be unchanged (i.e. the proxy would not be
appended).

+  int port = 0;
+  onc_proxy_location.GetIntegerWithoutPathExpansion(::onc::proxy::kPort, &port);
+
+  // Replace the port parsed from |host| by the provided |port|.
+  return net::ProxyServer(
+      proxy_server.scheme(),
+      net::HostPortPair(proxy_server.host_port_pair().host(),

[eroman, 2015/07/08 21:26:38]

proxy_server may be invalid, in which case calling host_port_pair() is not
valid.

[stevenjb, 2015/07/08 22:13:08]

Since the scheme is invalid, the host_port_pair value shouldn't matter here.

[eroman, 2015/07/13 20:08:38]

It will hit a DCHECK in ProxyServer::host_port_pair()

+                        static_cast<uint16>(port)));

[eroman, 2015/07/08 21:26:38]

possibility of underflow.
Is the input considered preference considered valid?

[stevenjb, 2015/07/08 22:13:08]

The ONC input should be validated already. Otherwise it appears that the port
gets clamped to a valid range elsewhere (I'm not super familiar with the proxy
code myself, this particular code was just re-located in this CL).

[pneubeck (no reviews), 2015/07/09 07:20:32]

The port of the proxy location isn't validated currently by the ONC validator:
https://code.google.com/p/chromium/codesearch#chromium/src/chromeos/network/o...

probably something worth adding.

[stevenjb, 2015/07/09 18:24:28]

Probably. Currently we seem to at least be clamping the port somewhere - if I
enter an invalid port in the UI it comes back as a valid value after the
round-trip (ONC -> Shill -> ONC), so it's getting clamped *somewhere*.
Unfortunately this effort is the first time I have really looked at any of the
proxy code, and there seems to be a *lot* of it.

+}
+
+void AppendProxyServerForScheme(const base::DictionaryValue& onc_manual,
+                                const std::string& onc_scheme,
+                                std::string* spec) {
+  const base::DictionaryValue* onc_proxy_location = nullptr;
+  if (!onc_manual.GetDictionaryWithoutPathExpansion(onc_scheme,
+                                                    &onc_proxy_location)) {
+    return;
+  }
+
+  net::ProxyServer::Scheme default_proxy_scheme = net::ProxyServer::SCHEME_HTTP;
+  std::string url_scheme;
+  if (onc_scheme == ::onc::proxy::kFtp) {
+    url_scheme = url::kFtpScheme;
+  } else if (onc_scheme == ::onc::proxy::kHttp) {
+    url_scheme = url::kHttpScheme;
+  } else if (onc_scheme == ::onc::proxy::kHttps) {
+    url_scheme = url::kHttpsScheme;
+  } else if (onc_scheme == ::onc::proxy::kSocks) {
+    default_proxy_scheme = net::ProxyServer::SCHEME_SOCKS4;
+    url_scheme = kSocksScheme;
+  } else {
+    NOTREACHED();
+  }
+
+  net::ProxyServer proxy_server = ConvertOncProxyLocationToHostPort(
+      default_proxy_scheme, *onc_proxy_location);
+
+  ProxyConfigDictionary::EncodeAndAppendProxyServer(url_scheme, proxy_server,
+                                                    spec);
+}
+
+net::ProxyBypassRules ConvertOncExcludeDomainsToBypassRules(
+    const base::ListValue& onc_exclude_domains) {
+  net::ProxyBypassRules rules;
+  for (base::ListValue::const_iterator it = onc_exclude_domains.begin();
+       it != onc_exclude_domains.end(); ++it) {
+    std::string rule;
+    (*it)->GetAsString(&rule);
+    rules.AddRuleFromString(rule);
+  }
+  return rules;
+}
+
+void SetProxyForScheme(const net::ProxyConfig::ProxyRules& proxy_rules,
+                       const std::string& scheme,
+                       const std::string& onc_scheme,
+                       base::DictionaryValue* dict) {
+  const net::ProxyList* proxy_list = nullptr;
+  if (proxy_rules.type == net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY) {
+    proxy_list = &proxy_rules.single_proxies;
+  } else if (proxy_rules.type ==
+             net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME) {
+    proxy_list = proxy_rules.MapUrlSchemeToProxyList(scheme);
+  }
+  if (!proxy_list || proxy_list->IsEmpty())
+    return;
+  const net::ProxyServer& server = proxy_list->Get();
+  scoped_ptr<base::DictionaryValue> url_dict(new base::DictionaryValue);
+  std::string host = server.host_port_pair().host();
+  // Special case: Include the scheme for socks5 only. For all proxy types
+  // except SOCKS, the default scheme of the proxy host is HTTP and we do not
+  // prefix the host.
+  if (server.scheme() == net::ProxyServer::SCHEME_SOCKS5)
+    host = "socks5://" + host;
+  url_dict->SetStringWithoutPathExpansion(::onc::proxy::kHost, host);
+  url_dict->SetIntegerWithoutPathExpansion(::onc::proxy::kPort,
+                                           server.host_port_pair().port());
+  dict->SetWithoutPathExpansion(onc_scheme, url_dict.release());
+}
+
+}  // namespace
+
+scoped_ptr<base::DictionaryValue> ConvertOncProxySettingsToProxyConfig(
+    const base::DictionaryValue& onc_proxy_settings) {
+  std::string type;
+  onc_proxy_settings.GetStringWithoutPathExpansion(::onc::proxy::kType, &type);
+  scoped_ptr<base::DictionaryValue> proxy_dict;
+
+  if (type == ::onc::proxy::kDirect) {
+    proxy_dict.reset(ProxyConfigDictionary::CreateDirect());
+  } else if (type == ::onc::proxy::kWPAD) {
+    proxy_dict.reset(ProxyConfigDictionary::CreateAutoDetect());
+  } else if (type == ::onc::proxy::kPAC) {
+    std::string pac_url;
+    onc_proxy_settings.GetStringWithoutPathExpansion(::onc::proxy::kPAC,
+                                                     &pac_url);
+    GURL url(pac_url);
+    DCHECK(url.is_valid()) << "Invalid URL in ProxySettings.PAC";
+    proxy_dict.reset(ProxyConfigDictionary::CreatePacScript(url.spec(), false));
+  } else if (type == ::onc::proxy::kManual) {
+    const base::DictionaryValue* manual_dict = nullptr;
+    onc_proxy_settings.GetDictionaryWithoutPathExpansion(::onc::proxy::kManual,

[eroman, 2015/07/08 21:26:38]

The error checking throughout is pretty weak.
For instance here it is assumed that this will succeed and manual_proxy be
non-null.

[stevenjb, 2015/07/08 22:13:08]

Here ONC validation will definitely ensure that if type == Manual then a Manual
dictionary exists.

+                                                         &manual_dict);
+    std::string manual_spec;
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kFtp, &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kHttp, &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kSocks,
+                               &manual_spec);
+    AppendProxyServerForScheme(*manual_dict, ::onc::proxy::kHttps,
+                               &manual_spec);
+
+    const base::ListValue* exclude_domains = nullptr;
+    net::ProxyBypassRules bypass_rules;
+    if (onc_proxy_settings.GetListWithoutPathExpansion(
+            ::onc::proxy::kExcludeDomains, &exclude_domains)) {
+      bypass_rules.AssignFrom(
+          ConvertOncExcludeDomainsToBypassRules(*exclude_domains));
+    }
+    proxy_dict.reset(ProxyConfigDictionary::CreateFixedServers(
+        manual_spec, bypass_rules.ToString()));
+  } else {
+    NOTREACHED();
+  }
+  return proxy_dict.Pass();
+}
+
+scoped_ptr<base::DictionaryValue> ConvertProxyConfigToOncProxySettings(
+    const base::DictionaryValue& proxy_config_value) {
+  // Create a ProxyConfigDictionary from the DictionaryValue.
+  scoped_ptr<ProxyConfigDictionary> proxy_config(
+      new ProxyConfigDictionary(&proxy_config_value));
+
+  // Create the result DictionaryValue and populate it.
+  scoped_ptr<base::DictionaryValue> proxy_settings(new base::DictionaryValue);
+  ProxyPrefs::ProxyMode mode;
+  if (!proxy_config->GetMode(&mode))
+    return nullptr;
+  switch (mode) {
+    case ProxyPrefs::MODE_DIRECT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kDirect);
+      break;
+    }
+    case ProxyPrefs::MODE_AUTO_DETECT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kWPAD);
+      break;
+    }
+    case ProxyPrefs::MODE_PAC_SCRIPT: {
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kType,
+                                                    ::onc::proxy::kPAC);
+      std::string pac_url;
+      proxy_config->GetPacUrl(&pac_url);
+      proxy_settings->SetStringWithoutPathExpansion(::onc::proxy::kPAC,
+                                                    pac_url);
+      break;
+    }
+    case ProxyPrefs::MODE_FIXED_SERVERS: {
+      proxy_settings->SetString(::onc::proxy::kType, ::onc::proxy::kManual);
+      scoped_ptr<base::DictionaryValue> manual(new base::DictionaryValue);
+      std::string proxy_rules_string;
+      if (proxy_config->GetProxyServer(&proxy_rules_string)) {
+        net::ProxyConfig::ProxyRules proxy_rules;
+        proxy_rules.ParseFromString(proxy_rules_string);
+        SetProxyForScheme(proxy_rules, url::kFtpScheme, ::onc::proxy::kFtp,
+                          manual.get());
+        SetProxyForScheme(proxy_rules, url::kHttpScheme, ::onc::proxy::kHttp,
+                          manual.get());
+        SetProxyForScheme(proxy_rules, url::kHttpsScheme, ::onc::proxy::kHttps,
+                          manual.get());
+        SetProxyForScheme(proxy_rules, kSocksScheme, ::onc::proxy::kSocks,
+                          manual.get());
+      }
+      proxy_settings->SetWithoutPathExpansion(::onc::proxy::kManual,
+                                              manual.release());
+
+      // Convert the 'bypass_list' string into dictionary entries.
+      std::string bypass_rules_string;
+      if (proxy_config->GetBypassList(&bypass_rules_string)) {
+        net::ProxyBypassRules bypass_rules;
+        bypass_rules.ParseFromString(bypass_rules_string);
+        scoped_ptr<base::ListValue> exclude_domains(new base::ListValue);
+        for (const net::ProxyBypassRules::Rule* rule : bypass_rules.rules())
+          exclude_domains->AppendString(rule->ToString());
+        if (!exclude_domains->empty()) {
+          proxy_settings->SetWithoutPathExpansion(::onc::proxy::kExcludeDomains,
+                                                  exclude_domains.release());
+        }
+      }
+      break;
+    }
+    default: {
+      LOG(ERROR) << "Unexpected proxy mode in Shill config: " << mode;
+      return nullptr;
+    }
+  }
+  return proxy_settings.Pass();
+}
+
 }  // namespace onc
 }  // namespace chromeos