Skip to content

Commit

Permalink
Resolve certificate references in ONC by PEM.
Browse files Browse the repository at this point in the history 
In ONC, Server and CA certificates are referenced by GUID.
Before, the GUID was stored in the nickname of each certificate and used to identify each certificate.
After this change, the GUID is resolved and replaced by the PEM encoding of the certificate during import. The nickname is not used.

This commit only affects Server and CA certificates (including IssuerCARef in CertificatePatterns).
Client certificates are still identified by GUID.

This CL also 
- uses the new *CaCertPEMProperty fields of Shill.
- prepares for a list of CaCerts (for EAP, IPsec and OpenVPN)

Side-effect of this CL:
IssuerCARef is stored in the UIData service-property in Shill. Because this CL replaces IssuerCARef by IssuerCAPEMs, IssuerCARef entries of old UIData properties are ignored.
This may break network configurations which were configured via chrome://net-internals.
Reimporting such a configuration will fix the problem.

BUG=208986
TBR=eroman@chromium.org (for net_internals_ui.cc)

Review URL: https://chromiumcodereview.appspot.com/16946002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210019 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
pneubeck@chromium.org committed Jul 3, 2013
1 parent 3e15934 commit 823e3cd
Show file tree
Hide file tree
Showing 62 changed files with 1,025 additions and 341 deletions.
31 changes: 20 additions & 11 deletions chrome/browser/chromeos/cros/cert_library.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "chromeos/dbus/cryptohome_client.h"
#include "chromeos/dbus/dbus_thread_manager.h"
#include "chromeos/login/login_state.h"
#include "chromeos/network/onc/onc_utils.h"
#include "content/public/browser/browser_thread.h"
#include "crypto/nss_util.h"
#include "grit/generated_resources.h"
Expand Down Expand Up @@ -61,6 +62,16 @@ string16 GetDisplayString(net::X509Certificate* cert, bool hardware_backed) {
}
}

std::string CertToPEM(const net::X509Certificate& cert) {
std::string pem_encoded_cert;
if (!net::X509Certificate::GetPEMEncoded(cert.os_cert_handle(),
&pem_encoded_cert)) {
LOG(ERROR) << "Couldn't PEM-encode certificate";
return std::string();
}
return pem_encoded_cert;
}

} // namespace

