From af59cc8ae82bbae4b4856583a9f2fd2572d69b8e Mon Sep 17 00:00:00 2001 From: Lutz Justen Date: Fri, 25 Jan 2019 14:19:45 +0000 Subject: [PATCH] Revert "Use install_attributes.proto from system_api" This reverts commit b0afc521fa85f664f3de0d2c13a99990c4944097. Reason for revert: Didn't realize that the Chrome OS protofiles ebuild and libbrillo still used components/policy/proto/install_attributes.proto. Pointing libbrillo to the system_api version caused build failures for Tael. Going for a different solution now: Make all of Chrome OS use the Chrome version of the file. Original change's description: > Use install_attributes.proto from system_api > > Gets rid of Chrome's own copy of install_attributes.proto and uses the > one from system_api instead, which was recently moved there by > CL:1402785. > > BUG=chromium:775087 > TEST=Tryjobs > > Change-Id: I5e9166eaee380d9f0e47c3312f4f19cb4ef62d72 > Reviewed-on: https://chromium-review.googlesource.com/c/1402876 > Commit-Queue: Lutz Justen > Reviewed-by: Sergey Poromov > Reviewed-by: Steven Bennetts > Reviewed-by: Roman Sorokin [CET] > Cr-Commit-Position: refs/heads/master@{#624055} TBR=stevenjb@chromium.org,rsorokin@chromium.org,ljusten@chromium.org,poromov@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: chromium:775087 Change-Id: Ic56cfecdf94fd9d2dfe0c41c46158120e7149cd2 Reviewed-on: https://chromium-review.googlesource.com/c/1436048 Reviewed-by: Lutz Justen Commit-Queue: Lutz Justen Auto-Submit: Lutz Justen Cr-Commit-Position: refs/heads/master@{#626071} --- chromeos/dbus/BUILD.gn | 1 - chromeos/dbus/fake_cryptohome_client.cc | 8 +++----- chromeos/tpm/install_attributes.cc | 8 ++------ chromeos/tpm/install_attributes_unittest.cc | 6 +----- components/policy/proto/BUILD.gn | 5 ++++- .../policy/proto/install_attributes.proto | 20 +++++++++++++++++++ 6 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 components/policy/proto/install_attributes.proto diff --git a/chromeos/dbus/BUILD.gn b/chromeos/dbus/BUILD.gn index 76185613230121..3978a5f674ea9e 100644 --- a/chromeos/dbus/BUILD.gn +++ b/chromeos/dbus/BUILD.gn @@ -362,7 +362,6 @@ proto_library("concierge_proto") { proto_library("cryptohome_proto") { sources = [ - "//third_party/cros_system_api/dbus/cryptohome/install_attributes.proto", "//third_party/cros_system_api/dbus/cryptohome/key.proto", "//third_party/cros_system_api/dbus/cryptohome/rpc.proto", ] diff --git a/chromeos/dbus/fake_cryptohome_client.cc b/chromeos/dbus/fake_cryptohome_client.cc index b8ab1368aad796..795006987dea4e 100644 --- a/chromeos/dbus/fake_cryptohome_client.cc +++ b/chromeos/dbus/fake_cryptohome_client.cc @@ -20,10 +20,10 @@ #include "base/threading/thread_task_runner_handle.h" #include "chromeos/dbus/attestation/attestation.pb.h" #include "chromeos/dbus/constants/dbus_paths.h" -#include "chromeos/dbus/cryptohome/install_attributes.pb.h" #include "chromeos/dbus/cryptohome/key.pb.h" #include "chromeos/dbus/cryptohome/rpc.pb.h" #include "chromeos/dbus/util/account_identifier_operators.h" +#include "components/policy/proto/install_attributes.pb.h" #include "third_party/cros_system_api/dbus/service_constants.h" namespace chromeos { @@ -280,10 +280,8 @@ bool FakeCryptohomeClient::InstallAttributesFinalize(bool* successful) { value.data() + value.size()); } - // Set default version (note that version is required). - install_attrs_proto.set_version(install_attrs_proto.version()); - std::string result = install_attrs_proto.SerializeAsString(); - DCHECK(!result.empty()); + std::string result; + install_attrs_proto.SerializeToString(&result); // The real implementation does a blocking wait on the dbus call; the fake // implementation must have this file written before returning. diff --git a/chromeos/tpm/install_attributes.cc b/chromeos/tpm/install_attributes.cc index 151df581f58f76..8ba4772b520469 100644 --- a/chromeos/tpm/install_attributes.cc +++ b/chromeos/tpm/install_attributes.cc @@ -21,10 +21,10 @@ #include "base/threading/thread_task_runner_handle.h" #include "base/time/time.h" #include "chromeos/dbus/constants/dbus_paths.h" -#include "chromeos/dbus/cryptohome/install_attributes.pb.h" #include "chromeos/dbus/cryptohome/rpc.pb.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/util/tpm_util.h" +#include "components/policy/proto/install_attributes.pb.h" #include "google_apis/gaia/gaia_auth_util.h" #include "third_party/cros_system_api/dbus/service_constants.h" @@ -138,11 +138,7 @@ InstallAttributes::GetEnterpriseOwnedInstallAttributesBlobForTesting( attribute->set_name(InstallAttributes::kAttrEnterpriseUser); attribute->set_value(user_name); - // Set default version (note that version is required). - install_attrs_proto.set_version(install_attrs_proto.version()); - std::string result = install_attrs_proto.SerializeAsString(); - DCHECK(!result.empty()); - return result; + return install_attrs_proto.SerializeAsString(); } InstallAttributes::InstallAttributes(CryptohomeClient* cryptohome_client) diff --git a/chromeos/tpm/install_attributes_unittest.cc b/chromeos/tpm/install_attributes_unittest.cc index 6ce8c31a04f284..5002a28ad1ae7e 100644 --- a/chromeos/tpm/install_attributes_unittest.cc +++ b/chromeos/tpm/install_attributes_unittest.cc @@ -15,11 +15,11 @@ #include "base/test/metrics/histogram_tester.h" #include "base/test/scoped_task_environment.h" #include "chromeos/dbus/constants/dbus_paths.h" -#include "chromeos/dbus/cryptohome/install_attributes.pb.h" #include "chromeos/dbus/cryptohome/rpc.pb.h" #include "chromeos/dbus/cryptohome_client.h" #include "chromeos/dbus/dbus_thread_manager.h" #include "chromeos/dbus/util/tpm_util.h" +#include "components/policy/proto/install_attributes.pb.h" #include "google_apis/gaia/gaia_auth_util.h" #include "testing/gtest/include/gtest/gtest.h" @@ -288,8 +288,6 @@ TEST_F(InstallAttributesTest, Init) { "true"); SetAttribute(&install_attrs_proto, InstallAttributes::kAttrEnterpriseUser, kTestUserDeprecated); - // Set default version (note that version is required). - install_attrs_proto.set_version(install_attrs_proto.version()); const std::string blob(install_attrs_proto.SerializeAsString()); ASSERT_EQ(static_cast(blob.size()), base::WriteFile(GetTempPath(), blob.c_str(), blob.size())); @@ -304,8 +302,6 @@ TEST_F(InstallAttributesTest, InitForConsumerKiosk) { cryptohome::SerializedInstallAttributes install_attrs_proto; SetAttribute(&install_attrs_proto, InstallAttributes::kAttrConsumerKioskEnabled, "true"); - // Set default version (note that version is required). - install_attrs_proto.set_version(install_attrs_proto.version()); const std::string blob(install_attrs_proto.SerializeAsString()); ASSERT_EQ(static_cast(blob.size()), base::WriteFile(GetTempPath(), blob.c_str(), blob.size())); diff --git a/components/policy/proto/BUILD.gn b/components/policy/proto/BUILD.gn index 7d6cb62f12b1fa..a7039ac9e50398 100644 --- a/components/policy/proto/BUILD.gn +++ b/components/policy/proto/BUILD.gn @@ -23,7 +23,10 @@ proto_library("proto_internal") { ] if (is_chromeos) { - sources += [ "chrome_device_policy.proto" ] + sources += [ + "chrome_device_policy.proto", + "install_attributes.proto", + ] } if (!is_android && !is_ios) { diff --git a/components/policy/proto/install_attributes.proto b/components/policy/proto/install_attributes.proto new file mode 100644 index 00000000000000..396794ba421668 --- /dev/null +++ b/components/policy/proto/install_attributes.proto @@ -0,0 +1,20 @@ +// Copyright 2013 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +syntax = "proto2"; + +option optimize_for = LITE_RUNTIME; + +package cryptohome; + +// This must match install_attributes.proto in the Chromium OS cryptohome +// repository. Only the fields that are relevant to the browser are present +// here. +message SerializedInstallAttributes { + message Attribute { + required string name = 1; + required bytes value = 2; + } + repeated Attribute attributes = 2; +}