impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Xianda Ke (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-6128: Add support for AES-CTR encryption when spilling to disk CFB mode is a stream cipher and is secure when used with a different nonce/IV for every message. However it can be a performance bottleneck. CTR mode is also stream cipher and is secure
Date Fri, 29 Dec 2017 01:07:23 GMT
Xianda Ke has posted comments on this change. ( http://gerrit.cloudera.org:8080/8861 )

Change subject: IMPALA-6128: Add support for AES-CTR encryption when spilling to disk CFB
mode is a stream cipher and is secure when used with a different nonce/IV for every message.
However it can be a performance bottleneck. CTR mode is also stream cipher and is secure
......................................................................


Patch Set 3:

(11 comments)

Thank Bikramjeet for the review. Fixed.
And UT ars finished successfully. https://jenkins.impala.io/job/gerrit-verify-dryrun/1654/

 > (11 comments)

http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@7
PS2, Line 7: Add support for AES-CTR encryption when spilling t
> nit, how about this?:
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@8
PS2, Line 8:  
> nit:
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@8
PS2, Line 8: n u
> nit: used with a
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@9
PS2, Line 9: can b
> nit: can
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@14
PS2, Line 14: fall back to using CFB mode.
> nit,add:
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@15
PS2, Line 15: 
> nit: using
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@18
PS2, Line 18: buffered-tuple-stream-test
> nit: long line, wrap around after 72 characters.
Done


http://gerrit.cloudera.org:8080/#/c/8861/2//COMMIT_MSG@19
PS2, Line 19: The ut case openssl-util-test.EncryptInPlace tests encryption in both modes.
> please also mention that you added a test that tests encryption in both mod
Done


http://gerrit.cloudera.org:8080/#/c/8861/2/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/8861/2/be/src/util/openssl-util.h@59
PS2, Line 59: CTR/CFB
> maybe add a line explaining when each mode is used.
Done


http://gerrit.cloudera.org:8080/#/c/8861/2/be/src/util/openssl-util.h@89
PS2, Line 89: 
> nit, add:
Done


http://gerrit.cloudera.org:8080/#/c/8861/2/be/src/util/openssl-util.cc
File be/src/util/openssl-util.cc:

http://gerrit.cloudera.org:8080/#/c/8861/2/be/src/util/openssl-util.cc@104
PS2, Line 104:  
> nit:
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/8861
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9debc240615dd8cdbf00ec8730cff62ffef52aff
Gerrit-Change-Number: 8861
Gerrit-PatchSet: 3
Gerrit-Owner: Xianda Ke <kexianda@gmail.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet.vig@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Xianda Ke <kexianda@gmail.com>
Gerrit-Comment-Date: Fri, 29 Dec 2017 01:07:23 +0000
Gerrit-HasComments: Yes

Mime
  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message