auth: return error code from encrypt/decrypt; make error string optional

This is simpler for a lot of callers.

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

Conflicts:
	src/test/crypto.cc (trivial)

diff --git a/src/auth/Crypto.cc b/src/auth/Crypto.cc
index cd53fbe8cd8853675ec2669e6f69d97272ef019a..5223f5a48e88968999bbdc6fc6dfd963f75e3dfa 100644 (file)
--- a/src/auth/Crypto.cc
+++ b/src/auth/Crypto.cc
@@ -67,13 +67,15 @@ uint64_t get_random(uint64_t min_val, uint64_t max_val)
 
 class CryptoNoneKeyHandler : public CryptoKeyHandler {
 public:
-  void encrypt(const bufferlist& in,
-              bufferlist& out, std::string &error) const {
+  int encrypt(const bufferlist& in,
+              bufferlist& out, std::string *error) const {
     out = in;
+    return 0;
   }
-  void decrypt(const bufferlist& in,
-              bufferlist& out, std::string &error) const {
+  int decrypt(const bufferlist& in,
+             bufferlist& out, std::string *error) const {
     out = in;
+    return 0;
   }
 };
 
@@ -139,8 +141,8 @@ public:
     return 0;
   }
 
-  void encrypt(const bufferlist& in,
-              bufferlist& out, std::string &error) const {
+  int encrypt(const bufferlist& in,
+             bufferlist& out, std::string *error) const {
     string ciphertext;
     CryptoPP::StringSink *sink = new CryptoPP::StringSink(ciphertext);
     CryptoPP::CBC_Mode_ExternalCipher::Encryption cbc(
@@ -155,16 +157,19 @@ public:
     try {
       stfEncryptor.MessageEnd();
     } catch (CryptoPP::Exception& e) {
-      ostringstream oss;
-      oss << "encryptor.MessageEnd::Exception: " << e.GetWhat();
-      error = oss.str();
-      return;
+      if (error) {
+       ostringstream oss;
+       oss << "encryptor.MessageEnd::Exception: " << e.GetWhat();
+       *error = oss.str();
+      }
+      return -1;
     }
     out.append((const char *)ciphertext.c_str(), ciphertext.length());
+    return 0;
   }
 
-  void decrypt(const bufferlist& in,
-              bufferlist& out, std::string &error) const {
+  int decrypt(const bufferlist& in,
+             bufferlist& out, std::string *error) const {
     string decryptedtext;
     CryptoPP::StringSink *sink = new CryptoPP::StringSink(decryptedtext);
     CryptoPP::CBC_Mode_ExternalCipher::Decryption cbc(
@@ -179,13 +184,16 @@ public:
     try {
       stfDecryptor.MessageEnd();
     } catch (CryptoPP::Exception& e) {
-      ostringstream oss;
-      oss << "decryptor.MessageEnd::Exception: " << e.GetWhat();
-      error = oss.str();
-      return;
+      if (error) {
+       ostringstream oss;
+       oss << "decryptor.MessageEnd::Exception: " << e.GetWhat();
+       *error = oss.str();
+      }
+      return -1;
     }
 
     out.append((const char *)decryptedtext.c_str(), decryptedtext.length());
+    return 0;
   }
 };
 
@@ -194,12 +202,12 @@ public:
 # define AES_KEY_LEN   16
 # define AES_BLOCK_LEN   16
 
-static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
-                             CK_MECHANISM_TYPE mechanism,
-                             PK11SymKey *key,
-                             SECItem *param,
-                             const bufferlist& in, bufferlist& out,
-                             std::string &error)
+static int nss_aes_operation(CK_ATTRIBUTE_TYPE op,
+                            CK_MECHANISM_TYPE mechanism,
+                            PK11SymKey *key,
+                            SECItem *param,
+                            const bufferlist& in, bufferlist& out,
+                            std::string *error)
 {
   // sample source said this has to be at least size of input + 8,
   // but i see 15 still fail with SEC_ERROR_OUTPUT_LEN
@@ -221,10 +229,12 @@ static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
                      in_buf, in.length());
   if (ret != SECSuccess) {
     PK11_DestroyContext(ectx, PR_TRUE);
-    ostringstream oss;
-    oss << "NSS AES failed: " << PR_GetError();
-    error = oss.str();
-    return;
+    if (error) {
+      ostringstream oss;
+      oss << "NSS AES failed: " << PR_GetError();
+      *error = oss.str();
+    }
+    return -1;
   }
 
   unsigned int written2;
@@ -234,14 +244,17 @@ static void nss_aes_operation(CK_ATTRIBUTE_TYPE op,
   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;
+    if (error) {
+      ostringstream oss;
+      oss << "NSS AES final round failed: " << PR_GetError();
+      *error = oss.str();
+    }
+    return -1;
   }
 
   out_tmp.set_length(written + written2);
   out.append(out_tmp);
+  return 0;
 }
 
 class CryptoAESKeyHandler : public CryptoKeyHandler {
@@ -298,13 +311,13 @@ public:
     return 0;
   }
 
-  void encrypt(const bufferlist& in,
-              bufferlist& out, std::string &error) const {
-    nss_aes_operation(CKA_ENCRYPT, mechanism, key, param, in, out, error);
+  int encrypt(const bufferlist& in,
+             bufferlist& out, std::string *error) const {
+    return 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, mechanism, key, param, in, out, error);
+  int decrypt(const bufferlist& in,
+              bufferlist& out, std::string *error) const {
+    return nss_aes_operation(CKA_DECRYPT, mechanism, key, param, in, out, error);
   }
 };
 

diff --git a/src/auth/Crypto.h b/src/auth/Crypto.h
index 97b796165b6ddf83ce9074cf361e22af6b3cb310..3bfc5aabd17a8174c837c5b9dee0d16c9c986a32 100644 (file)
--- a/src/auth/Crypto.h
+++ b/src/auth/Crypto.h
@@ -37,10 +37,10 @@ public:
 
   virtual ~CryptoKeyHandler() {}
 
-  virtual void encrypt(const bufferlist& in,
-                      bufferlist& out, std::string &error) const = 0;
-  virtual void decrypt(const bufferlist& in,
-                      bufferlist& out, std::string &error) const = 0;
+  virtual int encrypt(const bufferlist& in,
+                      bufferlist& out, std::string *error) const = 0;
+  virtual int decrypt(const bufferlist& in,
+                      bufferlist& out, std::string *error) const = 0;
 };
 
 /*
@@ -105,13 +105,13 @@ public:
 
   // --
   int create(CephContext *cct, int type);
-  void encrypt(CephContext *cct, const bufferlist& in, bufferlist& out,
-              std::string &error) const {
-    ckh->encrypt(in, out, error);
+  int encrypt(CephContext *cct, const bufferlist& in, bufferlist& out,
+              std::string *error) const {
+    return ckh->encrypt(in, out, error);
   }
-  void decrypt(CephContext *cct, const bufferlist& in, bufferlist& out,
-              std::string &error) const {
-    ckh->decrypt(in, out, error);
+  int decrypt(CephContext *cct, const bufferlist& in, bufferlist& out,
+              std::string *error) const {
+    return ckh->decrypt(in, out, error);
   }
 
   void to_str(std::string& s) const;

diff --git a/src/auth/cephx/CephxProtocol.h b/src/auth/cephx/CephxProtocol.h
index d72a23d87d61fcf5d079c5068d6c2b22ae6ccffd..f08f07d84b9ff172e01bc007f202141683bde155 100644 (file)
--- a/src/auth/cephx/CephxProtocol.h
+++ b/src/auth/cephx/CephxProtocol.h
@@ -433,8 +433,7 @@ void decode_decrypt_enc_bl(CephContext *cct, T& t, CryptoKey key, bufferlist& bl
   uint64_t magic;
   bufferlist bl;
 
-  key.decrypt(cct, bl_enc, bl, error);
-  if (!error.empty())
+  if (key.decrypt(cct, bl_enc, bl, &error) < 0)
     return;
 
   bufferlist::iterator iter2 = bl.begin();
@@ -462,7 +461,7 @@ void encode_encrypt_enc_bl(CephContext *cct, const T& t, const CryptoKey& key,
   ::encode(magic, bl);
   ::encode(t, bl);
 
-  key.encrypt(cct, bl, out, error);
+  key.encrypt(cct, bl, out, &error);
 }
 
 template <typename T>

diff --git a/src/auth/cephx/CephxSessionHandler.cc b/src/auth/cephx/CephxSessionHandler.cc
index 71c8ab3c161b54db4d5667e1424112bc35bcd88c..eaebd152fb2635283da4d9e0fbb3d5b19e845b15 100644 (file)
--- a/src/auth/cephx/CephxSessionHandler.cc
+++ b/src/auth/cephx/CephxSessionHandler.cc
@@ -28,7 +28,6 @@ int CephxSessionHandler::_calc_signature(Message *m, uint64_t *psig)
 {
   const ceph_msg_header& header = m->get_header();
   const ceph_msg_footer& footer = m->get_footer();
-  std::string error;
 
   // optimized signature calculation
   // - avoid temporary allocated buffers from encode_encrypt[_enc_bl]
@@ -49,7 +48,10 @@ int CephxSessionHandler::_calc_signature(Message *m, uint64_t *psig)
   bl_plaintext.append(buffer::create_static(sizeof(sigblock), (char*)&sigblock));
 
   bufferlist bl_ciphertext;
-  key.encrypt(cct, bl_plaintext, bl_ciphertext, error);
+  if (key.encrypt(cct, bl_plaintext, bl_ciphertext, NULL) < 0) {
+    lderr(cct) << __func__ << " failed to encrypt signature block" << dendl;
+    return -1;
+  }
 
   bufferlist::iterator ci = bl_ciphertext.begin();
   ::decode(*psig, ci);

diff --git a/src/test/crypto.cc b/src/test/crypto.cc
index be9462117341bb8f53faf84df2676b7f4d76ff9c..150d2a45cc5520af39d8f88f02b0a73f64948411 100644 (file)
--- a/src/test/crypto.cc
+++ b/src/test/crypto.cc
@@ -53,7 +53,8 @@ TEST(AES, Encrypt) {
   bufferlist cipher;
   std::string error;
   CryptoKeyHandler *kh = h->get_key_handler(secret, error);
-  kh->encrypt(plaintext, cipher, error);
+  int r = kh->encrypt(plaintext, cipher, &error);
+  ASSERT_EQ(r, 0);
   ASSERT_EQ(error, "");
 
   unsigned char want_cipher[] = {
@@ -98,7 +99,8 @@ TEST(AES, Decrypt) {
   std::string error;
   bufferlist plaintext;
   CryptoKeyHandler *kh = h->get_key_handler(secret, error);
-  kh->decrypt(cipher, plaintext, error);
+  int r = kh->decrypt(cipher, plaintext, &error);
+  ASSERT_EQ(r, 0);
   ASSERT_EQ(error, "");
 
   ASSERT_EQ(sizeof(plaintext_s), plaintext.length());
@@ -131,7 +133,8 @@ TEST(AES, Loop) {
 
       std::string error;
       CryptoKeyHandler *kh = h->get_key_handler(secret, error);
-      kh->encrypt(plaintext, cipher, error);
+      int r = kh->encrypt(plaintext, cipher, &error);
+      ASSERT_EQ(r, 0);
       ASSERT_EQ(error, "");
     }
     plaintext.clear();
@@ -140,7 +143,8 @@ TEST(AES, Loop) {
       CryptoHandler *h = g_ceph_context->get_crypto_handler(CEPH_CRYPTO_AES);
       std::string error;
       CryptoKeyHandler *ckh = h->get_key_handler(secret, error);
-      ckh->decrypt(cipher, plaintext, error);
+      int r = ckh->decrypt(cipher, plaintext, &error);
+      ASSERT_EQ(r, 0);
       ASSERT_EQ(error, "");
     }
   }
@@ -150,3 +154,28 @@ TEST(AES, Loop) {
   err = memcmp(plaintext_s, orig_plaintext_s, sizeof(orig_plaintext_s));
   ASSERT_EQ(0, err);
 }
+
+TEST(AES, LoopKey) {
+  bufferptr k(16);
+  get_random_bytes(k.c_str(), k.length());
+  CryptoKey key(CEPH_CRYPTO_AES, ceph_clock_now(NULL), k);
+
+  bufferlist data;
+  bufferptr r(128);
+  get_random_bytes(r.c_str(), r.length());
+  data.append(r);
+
+  utime_t start = ceph_clock_now(NULL);
+  int n = 100000;
+
+  for (int i=0; i<n; ++i) {
+    bufferlist encoded;
+    string error;
+    int r = key.encrypt(g_ceph_context, data, encoded, &error);
+    ASSERT_EQ(r, 0);
+  }
+
+  utime_t end = ceph_clock_now(NULL);
+  utime_t dur = end - start;
+  cout << n << " encoded in " << dur << std::endl;
+}

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