From 0006a625b1e202a2489b6c6911fde8d9accd1f84 Mon Sep 17 00:00:00 2001 From: Benjamin Wright Date: Fri, 12 Apr 2019 14:35:13 -0700 Subject: [PATCH] Remove HKDF implementation from WebRTC. We no longer have a need for a HKDF implementation in WebRTC. To keep code quality high it makes sense to delete this dead code path. Bug: webrtc:9600 Change-Id: Ibe6ee9150acd9dbf59452372242d857c5ffa65c4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132802 Reviewed-by: Peter Slatala Reviewed-by: Qingsi Wang Reviewed-by: Seth Hampson Commit-Queue: Benjamin Wright Cr-Commit-Position: refs/heads/master@{#27604} --- pc/jsep_transport_controller.cc | 1 - rtc_base/BUILD.gn | 5 - rtc_base/key_derivation.cc | 32 ------ rtc_base/key_derivation.h | 72 ------------ rtc_base/openssl_key_derivation_hkdf.cc | 108 ------------------ rtc_base/openssl_key_derivation_hkdf.h | 54 --------- .../openssl_key_derivation_hkdf_unittest.cc | 107 ----------------- 7 files changed, 379 deletions(-) delete mode 100644 rtc_base/key_derivation.cc delete mode 100644 rtc_base/key_derivation.h delete mode 100644 rtc_base/openssl_key_derivation_hkdf.cc delete mode 100644 rtc_base/openssl_key_derivation_hkdf.h delete mode 100644 rtc_base/openssl_key_derivation_hkdf_unittest.cc diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index e92dab5925..fd9551a328 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -21,7 +21,6 @@ #include "pc/srtp_filter.h" #include "rtc_base/bind.h" #include "rtc_base/checks.h" -#include "rtc_base/key_derivation.h" #include "rtc_base/thread.h" using webrtc::SdpType; diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 05cecd178a..59eca29cf9 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -825,8 +825,6 @@ rtc_static_library("rtc_base") { "ip_address.cc", "ip_address.h", "keep_ref_until_done.h", - "key_derivation.cc", - "key_derivation.h", "mdns_responder_interface.h", "message_digest.cc", "message_digest.h", @@ -855,8 +853,6 @@ rtc_static_library("rtc_base") { "openssl_digest.h", "openssl_identity.cc", "openssl_identity.h", - "openssl_key_derivation_hkdf.cc", - "openssl_key_derivation_hkdf.h", "openssl_session_cache.cc", "openssl_session_cache.h", "openssl_stream_adapter.cc", @@ -1363,7 +1359,6 @@ if (rtc_include_tests) { if (is_posix || is_fuchsia) { sources += [ "openssl_adapter_unittest.cc", - "openssl_key_derivation_hkdf_unittest.cc", "openssl_session_cache_unittest.cc", "openssl_utility_unittest.cc", "ssl_adapter_unittest.cc", diff --git a/rtc_base/key_derivation.cc b/rtc_base/key_derivation.cc deleted file mode 100644 index 6c9a254646..0000000000 --- a/rtc_base/key_derivation.cc +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2018 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "rtc_base/key_derivation.h" - -#include "absl/memory/memory.h" -#include "rtc_base/checks.h" -#include "rtc_base/openssl_key_derivation_hkdf.h" - -namespace rtc { - -KeyDerivation::KeyDerivation() = default; -KeyDerivation::~KeyDerivation() = default; - -// static -std::unique_ptr KeyDerivation::Create( - KeyDerivationAlgorithm key_derivation_algorithm) { - switch (key_derivation_algorithm) { - case KeyDerivationAlgorithm::HKDF_SHA256: - return absl::make_unique(); - } - RTC_NOTREACHED(); -} - -} // namespace rtc diff --git a/rtc_base/key_derivation.h b/rtc_base/key_derivation.h deleted file mode 100644 index f35e7ff990..0000000000 --- a/rtc_base/key_derivation.h +++ /dev/null @@ -1,72 +0,0 @@ -/* - * Copyright 2018 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef RTC_BASE_KEY_DERIVATION_H_ -#define RTC_BASE_KEY_DERIVATION_H_ - -#include -#include -#include - -#include "absl/types/optional.h" -#include "api/array_view.h" -#include "rtc_base/buffer.h" -#include "rtc_base/constructor_magic.h" - -namespace rtc { - -// Defines the set of key derivation algorithms that are supported. It is ideal -// to keep this list as small as possible. -enum class KeyDerivationAlgorithm { - // This algorithm is not suitable to generate a key from a password. Please - // only use with a cryptographically random master secret. - HKDF_SHA256 -}; - -// KeyDerivation provides a generic interface for deriving keys in WebRTC. This -// class should be used over directly accessing openssl or boringssl primitives -// so that we can maintain seperate implementations. -// Example: -// auto kd = KeyDerivation::Create(KeyDerivationAlgorithm::HDKF_SHA526); -// if (kd == nullptr) return; -// auto derived_key_or = kd->DeriveKey(secret, salt, label); -// if (!derived_key_or.ok()) return; -// DoSomethingWithKey(derived_key_or.value()); -class KeyDerivation { - public: - KeyDerivation(); - virtual ~KeyDerivation(); - - // Derives a new key from existing key material. - // secret - The random secret value you wish to derive a key from. - // salt - Optional but recommended (non secret) cryptographically random. - // label - A non secret but unique label value to determine the derivation. - // derived_key_byte_size - This must be at least 128 bits. - // return - An optional ZeroOnFreeBuffer containing the derived key or - // absl::nullopt. Nullopt indicates a failure in derivation. - virtual absl::optional> DeriveKey( - rtc::ArrayView secret, - rtc::ArrayView salt, - rtc::ArrayView label, - size_t derived_key_byte_size) = 0; - - // Static factory that will return an implementation that is capable of - // handling the key derivation with the requested algorithm. If no - // implementation is available nullptr will be returned. - static std::unique_ptr Create( - KeyDerivationAlgorithm key_derivation_algorithm); - - private: - RTC_DISALLOW_COPY_AND_ASSIGN(KeyDerivation); -}; - -} // namespace rtc - -#endif // RTC_BASE_KEY_DERIVATION_H_ diff --git a/rtc_base/openssl_key_derivation_hkdf.cc b/rtc_base/openssl_key_derivation_hkdf.cc deleted file mode 100644 index d82f07bc4a..0000000000 --- a/rtc_base/openssl_key_derivation_hkdf.cc +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2018 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "rtc_base/openssl_key_derivation_hkdf.h" - -#include - -#include -#ifdef OPENSSL_IS_BORINGSSL -#include -#include -#else -#include -#include -#endif -#include -#include - -#include "absl/algorithm/container.h" -#include "rtc_base/buffer.h" -#include "rtc_base/openssl.h" - -namespace rtc { - -#ifndef OPENSSL_IS_BORINGSSL -namespace { - -// HKDF is static within OpenSSL and hence not accessible to the caller. -// This internal implementation allows for compatibility with BoringSSL. -static int HKDF(uint8_t* out_key, - size_t out_len, - const EVP_MD* digest, - const uint8_t* secret, - size_t secret_len, - const uint8_t* salt, - size_t salt_len, - const uint8_t* info, - size_t info_len) { - EVP_PKEY_CTX* pctx = EVP_PKEY_CTX_new_id(EVP_PKEY_HKDF, NULL); - - if (EVP_PKEY_derive_init(pctx) <= 0 || - EVP_PKEY_CTX_set_hkdf_md(pctx, digest) <= 0 || - EVP_PKEY_CTX_set1_hkdf_salt(pctx, salt, salt_len) <= 0 || - EVP_PKEY_CTX_set1_hkdf_key(pctx, secret, secret_len) <= 0 || - EVP_PKEY_CTX_add1_hkdf_info(pctx, info, info_len) <= 0 || - EVP_PKEY_derive(pctx, out_key, &out_len) <= 0) { - EVP_PKEY_CTX_free(pctx); - return 0; - } - EVP_PKEY_CTX_free(pctx); - return 1; -} - -} // namespace -#endif - -OpenSSLKeyDerivationHKDF::OpenSSLKeyDerivationHKDF() = default; -OpenSSLKeyDerivationHKDF::~OpenSSLKeyDerivationHKDF() = default; - -const size_t OpenSSLKeyDerivationHKDF::kMinKeyByteSize = 16; -const size_t OpenSSLKeyDerivationHKDF::kMaxKeyByteSize = - 255 * SHA256_DIGEST_LENGTH; -const size_t OpenSSLKeyDerivationHKDF::kMinSecretByteSize = 16; - -absl::optional> OpenSSLKeyDerivationHKDF::DeriveKey( - rtc::ArrayView secret, - rtc::ArrayView salt, - rtc::ArrayView label, - size_t derived_key_byte_size) { - // Prevent deriving less than 128 bits of key material or more than the max. - if (derived_key_byte_size < kMinKeyByteSize || - derived_key_byte_size > kMaxKeyByteSize) { - return absl::nullopt; - } - // The secret must reach the minimum number of bits to be secure. - if (secret.data() == nullptr || secret.size() < kMinSecretByteSize) { - return absl::nullopt; - } - // Empty labels are always invalid in derivation. - if (label.data() == nullptr || label.size() == 0) { - return absl::nullopt; - } - // If a random salt is not provided use all zeros. - rtc::Buffer salt_buffer; - if (salt.data() == nullptr || salt.size() == 0) { - salt_buffer.SetSize(SHA256_DIGEST_LENGTH); - absl::c_fill(salt_buffer, 0); - salt = salt_buffer; - } - // This buffer will erase itself on release. - ZeroOnFreeBuffer derived_key_buffer(derived_key_byte_size, 0); - if (!HKDF(derived_key_buffer.data(), derived_key_buffer.size(), EVP_sha256(), - secret.data(), secret.size(), salt.data(), salt.size(), - label.data(), label.size())) { - return absl::nullopt; - } - return absl::optional>( - std::move(derived_key_buffer)); -} - -} // namespace rtc diff --git a/rtc_base/openssl_key_derivation_hkdf.h b/rtc_base/openssl_key_derivation_hkdf.h deleted file mode 100644 index 7f882527e2..0000000000 --- a/rtc_base/openssl_key_derivation_hkdf.h +++ /dev/null @@ -1,54 +0,0 @@ -/* - * Copyright 2018 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef RTC_BASE_OPENSSL_KEY_DERIVATION_HKDF_H_ -#define RTC_BASE_OPENSSL_KEY_DERIVATION_HKDF_H_ - -#include "rtc_base/constructor_magic.h" -#include "rtc_base/key_derivation.h" - -namespace rtc { - -// OpenSSLKeyDerivationHKDF provides a concrete implementation of the -// KeyDerivation interface to support the HKDF algorithm using the -// OpenSSL/BoringSSL internal implementation. -class OpenSSLKeyDerivationHKDF final : public KeyDerivation { - public: - OpenSSLKeyDerivationHKDF(); - ~OpenSSLKeyDerivationHKDF() override; - - // General users shouldn't be generating keys smaller than 128 bits. - static const size_t kMinKeyByteSize; - // The maximum available derivation size 255*DIGEST_LENGTH - static const size_t kMaxKeyByteSize; - // The minimum acceptable secret size. - static const size_t kMinSecretByteSize; - - // Derives a new key from existing key material using HKDF. - // secret - The random secret value you wish to derive a key from. - // salt - Optional (non secret) cryptographically random value. - // label - A non secret but unique label value to determine the derivation. - // derived_key_byte_size - The size of the derived key. - // return - A ZeroOnFreeBuffer containing the derived key or an error - // condition. Checking error codes is explicit in the API and error should - // never be ignored. - absl::optional> DeriveKey( - rtc::ArrayView secret, - rtc::ArrayView salt, - rtc::ArrayView label, - size_t derived_key_byte_size) override; - - private: - RTC_DISALLOW_COPY_AND_ASSIGN(OpenSSLKeyDerivationHKDF); -}; - -} // namespace rtc - -#endif // RTC_BASE_OPENSSL_KEY_DERIVATION_HKDF_H_ diff --git a/rtc_base/openssl_key_derivation_hkdf_unittest.cc b/rtc_base/openssl_key_derivation_hkdf_unittest.cc deleted file mode 100644 index 92df42ffd3..0000000000 --- a/rtc_base/openssl_key_derivation_hkdf_unittest.cc +++ /dev/null @@ -1,107 +0,0 @@ -/* - * Copyright 2018 The WebRTC Project Authors. All rights reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "rtc_base/openssl_key_derivation_hkdf.h" - -#include - -#include "test/gmock.h" - -namespace rtc { -namespace { - -// Validates that a basic valid call works correctly. -TEST(OpenSSLKeyDerivationHKDF, DerivationBasicTest) { - rtc::Buffer secret(32); - rtc::Buffer salt(32); - rtc::Buffer label(32); - const size_t derived_key_byte_size = 16; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or = hkdf.DeriveKey(secret, salt, label, derived_key_byte_size); - EXPECT_TRUE(key_or.has_value()); - ZeroOnFreeBuffer key = std::move(key_or.value()); - EXPECT_EQ(derived_key_byte_size, key.size()); -} - -// Derivation fails if output is too small. -TEST(OpenSSLKeyDerivationHKDF, DerivationFailsIfOutputIsTooSmall) { - rtc::Buffer secret(32); - rtc::Buffer salt(32); - rtc::Buffer label(32); - const size_t derived_key_byte_size = 15; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or = hkdf.DeriveKey(secret, salt, label, derived_key_byte_size); - EXPECT_FALSE(key_or.has_value()); -} - -// Derivation fails if output is too large. -TEST(OpenSSLKeyDerivationHKDF, DerivationFailsIfOutputIsTooLarge) { - rtc::Buffer secret(32); - rtc::Buffer salt(32); - rtc::Buffer label(32); - const size_t derived_key_byte_size = 256 * 32; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or = hkdf.DeriveKey(secret, salt, label, derived_key_byte_size); - EXPECT_FALSE(key_or.has_value()); -} - -// Validates that too little key material causes a failure. -TEST(OpenSSLKeyDerivationHKDF, DerivationFailsWithInvalidSecret) { - rtc::Buffer secret(15); - rtc::Buffer salt(32); - rtc::Buffer label(32); - const size_t derived_key_byte_size = 16; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or_0 = hkdf.DeriveKey(secret, salt, label, derived_key_byte_size); - EXPECT_FALSE(key_or_0.has_value()); - - auto key_or_1 = hkdf.DeriveKey(nullptr, salt, label, derived_key_byte_size); - EXPECT_FALSE(key_or_1.has_value()); - - rtc::Buffer secret_empty; - auto key_or_2 = - hkdf.DeriveKey(secret_empty, salt, label, derived_key_byte_size); - EXPECT_FALSE(key_or_2.has_value()); -} - -// Validates that HKDF works without a salt being set. -TEST(OpenSSLKeyDerivationHKDF, DerivationWorksWithNoSalt) { - rtc::Buffer secret(32); - rtc::Buffer label(32); - const size_t derived_key_byte_size = 16; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or = hkdf.DeriveKey(secret, nullptr, label, derived_key_byte_size); - EXPECT_TRUE(key_or.has_value()); -} - -// Validates that a label is required to work correctly. -TEST(OpenSSLKeyDerivationHKDF, DerivationRequiresLabel) { - rtc::Buffer secret(32); - rtc::Buffer salt(32); - rtc::Buffer label(1); - const size_t derived_key_byte_size = 16; - - OpenSSLKeyDerivationHKDF hkdf; - auto key_or_0 = hkdf.DeriveKey(secret, salt, label, derived_key_byte_size); - EXPECT_TRUE(key_or_0.has_value()); - ZeroOnFreeBuffer key = std::move(key_or_0.value()); - EXPECT_EQ(key.size(), derived_key_byte_size); - - auto key_or_1 = hkdf.DeriveKey(secret, salt, nullptr, derived_key_byte_size); - EXPECT_FALSE(key_or_1.has_value()); -} - -} // namespace -} // namespace rtc