New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 719073 link

Starred by 2 users

Issue metadata

Status: Archived
Owner:
Last visit 15 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug



Sign in to add a comment

Remove dbus-c++ dependency from chaps

Project Member Reported by ejcaruso@chromium.org, May 5 2017

Issue description

chaps still uses dbus-c++, which we're trying to get rid of. Refactor it to use chromeos-dbus-bindings.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2fe96e4c1a555f958613ee6c1c8bf88d53684fc5

commit 2fe96e4c1a555f958613ee6c1c8bf88d53684fc5
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat May 13 03:57:57 2017

chaps: canonicalize XML D-Bus files

The XML specification for the D-Bus interface generally goes in
a dbus_bindings directory for each of our daemons. On top of that,
the file is usually named <service name>.xml. Rename the file and
to fit in with the rest of the daemons and fix the include
directives.

BUG= chromium:719073 
TEST=emerge

Change-Id: I73c477108bc380b96080515758dd1f9c174591ee
Reviewed-on: https://chromium-review.googlesource.com/500667
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/2fe96e4c1a555f958613ee6c1c8bf88d53684fc5/chaps/chaps.gyp
[modify] https://crrev.com/2fe96e4c1a555f958613ee6c1c8bf88d53684fc5/chaps/chaps_proxy.h
[modify] https://crrev.com/2fe96e4c1a555f958613ee6c1c8bf88d53684fc5/chaps/chaps_adaptor.h
[rename] https://crrev.com/2fe96e4c1a555f958613ee6c1c8bf88d53684fc5/chaps/dbus_bindings/org.chromium.Chaps.xml

Project Member

Comment 2 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/073253384288ded6dbe1d5ed96aea7820887c13a

commit 073253384288ded6dbe1d5ed96aea7820887c13a
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat May 13 03:57:58 2017

chaps: use protos to serialize CK_*_INFO structs

Packaging these arguments into protos simplifies a lot of
code and makes the D-Bus interface a lot less messy.

CQ-DEPEND=CL:505211
BUG= chromium:719073 
TEST=unit tests

Change-Id: Iecd68eda02a0b316b99b8736d46f241e5bc7cd11
Reviewed-on: https://chromium-review.googlesource.com/503551
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_service_redirect.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_proxy.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_service_redirect.cc
[add] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/proto_conversion.h
[add] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/proto_conversion.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_interface.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_test.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chapsd_test.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_adaptor.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps.gyp
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/cryptohome/cryptohome.gyp
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_service_test.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_adaptor.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/dbus_bindings/org.chromium.Chaps.xml
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_proxy_mock.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_service.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_service.h
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps_proxy.cc
[modify] https://crrev.com/073253384288ded6dbe1d5ed96aea7820887c13a/chaps/chaps.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/140fbe285fd4cd002dfab5d5918536a5590478a3

commit 140fbe285fd4cd002dfab5d5918536a5590478a3
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat May 13 03:57:57 2017

system_api: add chaps proto headers

These proto headers will be included by chaps and cryptohome
in a future CL.

CQ-DEPEND=CL:505176
BUG= chromium:719073 
TEST=emerge

Change-Id: I59c4a65581022637f95e65e16efa90dad24a0a4d
Reviewed-on: https://chromium-review.googlesource.com/505210
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/140fbe285fd4cd002dfab5d5918536a5590478a3/chromeos-base/system_api/system_api-9999.ebuild

Project Member

Comment 4 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/5c0356c2c1fde90183c09f5c3848a04f6c90ddee

commit 5c0356c2c1fde90183c09f5c3848a04f6c90ddee
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat May 13 03:57:58 2017

chaps, cryptohome: add dependency on system_api

Protos introduced in CL:505176 need to be included by both
chaps and cryptohome, as they reference the ChapsInterface
which uses them.

CQ-DEPEND=CL:503551
BUG= chromium:719073 
TEST=emerge

Change-Id: I51c94d9b5e775aa4cfca4ccf00b93a4a00c17d91
Reviewed-on: https://chromium-review.googlesource.com/505211
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/5c0356c2c1fde90183c09f5c3848a04f6c90ddee/chromeos-base/chaps/chaps-9999.ebuild
[modify] https://crrev.com/5c0356c2c1fde90183c09f5c3848a04f6c90ddee/chromeos-base/cryptohome/cryptohome-9999.ebuild

