[
https://issues.apache.org/jira/browse/HIVE-20544?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Karen Coppage updated HIVE-20544:
---------------------------------
Description:
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=====
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.
was:
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===
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 paramater. Since all fields 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 (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 valiate() 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.
> 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.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=====
> 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)
