KeyChain: Wire user-selectability into KeyChain.
NOTE: Seeking early feedback, particularly on how to test.
Wire the ability to mark individual keys as non-user-selectable
into KeyChain Service:
* Add methods in the aidl to set/get user-selectability.
* Use the information in the AliasLoader activity to avoid loading
aliases that are not user-selectable.
* When granting access to an alias in the AliasLoader, check that the
alias is not one that should not be selected by user. This is an
additional safety check since such aliases should not be selected
by the user anyway.
Bug: 65624467
Test: To be added.
Change-Id: Ibaba2ddd4f94fced1a2a7bfcfb91189302ec7f3a
diff --git a/src/com/android/keychain/KeyChainActivity.java b/src/com/android/keychain/KeyChainActivity.java
index 79ecc79..2eb7c89 100644
--- a/src/com/android/keychain/KeyChainActivity.java
+++ b/src/com/android/keychain/KeyChainActivity.java
@@ -147,8 +147,24 @@
private void chooseCertificate() {
// Start loading the set of certs to choose from now- if device policy doesn't return an
// alias, having aliases loading already will save some time waiting for UI to start.
- // TODO: Once KeyChainService supports this interface, replace with that instance.
- final AliasLoader loader = new AliasLoader(mKeyStore, this, (alias) -> { return true; });
+ KeyInfoProvider keyInfoProvider = new KeyInfoProvider() {
+ public boolean isUserSelectable(String alias) {
+ try (KeyChain.KeyChainConnection connection =
+ KeyChain.bind(KeyChainActivity.this)) {
+ return connection.getService().isUserSelectable(alias);
+ }
+ catch (InterruptedException ignored) {
+ Log.e(TAG, "interrupted while checking if key is user-selectable", ignored);
+ Thread.currentThread().interrupt();
+ return false;
+ } catch (Exception ignored) {
+ Log.e(TAG, "error while checking if key is user-selectable", ignored);
+ return false;
+ }
+ }
+ };
+
+ final AliasLoader loader = new AliasLoader(mKeyStore, this, keyInfoProvider);
loader.execute();
final IKeyChainAliasCallback.Stub callback = new IKeyChainAliasCallback.Stub() {
@@ -479,6 +495,11 @@
if (mAlias != null) {
KeyChain.KeyChainConnection connection = KeyChain.bind(KeyChainActivity.this);
try {
+ if (!connection.getService().isUserSelectable(mAlias)) {
+ Log.w(TAG, String.format("Alias %s not user-selectable.", mAlias));
+ //TODO: Should we invoke the callback with null here to indicate error?
+ return null;
+ }
connection.getService().setGrant(mSenderUid, mAlias, true);
} finally {
connection.close();
diff --git a/src/com/android/keychain/KeyChainService.java b/src/com/android/keychain/KeyChainService.java
index f4366ee..09bdc6c 100644
--- a/src/com/android/keychain/KeyChainService.java
+++ b/src/com/android/keychain/KeyChainService.java
@@ -98,14 +98,33 @@
return mKeyStore.get(Credentials.CA_CERTIFICATE + alias);
}
- private void checkArgs(String alias) {
+ @Override public boolean isUserSelectable(String alias) {
+ validateAlias(alias);
+ return mGrantsDb.isUserSelectable(alias);
+ }
+
+ @Override public void setUserSelectable(String alias, boolean isUserSelectable) {
+ validateAlias(alias);
+ checkSystemCaller();
+ mGrantsDb.setIsUserSelectable(alias, isUserSelectable);
+ }
+
+ private void validateAlias(String alias) {
if (alias == null) {
throw new NullPointerException("alias == null");
}
+ }
+
+ private void validateKeyStoreState() {
if (!mKeyStore.isUnlocked()) {
throw new IllegalStateException("keystore is "
+ mKeyStore.state().toString());
}
+ }
+
+ private void checkArgs(String alias) {
+ validateAlias(alias);
+ validateKeyStoreState();
final int callingUid = getCallingUid();
if (!mGrantsDb.hasGrant(callingUid, alias)) {