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_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;