Mailing-List: contact reviews-help@impala.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201712280053.vBS0rjeL031241@ip-10-146-233-104.ec2.internal>
Date: Thu, 28 Dec 2017 00:53:44 +0000
From: "Bikramjeet Vig (Code Review)" <gerrit@cloudera.org>
To: Xianda Ke <kexianda@gmail.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Sailesh Mukil <sailesh@cloudera.com>,
        Tim Armstrong <tarmstrong@cloudera.com>
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-6128=3A_Spill-to-disk_Encryption=28AES-CFB_+_SHA256=29_is_slow_CFB_mode_is_stream_cipher_and_is_secure_when_use_different_nonce/IV_for_every_message=2E_However_it_would_be_a_performance_bottleneck=2E_CTR_mode_is_also_stream_cipher_and_is_secure=2C_4=7E6x_faster_=0A?=
In-Reply-To: <gerrit.1513494341000.I9debc240615dd8cdbf00ec8730cff62ffef52aff@gerrit.cloudera.org>
References: <gerrit.1513494341000.I9debc240615dd8cdbf00ec8730cff62ffef52aff@gerrit.cloudera.org>
Reply-To: bikramjeet.vig@cloudera.com, impala-cr@cloudera.com,
        sailesh@cloudera.com, kexianda@gmail.com, marcelk@gmail.com,
        tarmstrong@cloudera.com, reviews@impala.incubator.apache.org
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.14.2
Content-Type: multipart/alternative; boundary="+FJGJ3ZuGPM="; charset=UTF-8
archived-at: Thu, 28 Dec 2017 00:53:51 -0000

--+FJGJ3ZuGPM=
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Bikramjeet Vig has posted comments on this change=2E ( http://gerrit=2Eclou=
dera=2Eorg:8080/8861 )

Change subject: IMPALA-6128: Spill-to-disk Encrypti=
on(AES-CFB + SHA256) is slow CFB mode is stream cipher and is secure when u=
se different nonce/IV for every message=2E However it would be a performanc=
e bottleneck=2E CTR mode is also stream cipher and is secure, 4~6x faster 
=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=
=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E=2E


Patch Set 2:=


(11 comments)

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMIT_MSG=

Commit Message:

http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMIT_MS=
G@7
PS2, Line 7: Spill-to-disk Encryption(AES-CFB + SHA256) is slow
nit, ho=
w about this?:
Add support for AES-CTR encryption when spilling to disk


h=
ttp://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMIT_MSG@8
PS2, Line 8:  
=
nit:
is a stream


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMIT_M=
SG@8
PS2, Line 8: use
nit: used with a


http://gerrit=2Ecloudera=2Eorg:808=
0/#/c/8861/2//COMMIT_MSG@9
PS2, Line 9: would
nit: can


http://gerrit=2Ecl=
oudera=2Eorg:8080/#/c/8861/2//COMMIT_MSG@14
PS2, Line 14: CTR mode is used =
if OpenSSL version>=3D1=2E0=2E1 at runtime, otherwise
nit,add:
With this pa=
tch


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMIT_MSG@15
PS2, Li=
ne 15:  
nit: using


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COMMI=
T_MSG@18
PS2, Line 18: run runtime tmp-file-mgr-test, openssl-util-test, bu=
ffer-pool-test and buffered-tuple-stream-test
nit: long line, wrap around a=
fter 72 characters=2E


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2//COM=
MIT_MSG@19
PS2, Line 19: 
please also mention that you added a test that te=
sts encryption in both modes


http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861=
/2/be/src/util/openssl-util=2Eh
File be/src/util/openssl-util=2Eh:

http://=
gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2/be/src/util/openssl-util=2Eh@59
PS2=
, Line 59: CTR/CFB
maybe add a line explaining when each mode is used=2E


=
http://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2/be/src/util/openssl-util=2Eh=
@89
PS2, Line 89: Maybe
nit, add:
Currently used only for testing but


htt=
p://gerrit=2Ecloudera=2Eorg:8080/#/c/8861/2/be/src/util/openssl-util=2Ecc
F=
ile be/src/util/openssl-util=2Ecc:

http://gerrit=2Ecloudera=2Eorg:8080/#/c=
/8861/2/be/src/util/openssl-util=2Ecc@104
PS2, Line 104:  
nit:
either CTR =
or CFB(stream cipher), both of which support arbitrary length ciphertexts -=
 it doesn't have to be a multiple of 16 bytes=2E Additionally, CTR mode is =
well-optimized(instruction level parallelism) with hardware acceleration on=
 x86 and PowerPC=2E


-- 
To view, visit http://gerrit=2Ecloudera=2Eorg:80=
80/8861
To unsubscribe, visit http://gerrit=2Ecloudera=2Eorg:8080/settings
=

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comme=
nt
Gerrit-Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff
Gerrit-Chang=
e-Number: 8861
Gerrit-PatchSet: 2
Gerrit-Owner: Xianda Ke <kexianda@gmail=
=2Ecom>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet=2Evig@cloudera=2Ecom>
G=
errit-Reviewer: Sailesh Mukil <sailesh@cloudera=2Ecom>
Gerrit-Reviewer: Tim=
 Armstrong <tarmstrong@cloudera=2Ecom>
Gerrit-Reviewer: Xianda Ke <kexianda=
@gmail=2Ecom>
Gerrit-Comment-Date: Thu, 28 Dec 2017 00:53:44 +0000
Gerrit-H=
asComments: Yes

--+FJGJ3ZuGPM=--