Attention is currently required from: Adam Langley.
1 comment:
Patchset:
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.
Attention is currently required from: Alex Ilin, Kristian Monsen.
Patch set 1:Code-Review +1
3 comments:
Patchset:
I'm fine with it if +kristianm is.
File crypto/unexportable_key.h:
Patch Set #1, Line 194: retuns
typo: "returns"
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.
Attention is currently required from: Adam Langley, Alex Ilin.
Patch set 1:Code-Review +1
1 comment:
Patchset:
I'm fine with it if +kristianm is.
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.
Attention is currently required from: Adam Langley.
Patch set 3:Commit-Queue +2
3 comments:
Patchset:
I think this is good as long as we make sure to not ship it outside development. […]
Thanks!
File crypto/unexportable_key.h:
Patch Set #1, Line 194: retuns
typo: "returns"
Done
File crypto/unexportable_key.cc:
Patch Set #1, Line 56: return GetUnexportableKeyProviderSoftwareUnsecure();
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.
Chromium LUCI CQ submitted this 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>();
}
```
[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(-)
Kristian Monsen has created a revert of this change.
To view, visit change 4675882. To unsubscribe, or for help writing mail filters, visit settings.
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |