From 5eedf5a3734308ee5ebe8a5ea2f73b6e6a0cb1fe Mon Sep 17 00:00:00 2001 From: Eran Messeri Date: Wed, 2 Feb 2022 22:50:50 +0000 Subject: [PATCH] Keystore: Surface service error message Surface the service-specific error message. To avoid API changes, the error message is surfaced in the toString / getMessage methods. Test: atest android.security.keystore.KeyStoreExceptionTest Bug: 217593122 Change-Id: Id4090564b46db9b3b10ea390390f6683f7314463 --- keystore/java/android/security/KeyStore2.java | 34 ++++++++------ .../android/security/KeyStoreException.java | 10 +++++ .../android/security/KeyStoreOperation.java | 5 ++- .../security/KeyStoreSecurityLevel.java | 6 +-- .../keystore/KeyStoreExceptionTest.java | 44 +++++++++++++++++++ 5 files changed, 81 insertions(+), 18 deletions(-) create mode 100644 keystore/tests/src/android/security/keystore/KeyStoreExceptionTest.java diff --git a/keystore/java/android/security/KeyStore2.java b/keystore/java/android/security/KeyStore2.java index 1034847b761b..3d53cfb388e1 100644 --- a/keystore/java/android/security/KeyStore2.java +++ b/keystore/java/android/security/KeyStore2.java @@ -108,7 +108,7 @@ public class KeyStore2 { try { return request.execute(service); } catch (ServiceSpecificException e) { - throw getKeyStoreException(e.errorCode); + throw getKeyStoreException(e.errorCode, e.getMessage()); } catch (RemoteException e) { if (firstTry) { Log.w(TAG, "Looks like we may have lost connection to the Keystore " @@ -120,7 +120,7 @@ public class KeyStore2 { firstTry = false; } else { Log.e(TAG, "Cannot connect to Keystore daemon.", e); - throw new KeyStoreException(ResponseCode.SYSTEM_ERROR, ""); + throw new KeyStoreException(ResponseCode.SYSTEM_ERROR, "", e.getMessage()); } } } @@ -322,26 +322,32 @@ public class KeyStore2 { } } - static KeyStoreException getKeyStoreException(int errorCode) { + static KeyStoreException getKeyStoreException(int errorCode, String serviceErrorMessage) { if (errorCode > 0) { // KeyStore layer error switch (errorCode) { case ResponseCode.LOCKED: - return new KeyStoreException(errorCode, "User authentication required"); + return new KeyStoreException(errorCode, "User authentication required", + serviceErrorMessage); case ResponseCode.UNINITIALIZED: - return new KeyStoreException(errorCode, "Keystore not initialized"); + return new KeyStoreException(errorCode, "Keystore not initialized", + serviceErrorMessage); case ResponseCode.SYSTEM_ERROR: - return new KeyStoreException(errorCode, "System error"); + return new KeyStoreException(errorCode, "System error", serviceErrorMessage); case ResponseCode.PERMISSION_DENIED: - return new KeyStoreException(errorCode, "Permission denied"); + return new KeyStoreException(errorCode, "Permission denied", + serviceErrorMessage); case ResponseCode.KEY_NOT_FOUND: - return new KeyStoreException(errorCode, "Key not found"); + return new KeyStoreException(errorCode, "Key not found", serviceErrorMessage); case ResponseCode.VALUE_CORRUPTED: - return new KeyStoreException(errorCode, "Key blob corrupted"); + return new KeyStoreException(errorCode, "Key blob corrupted", + serviceErrorMessage); case ResponseCode.KEY_PERMANENTLY_INVALIDATED: - return new KeyStoreException(errorCode, "Key permanently invalidated"); + return new KeyStoreException(errorCode, "Key permanently invalidated", + serviceErrorMessage); default: - return new KeyStoreException(errorCode, String.valueOf(errorCode)); + return new KeyStoreException(errorCode, String.valueOf(errorCode), + serviceErrorMessage); } } else { // Keymaster layer error @@ -350,10 +356,12 @@ public class KeyStore2 { // The name of this parameter significantly differs between Keymaster and // framework APIs. Use the framework wording to make life easier for developers. return new KeyStoreException(errorCode, - "Invalid user authentication validity duration"); + "Invalid user authentication validity duration", + serviceErrorMessage); default: return new KeyStoreException(errorCode, - KeymasterDefs.getErrorMessage(errorCode)); + KeymasterDefs.getErrorMessage(errorCode), + serviceErrorMessage); } } } diff --git a/keystore/java/android/security/KeyStoreException.java b/keystore/java/android/security/KeyStoreException.java index 6db2745840f4..54184dbf6e08 100644 --- a/keystore/java/android/security/KeyStoreException.java +++ b/keystore/java/android/security/KeyStoreException.java @@ -157,6 +157,16 @@ public class KeyStoreException extends Exception { mErrorCode = errorCode; } + /** + * @hide + */ + public KeyStoreException(int errorCode, @Nullable String message, + @Nullable String keystoreErrorMessage) { + super(message + " (internal Keystore code: " + errorCode + " message: " + + keystoreErrorMessage + ")"); + mErrorCode = errorCode; + } + /** * Returns the internal error code. Only for use by the platform. * diff --git a/keystore/java/android/security/KeyStoreOperation.java b/keystore/java/android/security/KeyStoreOperation.java index e6c1ea827118..737ff2b4822f 100644 --- a/keystore/java/android/security/KeyStoreOperation.java +++ b/keystore/java/android/security/KeyStoreOperation.java @@ -75,7 +75,7 @@ public class KeyStoreOperation { ); } default: - throw KeyStore2.getKeyStoreException(e.errorCode); + throw KeyStore2.getKeyStoreException(e.errorCode, e.getMessage()); } } catch (RemoteException e) { // Log exception and report invalid operation handle. @@ -85,7 +85,8 @@ public class KeyStoreOperation { "Remote exception while advancing a KeyStoreOperation.", e ); - throw new KeyStoreException(KeymasterDefs.KM_ERROR_INVALID_OPERATION_HANDLE, ""); + throw new KeyStoreException(KeymasterDefs.KM_ERROR_INVALID_OPERATION_HANDLE, "", + e.getMessage()); } } diff --git a/keystore/java/android/security/KeyStoreSecurityLevel.java b/keystore/java/android/security/KeyStoreSecurityLevel.java index b85dd742cc49..9c0b46c8e87b 100644 --- a/keystore/java/android/security/KeyStoreSecurityLevel.java +++ b/keystore/java/android/security/KeyStoreSecurityLevel.java @@ -54,12 +54,12 @@ public class KeyStoreSecurityLevel { try { return request.execute(); } catch (ServiceSpecificException e) { - throw KeyStore2.getKeyStoreException(e.errorCode); + throw KeyStore2.getKeyStoreException(e.errorCode, e.getMessage()); } catch (RemoteException e) { // Log exception and report invalid operation handle. // This should prompt the caller drop the reference to this operation and retry. Log.e(TAG, "Could not connect to Keystore.", e); - throw new KeyStoreException(ResponseCode.SYSTEM_ERROR, ""); + throw new KeyStoreException(ResponseCode.SYSTEM_ERROR, "", e.getMessage()); } } @@ -117,7 +117,7 @@ public class KeyStoreSecurityLevel { break; } default: - throw KeyStore2.getKeyStoreException(e.errorCode); + throw KeyStore2.getKeyStoreException(e.errorCode, e.getMessage()); } } catch (RemoteException e) { Log.w(TAG, "Cannot connect to keystore", e); diff --git a/keystore/tests/src/android/security/keystore/KeyStoreExceptionTest.java b/keystore/tests/src/android/security/keystore/KeyStoreExceptionTest.java new file mode 100644 index 000000000000..31c742289a61 --- /dev/null +++ b/keystore/tests/src/android/security/keystore/KeyStoreExceptionTest.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.security.keystore; + +import static org.junit.Assert.assertTrue; + +import android.security.KeyStoreException; + +import androidx.test.runner.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +@RunWith(AndroidJUnit4.class) +public class KeyStoreExceptionTest { + @Test + public void testKeystoreMessageIsIncluded() { + final String primaryMessage = "some_message"; + final String keystoreMessage = "ks_message"; + KeyStoreException exception = new KeyStoreException(-1, primaryMessage, keystoreMessage); + + String exceptionMessage = exception.getMessage(); + assertTrue(exceptionMessage.contains(primaryMessage)); + assertTrue(exceptionMessage.contains(keystoreMessage)); + + String exceptionString = exception.toString(); + assertTrue(exceptionString.contains(primaryMessage)); + assertTrue(exceptionString.contains(keystoreMessage)); + } +}