> On July 23, 2013, 9:48 p.m., Thejas Nair wrote: > > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 142 > > <https://reviews.apache.org/r/12824/diff/1/?file=324969#file324969line142> > > > > the HIVE_AUTH_TYPE env variable is called "auth". > > Should we use something more descriptive like "sasl.qop" as the > > variable that sets the QOP level. > > > > Arup Malakar wrote: > I am totally agree that a different key name should be used for qop > settings. As the current HIVE_AUTH_TYPE configuration key is overloaded. > Original idea was to clean up the configuration keys which is being taken > care of in: https://issues.apache.org/jira/browse/HIVE-4232. Once the auth > params are taken care of, I had plans of introducing a new parameter called > qop which would be used to configure the QoP alone. But since HIVE-4232 is > not yet committed, I ended up using the HIVE_AUTH_TYPE. I can rebase if > HIVE-4232 goes in.
I am totally agree that a different key name should be used for qop settings. As the current HIVE_AUTH_TYPE configuration key is overloaded. Original idea was to clean up the configuration keys which is being taken care of in: https://issues.apache.org/jira/browse/HIVE-4232. Once the auth params are taken care of, I had plans of introducing a new parameter called qop which would be used to configure the QoP alone. But since HIVE-4232 is not yet committed, I ended up using the HIVE_AUTH_TYPE. I can rebase if HIVE-4232 goes in. > On July 23, 2013, 9:48 p.m., Thejas Nair wrote: > > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java, line 142 > > <https://reviews.apache.org/r/12824/diff/1/?file=324969#file324969line142> > > > > the HIVE_AUTH_TYPE env variable is called "auth". > > Should we use something more descriptive like "sasl.qop" as the > > variable that sets the QOP level. > > > > Arup Malakar wrote: > I am totally agree that a different key name should be used for qop > settings. As the current HIVE_AUTH_TYPE configuration key is overloaded. > Original idea was to clean up the configuration keys which is being taken > care of in: https://issues.apache.org/jira/browse/HIVE-4232. Once the auth > params are taken care of, I had plans of introducing a new parameter called > qop which would be used to configure the QoP alone. But since HIVE-4232 is > not yet committed, I ended up using the HIVE_AUTH_TYPE. I can rebase if > HIVE-4232 goes in. I am totally agree that a different key name should be used for qop settings. As the current HIVE_AUTH_TYPE configuration key is overloaded. Original idea was to clean up the configuration keys which is being taken care of in: https://issues.apache.org/jira/browse/HIVE-4232. Once the auth params are taken care of, I had plans of introducing a new parameter called qop which would be used to configure the QoP alone. But since HIVE-4232 is not yet committed, I ended up using the HIVE_AUTH_TYPE. I can rebase if HIVE-4232 goes in. > On July 23, 2013, 9:48 p.m., Thejas Nair wrote: > > shims/src/common-secure/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java, > > line 111 > > <https://reviews.apache.org/r/12824/diff/1/?file=324973#file324973line111> > > > > This function is called from hive metastore client. Using > > SaslRpcServer.SASL_PROPS here means that setting hadoop.rpc.protection will > > determine the QOP level, if we make a call to SaslRpcServer.init(conf) from > > anywhere in the code. But that function is not being called. > > > > I think it makes sense to use hadoop.rpc.protection for metastore QOP, > > since metastore usually not exposed 'outside' the cluster unlike hive > > server2. It is often viewed as something 'inside the cluster'. > > > > Should we change this function to take in a configuration object and > > use that to call SaslRpcServer.init(conf) ? The current createClientTransport method (without this patch) uses SaslRpcServer.SASL_PROPS too, but it doesn't call SaslRpcServer.init(conf) so I assumed SaslRpcServer.init(conf) is being called before reaching this method. But looking at https://issues.apache.org/jira/browse/HIVE-4232 I realized that this is indeed a bug in current code. Rather than doing init() in createTransportFactory() and createClientTransport() I removed the default method that uses SaslRpcServer.SASL_PROPS. Both these methods now only takes Map<String, String>. In case of both metastore client/server the code gets the Sasl propeties from MetaStoreUtils.getMetaStoreSaslProperties(conf) and passes it to the methods in HadoopThriftAuthBridge20S. Reasons: 1. We could remove the redundant methods that defaults to SaslRpcServer.SASL_PROPS 2. Changing meta store to use different configuration can be easily accomplished by modifying in only one place MetaStoreUtils.getMetaStoreSaslProperties(conf) Let me know what you think of it. I have a question though, is it okay to access SaslRpcSercer.SASL_PROPS from MetaStoreUtils class, I mean compiling with hadoop 20 wise? - Arup ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12824/#review23711 ----------------------------------------------------------- On July 24, 2013, 4:43 p.m., Arup Malakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12824/ > ----------------------------------------------------------- > > (Updated July 24, 2013, 4:43 p.m.) > > > Review request for hive. > > > Bugs: HIVE-4911 > https://issues.apache.org/jira/browse/HIVE-4911 > > > Repository: hive-git > > > Description > ------- > > The QoP for hive server 2 should be configurable to enable encryption. A new > configuration should be exposed "hive.server2.thrift.rpc.protection". This > would give greater control configuring hive server 2 service. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java > 11c31216495d0c4e454f2627af5c93a9f270b1fe > conf/hive-default.xml.template 603b475802152a4bd5ab92a4c7146b56f6be020d > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java > 00f43511b478c687b7811fc8ad66af2b507a3626 > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 72eac989394a3899998e52d3845b02bb38ebeaad > > metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > cef50f40ccb047a8135f704b2997968a2cf477b8 > metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java > 88151a1d48b12cf3a8346ae94b6d1a182a331992 > service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java > 1809e1b26ceee5de14a354a0e499aa8c0ab793bf > service/src/java/org/apache/hive/service/auth/KerberosSaslHelper.java > 379dafb8377aed55e74f0ae18407996bb9e1216f > service/src/java/org/apache/hive/service/auth/SaslQOP.java PRE-CREATION > > shims/src/common-secure/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java > 777226f8da0af2235d4294cd6a676fa8192c89e4 > > shims/src/common/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge.java > 9b0ec0a75563b41339e6fc747556440fdf83e31e > > Diff: https://reviews.apache.org/r/12824/diff/ > > > Testing > ------- > > > Thanks, > > Arup Malakar > >
