diff options
author | davidben <davidben@chromium.org> | 2015-05-26 13:25:45 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-05-26 20:26:27 +0000 |
commit | ee92e3890448e5d0fa00634d18308f0adb079211 (patch) | |
tree | c222a23135677b2bf91917ede3811f2cb23d312d /components | |
parent | 0bc8b58d4c25353d7d8bce3d9ccdda36e2701ebd (diff) | |
download | chromium_src-ee92e3890448e5d0fa00634d18308f0adb079211.zip chromium_src-ee92e3890448e5d0fa00634d18308f0adb079211.tar.gz chromium_src-ee92e3890448e5d0fa00634d18308f0adb079211.tar.bz2 |
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}
Diffstat (limited to 'components')
-rw-r--r-- | components/BUILD.gn | 5 | ||||
-rw-r--r-- | components/components_tests.gyp | 4 | ||||
-rw-r--r-- | components/ownership.gypi | 74 | ||||
-rw-r--r-- | components/ownership/BUILD.gn | 82 | ||||
-rw-r--r-- | components/ownership/mock_owner_key_util.cc | 27 | ||||
-rw-r--r-- | components/ownership/mock_owner_key_util.h | 13 | ||||
-rw-r--r-- | components/ownership/owner_key_util.cc | 4 | ||||
-rw-r--r-- | components/ownership/owner_key_util.h | 19 | ||||
-rw-r--r-- | components/ownership/owner_key_util_impl.cc | 16 | ||||
-rw-r--r-- | components/ownership/owner_key_util_impl.h | 7 | ||||
-rw-r--r-- | components/ownership/owner_settings_service.cc | 42 |
11 files changed, 155 insertions, 138 deletions
diff --git a/components/BUILD.gn b/components/BUILD.gn index cef6b23..c4987b4 100644 --- a/components/BUILD.gn +++ b/components/BUILD.gn @@ -285,7 +285,6 @@ test("components_unittests") { "//components/metrics:unit_tests", "//components/mime_util:unit_tests", "//components/omnibox:unit_tests", - "//components/ownership:unit_tests", "//components/packed_ct_ev_whitelist:unit_tests", "//components/undo:unit_tests", "//components/update_client:unit_tests", @@ -308,6 +307,10 @@ test("components_unittests") { deps += [ "//components/proximity_auth:unit_tests" ] } + if (is_chromeos) { + deps += [ "//components/ownership:unit_tests" ] + } + # TODO(GYP) need this target. #'breakpad/app/crash_keys_win_unittest.cc', diff --git a/components/components_tests.gyp b/components/components_tests.gyp index a32b943..22dce6b 100644 --- a/components/components_tests.gyp +++ b/components/components_tests.gyp @@ -691,7 +691,6 @@ '<@(network_time_unittest_sources)', '<@(omnibox_unittest_sources)', '<@(os_crypt_unittest_sources)', - '<@(ownership_unittest_sources)', '<@(packed_ct_ev_whitelist_unittest_sources)', '<@(password_manager_unittest_sources)', '<@(precache_unittest_sources)', @@ -797,7 +796,6 @@ 'components.gyp:omnibox', 'components.gyp:omnibox_test_support', 'components.gyp:os_crypt', - 'components.gyp:ownership', 'components.gyp:packed_ct_ev_whitelist', 'components.gyp:password_manager_core_browser', 'components.gyp:password_manager_core_browser', @@ -1073,12 +1071,14 @@ 'wifi_sync/wifi_credential_unittest.cc', 'wifi_sync/wifi_security_class_chromeos_unittest.cc', 'wifi_sync/wifi_security_class_unittest.cc', + '<@(ownership_unittest_sources)', ], 'sources!': [ 'storage_monitor/storage_monitor_linux_unittest.cc', ], 'dependencies': [ '../chromeos/chromeos.gyp:chromeos_test_support', + 'components.gyp:ownership', 'components.gyp:pairing', 'components.gyp:user_manager_test_support', 'components.gyp:wifi_sync', diff --git a/components/ownership.gypi b/components/ownership.gypi index 4784259..9415ade 100644 --- a/components/ownership.gypi +++ b/components/ownership.gypi @@ -3,43 +3,47 @@ # found in the LICENSE file. { - 'targets': [{ - 'target_name': 'ownership', - 'type': '<(component)', - 'dependencies': [ - '<(DEPTH)/base/base.gyp:base', - '<(DEPTH)/components/components.gyp:keyed_service_core', - '<(DEPTH)/components/components.gyp:policy_component_common', - '<(DEPTH)/crypto/crypto.gyp:crypto', - ], - 'defines': [ - 'OWNERSHIP_IMPLEMENTATION', - ], - 'include_dirs': [ - '<(SHARED_INTERMEDIATE_DIR)', - ], - 'sources': [ - 'ownership/mock_owner_key_util.cc', - 'ownership/mock_owner_key_util.h', - 'ownership/owner_key_util.cc', - 'ownership/owner_key_util.h', - 'ownership/owner_key_util_impl.cc', - 'ownership/owner_key_util_impl.h', - 'ownership/owner_settings_service.cc', - 'ownership/owner_settings_service.h', - ], - 'conditions': [ - ['configuration_policy==1', { + 'conditions': [ + ['chromeos==1', { + 'targets': [{ + 'target_name': 'ownership', + 'type': '<(component)', 'dependencies': [ - '<(DEPTH)/components/components.gyp:cloud_policy_proto', - '<(DEPTH)/components/components.gyp:policy', + '<(DEPTH)/base/base.gyp:base', + '<(DEPTH)/components/components.gyp:keyed_service_core', + '<(DEPTH)/components/components.gyp:policy_component_common', + '<(DEPTH)/crypto/crypto.gyp:crypto', ], - }], - ['use_nss_certs==1', { - 'dependencies': [ - '../build/linux/system.gyp:ssl', + 'defines': [ + 'OWNERSHIP_IMPLEMENTATION', + ], + 'include_dirs': [ + '<(SHARED_INTERMEDIATE_DIR)', + ], + 'sources': [ + 'ownership/mock_owner_key_util.cc', + 'ownership/mock_owner_key_util.h', + 'ownership/owner_key_util.cc', + 'ownership/owner_key_util.h', + 'ownership/owner_key_util_impl.cc', + 'ownership/owner_key_util_impl.h', + 'ownership/owner_settings_service.cc', + 'ownership/owner_settings_service.h', + ], + 'conditions': [ + ['configuration_policy==1', { + 'dependencies': [ + '<(DEPTH)/components/components.gyp:cloud_policy_proto', + '<(DEPTH)/components/components.gyp:policy', + ], + }], + ['use_nss_certs==1', { + 'dependencies': [ + '../build/linux/system.gyp:ssl', + ], + }], ], }], - ], - }], + }], + ], } 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 <pk11pub.h> + #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<uint8>* output) { return !public_key_.empty(); } -#if defined(USE_NSS_CERTS) -crypto::RSAPrivateKey* MockOwnerKeyUtil::FindPrivateKeyInSlot( +crypto::ScopedSECKEYPrivateKey MockOwnerKeyUtil::FindPrivateKeyInSlot( const std::vector<uint8>& 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<uint8>& key) { void MockOwnerKeyUtil::SetPublicKeyFromPrivateKey( const crypto::RSAPrivateKey& key) { - key.ExportPublicKey(&public_key_); + CHECK(key.ExportPublicKey(&public_key_)); } void MockOwnerKeyUtil::SetPrivateKey(scoped_ptr<crypto::RSAPrivateKey> key) { - private_key_ = key.Pass(); - private_key_->ExportPublicKey(&public_key_); + CHECK(key->ExportPublicKey(&public_key_)); + + std::vector<uint8_t> 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<uint8>* output) override; -#if defined(USE_NSS_CERTS) - crypto::RSAPrivateKey* FindPrivateKeyInSlot(const std::vector<uint8>& key, - PK11SlotInfo* slot) override; -#endif // defined(USE_NSS_CERTS) + crypto::ScopedSECKEYPrivateKey FindPrivateKeyInSlot( + const std::vector<uint8>& 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<uint8> public_key_; - scoped_ptr<crypto::RSAPrivateKey> 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<PrivateKey> { 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<PrivateKey>; virtual ~PrivateKey(); - scoped_ptr<crypto::RSAPrivateKey> 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<uint8>* 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<uint8>& 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 <keythi.h> #include "crypto/nss_key_util.h" -#include "crypto/rsa_private_key.h" -#endif namespace ownership { @@ -55,8 +52,7 @@ bool OwnerKeyUtilImpl::ImportPublicKey(std::vector<uint8>* output) { return data_read == safe_file_size; } -#if defined(USE_NSS_CERTS) -crypto::RSAPrivateKey* OwnerKeyUtilImpl::FindPrivateKeyInSlot( +crypto::ScopedSECKEYPrivateKey OwnerKeyUtilImpl::FindPrivateKeyInSlot( const std::vector<uint8>& 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<uint8>* output) override; -#if defined(USE_NSS_CERTS) - crypto::RSAPrivateKey* FindPrivateKeyInSlot(const std::vector<uint8>& key, - PK11SlotInfo* slot) override; -#endif // defined(USE_NSS_CERTS) + crypto::ScopedSECKEYPrivateKey FindPrivateKeyInSlot( + const std::vector<uint8>& 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 <cryptohi.h> +#include <keyhi.h> + #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<SGNContext, + crypto::NSSDestroyer1<SGNContext, SGN_DestroyContext, PR_TRUE>>; + scoped_ptr<em::PolicyFetchResponse> AssembleAndSignPolicy( scoped_ptr<em::PolicyData> policy, - crypto::RSAPrivateKey* private_key) { + SECKEYPrivateKey* private_key) { // Assemble the policy. scoped_ptr<em::PolicyFetchResponse> policy_response( new em::PolicyFetchResponse()); @@ -33,23 +40,28 @@ scoped_ptr<em::PolicyFetchResponse> AssembleAndSignPolicy( return scoped_ptr<em::PolicyFetchResponse>(nullptr).Pass(); } - // Generate the signature. - scoped_ptr<crypto::SignatureCreator> signature_creator( - crypto::SignatureCreator::Create(private_key, - crypto::SignatureCreator::SHA1)); - signature_creator->Update( - reinterpret_cast<const uint8*>(policy_response->policy_data().c_str()), - policy_response->policy_data().size()); - std::vector<uint8> 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<const uint8*>( + 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<em::PolicyFetchResponse>(nullptr).Pass(); + return nullptr; } policy_response->mutable_policy_data_signature()->assign( - reinterpret_cast<const char*>(vector_as_array(&signature_bytes)), - signature_bytes.size()); + reinterpret_cast<const char*>(signature_item.data), signature_item.len); + SECITEM_FreeItem(&signature_item, PR_FALSE); + return policy_response.Pass(); } |