Skip to content

Commit

Permalink
Remove the recursive mutex from the keyset_manager.
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 214781490
GitOrigin-RevId: 4d367cd5c12b14a50a1fbebbfcf9811fda1d1db4
  • Loading branch information
@tholenst
tholenst authored and Tink Team committed Oct 2, 2018
1 parent 5d91b98 commit c57b27a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 29 deletions.
2 changes: 2 additions & 0 deletions cc/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,8 @@ cc_library(
"//cc/util:statusor",
"//proto:tink_cc_proto",
"@com_google_absl//absl/memory",
"@com_google_absl//absl/base:core_headers",
"@com_google_absl//absl/synchronization",
],
)

Expand Down
35 changes: 19 additions & 16 deletions cc/core/keyset_manager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ StatusOr<std::unique_ptr<KeysetManager>> KeysetManager::New(
StatusOr<std::unique_ptr<KeysetManager>> KeysetManager::New(
const KeysetHandle& keyset_handle) {
auto manager = absl::make_unique<KeysetManager>();
absl::MutexLock lock(&manager->keyset_mutex_);
manager->keyset_ = keyset_handle.get_keyset();
return std::move(manager);
}

uint32_t KeysetManager::GenerateNewKeyId() {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
while (true) {
uint32_t key_id = NewKeyId();
bool already_exists = false;
Expand All @@ -81,15 +81,20 @@ uint32_t KeysetManager::GenerateNewKeyId() {
}

std::unique_ptr<KeysetHandle> KeysetManager::GetKeysetHandle() {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
std::unique_ptr<Keyset> keyset_copy(new Keyset(keyset_));
std::unique_ptr<KeysetHandle> handle(
new KeysetHandle(std::move(keyset_copy)));
return handle;
}

StatusOr<uint32_t> KeysetManager::Add(const KeyTemplate& key_template) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
return Add(key_template, false);
}

crypto::tink::util::StatusOr<uint32_t> KeysetManager::Add(
const google::crypto::tink::KeyTemplate& key_template, bool as_primary) {
absl::MutexLock lock(&keyset_mutex_);
auto key_data_result = Registry::NewKeyData(key_template);
if (!key_data_result.ok()) return key_data_result.status();
auto key_data = std::move(key_data_result.ValueOrDie());
Expand All @@ -99,22 +104,19 @@ StatusOr<uint32_t> KeysetManager::Add(const KeyTemplate& key_template) {
key->set_status(KeyStatusType::ENABLED);
key->set_key_id(key_id);
key->set_output_prefix_type(key_template.output_prefix_type());
if (as_primary) {
keyset_.set_primary_key_id(key_id);
}
return key_id;
}

StatusOr<uint32_t> KeysetManager::Rotate(const KeyTemplate& key_template) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
auto add_result = Add(key_template);
if (!add_result.ok()) return add_result.status();
auto key_id = add_result.ValueOrDie();
auto status = SetPrimary(key_id);
if (!status.ok()) return status;
return key_id;
return Add(key_template, true);
}


Status KeysetManager::Enable(uint32_t key_id) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
for (auto& key : *(keyset_.mutable_key())) {
if (key.key_id() == key_id) {
if (key.status() != KeyStatusType::DISABLED &&
Expand All @@ -134,7 +136,7 @@ Status KeysetManager::Enable(uint32_t key_id) {
}

Status KeysetManager::Disable(uint32_t key_id) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
if (keyset_.primary_key_id() == key_id) {
return ToStatusF(util::error::INVALID_ARGUMENT,
"Cannot disable primary key (key_id %" PRIu32 ").",
Expand All @@ -159,7 +161,7 @@ Status KeysetManager::Disable(uint32_t key_id) {
}

Status KeysetManager::Delete(uint32_t key_id) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
if (keyset_.primary_key_id() == key_id) {
return ToStatusF(util::error::INVALID_ARGUMENT,
"Cannot delete primary key (key_id %" PRIu32 ").",
Expand All @@ -181,7 +183,7 @@ Status KeysetManager::Delete(uint32_t key_id) {
}

Status KeysetManager::Destroy(uint32_t key_id) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
if (keyset_.primary_key_id() == key_id) {
return ToStatusF(util::error::INVALID_ARGUMENT,
"Cannot destroy primary key (key_id %" PRIu32 ").",
Expand All @@ -208,7 +210,7 @@ Status KeysetManager::Destroy(uint32_t key_id) {
}

Status KeysetManager::SetPrimary(uint32_t key_id) {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
for (auto& key : keyset_.key()) {
if (key.key_id() == key_id) {
if (key.status() != KeyStatusType::ENABLED) {
Expand All @@ -225,8 +227,9 @@ Status KeysetManager::SetPrimary(uint32_t key_id) {
key_id);
}


int KeysetManager::KeyCount() const {
std::lock_guard<std::recursive_mutex> lock(keyset_mutex_);
absl::MutexLock lock(&keyset_mutex_);
return keyset_.key_size();
}

Expand Down
38 changes: 25 additions & 13 deletions cc/keyset_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@
// limitations under the License.
//
///////////////////////////////////////////////////////////////////////////////

#ifndef TINK_KEYSET_MANAGER_H_
#define TINK_KEYSET_MANAGER_H_

#include <mutex> // NOLINT(build/c++11)

#include "absl/base/thread_annotations.h"
#include "absl/synchronization/mutex.h"
#include "tink/util/status.h"
#include "tink/util/statusor.h"
#include "proto/tink.pb.h"
Expand Down Expand Up @@ -51,53 +52,64 @@ class KeysetManager {
// 'keyset_template' and returns the key_id of the added key.
// The added key has status 'ENABLED'.
crypto::tink::util::StatusOr<uint32_t> Add(
const google::crypto::tink::KeyTemplate& key_template);
const google::crypto::tink::KeyTemplate& key_template)
LOCKS_EXCLUDED(keyset_mutex_);

// Adds to the managed keyset a fresh key generated according to
// 'keyset_template', sets the new key as the primary,
// and returns the key_id of the added key.
// The key that was primary prior to rotation remains 'ENABLED'.
crypto::tink::util::StatusOr<uint32_t> Rotate(
const google::crypto::tink::KeyTemplate& key_template);
const google::crypto::tink::KeyTemplate& key_template)
LOCKS_EXCLUDED(keyset_mutex_);

// Sets the status of the specified key to 'ENABLED'.
// Succeeds only if before the call the specified key
// has status 'DISABLED' or 'ENABLED'.
crypto::tink::util::Status Enable(uint32_t key_id);
crypto::tink::util::Status Enable(uint32_t key_id)
LOCKS_EXCLUDED(keyset_mutex_);

// Sets the status of the specified key to 'DISABLED'.
// Succeeds only if before the call the specified key
// is not primary and has status 'DISABLED' or 'ENABLED'.
crypto::tink::util::Status Disable(uint32_t key_id);
crypto::tink::util::Status Disable(uint32_t key_id)
LOCKS_EXCLUDED(keyset_mutex_);

// Sets the status of the specified key to 'DESTROYED',
// and removes the corresponding key material, if any.
// Succeeds only if before the call the specified key
// is not primary and has status 'DISABLED', or 'ENABLED',
// or 'DESTROYED'
crypto::tink::util::Status Destroy(uint32_t key_id);
// or 'DESTROYED'.
crypto::tink::util::Status Destroy(uint32_t key_id)
LOCKS_EXCLUDED(keyset_mutex_);

// Removes the specifed key from the managed keyset.
// Succeeds only if the specified key is not primary.
// After deletion the keyset contains one key fewer.
crypto::tink::util::Status Delete(uint32_t key_id);
crypto::tink::util::Status Delete(uint32_t key_id)
LOCKS_EXCLUDED(keyset_mutex_);

// Sets the specified key as the primary.
// Succeeds only if the specified key is 'ENABLED'.
crypto::tink::util::Status SetPrimary(uint32_t key_id);
crypto::tink::util::Status SetPrimary(uint32_t key_id)
LOCKS_EXCLUDED(keyset_mutex_);

// Returns the count of all keys in the keyset.
int KeyCount() const;

// Returns a handle with a copy of the managed keyset.
std::unique_ptr<KeysetHandle> GetKeysetHandle();
std::unique_ptr<KeysetHandle> GetKeysetHandle() LOCKS_EXCLUDED(keyset_mutex_);

private:
mutable std::recursive_mutex keyset_mutex_;
google::crypto::tink::Keyset keyset_; // guarded by keyset_mutex_
crypto::tink::util::StatusOr<uint32_t> Add(
const google::crypto::tink::KeyTemplate& key_template, bool as_primary)
LOCKS_EXCLUDED(keyset_mutex_);

mutable absl::Mutex keyset_mutex_;
google::crypto::tink::Keyset keyset_ GUARDED_BY(keyset_mutex_);

// Generates a new key_id avoiding collisions in the managed keyset.
uint32_t GenerateNewKeyId();
uint32_t GenerateNewKeyId() SHARED_LOCKS_REQUIRED(keyset_mutex_);
};

} // namespace tink
Expand Down

0 comments on commit c57b27a

Please sign in to comment.