Project Member

Comment 5 by bugdroid1@chromium.org, May 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/system_api/+/ce280bc0393d706fea14d5ca32369986fdd01850

commit ce280bc0393d706fea14d5ca32369986fdd01850
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat May 13 03:57:57 2017

system_api: add protos for pkcs11 structs

Some chaps D-Bus calls return what would normally be a crypto
token interface struct, but split into many out-arguments. This
is really gross, so we should package them into protobufs instead.

BUG= chromium:719073 
TEST=emerge

Change-Id: I05851ea58dcb4c519456fa4834459bf257ac868d
Reviewed-on: https://chromium-review.googlesource.com/505176
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[add] https://crrev.com/ce280bc0393d706fea14d5ca32369986fdd01850/dbus/chaps/OWNERS
[modify] https://crrev.com/ce280bc0393d706fea14d5ca32369986fdd01850/system_api.gyp
[add] https://crrev.com/ce280bc0393d706fea14d5ca32369986fdd01850/dbus/chaps/ck_structs.proto
[modify] https://crrev.com/ce280bc0393d706fea14d5ca32369986fdd01850/system_api.pc

chromeos-dbus-bindings might be a no-go, since I just recently learned that chaps-on-linux is a thing and we're not going to have the ability to generate our bindings there. Using regular libchrome bindings should still be fine, though.
Project Member

Comment 7 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6595fb9f888f839f16f387d70810157be7859380

commit 6595fb9f888f839f16f387d70810157be7859380
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Fri May 19 20:57:57 2017

chaps: add D-Bus constants

Because of chaps-on-linux, we can't use chromeos-dbus-bindings to
generate the D-Bus adaptors and proxies for chaps, and we're going
to need to use the libchrome bindings and libbrillo helpers
directly. This means we need to put the strings for method names
somewhere. Luckily, the proxy and adaptor implementations both
live in this repo so we don't need to export it in system_api, and
we can just use them internally in chaps.

BUG= chromium:719073 
TEST=emerge

Change-Id: Ie804d539fddfd75504e334e164a72eb0fb2ed7b4
Reviewed-on: https://chromium-review.googlesource.com/508254
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[add] https://crrev.com/6595fb9f888f839f16f387d70810157be7859380/chaps/dbus_bindings/constants.h
[modify] https://crrev.com/6595fb9f888f839f16f387d70810157be7859380/chaps/chaps_adaptor.cc
[modify] https://crrev.com/6595fb9f888f839f16f387d70810157be7859380/chaps/chaps_proxy.cc
[modify] https://crrev.com/6595fb9f888f839f16f387d70810157be7859380/chaps/chaps_utility.cc
[modify] https://crrev.com/6595fb9f888f839f16f387d70810157be7859380/chaps/chaps.h

Project Member

Comment 8 by bugdroid1@chromium.org, May 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/deaae1b337c18734dcc4e99780036830d8aa64b0

commit deaae1b337c18734dcc4e99780036830d8aa64b0
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Thu May 25 19:25:21 2017

chaps: simplify ChapsAdaptor

This uses the libbrillo D-Bus utils and libchrome's bindings
instead of dbus-c++ and its code generator.

BUG= chromium:719073 
TEST=emerge, use pkcs11-tool on DUT, run autotests:
  platform_Pkcs11Events platform_Pkcs11LoadPerf
  platform_Pkcs11InitOnLogin platform_Pkcs11ChangeAuthData
  platform_TPMEvict platform_Pkcs11InitUnderErrors
  introduce delay in extra init threads, send signals and
  ensure the daemon still cleans itself up properly

Change-Id: Ib844b7e82a9c4dfe3be90f8fb8618d9d6559c474
Reviewed-on: https://chromium-review.googlesource.com/508255
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>

[modify] https://crrev.com/deaae1b337c18734dcc4e99780036830d8aa64b0/chaps/chaps.gyp
[modify] https://crrev.com/deaae1b337c18734dcc4e99780036830d8aa64b0/chaps/chaps_adaptor.cc
[modify] https://crrev.com/deaae1b337c18734dcc4e99780036830d8aa64b0/chaps/chaps_adaptor.h
[modify] https://crrev.com/deaae1b337c18734dcc4e99780036830d8aa64b0/chaps/chapsd.cc
[modify] https://crrev.com/deaae1b337c18734dcc4e99780036830d8aa64b0/chaps/Makefile

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/91f6440261bb2bb7ed7037fb9f6e956bf5db91e9

