Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Secure Area Test App. #395

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Conversation

davidz25
Copy link
Contributor

This is taken from the experimental-cloud-secure-area branch and adapted to show just AndroidKeystoreSecureArea and SoftwareSecureArea (will rebase that branch for all three secure areas once this is merged).

Also rework how Android Keystore capabilities are reported and show these in the UI in the new SA test app.

Since it's now possible to test AndroidKeystoreSecureArea under various conditions (for example when a Secure Lock Screen has not been set up) it's easier to verify our error handling paths. To that end, fix up propogated exceptions so they are easier to parse from the top-level exception message and not just the cause.

Test: Manually tested

@davidz25 davidz25 force-pushed the android-keystore-capabilities branch from fb36e46 to fef76fd Compare October 27, 2023 16:21
Copy link
Contributor

@suzannajiwani suzannajiwani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just had some questions, nothing blocking though

Comment on lines +1196 to +1201
* <p>In general this is implemented by examining
* <a href="https://developer.android.com/reference/android/content/pm/PackageManager#FEATURE_HARDWARE_KEYSTORE">
* FEATURE_HARDWARE_KEYSTORE</a> and
* <a href="https://developer.android.com/reference/android/content/pm/PackageManager#FEATURE_STRONGBOX_KEYSTORE">
* FEATURE_STRONGBOX_KEYSTORE</a> to determine the KeyMint version for both
* the normal hardware-backed keystore and - if available - the StrongBox-backed keystore.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just wondering what's the reasoning behind detailing implementation in the docstring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each method references the conditions of the particular capability and a lot of them reference KeyMint versions. I thought it was useful to at least link to this from somewhere.

@@ -0,0 +1,70 @@
package com.android.identity.secure_area_test_app.ui.theme
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do files like this (+ Color.kt, Type.kt) need copyright at the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably can't hurt. Added.

This is taken from the experimental-cloud-secure-area branch and adapted
to show just AndroidKeystoreSecureArea and SoftwareSecureArea (will rebase
that branch for all three secure areas once this is merged).

Also rework how Android Keystore capabilities are reported and show these
in the UI in the new SA test app.

Since it's now possible to test AndroidKeystoreSecureArea under various
conditions (for example when a Secure Lock Screen has not been set up)
it's easier to verify our error handling paths. To that end, fix up
propogated exceptions so they are easier to parse from the top-level
exception message and not just the cause.

Test: Manually tested
@davidz25 davidz25 force-pushed the android-keystore-capabilities branch from fef76fd to 1f020f4 Compare October 27, 2023 18:17
@davidz25 davidz25 merged commit 4ad39f6 into master Oct 27, 2023
4 checks passed
@davidz25 davidz25 deleted the android-keystore-capabilities branch October 27, 2023 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants