[DBSC] Make it easier to test binding keys on Linux and Mac [chromium/src : main]

0 views
Skip to first unread message

Alex Ilin (Gerrit)

unread,
Jul 10, 2023, 2:21:08 PM7/10/23
to Alex Ilin, Adam Langley, chromium...@chromium.org

Attention is currently required from: Adam Langley.

View Change

1 comment:

  • Patchset:

    • Patch Set #1:

      Adam, WDYT about this CL?

      If you think it's too dangerous to expose an insecure implementation from //crypto, we could add more warning signs or/and make it usable only to restricted clients.

      If it's still not enough, I think I could add my own fake implementation to components/unexportable_keys that won't do real crypto but just return fake values.

To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Attention: Adam Langley <a...@chromium.org>
Gerrit-Comment-Date: Mon, 10 Jul 2023 18:21:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Adam Langley (Gerrit)

unread,
Jul 10, 2023, 6:05:58 PM7/10/23
to Alex Ilin, Kristian Monsen, Tricium, chromium...@chromium.org

Attention is currently required from: Alex Ilin, Kristian Monsen.

Patch set 1:Code-Review +1

View Change

3 comments:

  • Patchset:

  • File crypto/unexportable_key.h:

  • File crypto/unexportable_key.cc:

    • Patch Set #1, Line 56: return GetUnexportableKeyProviderSoftwareUnsecure();

      Could the function in `unexportable_key_software_unsecure.cc` just be called `GetSoftwareUnsecureUnexportableKeyProvider` to avoid needing this function?

To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Reviewer: Kristian Monsen <kris...@chromium.org>
Gerrit-Attention: Kristian Monsen <kris...@chromium.org>
Gerrit-Attention: Alex Ilin <alex...@chromium.org>
Gerrit-Comment-Date: Mon, 10 Jul 2023 22:05:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Kristian Monsen (Gerrit)

unread,
Jul 10, 2023, 6:19:07 PM7/10/23
to Alex Ilin, Adam Langley, Tricium, chromium...@chromium.org

Attention is currently required from: Adam Langley, Alex Ilin.

Patch set 1:Code-Review +1

View Change

1 comment:

  • Patchset:

    • I think this is good as long as we make sure to not ship it outside development.

      Ideally it should not be needed, but development on windows is such a pain these days and we want the server side team to be able to test their changes.

To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Reviewer: Kristian Monsen <kris...@chromium.org>
Gerrit-Attention: Adam Langley <a...@chromium.org>
Gerrit-Attention: Alex Ilin <alex...@chromium.org>
Gerrit-Comment-Date: Mon, 10 Jul 2023 22:18:59 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Adam Langley <a...@chromium.org>

Alex Ilin (Gerrit)

unread,
Jul 11, 2023, 5:27:18 AM7/11/23
to Alex Ilin, Kristian Monsen, Adam Langley, Tricium, chromium...@chromium.org

Attention is currently required from: Adam Langley.

Patch set 3:Commit-Queue +2

View Change

