Separate the binder backend from android_engine
Create a pure virtual class "KeystoreBackend" which supplies the
crypto methods used by android_engine. Create a KestoreBackendBinder
class which implements the binder backend as a no-op change that
will allow future backends to be added.
Bug: 34603782
Test: Compiles
Change-Id: I16620aba569bd53290145b2b30242c4888106d0a
diff --git a/keystore-engine/Android.mk b/keystore-engine/Android.mk
index 6d6abe0..955508b 100644
--- a/keystore-engine/Android.mk
+++ b/keystore-engine/Android.mk
@@ -19,7 +19,8 @@
LOCAL_MODULE := libkeystore-engine
LOCAL_SRC_FILES := \
- android_engine.cpp
+ android_engine.cpp \
+ keystore_backend_binder.cpp
LOCAL_MODULE_TAGS := optional
LOCAL_CFLAGS := -fvisibility=hidden -Wall -Werror
diff --git a/keystore-engine/android_engine.cpp b/keystore-engine/android_engine.cpp
index 6295873..8324b55 100644
--- a/keystore-engine/android_engine.cpp
+++ b/keystore-engine/android_engine.cpp
@@ -21,14 +21,18 @@
* THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
#define LOG_TAG "keystore-engine"
+#include "keystore_backend_binder.h"
#include <UniquePtr.h>
+#include <pthread.h>
#include <sys/socket.h>
#include <stdarg.h>
#include <string.h>
#include <unistd.h>
+#include <cutils/log.h>
+
#include <openssl/bn.h>
#include <openssl/ec.h>
#include <openssl/ec_key.h>
@@ -38,16 +42,7 @@
#include <openssl/rsa.h>
#include <openssl/x509.h>
-#include <binder/IServiceManager.h>
-#include <keystore/keystore.h>
-#include <keystore/IKeystoreService.h>
-#include <keystore/keystore_hidl_support.h>
-
-using namespace android;
-
namespace {
-using namespace keystore;
-
extern const RSA_METHOD keystore_rsa_method;
extern const ECDSA_METHOD keystore_ecdsa_method;
@@ -111,11 +106,13 @@
pthread_once_t g_keystore_engine_once = PTHREAD_ONCE_INIT;
KeystoreEngine *g_keystore_engine;
+KeystoreBackend *g_keystore_backend;
/* init_keystore_engine is called to initialize |g_keystore_engine|. This
* should only be called by |pthread_once|. */
void init_keystore_engine() {
g_keystore_engine = new KeystoreEngine;
+ g_keystore_backend = new KeystoreBackendBinder;
}
/* ensure_keystore_engine ensures that |g_keystore_engine| is pointing to a
@@ -149,39 +146,35 @@
return 0;
}
- sp<IServiceManager> sm = defaultServiceManager();
- sp<IBinder> binder = sm->getService(String16("android.security.keystore"));
- sp<IKeystoreService> service = interface_cast<IKeystoreService>(binder);
-
- if (service == NULL) {
- ALOGE("could not contact keystore");
+ uint8_t* reply = NULL;
+ size_t reply_len;
+ int32_t ret = g_keystore_backend->sign(key_id, in, len, &reply, &reply_len);
+ if (ret < 0) {
+ ALOGW("There was an error during rsa_decrypt: could not connect");
return 0;
- }
- auto inBlob = blob2hidlVec(in ,len);
- hidl_vec<uint8_t> reply;
-
- auto ret = service->sign(String16(key_id), inBlob, &reply);
- if (!ret.isOk()) {
- ALOGW("Error during sign from keystore: %d", int32_t(ret));
+ } else if (ret != 0) {
+ ALOGW("Error during sign from keystore: %d", ret);
return 0;
- } else if (reply.size() == 0) {
+ } else if (reply_len == 0 || reply == NULL) {
ALOGW("No valid signature returned");
return 0;
}
- if (reply.size() > len) {
+ if (reply_len > len) {
/* The result of the RSA operation can never be larger than the size of
* the modulus so we assume that the result has extra zeros on the
* left. This provides attackers with an oracle, but there's nothing
* that we can do about it here. */
- memcpy(out, &reply[reply.size() - len], len);
- } else if (reply.size() < len) {
+ ALOGW("Reply len %zu greater than expected %zu", reply_len, len);
+ memcpy(out, &reply[reply_len - len], len);
+ } else if (reply_len < len) {
/* If the Keystore implementation returns a short value we assume that
* it's because it removed leading zeros from the left side. This is
* bad because it provides attackers with an oracle but we cannot do
* anything about a broken Keystore implementation here. */
+ ALOGW("Reply len %zu lesser than expected %zu", reply_len, len);
memset(out, 0, len);
- memcpy(out + len - reply.size(), &reply[0], reply.size());
+ memcpy(out + len - reply_len, &reply[0], reply_len);
} else {
memcpy(out, &reply[0], len);
}
@@ -240,35 +233,26 @@
return 0;
}
- sp<IServiceManager> sm = defaultServiceManager();
- sp<IBinder> binder = sm->getService(String16("android.security.keystore"));
- sp<IKeystoreService> service = interface_cast<IKeystoreService>(binder);
-
- if (service == NULL) {
- ALOGE("could not contact keystore");
- return 0;
- }
-
size_t ecdsa_size = ECDSA_size(ec_key);
- auto hidlDigest = blob2hidlVec(digest, digest_len);
- hidl_vec<uint8_t> reply;
- auto ret = service->sign(String16(reinterpret_cast<const char*>(key_id)),
- hidlDigest, &reply);
- if (!ret.isOk()) {
- ALOGW("Error during sign from keystore: %d", int32_t(ret));
+ uint8_t* reply = NULL;
+ size_t reply_len;
+ int32_t ret = g_keystore_backend->sign(
+ key_id, digest, digest_len, &reply, &reply_len);
+ if (ret < 0) {
+ ALOGW("There was an error during ecdsa_sign: could not connect");
return 0;
- } else if (reply.size() == 0) {
+ } else if (reply_len == 0 || reply == NULL) {
ALOGW("No valid signature returned");
return 0;
- } else if (reply.size() > ecdsa_size) {
+ } else if (reply_len > ecdsa_size) {
ALOGW("Signature is too large");
return 0;
}
// Reviewer: should't sig_len be checked here? Or is it just assumed that it is at least ecdsa_size?
- memcpy(sig, &reply[0], reply.size());
- *sig_len = reply.size();
+ memcpy(sig, &reply[0], reply_len);
+ *sig_len = reply_len;
ALOGV("ecdsa_sign(%p, %u, %p) => success", digest, (unsigned)digest_len,
ec_key);
@@ -395,24 +379,19 @@
EVP_PKEY* EVP_PKEY_from_keystore(const char* key_id) {
ALOGV("EVP_PKEY_from_keystore(\"%s\")", key_id);
- sp<IServiceManager> sm = defaultServiceManager();
- sp<IBinder> binder = sm->getService(String16("android.security.keystore"));
- sp<IKeystoreService> service = interface_cast<IKeystoreService>(binder);
-
- if (service == NULL) {
- ALOGE("could not contact keystore");
- return 0;
- }
-
- hidl_vec<uint8_t> pubkey;
- auto ret = service->get_pubkey(String16(key_id), &pubkey);
- if (!ret.isOk()) {
- ALOGW("keystore reports error: %d", int32_t(ret));
+ uint8_t *pubkey = NULL;
+ size_t pubkey_len;
+ int32_t ret = g_keystore_backend->get_pubkey(key_id, &pubkey, &pubkey_len);
+ if (ret < 0) {
+ ALOGW("could not contact keystore");
+ return NULL;
+ } else if (ret != 0) {
+ ALOGW("keystore reports error: %d", ret);
return NULL;
}
const uint8_t *inp = &pubkey[0];
- Unique_EVP_PKEY pkey(d2i_PUBKEY(NULL, &inp, pubkey.size()));
+ Unique_EVP_PKEY pkey(d2i_PUBKEY(NULL, &inp, pubkey_len));
if (pkey.get() == NULL) {
ALOGW("Cannot convert pubkey");
return NULL;
diff --git a/keystore-engine/keystore_backend.h b/keystore-engine/keystore_backend.h
new file mode 100644
index 0000000..19fc183
--- /dev/null
+++ b/keystore-engine/keystore_backend.h
@@ -0,0 +1,33 @@
+/* Copyright 2017 The Android Open Source Project
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
+#include <stdint.h>
+
+class KeystoreBackend {
+ public:
+ virtual ~KeystoreBackend() {}
+ virtual int32_t sign(const char *key_id, const uint8_t* in, size_t len,
+ uint8_t** reply, size_t* reply_len) = 0;
+ virtual int32_t get_pubkey(const char *key_id, uint8_t** pubkey,
+ size_t* reply_len) = 0;
+};
+
diff --git a/keystore-engine/keystore_backend_binder.cpp b/keystore-engine/keystore_backend_binder.cpp
new file mode 100644
index 0000000..181bd65
--- /dev/null
+++ b/keystore-engine/keystore_backend_binder.cpp
@@ -0,0 +1,82 @@
+/* Copyright 2017 The Android Open Source Project
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
+#include "keystore_backend_binder.h"
+
+#include <binder/IServiceManager.h>
+#include <keystore/keystore.h>
+#include <keystore/IKeystoreService.h>
+#include <keystore/keystore_hidl_support.h>
+
+using namespace android;
+using keystore::blob2hidlVec;
+using keystore::hidl_vec;
+
+namespace {
+const char keystore_service_name[] = "android.security.keystore";
+};
+
+int32_t KeystoreBackendBinder::sign(
+ const char *key_id, const uint8_t* in, size_t len, uint8_t** reply,
+ size_t* reply_len) {
+ sp<IServiceManager> sm = defaultServiceManager();
+ sp<IBinder> binder = sm->getService(String16(keystore_service_name));
+ sp<IKeystoreService> service = interface_cast<IKeystoreService>(binder);
+
+ if (service == NULL) {
+ ALOGE("could not contact keystore");
+ return -1;
+ }
+
+ auto inBlob = blob2hidlVec(in ,len);
+ hidl_vec<uint8_t> reply_vec;
+ auto ret = service->sign(String16(key_id), inBlob, &reply_vec);
+ if (!ret.isOk()) {
+ return -1;
+ }
+
+ *reply = reply_vec.data();
+ *reply_len = reply_vec.size();
+ return 0;
+}
+
+int32_t KeystoreBackendBinder::get_pubkey(
+ const char *key_id, uint8_t** pubkey, size_t* pubkey_len) {
+ sp<IServiceManager> sm = defaultServiceManager();
+ sp<IBinder> binder = sm->getService(String16(keystore_service_name));
+ sp<IKeystoreService> service = interface_cast<IKeystoreService>(binder);
+
+ if (service == NULL) {
+ ALOGE("could not contact keystore");
+ return -1;
+ }
+
+ hidl_vec<uint8_t> pubkey_vec;
+ auto ret = service->get_pubkey(String16(key_id), &pubkey_vec);
+ if (!ret.isOk()) {
+ return -1;
+ }
+
+ *pubkey = pubkey_vec.data();
+ *pubkey_len = pubkey_vec.size();
+ return 0;
+}
diff --git a/keystore-engine/keystore_backend_binder.h b/keystore-engine/keystore_backend_binder.h
new file mode 100644
index 0000000..0fcf8de
--- /dev/null
+++ b/keystore-engine/keystore_backend_binder.h
@@ -0,0 +1,34 @@
+/* Copyright 2017 The Android Open Source Project
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */
+
+#include "keystore_backend.h"
+
+class KeystoreBackendBinder : public KeystoreBackend {
+ public:
+ KeystoreBackendBinder() {}
+ virtual ~KeystoreBackendBinder() {}
+ int32_t sign(const char *key_id, const uint8_t* in, size_t len,
+ uint8_t** reply, size_t* reply_len) override;
+ int32_t get_pubkey(const char *key_id, uint8_t** pubkey,
+ size_t* reply_len) override;
+};
+