[ 
https://issues.apache.org/jira/browse/HIVE-20544?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16631282#comment-16631282
 ] 

Hive QA commented on HIVE-20544:
--------------------------------

| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
|| || || || {color:brown} Prechecks {color} ||
| {color:green}+1{color} | {color:green} @author {color} | {color:green}  0m  
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
|| || || || {color:brown} master Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
47s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  7m 
25s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
55s{color} | {color:green} master passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green}  0m 
24s{color} | {color:green} master passed {color} |
| {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue}  0m 
41s{color} | {color:blue} itests/hive-unit in master has 2 extant Findbugs 
warnings. {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
42s{color} | {color:green} master passed {color} |
|| || || || {color:brown} Patch Compile Tests {color} ||
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue}  0m 
12s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green}  0m 
59s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green}  0m 
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green}  0m 
55s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red}  0m 
17s{color} | {color:red} itests/hive-unit: The patch generated 1 new + 0 
unchanged - 0 fixed = 1 total (was 0) {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green}  0m 
 0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} xml {color} | {color:green}  0m  
1s{color} | {color:green} The patch has no ill-formed XML file. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green}  0m 
49s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green}  0m 
41s{color} | {color:green} the patch passed {color} |
|| || || || {color:brown} Other Tests {color} ||
| {color:red}-1{color} | {color:red} asflicense {color} | {color:red}  0m 
15s{color} | {color:red} The patch generated 1 ASF License warnings. {color} |
| {color:black}{color} | {color:black} {color} | {color:black} 15m 47s{color} | 
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Optional Tests |  asflicense  javac  javadoc  findbugs  checkstyle  compile  
xml  |
| uname | Linux hiveptest-server-upstream 3.16.0-4-amd64 #1 SMP Debian 
3.16.36-1+deb8u1 (2016-09-03) x86_64 GNU/Linux |
| Build tool | maven |
| Personality | 
/data/hiveptest/working/yetus_PreCommit-HIVE-Build-14094/dev-support/hive-personality.sh
 |
| git revision | master / 727e4b2 |
| Default Java | 1.8.0_111 |
| findbugs | v3.0.0 |
| checkstyle | 
http://104.198.109.242/logs//PreCommit-HIVE-Build-14094/yetus/diff-checkstyle-itests_hive-unit.txt
 |
| asflicense | 
http://104.198.109.242/logs//PreCommit-HIVE-Build-14094/yetus/patch-asflicense-problems.txt
 |
| modules | C: service-rpc itests/hive-unit U: . |
| Console output | 
http://104.198.109.242/logs//PreCommit-HIVE-Build-14094/yetus.txt |
| Powered by | Apache Yetus    http://yetus.apache.org |


This message was automatically generated.



> TOpenSessionReq logs password and username
> ------------------------------------------
>
>                 Key: HIVE-20544
>                 URL: https://issues.apache.org/jira/browse/HIVE-20544
>             Project: Hive
>          Issue Type: Bug
>          Components: Hive
>    Affects Versions: 4.0.0
>            Reporter: Karen Coppage
>            Assignee: Karen Coppage
>            Priority: Major
>              Labels: beginner, patch, security
>         Attachments: HIVE-20544.1.patch, HIVE-20544.2.patch, 
> HIVE-20544.3.patch, HIVE-20544.3.patch, HIVE-20544.4.patch, 
> HIVE-20544.4.patch, HIVE-20544.patch, non-solution.patch, 
> working-solution.patch
>
>
> In 
> service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TOpenSessionReq,
>  if client protocol is unset, validate() and toString() prints both username 
> and password to logs.
> Logging a password is a security risk. We should hide the *******.
> =====Edit===== (no longer relevant, see comments)
> This issue is tricky since it is caused in a fully generated class. I've been 
> playing around and have found one working solution, butI'd truly appreciate 
> ideas for a more elegant solution or input.
> The problem:
>  TCLIService.thrift is the template for generating all classes in 
> service-rpc. Struct TOpenSessionReq is OpenSession()'s one parameter and is 
> defined thus:
> {noformat}
> struct TOpenSessionReq {
>   1: required TProtocolVersion client_protocol = 
> TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V10
>   2: optional string username
>   3: optional string password
>   4: optional map<string, string> configuration
> }
> {noformat}
> In the generated class TOpenSessionReq.java, client_protocol is checked by a 
> validate() method, which is called quite a few times; if client_protocol is 
> not set, it throws a TProtocolException, passing along a toString(). This 
> toString() gets the names and values of all fields, including username and 
> password.
> Working solution:
>  * Create a separate struct containing only the username and password, and 
> pass it to OpenSession() as a second parameter. Since all fields in the new 
> struct are "optional", the generated validate() is empty – toString() is 
> never used. This involves changing core classes and breaks the "Each function 
> should take exactly one parameter" coding convention (detailed at 
> service-rpc/if/TCLIService.thrift:27).
>  See working-solution.patch.
> What doesn't work:
>  * Making client_protocol optional instead of required. Apparently this will 
> break everything.
>  * Overwriting toString() – TOpenSessionReq is a struct.
>  * Creating two Thrift structs, one struct for required (TRequiredReq) and 
> one for optional (TOptionalReq) fields, and nesting them in struct 
> TOpenSessionReq. This doesn't work because validate() in TOpenSessionReq can 
> call TOptionalReq.toString(), which prints the password to logs. This will 
> happen if TRequiredReq.client_protocol isn't set.
>  See non-solution.patch
>  * Asking Thrift devs to change their code. I wrote them an email but have no 
> expectations.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to