3 comments:

  • Patchset:

    • Patch Set #1:

      I think this is good as long as we make sure to not ship it outside development. […]

      Thanks!

  • File crypto/unexportable_key.h:

    • Done

  • File crypto/unexportable_key.cc:

    • Could the function in `unexportable_key_software_unsecure. […]

      Indeed. Thanks!

To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Reviewer: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Kristian Monsen <kris...@chromium.org>
Gerrit-Attention: Adam Langley <a...@chromium.org>
Gerrit-Comment-Date: Tue, 11 Jul 2023 09:27:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Kristian Monsen <kris...@chromium.org>
Comment-In-Reply-To: Adam Langley <a...@chromium.org>

Chromium LUCI CQ (Gerrit)

unread,
Jul 11, 2023, 6:18:09 AM7/11/23
to Alex Ilin, Kristian Monsen, Adam Langley, Tricium, chromium...@chromium.org

Chromium LUCI CQ submitted this change.

View Change



1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: crypto/unexportable_key.cc
Insertions: 0, Deletions: 5.

@@ -51,11 +51,6 @@
#endif
}

-std::unique_ptr<UnexportableKeyProvider>
-GetSoftwareUnsecureUnexportableKeyProvider() {
- return GetUnexportableKeyProviderSoftwareUnsecure();
-}
-
namespace internal {

void SetUnexportableKeyProviderForTesting(
```
```
The name of the file: crypto/unexportable_key.h
Insertions: 1, Deletions: 1.

@@ -191,7 +191,7 @@
CRYPTO_EXPORT std::unique_ptr<VirtualUnexportableKeyProvider>
GetVirtualUnexportableKeyProvider_DO_NOT_USE_METRICS_ONLY();

-// `GetSoftwareUnsecureUnexportableKeyProvider()` retuns a mock software
+// `GetSoftwareUnsecureUnexportableKeyProvider()` returns a mock software
// implementation of `UnexportableKeyProvider` that can be used on platforms
// that do not have a native secure implementation.
// This should be used for development purposes only since these keys are not
```
```
The name of the file: crypto/unexportable_key_software_unsecure.cc
Insertions: 1, Deletions: 1.

@@ -111,7 +111,7 @@
} // namespace

std::unique_ptr<UnexportableKeyProvider>
-GetUnexportableKeyProviderSoftwareUnsecure() {
+GetSoftwareUnsecureUnexportableKeyProvider() {
return std::make_unique<SoftwareProvider>();
}

```

Approvals: Adam Langley: Looks good to me Kristian Monsen: Looks good to me Alex Ilin: Commit
[DBSC] Make it easier to test binding keys on Linux and Mac

//crypto currently has an UnexportableKeyProvider support on Windows
only. Since most of the future developers work on Linux and Mac, it will
be helpful to be able to test the feature on these platforms.

To facilite local development, this CL adds a mock software-backed
implementation of an UnexportableKeyProvider behind a feature flag. This
feature flag is expected to never be shipped to end users.

The flag is checked on the UnexportableKeyService level, so only
DBSC-related code will switch to an insecure implementation.

Bug: b/290640296
Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4675882
Reviewed-by: Adam Langley <a...@chromium.org>
Reviewed-by: Kristian Monsen <kris...@chromium.org>
Commit-Queue: Alex Ilin <alex...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1168616}
---
M components/unexportable_keys/unexportable_key_service_impl.cc
M components/unexportable_keys/unexportable_key_task_manager.cc
M components/unexportable_keys/unexportable_key_task_manager.h
M crypto/BUILD.gn
M crypto/scoped_mock_unexportable_key_provider.cc
M crypto/unexportable_key.cc
M crypto/unexportable_key.h
A crypto/unexportable_key_software_unsecure.cc
8 files changed, 160 insertions(+), 104 deletions(-)


To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: merged
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Reviewer: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>

Kristian Monsen (Gerrit)

unread,
Jul 12, 2023, 12:15:33 PM7/12/23
to Chromium LUCI CQ, Alex Ilin, Adam Langley, Tricium, chromium...@chromium.org

Kristian Monsen has created a revert of this change.

View Change

To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-MessageType: revert

Beto Hernández (Gerrit)

unread,
8:35 PM (3 hours ago) 8:35 PM
to Alex Ilin, Chromium LUCI CQ, Kristian Monsen, Adam Langley, chromium...@chromium.org

Beto Hernández added 1 comment

Patchset-level comments
File-level comment, Patchset 4 (Latest):
Beto Hernández . resolved

Wifi

Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement satisfiedCode-Review
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: Iccd1d0c1733d3f5472380023377bd81df4e3f933
Gerrit-Change-Number: 4675882
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Adam Langley <a...@chromium.org>
Gerrit-Reviewer: Alex Ilin <alex...@chromium.org>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Reviewer: Kristian Monsen <kris...@chromium.org>
Gerrit-CC: Beto Hernández <betoher...@gmail.com>
Gerrit-CC: Deleted User
Gerrit-Comment-Date: Wed, 14 Jan 2026 01:35:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages