auth/Crypto: optimize libnss key

Allocate the key and slot once per key instead of once per encrypt/
decrypt operation.  This improves things by a factor of almost 4x
for my microbenchmark.

Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 973cd1c00a7811e95ff0406a90386f6ead5491c4)

diff --git a/src/auth/Crypto.cc b/src/auth/Crypto.cc
index b656d9d0cc25919e06f116d0821de111a2eb850e..b3898bfe23add98ea10748ead88c34edd94f0b47 100644 (file)
--- a/src/auth/Crypto.cc
+++ b/src/auth/Crypto.cc
@@ -185,51 +185,12 @@ public:
 # define AES_BLOCK_LEN   16
 
 static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
-                             const bufferptr& secret,
+                             CK_MECHANISM_TYPE mechanism,
+                             PK11SymKey *key,
+                             SECItem *param,
                              const bufferlist& in, bufferlist& out,
-                             std::string& error)
+                             std::string &error)
 {
-  CK_MECHANISM_TYPE mechanism = CKM_AES_CBC_PAD;
-  PK11SlotInfo *slot;
-  PK11SymKey *key;
-  SECItem *param;
-
-  slot = PK11_GetBestSlot(mechanism, NULL);
-  if (!slot) {
-    ostringstream err;
-    err << "cannot find NSS slot to use: " << PR_GetError();
-    error = err.str();
-    return;
-  }
-
-  SECItem keyItem;
-  keyItem.type = siBuffer;
-  keyItem.data = (unsigned char*)secret.c_str();
-  keyItem.len = secret.length();
-  key = PK11_ImportSymKey(slot, mechanism, PK11_OriginUnwrap, CKA_ENCRYPT,
-                         &keyItem, NULL);
-  if (!key) {
-    ostringstream err;
-    err << "cannot convert AES key for NSS: " << PR_GetError();
-    error = err.str();
-    return;
-  }
-
-  SECItem ivItem;
-  ivItem.type = siBuffer;
-  // losing constness due to SECItem.data; IV should never be
-  // modified, regardless
-  ivItem.data = (unsigned char*)CEPH_AES_IV;
-  ivItem.len = sizeof(CEPH_AES_IV);
-
-  param = PK11_ParamFromIV(mechanism, &ivItem);
-  if (!param) {
-    ostringstream err;
-    err << "cannot set NSS IV param: " << PR_GetError();
-    error = err.str();
-    return;
-  }
-
   // sample source said this has to be at least size of input + 8,
   // but i see 15 still fail with SEC_ERROR_OUTPUT_LEN
   bufferptr out_tmp(in.length()+16);
@@ -249,6 +210,7 @@ static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
                      (unsigned char*)out_tmp.c_str(), &written, out_tmp.length(),
                      in_buf, in.length());
   if (ret != SECSuccess) {
+    PK11_DestroyContext(ectx, PR_TRUE);
     ostringstream oss;
     oss << "NSS AES failed: " << PR_GetError();
     error = oss.str();
@@ -259,36 +221,80 @@ static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
   ret = PK11_DigestFinal(ectx,
                         (unsigned char*)out_tmp.c_str()+written, &written2,
                         out_tmp.length()-written);
+  PK11_DestroyContext(ectx, PR_TRUE);
   if (ret != SECSuccess) {
+    PK11_DestroyContext(ectx, PR_TRUE);
     ostringstream oss;
     oss << "NSS AES final round failed: " << PR_GetError();
     error = oss.str();
     return;
   }
 
-  PK11_DestroyContext(ectx, PR_TRUE);
-  SECITEM_FreeItem(param, PR_TRUE);
-  PK11_FreeSymKey(key);
-  PK11_FreeSlot(slot);
-
   out_tmp.set_length(written + written2);
   out.append(out_tmp);
 }
 
 class CryptoAESKeyHandler : public CryptoKeyHandler {
+  CK_MECHANISM_TYPE mechanism;
+  PK11SlotInfo *slot;
+  PK11SymKey *key;
+  SECItem *param;
+
 public:
+  CryptoAESKeyHandler()
+    : mechanism(CKM_AES_CBC_PAD),
+      slot(NULL),
+      key(NULL),
+      param(NULL) {}
+  ~CryptoAESKeyHandler() {
+    SECITEM_FreeItem(param, PR_TRUE);
+    PK11_FreeSymKey(key);
+    PK11_FreeSlot(slot);
+  }
+
   int init(const bufferptr& s, ostringstream& err) {
     secret = s;
+
+    slot = PK11_GetBestSlot(mechanism, NULL);
+    if (!slot) {
+      err << "cannot find NSS slot to use: " << PR_GetError();
+      return -1;
+    }
+
+    SECItem keyItem;
+    keyItem.type = siBuffer;
+    keyItem.data = (unsigned char*)secret.c_str();
+    keyItem.len = secret.length();
+    key = PK11_ImportSymKey(slot, mechanism, PK11_OriginUnwrap, CKA_ENCRYPT,
+                           &keyItem, NULL);
+    if (!key) {
+      err << "cannot convert AES key for NSS: " << PR_GetError();
+      return -1;
+    }
+
+    SECItem ivItem;
+    ivItem.type = siBuffer;
+    // losing constness due to SECItem.data; IV should never be
+    // modified, regardless
+    ivItem.data = (unsigned char*)CEPH_AES_IV;
+    ivItem.len = sizeof(CEPH_AES_IV);
+
+    param = PK11_ParamFromIV(mechanism, &ivItem);
+    if (!param) {
+      err << "cannot set NSS IV param: " << PR_GetError();
+      return -1;
+    }
+
     return 0;
   }
 
   void encrypt(const bufferlist& in,
               bufferlist& out, std::string &error) const {
-    nss_aes_operation(CKA_ENCRYPT, secret, in, out, error);
+    nss_aes_operation(CKA_ENCRYPT, mechanism, key, param, in, out, error);
   }
   void decrypt(const bufferlist& in,
               bufferlist& out, std::string &error) const {
-    nss_aes_operation(CKA_DECRYPT, secret, in, out, error);
+    nss_aes_operation(CKA_DECRYPT, mechanism, key, param, in, out, error);
   }
 };
 

diff --git a/src/test/testcrypto.cc b/src/test/testcrypto.cc
index 0b7a9d5474250ae5b9125ea4f975f22629b9c4ad..0c676e7ce08a7576578e38f3d4b0b618ce5cdb6f 100644 (file)
--- a/src/test/testcrypto.cc
+++ b/src/test/testcrypto.cc
@@ -25,8 +25,8 @@ int main(int argc, char *argv[])
 
   bufferlist enc_out;
   std::string error;
-  key.encrypt(g_ceph_context, enc_in, enc_out, error);
-  if (!error.empty()) {
+  if (key.encrypt(g_ceph_context, enc_in, enc_out, &error) < 0) {
+    ASSERT_TRUE(!error.empty());
     dout(0) << "couldn't encode! error " << error << dendl;
     exit(1);
   }
@@ -42,8 +42,8 @@ int main(int argc, char *argv[])
 
   dec_in = enc_out;
 
-  key.decrypt(g_ceph_context, dec_in, dec_out, error);
-  if (!error.empty()) {
+  if (key.decrypt(g_ceph_context, dec_in, dec_out, &error) < 0) {
+    ASSERT_TRUE(!error.empty());
     dout(0) << "couldn't decode! error " << error << dendl;
     exit(1);
   }