You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2018/04/27 20:47:01 UTC
impala git commit: IMPALA-6892: CheckHashAndDecrypt() includes file
and host
Repository: impala
Updated Branches:
refs/heads/2.x a84053ddf -> 2c893f46b
IMPALA-6892: CheckHashAndDecrypt() includes file and host
The error text with AES-GCM enabled looks like:
Error reading 44 bytes from scratch file
'/tmp/impala-scratch/0:0_d43635d0-8f55-485e-8899-907af289ac86' on
backend tarmstrong-box:22000 at offset 0: verification of read data
failed.
OpenSSL error in EVP_DecryptFinal:
139634997483216:error:0607C083:digital envelope
routines:EVP_CIPHER_CTX_ctrl:no cipher set:evp_enc.c:610:
139634997483216:error:0607C083:digital envelope
routines:EVP_CIPHER_CTX_ctrl:no cipher set:evp_enc.c:610:
139634997483216:error:0607C083:digital envelope
routines:EVP_CIPHER_CTX_ctrl:no cipher set:evp_enc.c:610:
139634997483216:error:0607C083:digital envelope
routines:EVP_CIPHER_CTX_ctrl:no cipher set:evp_enc.c:610:
Testing:
Added a backend test to exercise the code path and verify the error
code.
Change-Id: I0652d6cdfbb4e543dd0ca46b7cc65edc4e41a2d8
Reviewed-on: http://gerrit.cloudera.org:8080/10204
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-on: http://gerrit.cloudera.org:8080/10232
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/2c893f46
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/2c893f46
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/2c893f46
Branch: refs/heads/2.x
Commit: 2c893f46bd630b42b96ade7694cb166515f20ef3
Parents: a84053d
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Wed Apr 25 11:32:30 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Apr 27 19:47:38 2018 +0000
----------------------------------------------------------------------
be/src/exec/exec-node.cc | 2 --
be/src/runtime/tmp-file-mgr-test.cc | 53 ++++++++++++++++++++++++++++++
be/src/runtime/tmp-file-mgr.cc | 15 +++++++--
common/thrift/generate_error_codes.py | 4 +++
4 files changed, 70 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/2c893f46/be/src/exec/exec-node.cc
----------------------------------------------------------------------
diff --git a/be/src/exec/exec-node.cc b/be/src/exec/exec-node.cc
index c94f38c..8553459 100644
--- a/be/src/exec/exec-node.cc
+++ b/be/src/exec/exec-node.cc
@@ -68,8 +68,6 @@
using strings::Substitute;
-DECLARE_int32(be_port);
-DECLARE_string(hostname);
DEFINE_bool_hidden(enable_partitioned_hash_join, true, "Deprecated - has no effect");
DEFINE_bool_hidden(enable_partitioned_aggregation, true, "Deprecated - has no effect");
http://git-wip-us.apache.org/repos/asf/impala/blob/2c893f46/be/src/runtime/tmp-file-mgr-test.cc
----------------------------------------------------------------------
diff --git a/be/src/runtime/tmp-file-mgr-test.cc b/be/src/runtime/tmp-file-mgr-test.cc
index a61f844..318fdbe 100644
--- a/be/src/runtime/tmp-file-mgr-test.cc
+++ b/be/src/runtime/tmp-file-mgr-test.cc
@@ -31,6 +31,7 @@
#include "service/fe-support.h"
#include "testutil/gtest-util.h"
#include "util/condition-variable.h"
+#include "util/cpu-info.h"
#include "util/filesystem-util.h"
#include "util/metrics.h"
@@ -169,6 +170,9 @@ class TmpFileMgrTest : public ::testing::Test {
while (cb_counter_ < val) cb_cv_.Wait(lock);
}
+ /// Implementation of TestBlockVerification(), which is run with different environments.
+ void TestBlockVerification();
+
ObjectPool obj_pool_;
scoped_ptr<MetricGroup> metrics_;
// Owned by 'obj_pool_'.
@@ -507,6 +511,55 @@ TEST_F(TmpFileMgrTest, TestEncryptionDuringCancellation) {
file_group.Close();
test_env_->TearDownQueries();
}
+
+TEST_F(TmpFileMgrTest, TestBlockVerification) {
+ TestBlockVerification();
+}
+
+TEST_F(TmpFileMgrTest, TestBlockVerificationGcmDisabled) {
+ // Disable AES-GCM to test that errors from non-AES-GCM verification are also correct.
+ CpuInfo::TempDisable t1(CpuInfo::PCLMULQDQ);
+ TestBlockVerification();
+}
+
+void TmpFileMgrTest::TestBlockVerification() {
+ FLAGS_disk_spill_encryption = true;
+ TUniqueId id;
+ TmpFileMgr::FileGroup file_group(test_env_->tmp_file_mgr(), io_mgr(), profile_, id);
+ string data = "the quick brown fox jumped over the lazy dog";
+ MemRange data_mem_range(reinterpret_cast<uint8_t*>(&data[0]), data.size());
+
+ // Start a write in flight, which should encrypt the data and write it to disk.
+ unique_ptr<TmpFileMgr::WriteHandle> handle;
+ WriteRange::WriteDoneCallback callback =
+ bind(mem_fn(&TmpFileMgrTest::SignalCallback), this, _1);
+ ASSERT_OK(file_group.Write(data_mem_range, callback, &handle));
+ string file_path = handle->TmpFilePath();
+
+ WaitForWrite(handle.get());
+ WaitForCallbacks(1);
+
+ // Modify the data in the scratch file and check that a read error occurs.
+ FILE* file = fopen(file_path.c_str(), "rb+");
+ fputc('?', file);
+ fclose(file);
+ vector<uint8_t> tmp;
+ tmp.resize(data.size());
+ Status read_status = file_group.Read(handle.get(), MemRange(tmp.data(), tmp.size()));
+ LOG(INFO) << read_status.GetDetail();
+ EXPECT_EQ(TErrorCode::SCRATCH_READ_VERIFY_FAILED, read_status.code())
+ << read_status.GetDetail();
+
+ // Modify the data in memory. Restoring the data should fail.
+ data[0] = '?';
+ Status restore_status = file_group.RestoreData(move(handle), data_mem_range);
+ LOG(INFO) << restore_status.GetDetail();
+ EXPECT_EQ(TErrorCode::SCRATCH_READ_VERIFY_FAILED, restore_status.code())
+ << restore_status.GetDetail();
+
+ file_group.Close();
+ test_env_->TearDownQueries();
+}
}
int main(int argc, char** argv) {
http://git-wip-us.apache.org/repos/asf/impala/blob/2c893f46/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 5adfcab..b44a5c4 100644
--- a/be/src/runtime/tmp-file-mgr.cc
+++ b/be/src/runtime/tmp-file-mgr.cc
@@ -608,15 +608,26 @@ Status TmpFileMgr::WriteHandle::EncryptAndHash(MemRange buffer) {
Status TmpFileMgr::WriteHandle::CheckHashAndDecrypt(MemRange buffer) {
DCHECK(FLAGS_disk_spill_encryption);
+ DCHECK(write_range_ != nullptr);
SCOPED_TIMER(encryption_timer_);
// GCM mode will verify the integrity by itself
if (!key_.IsGcmMode()) {
if (!hash_.Verify(buffer.data(), buffer.len())) {
- return Status("Block verification failure");
+ return Status(TErrorCode::SCRATCH_READ_VERIFY_FAILED, buffer.len(),
+ write_range_->file(), GetBackendString(), write_range_->offset());
}
}
- return key_.Decrypt(buffer.data(), buffer.len(), buffer.data());
+ Status decrypt_status = key_.Decrypt(buffer.data(), buffer.len(), buffer.data());
+ if (!decrypt_status.ok()) {
+ // Treat decryption failing as a verification failure, but include extra info from
+ // the decryption status.
+ Status result_status(TErrorCode::SCRATCH_READ_VERIFY_FAILED, buffer.len(),
+ write_range_->file(), GetBackendString(), write_range_->offset());
+ result_status.MergeStatus(decrypt_status);
+ return result_status;
+ }
+ return Status::OK();
}
string TmpFileMgr::WriteHandle::DebugString() {
http://git-wip-us.apache.org/repos/asf/impala/blob/2c893f46/common/thrift/generate_error_codes.py
----------------------------------------------------------------------
diff --git a/common/thrift/generate_error_codes.py b/common/thrift/generate_error_codes.py
index fc47b2a..7775ef6 100755
--- a/common/thrift/generate_error_codes.py
+++ b/common/thrift/generate_error_codes.py
@@ -359,6 +359,10 @@ error_codes = (
("LIB_VERSION_MISMATCH", 117,
"The library $0 last modified time $1 does not match the expected last "
"modified time $2. Run 'refresh functions <db name>'."),
+
+ ("SCRATCH_READ_VERIFY_FAILED", 118, "Error reading $0 bytes from scratch file '$1' "
+ "on backend $2 at offset $3: verification of read data failed."),
+
)
import sys