commit 91f6440261bb2bb7ed7037fb9f6e956bf5db91e9
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Sat Jun 17 08:12:42 2017

chaps: don't manually unmarshal protos in adaptor

The libbrillo bindings do have support for taking protos directly,
if they are subclasses of MessageLite (which all chaps protots are).
This means we don't have to take a vector and then unmarshal it;
the libbrillo bindings will do that if the method we register takes
a MessageLite subclass.

BUG= chromium:719073 
TEST=unit tests, list slots, tokens and mechanisms with pkcs11-tool

Change-Id: I02e4b999f657a8a59aae9e43b65ae50d17e72b01
Reviewed-on: https://chromium-review.googlesource.com/538855
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/91f6440261bb2bb7ed7037fb9f6e956bf5db91e9/chaps/chaps_adaptor.cc
[modify] https://crrev.com/91f6440261bb2bb7ed7037fb9f6e956bf5db91e9/chaps/chaps_adaptor.h

Project Member

Comment 10 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/85644bff23e5bad6bbe6b12dd607dcd997dcd278

commit 85644bff23e5bad6bbe6b12dd607dcd997dcd278
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Aug 22 05:03:53 2017

chaps: remove dbus-c++ dependency

CQ-DEPEND=CL:537958
BUG= chromium:719073 
TEST=emerge

Change-Id: I7c2219668fbeaf8ff2dbc5ceb1280146dddefbe6
Reviewed-on: https://chromium-review.googlesource.com/624722
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>

[modify] https://crrev.com/85644bff23e5bad6bbe6b12dd607dcd997dcd278/chromeos-base/chaps/chaps-9999.ebuild

Project Member

Comment 11 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/b1dc1722fcdb8bc3b0d993decc05e4e781750eda

commit b1dc1722fcdb8bc3b0d993decc05e4e781750eda
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Aug 22 05:03:52 2017

chaps: refactor ChapsProxy to stop using dbus-c++

Instead of using dbus-c++ generated bindings, use libbrillo
bindings to call methods and extract the parts of the reply.
This removes the rest of the dbus-c++ dependency.

BUG= chromium:719073 
TEST=platform TPM/PKCS11 autotests

Change-Id: I8f7730fb7206d90509d5949d023f37938c608aa9
Reviewed-on: https://chromium-review.googlesource.com/537957
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>

[modify] https://crrev.com/b1dc1722fcdb8bc3b0d993decc05e4e781750eda/chaps/chaps_proxy.h
[modify] https://crrev.com/b1dc1722fcdb8bc3b0d993decc05e4e781750eda/chaps/chaps_proxy.cc
[modify] https://crrev.com/b1dc1722fcdb8bc3b0d993decc05e4e781750eda/chaps/chaps.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Aug 22 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/6ec699954fc302d40f50a7ed0f61097119b65424

commit 6ec699954fc302d40f50a7ed0f61097119b65424
Author: Eric Caruso <ejcaruso@chromium.org>
Date: Tue Aug 22 05:03:52 2017

chaps: remove dbus-c++ and exceptions from gyp file

Also fix a comment that still references dbus-c++.

BUG= chromium:719073 
TEST=emerge

Change-Id: I4197e325b34391da574fabe43422f8ed86c8ebc1
Reviewed-on: https://chromium-review.googlesource.com/537958
Commit-Ready: Eric Caruso <ejcaruso@chromium.org>
Tested-by: Eric Caruso <ejcaruso@chromium.org>
Reviewed-by: Andrey Pronin <apronin@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/6ec699954fc302d40f50a7ed0f61097119b65424/chaps/chaps.gyp
[modify] https://crrev.com/6ec699954fc302d40f50a7ed0f61097119b65424/chaps/chaps_interface.h

Status: Fixed (was: Assigned)
Summary: Remove dbus-c++ dependency from chaps (was: Remove dbus-c++ dependency from chapsd)

Comment 14 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment