You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by kw...@apache.org on 2017/11/20 21:52:43 UTC

incubator-impala git commit: IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"

Repository: incubator-impala
Updated Branches:
  refs/heads/master 690e302a6 -> 3632ed4b9


IMPALA-6220: Revert "IMPALA-6128: Spill-to-disk Encryption(AES-CFB + SHA256) is slow"

Compilation failed on some platforms as ‘EVP_aes_256_ctr’ not declared in openssl-util.cc.
Reverting the change to unbreak the builds for now.

This reverts commit fb4c3b01240d8f65fc2c45bf27b668ae9b1fa5d2.

Change-Id: Id31d5fcfec5c6d777d4acee5c1be2d4fc4605efb
Reviewed-on: http://gerrit.cloudera.org:8080/8597
Reviewed-by: Xianda Ke <ke...@gmail.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Michael Ho <kw...@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/3632ed4b
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/3632ed4b
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/3632ed4b

Branch: refs/heads/master
Commit: 3632ed4b9e90d7a7a10300fed4d5710ae72c6ebf
Parents: 690e302
Author: Michael Ho <kw...@cloudera.com>
Authored: Mon Nov 20 00:49:30 2017 -0800
Committer: Michael Ho <kw...@cloudera.com>
Committed: Mon Nov 20 21:50:49 2017 +0000

----------------------------------------------------------------------
 be/src/runtime/tmp-file-mgr.cc |  2 +-
 be/src/util/openssl-util.cc    | 24 ++++++++----------------
 be/src/util/openssl-util.h     |  7 ++-----
 3 files changed, 11 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3632ed4b/be/src/runtime/tmp-file-mgr.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tmp-file-mgr.cc b/be/src/runtime/tmp-file-mgr.cc
index ac165a7..24217de 100644
--- a/be/src/runtime/tmp-file-mgr.cc
+++ b/be/src/runtime/tmp-file-mgr.cc
@@ -612,7 +612,7 @@ void TmpFileMgr::WriteHandle::WaitForWrite() {
 Status TmpFileMgr::WriteHandle::EncryptAndHash(MemRange buffer) {
   DCHECK(FLAGS_disk_spill_encryption);
   SCOPED_TIMER(encryption_timer_);
-  // Since we're using AES-CTR mode, we must take care not to reuse a key/IV pair.
+  // Since we're using AES-CFB mode, we must take care not to reuse a key/IV pair.
   // Regenerate a new key and IV for every data buffer we write.
   key_.InitializeRandom();
   RETURN_IF_ERROR(key_.Encrypt(buffer.data(), buffer.len(), buffer.data()));

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3632ed4b/be/src/util/openssl-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.cc b/be/src/util/openssl-util.cc
index 8eef4be..e3b2299 100644
--- a/be/src/util/openssl-util.cc
+++ b/be/src/util/openssl-util.cc
@@ -99,15 +99,13 @@ Status EncryptionKey::EncryptInternal(
   EVP_CIPHER_CTX_init(&ctx);
   EVP_CIPHER_CTX_set_padding(&ctx, 0);
 
-  // Start encryption/decryption.  We use a 256-bit AES key, and the cipher block mode
-  // is CTR. CTR is a stream cipher, which supports
-  //   (1). arbitrary length ciphertexts - it doesn't have to be a multiple of 16 bytes.
-  //   (2). well-optimized(instruction level parallelism) with
-  //        hardware acceleration on x86 or PowerPC
-  const EVP_CIPHER* evpCipher = IsCtrSupported() ? EVP_aes_256_ctr() : EVP_aes_256_cfb();
-  int success = encrypt ? EVP_EncryptInit_ex(&ctx, evpCipher, NULL, key_, iv_) :
-                          EVP_DecryptInit_ex(&ctx, evpCipher, NULL, key_, iv_);
+  int success;
 
+  // Start encryption/decryption.  We use a 256-bit AES key, and the cipher block mode
+  // is CFB because this gives us a stream cipher, which supports arbitrary
+  // length ciphertexts - it doesn't have to be a multiple of 16 bytes.
+  success = encrypt ? EVP_EncryptInit_ex(&ctx, EVP_aes_256_cfb(), NULL, key_, iv_) :
+                      EVP_DecryptInit_ex(&ctx, EVP_aes_256_cfb(), NULL, key_, iv_);
   if (success != 1) {
     return OpenSSLErr(encrypt ? "EVP_EncryptInit_ex" : "EVP_DecryptInit_ex");
   }
@@ -124,7 +122,7 @@ Status EncryptionKey::EncryptInternal(
     if (success != 1) {
       return OpenSSLErr(encrypt ? "EVP_EncryptUpdate" : "EVP_DecryptUpdate");
     }
-    // This is safe because we're using CTR mode without padding.
+    // This is safe because we're using CFB mode without padding.
     DCHECK_EQ(in_len, out_len);
     offset += in_len;
   }
@@ -136,14 +134,8 @@ Status EncryptionKey::EncryptInternal(
   if (success != 1) {
     return OpenSSLErr(encrypt ? "EVP_EncryptFinal" : "EVP_DecryptFinal");
   }
-  // Again safe due to CTR with no padding
+  // Again safe due to CFB with no padding
   DCHECK_EQ(final_out_len, 0);
   return Status::OK();
 }
-
-#define OPENSSL_VERSION_1_0_1 0x1000100L
-bool EncryptionKey::IsCtrSupported() const {
-  // aes_256_ctr was supported since v1.0.1
-  return (SSLeay() >= OPENSSL_VERSION_1_0_1);
-}
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3632ed4b/be/src/util/openssl-util.h
----------------------------------------------------------------------
diff --git a/be/src/util/openssl-util.h b/be/src/util/openssl-util.h
index cd1ca1b..4b32db6 100644
--- a/be/src/util/openssl-util.h
+++ b/be/src/util/openssl-util.h
@@ -47,7 +47,7 @@ class IntegrityHash {
 /// The key and initialization vector (IV) required to encrypt and decrypt a buffer of
 /// data. This should be regenerated for each buffer of data.
 ///
-/// We use AES with a 256-bit key and CTR cipher block mode, which gives us a stream
+/// We use AES with a 256-bit key and CFB cipher block mode, which gives us a stream
 /// cipher that can support arbitrary-length ciphertexts. The IV is used as an input to
 /// the cipher as the "block to supply before the first block of plaintext". This is
 /// required because all ciphers (except the weak ECB) are built such that each block
@@ -59,7 +59,7 @@ class EncryptionKey {
   EncryptionKey() : initialized_(false) {}
 
   /// Initialize a key for temporary use with randomly generated data. Reinitializes with
-  /// new random values if the key was already initialized. We use AES-CTR mode so key/IV
+  /// new random values if the key was already initialized. We use AES-CFB mode so key/IV
   /// pairs should not be reused. This function automatically reseeds the RNG
   /// periodically, so callers do not need to do it.
   void InitializeRandom();
@@ -84,9 +84,6 @@ class EncryptionKey {
   Status EncryptInternal(bool encrypt, const uint8_t* data, int64_t len,
       uint8_t* out) const WARN_UNUSED_RESULT;
 
-  /// check if CTR mode is supported at runtime
-  bool IsCtrSupported() const;
-
   /// Track whether this key has been initialized, to avoid accidentally using
   /// uninitialized keys.
   bool initialized_;