From ee92e3890448e5d0fa00634d18308f0adb079211 Mon Sep 17 00:00:00 2001 From: davidben Date: Tue, 26 May 2015 13:25:45 -0700 Subject: Make components/ownership transit SECKEYPrivateKey handles. We'll later want to make this code use CHAPS directly, possibly with some interface abstracting NSS and CHAPS. But RSAPrivateKey won't work as that interface as it can't abstract between multiple private key implementations at runtime. So switch it to SECKEYPrivateKey and NSS functions directly. With this, the chimera build should be functional on Linux and CrOS apart from client certificates. This tightens the build so that components/ownership is only build on CrOS. It's currently unused on other platforms, but still built as a test, with NSS pieces #ifdef'd out. BUG=478777 Review URL: https://codereview.chromium.org/1145613002 Cr-Commit-Position: refs/heads/master@{#331421} --- components/ownership/BUILD.gn | 82 ++++++++++++++------------ components/ownership/mock_owner_key_util.cc | 27 ++++++--- components/ownership/mock_owner_key_util.h | 13 ++-- components/ownership/owner_key_util.cc | 4 +- components/ownership/owner_key_util.h | 19 ++---- components/ownership/owner_key_util_impl.cc | 16 +---- components/ownership/owner_key_util_impl.h | 7 +-- components/ownership/owner_settings_service.cc | 42 ++++++++----- 8 files changed, 110 insertions(+), 100 deletions(-) (limited to 'components/ownership') diff --git a/components/ownership/BUILD.gn b/components/ownership/BUILD.gn index cdaf227..eb65f21 100644 --- a/components/ownership/BUILD.gn +++ b/components/ownership/BUILD.gn @@ -5,47 +5,51 @@ import("//build/config/crypto.gni") import("//build/config/features.gni") -component("ownership") { - sources = [ - "mock_owner_key_util.cc", - "mock_owner_key_util.h", - "owner_key_util.cc", - "owner_key_util.h", - "owner_key_util_impl.cc", - "owner_key_util_impl.h", - "owner_settings_service.cc", - "owner_settings_service.h", - ] - - defines = [ "OWNERSHIP_IMPLEMENTATION" ] - - deps = [ - "//base", - "//components/keyed_service/core", - "//components/policy/proto", - "//components/policy:policy_component_common", - "//crypto", - ] - - if (enable_configuration_policy) { - deps += [ "//components/policy" ] +if (is_chromeos) { + component("ownership") { + sources = [ + "mock_owner_key_util.cc", + "mock_owner_key_util.h", + "owner_key_util.cc", + "owner_key_util.h", + "owner_key_util_impl.cc", + "owner_key_util_impl.h", + "owner_settings_service.cc", + "owner_settings_service.h", + ] + + defines = [ "OWNERSHIP_IMPLEMENTATION" ] + + deps = [ + "//base", + "//components/keyed_service/core", + "//components/policy/proto", + "//components/policy:policy_component_common", + "//crypto", + ] + + if (enable_configuration_policy) { + deps += [ "//components/policy" ] + } + + if (use_nss_certs) { + public_deps = [ + "//crypto:platform", + ] + } } - if (use_nss_certs) { - deps += [ "//crypto:platform" ] - } -} + source_set("unit_tests") { + testonly = true + sources = [ + "owner_key_util_impl_unittest.cc", + ] -source_set("unit_tests") { - testonly = true - sources = [ - "owner_key_util_impl_unittest.cc", - ] + configs += [ "//build/config/compiler:no_size_t_to_int_warning" ] - configs += [ "//build/config/compiler:no_size_t_to_int_warning" ] - - deps = [ - ":ownership", - "//testing/gtest", - ] + deps = [ + ":ownership", + "//testing/gtest", + ] + } } diff --git a/components/ownership/mock_owner_key_util.cc b/components/ownership/mock_owner_key_util.cc index 703351d..3f5adb5 100644 --- a/components/ownership/mock_owner_key_util.cc +++ b/components/ownership/mock_owner_key_util.cc @@ -4,7 +4,11 @@ #include "components/ownership/mock_owner_key_util.h" +#include + #include "base/files/file_path.h" +#include "base/logging.h" +#include "crypto/nss_key_util.h" #include "crypto/rsa_private_key.h" namespace ownership { @@ -20,13 +24,14 @@ bool MockOwnerKeyUtil::ImportPublicKey(std::vector* output) { return !public_key_.empty(); } -#if defined(USE_NSS_CERTS) -crypto::RSAPrivateKey* MockOwnerKeyUtil::FindPrivateKeyInSlot( +crypto::ScopedSECKEYPrivateKey MockOwnerKeyUtil::FindPrivateKeyInSlot( const std::vector& key, PK11SlotInfo* slot) { - return private_key_.get() ? private_key_->Copy() : NULL; + if (!private_key_) + return nullptr; + return crypto::ScopedSECKEYPrivateKey( + SECKEY_CopyPrivateKey(private_key_.get())); } -#endif // defined(USE_NSS_CERTS) bool MockOwnerKeyUtil::IsPublicKeyPresent() { return !public_key_.empty(); @@ -43,12 +48,20 @@ void MockOwnerKeyUtil::SetPublicKey(const std::vector& key) { void MockOwnerKeyUtil::SetPublicKeyFromPrivateKey( const crypto::RSAPrivateKey& key) { - key.ExportPublicKey(&public_key_); + CHECK(key.ExportPublicKey(&public_key_)); } void MockOwnerKeyUtil::SetPrivateKey(scoped_ptr key) { - private_key_ = key.Pass(); - private_key_->ExportPublicKey(&public_key_); + CHECK(key->ExportPublicKey(&public_key_)); + + std::vector key_exported; + CHECK(key->ExportPrivateKey(&key_exported)); + + crypto::ScopedPK11Slot slot(PK11_GetInternalSlot()); + CHECK(slot); + private_key_ = crypto::ImportNSSKeyFromPrivateKeyInfo( + slot.get(), key_exported, false /* not permanent */); + CHECK(private_key_); } } // namespace ownership diff --git a/components/ownership/mock_owner_key_util.h b/components/ownership/mock_owner_key_util.h index 72fddc3..c36eb13 100644 --- a/components/ownership/mock_owner_key_util.h +++ b/components/ownership/mock_owner_key_util.h @@ -14,6 +14,10 @@ #include "components/ownership/owner_key_util.h" #include "components/ownership/ownership_export.h" +namespace crypto { +class RSAPrivateKey; +} + namespace ownership { // Implementation of OwnerKeyUtil which should be used only for @@ -24,10 +28,9 @@ class OWNERSHIP_EXPORT MockOwnerKeyUtil : public OwnerKeyUtil { // OwnerKeyUtil implementation: bool ImportPublicKey(std::vector* output) override; -#if defined(USE_NSS_CERTS) - crypto::RSAPrivateKey* FindPrivateKeyInSlot(const std::vector& key, - PK11SlotInfo* slot) override; -#endif // defined(USE_NSS_CERTS) + crypto::ScopedSECKEYPrivateKey FindPrivateKeyInSlot( + const std::vector& key, + PK11SlotInfo* slot) override; bool IsPublicKeyPresent() override; // Clears the public and private keys. @@ -47,7 +50,7 @@ class OWNERSHIP_EXPORT MockOwnerKeyUtil : public OwnerKeyUtil { ~MockOwnerKeyUtil() override; std::vector public_key_; - scoped_ptr private_key_; + crypto::ScopedSECKEYPrivateKey private_key_; DISALLOW_COPY_AND_ASSIGN(MockOwnerKeyUtil); }; diff --git a/components/ownership/owner_key_util.cc b/components/ownership/owner_key_util.cc index eb2cb62..a049729 100644 --- a/components/ownership/owner_key_util.cc +++ b/components/ownership/owner_key_util.cc @@ -4,8 +4,6 @@ #include "components/ownership/owner_key_util.h" -#include "crypto/rsa_private_key.h" - namespace ownership { /////////////////////////////////////////////////////////////////////////// @@ -20,7 +18,7 @@ PublicKey::~PublicKey() { /////////////////////////////////////////////////////////////////////////// // PrivateKey -PrivateKey::PrivateKey(crypto::RSAPrivateKey* key) : key_(key) { +PrivateKey::PrivateKey(crypto::ScopedSECKEYPrivateKey key) : key_(key.Pass()) { } PrivateKey::~PrivateKey() { diff --git a/components/ownership/owner_key_util.h b/components/ownership/owner_key_util.h index 3920180..ae083cf 100644 --- a/components/ownership/owner_key_util.h +++ b/components/ownership/owner_key_util.h @@ -14,15 +14,10 @@ #include "base/memory/scoped_ptr.h" #include "base/stl_util.h" #include "components/ownership/ownership_export.h" +#include "crypto/scoped_nss_types.h" -#if defined(USE_NSS_CERTS) struct PK11SlotInfoStr; typedef struct PK11SlotInfoStr PK11SlotInfo; -#endif // defined(USE_NSS_CERTS) - -namespace crypto { -class RSAPrivateKey; -} namespace ownership { @@ -53,21 +48,21 @@ class OWNERSHIP_EXPORT PublicKey DISALLOW_COPY_AND_ASSIGN(PublicKey); }; -// This class is a ref-counted wrapper around a crypto::RSAPrivateKey +// This class is a ref-counted wrapper around a SECKEYPrivateKey // instance. class OWNERSHIP_EXPORT PrivateKey : public base::RefCountedThreadSafe { public: - explicit PrivateKey(crypto::RSAPrivateKey* key); + explicit PrivateKey(crypto::ScopedSECKEYPrivateKey key); - crypto::RSAPrivateKey* key() { return key_.get(); } + SECKEYPrivateKey* key() { return key_.get(); } private: friend class base::RefCountedThreadSafe; virtual ~PrivateKey(); - scoped_ptr key_; + crypto::ScopedSECKEYPrivateKey key_; DISALLOW_COPY_AND_ASSIGN(PrivateKey); }; @@ -81,14 +76,12 @@ class OWNERSHIP_EXPORT OwnerKeyUtil // returns true and populates |output|. False on failure. virtual bool ImportPublicKey(std::vector* output) = 0; -#if defined(USE_NSS_CERTS) // Looks for the private key associated with |key| in the |slot| // and returns it if it can be found. Returns NULL otherwise. // Caller takes ownership. - virtual crypto::RSAPrivateKey* FindPrivateKeyInSlot( + virtual crypto::ScopedSECKEYPrivateKey FindPrivateKeyInSlot( const std::vector& key, PK11SlotInfo* slot) = 0; -#endif // defined(USE_NSS_CERTS) // Checks whether the public key is present in the file system. virtual bool IsPublicKeyPresent() = 0; diff --git a/components/ownership/owner_key_util_impl.cc b/components/ownership/owner_key_util_impl.cc index 5a400d5..6a95131 100644 --- a/components/ownership/owner_key_util_impl.cc +++ b/components/ownership/owner_key_util_impl.cc @@ -9,11 +9,8 @@ #include "base/files/file_util.h" #include "base/logging.h" -#if defined(USE_NSS_CERTS) #include #include "crypto/nss_key_util.h" -#include "crypto/rsa_private_key.h" -#endif namespace ownership { @@ -55,8 +52,7 @@ bool OwnerKeyUtilImpl::ImportPublicKey(std::vector* output) { return data_read == safe_file_size; } -#if defined(USE_NSS_CERTS) -crypto::RSAPrivateKey* OwnerKeyUtilImpl::FindPrivateKeyInSlot( +crypto::ScopedSECKEYPrivateKey OwnerKeyUtilImpl::FindPrivateKeyInSlot( const std::vector& key, PK11SlotInfo* slot) { if (!slot) @@ -66,16 +62,8 @@ crypto::RSAPrivateKey* OwnerKeyUtilImpl::FindPrivateKeyInSlot( crypto::FindNSSKeyFromPublicKeyInfoInSlot(key, slot)); if (!private_key || SECKEY_GetPrivateKeyType(private_key.get()) != rsaKey) return nullptr; -#if defined(USE_OPENSSL) - // TODO(davidben): This assumes that crypto::RSAPrivateKey also uses NSS. - // https://crbug.com/478777 - NOTIMPLEMENTED(); - return nullptr; -#else - return crypto::RSAPrivateKey::CreateFromKey(private_key.get()); -#endif + return private_key.Pass(); } -#endif // defined(USE_NSS_CERTS) bool OwnerKeyUtilImpl::IsPublicKeyPresent() { return base::PathExists(public_key_file_); diff --git a/components/ownership/owner_key_util_impl.h b/components/ownership/owner_key_util_impl.h index 4446ee6..137b4f7 100644 --- a/components/ownership/owner_key_util_impl.h +++ b/components/ownership/owner_key_util_impl.h @@ -21,10 +21,9 @@ class OWNERSHIP_EXPORT OwnerKeyUtilImpl : public OwnerKeyUtil { // OwnerKeyUtil implementation: bool ImportPublicKey(std::vector* output) override; -#if defined(USE_NSS_CERTS) - crypto::RSAPrivateKey* FindPrivateKeyInSlot(const std::vector& key, - PK11SlotInfo* slot) override; -#endif // defined(USE_NSS_CERTS) + crypto::ScopedSECKEYPrivateKey FindPrivateKeyInSlot( + const std::vector& key, + PK11SlotInfo* slot) override; bool IsPublicKeyPresent() override; private: diff --git a/components/ownership/owner_settings_service.cc b/components/ownership/owner_settings_service.cc index 204cdd1..42ccfe4 100644 --- a/components/ownership/owner_settings_service.cc +++ b/components/ownership/owner_settings_service.cc @@ -4,6 +4,9 @@ #include "components/ownership/owner_settings_service.h" +#include +#include + #include "base/basictypes.h" #include "base/bind.h" #include "base/callback.h" @@ -14,7 +17,7 @@ #include "base/task_runner_util.h" #include "base/values.h" #include "components/ownership/owner_key_util.h" -#include "crypto/signature_creator.h" +#include "crypto/scoped_nss_types.h" namespace em = enterprise_management; @@ -22,9 +25,13 @@ namespace ownership { namespace { +using ScopedSGNContext = + scoped_ptr>; + scoped_ptr AssembleAndSignPolicy( scoped_ptr policy, - crypto::RSAPrivateKey* private_key) { + SECKEYPrivateKey* private_key) { // Assemble the policy. scoped_ptr policy_response( new em::PolicyFetchResponse()); @@ -33,23 +40,28 @@ scoped_ptr AssembleAndSignPolicy( return scoped_ptr(nullptr).Pass(); } - // Generate the signature. - scoped_ptr signature_creator( - crypto::SignatureCreator::Create(private_key, - crypto::SignatureCreator::SHA1)); - signature_creator->Update( - reinterpret_cast(policy_response->policy_data().c_str()), - policy_response->policy_data().size()); - std::vector signature_bytes; - std::string policy_blob; - if (!signature_creator->Final(&signature_bytes)) { + ScopedSGNContext sign_context( + SGN_NewContext(SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION, private_key)); + if (!sign_context) { + NOTREACHED(); + return nullptr; + } + + SECItem signature_item; + if (SGN_Begin(sign_context.get()) != SECSuccess || + SGN_Update(sign_context.get(), + reinterpret_cast( + policy_response->policy_data().c_str()), + policy_response->policy_data().size()) != SECSuccess || + SGN_End(sign_context.get(), &signature_item) != SECSuccess) { LOG(ERROR) << "Failed to create policy signature."; - return scoped_ptr(nullptr).Pass(); + return nullptr; } policy_response->mutable_policy_data_signature()->assign( - reinterpret_cast(vector_as_array(&signature_bytes)), - signature_bytes.size()); + reinterpret_cast(signature_item.data), signature_item.len); + SECITEM_FreeItem(&signature_item, PR_FALSE); + return policy_response.Pass(); } -- cgit v1.1