class CertNameComparator {
Expand Down Expand Up @@ -148,9 +159,8 @@ string16 CertLibrary::GetCertDisplayStringAt(CertType type, int index) const {
return GetDisplayString(cert, hardware_backed);
}

std::string CertLibrary::GetCertNicknameAt(CertType type, int index) const {
net::X509Certificate* cert = GetCertificateAt(type, index);
return x509_certificate_model::GetNickname(cert->os_cert_handle());
std::string CertLibrary::GetCertPEMAt(CertType type, int index) const {
return CertToPEM(*GetCertificateAt(type, index));
}

std::string CertLibrary::GetCertPkcs11IdAt(CertType type, int index) const {
Expand All @@ -168,17 +178,16 @@ bool CertLibrary::IsCertHardwareBackedAt(CertType type, int index) const {
NetworkHandler::Get()->cert_loader()->tpm_token_name();
}

int CertLibrary::GetCertIndexByNickname(CertType type,
const std::string& nickname) const {
int CertLibrary::GetCertIndexByPEM(CertType type,
const std::string& pem_encoded) const {
int num_certs = NumCertificates(type);
for (int index = 0; index < num_certs; ++index) {
net::X509Certificate* cert = GetCertificateAt(type, index);
net::X509Certificate::OSCertHandle cert_handle = cert->os_cert_handle();
std::string nick = x509_certificate_model::GetNickname(cert_handle);
if (nick == nickname)
return index;
if (CertToPEM(*cert) != pem_encoded)
continue;
return index;
}
return -1; // Not found.
return -1;
}

int CertLibrary::GetCertIndexByPkcs11Id(CertType type,
Expand Down Expand Up @@ -272,4 +281,4 @@ const net::CertificateList& CertLibrary::GetCertificateListForType(
return certs_;
}

} // chromeos
} // namespace chromeos
10 changes: 7 additions & 3 deletions chrome/browser/chromeos/cros/cert_library.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,16 @@ class CertLibrary : public CertLoader::Observer {

// Retreives the certificate property for |type| at |index|.
string16 GetCertDisplayStringAt(CertType type, int index) const;
std::string GetCertNicknameAt(CertType type, int index) const;
std::string GetCertPEMAt(CertType type, int index) const;
std::string GetCertPkcs11IdAt(CertType type, int index) const;
bool IsCertHardwareBackedAt(CertType type, int index) const;

// Returns the index of a Certificate matching |nickname| or -1 if none found.
int GetCertIndexByNickname(CertType type, const std::string& nickname) const;
// Returns the index of a Certificate matching |pem_encoded| or -1 if none
// found. This function may be slow depending on the number of stored
// certificates.
// TOOD(pneubeck): Either make this more efficient, asynchronous or get rid of
// it.
int GetCertIndexByPEM(CertType type, const std::string& pem_encoded) const;
// Same as above but for a PKCS#11 id. TODO(stevenjb): Replace this with a
// better mechanism for uniquely idientifying certificates, crbug.com/236978.
int GetCertIndexByPkcs11Id(CertType type, const std::string& pkcs11_id) const;
Expand Down
37 changes: 15 additions & 22 deletions chrome/browser/chromeos/cros/native_network_parser.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@

#include <string>

#include "base/logging.h"
#include "base/strings/string_util.h"
#include "base/strings/stringprintf.h"
#include "base/values.h"
#include "chrome/browser/chromeos/cros/native_network_constants.h"
#include "chrome/browser/chromeos/cros/network_library.h"
#include "chrome/browser/chromeos/login/user_manager.h"
#include "chromeos/network/onc/onc_utils.h"
#include "third_party/cros_system_api/dbus/service_constants.h"

namespace chromeos {
Expand Down Expand Up @@ -48,8 +50,7 @@ EnumMapper<PropertyIndex>::Pair property_index_table[] = {
{ flimflam::kEapAnonymousIdentityProperty,
PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY },
{ flimflam::kEapCaCertIdProperty, PROPERTY_INDEX_EAP_CA_CERT_ID },
{ flimflam::kEapCaCertNssProperty, PROPERTY_INDEX_EAP_CA_CERT_NSS },
{ flimflam::kEapCaCertProperty, PROPERTY_INDEX_EAP_CA_CERT },
{ shill::kEapCaCertPemProperty, PROPERTY_INDEX_EAP_CA_CERT_PEM },
{ flimflam::kEapCertIdProperty, PROPERTY_INDEX_EAP_CERT_ID },
{ flimflam::kEapClientCertNssProperty, PROPERTY_INDEX_EAP_CLIENT_CERT_NSS },
{ flimflam::kEapClientCertProperty, PROPERTY_INDEX_EAP_CLIENT_CERT },
Expand Down Expand Up @@ -82,8 +83,8 @@ EnumMapper<PropertyIndex>::Pair property_index_table[] = {
{ flimflam::kIsActiveProperty, PROPERTY_INDEX_IS_ACTIVE },
{ flimflam::kL2tpIpsecAuthenticationType,
PROPERTY_INDEX_IPSEC_AUTHENTICATIONTYPE },
{ flimflam::kL2tpIpsecCaCertNssProperty,
PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS },
{ shill::kL2tpIpsecCaCertPemProperty,
PROPERTY_INDEX_L2TPIPSEC_CA_CERT_PEM },
{ flimflam::kL2tpIpsecClientCertIdProperty,
PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_ID },
{ flimflam::kL2tpIpsecClientCertSlotProp,
Expand Down Expand Up @@ -153,7 +154,7 @@ EnumMapper<PropertyIndex>::Pair property_index_table[] = {
PROPERTY_INDEX_OPEN_VPN_AUTHNOCACHE },
{ flimflam::kOpenVPNAuthUserPassProperty,
PROPERTY_INDEX_OPEN_VPN_AUTHUSERPASS },
{ flimflam::kOpenVPNCaCertNSSProperty, PROPERTY_INDEX_OPEN_VPN_CACERT },
{ shill::kOpenVPNCaCertPemProperty, PROPERTY_INDEX_OPEN_VPN_CA_CERT_PEM },
{ flimflam::kOpenVPNClientCertSlotProperty,
PROPERTY_INDEX_OPEN_VPN_CLIENT_CERT_SLOT },
{ flimflam::kOpenVPNCipherProperty, PROPERTY_INDEX_OPEN_VPN_CIPHER },
Expand Down Expand Up @@ -1237,12 +1238,11 @@ bool NativeWifiNetworkParser::ParseValue(PropertyIndex index,
wifi_network->set_eap_client_cert_pkcs11_id(eap_client_cert_pkcs11_id);
return true;
}
case PROPERTY_INDEX_EAP_CA_CERT_NSS: {
std::string eap_server_ca_cert_nss_nickname;
if (!value.GetAsString(&eap_server_ca_cert_nss_nickname))
case PROPERTY_INDEX_EAP_CA_CERT_PEM: {
std::string ca_cert_pem;
if (!value.GetAsString(&ca_cert_pem))
break;
wifi_network->set_eap_server_ca_cert_nss_nickname(
eap_server_ca_cert_nss_nickname);
wifi_network->set_eap_server_ca_cert_pem(ca_cert_pem);
return true;
}
case PROPERTY_INDEX_EAP_USE_SYSTEM_CAS: {
Expand All @@ -1259,13 +1259,6 @@ bool NativeWifiNetworkParser::ParseValue(PropertyIndex index,
wifi_network->set_eap_passphrase(eap_passphrase);
return true;
}
case PROPERTY_INDEX_EAP_CA_CERT: {
std::string eap_cert_nickname;
if (!value.GetAsString(&eap_cert_nickname))
break;
wifi_network->set_eap_server_ca_cert_nss_nickname(eap_cert_nickname);
return true;
}
case PROPERTY_INDEX_WIFI_AUTH_MODE:
case PROPERTY_INDEX_WIFI_PHY_MODE:
case PROPERTY_INDEX_EAP_CLIENT_CERT:
Expand Down Expand Up @@ -1379,12 +1372,12 @@ bool NativeVirtualNetworkParser::ParseProviderValue(PropertyIndex index,
network->set_provider_type(ParseProviderType(provider_type_string));
return true;
}
case PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS:
case PROPERTY_INDEX_OPEN_VPN_CACERT: {
std::string ca_cert_nss;
if (!value.GetAsString(&ca_cert_nss))
case PROPERTY_INDEX_L2TPIPSEC_CA_CERT_PEM:
case PROPERTY_INDEX_OPEN_VPN_CA_CERT_PEM: {
std::string ca_cert_pem;
if (!value.GetAsString(&ca_cert_pem))
break;
network->set_ca_cert_nss(ca_cert_nss);
network->set_ca_cert_pem(ca_cert_pem);
return true;
}
case PROPERTY_INDEX_L2TPIPSEC_PSK: {
Expand Down
7 changes: 3 additions & 4 deletions chrome/browser/chromeos/cros/network_constants.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ enum PropertyIndex {
PROPERTY_INDEX_DEVICES,
PROPERTY_INDEX_EAP,
PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY,
PROPERTY_INDEX_EAP_CA_CERT,
PROPERTY_INDEX_EAP_CA_CERT_ID,
PROPERTY_INDEX_EAP_CA_CERT_NSS,
PROPERTY_INDEX_EAP_CA_CERT_PEM,
PROPERTY_INDEX_EAP_CERT_ID,
PROPERTY_INDEX_EAP_CLIENT_CERT,
PROPERTY_INDEX_EAP_CLIENT_CERT_NSS,
Expand Down Expand Up @@ -70,7 +69,7 @@ enum PropertyIndex {
PROPERTY_INDEX_ISSUER_SUBJECT_PATTERN_ORGANIZATION,
PROPERTY_INDEX_ISSUER_SUBJECT_PATTERN_ORGANIZATIONAL_UNIT,
PROPERTY_INDEX_IS_ACTIVE,
PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS,
PROPERTY_INDEX_L2TPIPSEC_CA_CERT_PEM,
PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_ID,
PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_SLOT,
PROPERTY_INDEX_L2TPIPSEC_GROUP_NAME,
Expand All @@ -95,7 +94,7 @@ enum PropertyIndex {
PROPERTY_INDEX_OPEN_VPN_AUTHNOCACHE,
PROPERTY_INDEX_OPEN_VPN_AUTHRETRY,
PROPERTY_INDEX_OPEN_VPN_AUTHUSERPASS,
PROPERTY_INDEX_OPEN_VPN_CACERT,
PROPERTY_INDEX_OPEN_VPN_CA_CERT_PEM,
PROPERTY_INDEX_OPEN_VPN_CERT,
PROPERTY_INDEX_OPEN_VPN_CIPHER,
PROPERTY_INDEX_OPEN_VPN_CLIENT_CERT_ID,
Expand Down
32 changes: 20 additions & 12 deletions chrome/browser/chromeos/cros/network_library.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@
#include "chrome/browser/chromeos/cros/network_library_impl_cros.h"
#include "chrome/browser/chromeos/cros/network_library_impl_stub.h"
#include "chrome/common/net/x509_certificate_model.h"
#include "chromeos/network/cert_loader.h"
#include "chromeos/network/certificate_pattern.h"
#include "chromeos/network/certificate_pattern_matcher.h"
#include "chromeos/network/cros_network_functions.h"
#include "chromeos/network/network_state_handler.h"
#include "chromeos/network/onc/onc_utils.h"
#include "content/public/browser/browser_thread.h"
#include "grit/ash_strings.h"
#include "grit/generated_resources.h"
Expand Down Expand Up @@ -591,7 +593,7 @@ VirtualNetwork::VirtualNetwork(const std::string& service_path)
VirtualNetwork::~VirtualNetwork() {}

void VirtualNetwork::EraseCredentials() {
WipeString(&ca_cert_nss_);
WipeString(&ca_cert_pem_);
WipeString(&psk_passphrase_);
WipeString(&client_cert_id_);
WipeString(&user_passphrase_);
Expand Down Expand Up @@ -619,8 +621,8 @@ void VirtualNetwork::CopyCredentialsFromRemembered(Network* remembered) {
VirtualNetwork* remembered_vpn = static_cast<VirtualNetwork*>(remembered);
VLOG(1) << "Copy VPN credentials: " << name()
<< " username: " << remembered_vpn->username();
if (ca_cert_nss_.empty())
ca_cert_nss_ = remembered_vpn->ca_cert_nss();
if (ca_cert_pem_.empty())
ca_cert_pem_ = remembered_vpn->ca_cert_pem();
if (psk_passphrase_.empty())
psk_passphrase_ = remembered_vpn->psk_passphrase();
if (client_cert_id_.empty())
Expand Down Expand Up @@ -711,13 +713,16 @@ bool VirtualNetwork::IsUserPassphraseRequired() const {
return user_passphrase_required_ && user_passphrase_.empty();
}

void VirtualNetwork::SetCACertNSS(const std::string& ca_cert_nss) {
void VirtualNetwork::SetCACertPEM(const std::string& ca_cert_pem) {
VLOG(1) << "SetCACertPEM " << ca_cert_pem;
if (provider_type_ == PROVIDER_TYPE_OPEN_VPN) {
SetStringProperty(
flimflam::kOpenVPNCaCertNSSProperty, ca_cert_nss, &ca_cert_nss_);
ca_cert_pem_ = ca_cert_pem;
base::ListValue pem_list;
pem_list.AppendString(ca_cert_pem_);
SetValueProperty(shill::kOpenVPNCaCertPemProperty, pem_list);
} else {
SetStringProperty(
flimflam::kL2tpIpsecCaCertNssProperty, ca_cert_nss, &ca_cert_nss_);
shill::kL2tpIpsecCaCertPemProperty, ca_cert_pem, &ca_cert_pem_);
}
}

Expand Down Expand Up @@ -1109,6 +1114,7 @@ void WifiNetwork::SetPassphrase(const std::string& passphrase) {
void WifiNetwork::EraseCredentials() {
WipeString(&passphrase_);
WipeString(&user_passphrase_);
WipeString(&eap_server_ca_cert_pem_);
WipeString(&eap_client_cert_pkcs11_id_);
WipeString(&eap_identity_);
WipeString(&eap_anonymous_identity_);
Expand Down Expand Up @@ -1182,11 +1188,13 @@ void WifiNetwork::SetEAPPhase2Auth(EAPPhase2Auth auth) {
}
}

void WifiNetwork::SetEAPServerCaCertNssNickname(
const std::string& nss_nickname) {
VLOG(1) << "SetEAPServerCaCertNssNickname " << nss_nickname;
SetOrClearStringProperty(flimflam::kEapCaCertNssProperty,
nss_nickname, &eap_server_ca_cert_nss_nickname_);
void WifiNetwork::SetEAPServerCaCertPEM(
const std::string& ca_cert_pem) {
VLOG(1) << "SetEAPServerCaCertPEM " << ca_cert_pem;
eap_server_ca_cert_pem_ = ca_cert_pem;
base::ListValue pem_list;
pem_list.AppendString(ca_cert_pem);
SetValueProperty(shill::kEapCaCertPemProperty, pem_list);
}

void WifiNetwork::SetEAPClientCertPkcs11Id(const std::string& pkcs11_id) {
Expand Down
Loading

0 comments on commit 823e3cd

Please sign in to comment.