diff --git a/BUILD.gn b/BUILD.gn index 250ca73e4296a..e755ab827ad18 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -106,6 +106,7 @@ group("gn_all") { "//third_party/angle/src/tests:angle_end2end_tests", "//third_party/angle/src/tests:angle_unittests", "//third_party/angle/src/tests:angle_white_box_tests", + "//third_party/distributed_point_functions:distributed_point_functions_unittests", "//third_party/flatbuffers:flatbuffers_unittests", "//third_party/liburlpattern:liburlpattern_unittests", "//tools/binary_size:binary_size_trybot_py", diff --git a/content/browser/aggregation_service/DEPS b/content/browser/aggregation_service/DEPS index 4898e2d0e3e48..8589af7960650 100644 --- a/content/browser/aggregation_service/DEPS +++ b/content/browser/aggregation_service/DEPS @@ -1,4 +1,5 @@ include_rules = [ "+components/aggregation_service", - "+third_party/distributed_point_functions", + "+third_party/distributed_point_functions/shim", + "+third_party/distributed_point_functions/dpf/distributed_point_function.pb.h", ] diff --git a/content/browser/aggregation_service/aggregatable_report.cc b/content/browser/aggregation_service/aggregatable_report.cc index 66eb8838688e5..723a32c93de56 100644 --- a/content/browser/aggregation_service/aggregatable_report.cc +++ b/content/browser/aggregation_service/aggregatable_report.cc @@ -8,6 +8,7 @@ #include <stdint.h> #include <limits> +#include <optional> #include <ostream> #include <string> #include <type_traits> @@ -43,7 +44,8 @@ #include "third_party/abseil-cpp/absl/types/optional.h" #include "third_party/blink/public/mojom/private_aggregation/aggregatable_report.mojom.h" #include "third_party/boringssl/src/include/openssl/hpke.h" -#include "third_party/distributed_point_functions/code/dpf/distributed_point_function.h" +#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h" +#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h" #include "url/gurl.h" #include "url/origin.h" @@ -51,8 +53,6 @@ namespace content { namespace { -using DistributedPointFunction = - distributed_point_functions::DistributedPointFunction; using DpfKey = distributed_point_functions::DpfKey; using DpfParameters = distributed_point_functions::DpfParameters; @@ -112,31 +112,22 @@ std::vector<DpfKey> GenerateDpfKeys( blink::mojom::AggregationServiceMode::kExperimentalPoplar); DCHECK_EQ(contents.contributions.size(), 1u); - // absl::StatusOr is not allowed in the codebase, but this minimal usage is - // necessary to interact with //third_party/distributed_point_functions/. - absl::StatusOr<std::unique_ptr<DistributedPointFunction>> status_or_dpf = - DistributedPointFunction::CreateIncremental(ConstructDpfParameters()); - if (!status_or_dpf.ok()) { - return {}; - } - std::unique_ptr<DistributedPointFunction> dpf = - std::move(status_or_dpf).value(); - - // We want the same beta, no matter which prefix length is used. - absl::StatusOr<std::pair<DpfKey, DpfKey>> status_or_dpf_keys = - dpf->GenerateKeysIncremental( + std::optional<std::pair<DpfKey, DpfKey>> maybe_dpf_keys = + distributed_point_functions::GenerateKeysIncremental( + ConstructDpfParameters(), /*alpha=*/contents.contributions[0].bucket, - /*beta=*/std::vector<absl::uint128>( - AggregatableReport::kBucketDomainBitLength, - contents.contributions[0].value)); - if (!status_or_dpf_keys.ok()) { + // We want the same beta, no matter which prefix length is used. + /*beta=*/ + std::vector<absl::uint128>(AggregatableReport::kBucketDomainBitLength, + contents.contributions[0].value)); + + if (!maybe_dpf_keys.has_value()) { return {}; } std::vector<DpfKey> dpf_keys; - dpf_keys.push_back(std::move(status_or_dpf_keys->first)); - dpf_keys.push_back(std::move(status_or_dpf_keys->second)); - + dpf_keys.push_back(std::move(maybe_dpf_keys->first)); + dpf_keys.push_back(std::move(maybe_dpf_keys->second)); return dpf_keys; } diff --git a/third_party/distributed_point_functions/BUILD.gn b/third_party/distributed_point_functions/BUILD.gn index 4735400c69458..9eec02360165a 100644 --- a/third_party/distributed_point_functions/BUILD.gn +++ b/third_party/distributed_point_functions/BUILD.gn @@ -3,14 +3,9 @@ # found in the LICENSE file. import("//testing/libfuzzer/fuzzer_test.gni") +import("//testing/test.gni") import("//third_party/protobuf/proto_library.gni") -proto_library("proto") { - sources = [ "code/dpf/distributed_point_function.proto" ] - proto_out_dir = "third_party/distributed_point_functions/dpf" - cc_generator_options = "lite" -} - if (!defined(dpf_abseil_cpp_dir)) { dpf_abseil_cpp_dir = "//third_party/abseil-cpp" } @@ -18,7 +13,54 @@ if (!defined(dpf_highway_cpp_dir)) { dpf_highway_cpp_dir = "//third_party/highway" } -config("distributed_point_functions_includes") { +# This is Chromium's interface with the third-party distributed_point_functions +# library. It prevents macros like absl/log/check.h's CHECK from leaking into +# Chromium code via header includes. Chromium already defines CHECK in +# base/check.h, so direct includes can lead to macro redefinition errors. For +# more context, see <https://crbug.com/1499970>. +static_library("distributed_point_functions") { + sources = [ + "shim/distributed_point_function_shim.cc", + "shim/distributed_point_function_shim.h", + ] + deps = [ ":internal" ] + public_deps = [ ":proto" ] + configs += [ ":includes" ] +} + +proto_library("proto") { + sources = [ "code/dpf/distributed_point_function.proto" ] + proto_out_dir = "third_party/distributed_point_functions/dpf" + cc_generator_options = "lite" +} + +test("distributed_point_functions_unittests") { + sources = [ "shim/distributed_point_function_shim_unittest.cc" ] + deps = [ + ":distributed_point_functions", + "$dpf_abseil_cpp_dir:absl", + "//testing/gtest", + "//testing/gtest:gtest_main", + "//third_party/protobuf:protobuf_lite", + ] +} + +fuzzer_test("dpf_fuzzer") { + sources = [ "fuzz/dpf_fuzzer.cc" ] + deps = [ ":internal" ] + + # Do not apply Chromium code rules to this third-party code. + suppressed_configs = [ "//build/config/compiler:chromium_code" ] + additional_configs = [ "//build/config/compiler:no_chromium_code" ] + + additional_configs += [ ":includes" ] +} + +# The targets below this line are internal-only; they should only be visible to +# targets in this directory. +visibility = [ ":*" ] + +config("includes") { include_dirs = [ ".", "code", @@ -26,7 +68,7 @@ config("distributed_point_functions_includes") { ] } -source_set("distributed_point_functions") { +source_set("internal") { sources = [ "code/dpf/aes_128_fixed_key_hash.cc", "code/dpf/aes_128_fixed_key_hash.h", @@ -61,14 +103,5 @@ source_set("distributed_point_functions") { configs -= [ "//build/config/compiler:chromium_code" ] configs += [ "//build/config/compiler:no_chromium_code" ] - public_configs = [ ":distributed_point_functions_includes" ] -} - -fuzzer_test("dpf_fuzzer") { - sources = [ "fuzz/dpf_fuzzer.cc" ] - deps = [ ":distributed_point_functions" ] - - # Do not apply Chromium code rules to this third-party code. - suppressed_configs = [ "//build/config/compiler:chromium_code" ] - additional_configs = [ "//build/config/compiler:no_chromium_code" ] + configs += [ ":includes" ] } diff --git a/third_party/distributed_point_functions/DEPS b/third_party/distributed_point_functions/DEPS index c99f58cef539f..f0b5eeb46d24f 100644 --- a/third_party/distributed_point_functions/DEPS +++ b/third_party/distributed_point_functions/DEPS @@ -6,6 +6,7 @@ include_rules = [ "+gmock", "+google/protobuf", "+gtest", + "+testing", "+hwy", "+openssl", ] diff --git a/third_party/distributed_point_functions/shim/distributed_point_function_shim.cc b/third_party/distributed_point_functions/shim/distributed_point_function_shim.cc new file mode 100644 index 0000000000000..19e736a325399 --- /dev/null +++ b/third_party/distributed_point_functions/shim/distributed_point_function_shim.cc @@ -0,0 +1,40 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <memory> +#include <optional> +#include <utility> +#include <vector> + +#include "third_party/abseil-cpp/absl/numeric/int128.h" +#include "third_party/abseil-cpp/absl/status/status.h" +#include "third_party/abseil-cpp/absl/status/statusor.h" +#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h" +#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h" + +// NOTE: To avoid name-colliding macros named "CHECK", this header must not be +// transitively included into a compilation unit that also uses base/check.h. +// See <https://crbug.com/1499970>. +#include "third_party/distributed_point_functions/code/dpf/distributed_point_function.h" + +namespace distributed_point_functions { +std::optional<std::pair<DpfKey, DpfKey>> GenerateKeysIncremental( + std::vector<DpfParameters> parameters, + absl::uint128 alpha, + std::vector<absl::uint128> beta) { + // absl::StatusOr is not allowed in the codebase, but this minimal usage is + // necessary to interact with //third_party/distributed_point_functions/. + absl::StatusOr<std::unique_ptr<DistributedPointFunction>> status_or_dpf = + DistributedPointFunction::CreateIncremental(std::move(parameters)); + if (!status_or_dpf.ok() || !status_or_dpf.value()) { + return std::nullopt; + } + absl::StatusOr<std::pair<DpfKey, DpfKey>> status_or_dpf_keys = + status_or_dpf.value()->GenerateKeysIncremental(alpha, std::move(beta)); + if (!status_or_dpf_keys.ok()) { + return std::nullopt; + } + return std::move(status_or_dpf_keys.value()); +} +} // namespace distributed_point_functions diff --git a/third_party/distributed_point_functions/shim/distributed_point_function_shim.h b/third_party/distributed_point_functions/shim/distributed_point_function_shim.h new file mode 100644 index 0000000000000..bd5ffbae76dac --- /dev/null +++ b/third_party/distributed_point_functions/shim/distributed_point_function_shim.h @@ -0,0 +1,30 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_ +#define CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_ + +#include <optional> +#include <utility> +#include <vector> + +#include "third_party/abseil-cpp/absl/numeric/int128.h" +#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h" + +// This header is the interface between Chromium code and the third-party +// distributed_point_functions library. For more info, see the comment in +// ../BUILD.gn above the //third_party/distributed_point_functions target. + +namespace distributed_point_functions { + +// Generates a pair of keys for a DPF that evaluates to `beta` when given +// `alpha`. On failure, returns std::nullopt. +std::optional<std::pair<DpfKey, DpfKey>> GenerateKeysIncremental( + std::vector<DpfParameters> parameters, + absl::uint128 alpha, + std::vector<absl::uint128> beta); + +} // namespace distributed_point_functions + +#endif // CONTENT_BROWSER_AGGREGATION_SERVICE_DISTRIBUTED_POINT_FUNCTION_SHIM_H_ diff --git a/third_party/distributed_point_functions/shim/distributed_point_function_shim_unittest.cc b/third_party/distributed_point_functions/shim/distributed_point_function_shim_unittest.cc new file mode 100644 index 0000000000000..d39dcb9911b5c --- /dev/null +++ b/third_party/distributed_point_functions/shim/distributed_point_function_shim_unittest.cc @@ -0,0 +1,52 @@ +// Copyright 2023 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include <stddef.h> + +#include <optional> +#include <utility> +#include <vector> + +#include "testing/gtest/include/gtest/gtest.h" +#include "third_party/abseil-cpp/absl/numeric/int128.h" +#include "third_party/distributed_point_functions/dpf/distributed_point_function.pb.h" +#include "third_party/distributed_point_functions/shim/distributed_point_function_shim.h" + +namespace distributed_point_functions { + +// The shim's GenerateKeysIncremental() can return a value besides std::nullopt. +TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalConstructsKeys) { + constexpr size_t kBitLength = 32; + std::vector<DpfParameters> parameters(kBitLength); + for (size_t i = 0; i < parameters.size(); ++i) { + parameters[i].set_log_domain_size(i + 1); + parameters[i].mutable_value_type()->mutable_integer()->set_bitsize( + parameters.size()); + } + std::optional<std::pair<DpfKey, DpfKey>> maybe_dpf_keys = + GenerateKeysIncremental( + std::move(parameters), + /*alpha=*/absl::uint128{1}, + /*beta=*/std::vector<absl::uint128>(kBitLength, absl::uint128{1})); + EXPECT_TRUE(maybe_dpf_keys.has_value()); +} + +// When DistributedPointFunction::CreateIncremental() fails, the shim's +// GenerateKeysIncremental() should return std::nullopt. +TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalEmptyParameters) { + EXPECT_FALSE(GenerateKeysIncremental(/*parameters=*/{}, + /*alpha=*/absl::uint128{}, /*beta=*/{})); +} + +// When the length of beta does not match the number of parameters, the internal +// call to DistributedPointFunction::GenerateKeysIncremental() will fail, and +// the shim's GenerateKeysIncremental() should return std::nullopt. +TEST(DistributedPointFunctionShimTest, GenerateKeysIncrementalBetaWrongSize) { + std::vector<DpfParameters> parameters(3); + EXPECT_FALSE( + GenerateKeysIncremental(/*parameters=*/std::vector<DpfParameters>(3), + /*alpha=*/absl::uint128{}, /*beta=*/{1, 2, 3})); +} + +} // namespace distributed_point_functions