Hive
  1. Hive
  2. HIVE-6857

Refactor HiveServer2 TSetIpAddressProcessor

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: HiveServer2
    • Labels:
      None

      Description

      Excerpt from HIVE-6837 and related issues:
      1. SessionManager#openSession:

      public SessionHandle openSession(TProtocolVersion protocol, String username, String password,
            Map<String, String> sessionConf, boolean withImpersonation, String delegationToken)
                throws HiveSQLException {
          HiveSession session;
          if (withImpersonation) {
            HiveSessionImplwithUGI hiveSessionUgi = new HiveSessionImplwithUGI(protocol, username, password,
              hiveConf, sessionConf, TSetIpAddressProcessor.getUserIpAddress(), delegationToken);
            session = HiveSessionProxy.getProxy(hiveSessionUgi, hiveSessionUgi.getSessionUgi());
            hiveSessionUgi.setProxySession(session);
          } else {
            session = new HiveSessionImpl(protocol, username, password, hiveConf, sessionConf,
                TSetIpAddressProcessor.getUserIpAddress());
          }
          session.setSessionManager(this);
          session.setOperationManager(operationManager);
          session.open();
          handleToSession.put(session.getSessionHandle(), session);
      
          try {
            executeSessionHooks(session);
          } catch (Exception e) {
            throw new HiveSQLException("Failed to execute session hooks", e);
          }
          return session.getSessionHandle();
        }
      

      Notice that if withImpersonation is set to true, we're using TSetIpAddressProcessor.getUserIpAddress() to get the IP address which is wrong for a kerberized setup (should use HiveAuthFactory#getIpAddress).

      2. Also, in case of a kerberized setup, we're wrapping the transport in a doAs (with UGI of the HiveServer2 process) which doesn't make sense to me: https://github.com/apache/hive/blob/trunk/shims/common-secure/src/main/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java#L335.

      3. The name TSetIpAddressProcessor should be replaced with something more meaningful like TPlainSASLProcessor.

      4. Consolidate thread locals used for username, ipaddress

      5. Do not directly use TSetIpAddressProcessor; get it via factory like here:
      https://github.com/apache/hive/blob/trunk/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java#L161

        Issue Links

          Activity

          Vaibhav Gumashta created issue -
          Vaibhav Gumashta made changes -
          Field Original Value New Value
          Link This issue is related to HIVE-6837 [ HIVE-6837 ]
          Vaibhav Gumashta made changes -
          Description Check the discussion here: HIVE-6837 Excerpt HIVE-6837. Issues:
          1. SessionManager#openSession:
          {code}
          public SessionHandle openSession(TProtocolVersion protocol, String username, String password,
                Map<String, String> sessionConf, boolean withImpersonation, String delegationToken)
                    throws HiveSQLException {
              HiveSession session;
              if (withImpersonation) {
                HiveSessionImplwithUGI hiveSessionUgi = new HiveSessionImplwithUGI(protocol, username, password,
                  hiveConf, sessionConf, TSetIpAddressProcessor.getUserIpAddress(), delegationToken);
                session = HiveSessionProxy.getProxy(hiveSessionUgi, hiveSessionUgi.getSessionUgi());
                hiveSessionUgi.setProxySession(session);
              } else {
                session = new HiveSessionImpl(protocol, username, password, hiveConf, sessionConf,
                    TSetIpAddressProcessor.getUserIpAddress());
              }
              session.setSessionManager(this);
              session.setOperationManager(operationManager);
              session.open();
              handleToSession.put(session.getSessionHandle(), session);

              try {
                executeSessionHooks(session);
              } catch (Exception e) {
                throw new HiveSQLException("Failed to execute session hooks", e);
              }
              return session.getSessionHandle();
            }
          {code}
          Notice that if withImpersonation is set to true, we're using TSetIpAddressProcessor.getUserIpAddress() to get the IP address which is wrong for a kerberized setup (should use HiveAuthFactory#getIpAddress).

          2. Also, in case of a kerberized setup, we're wrapping the transport in a doAs (with UGI of the HiveServer2 process) which doesn't make sense to me: https://github.com/apache/hive/blob/trunk/shims/common-secure/src/main/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java#L335.

          3. The name TSetIpAddressProcessor should be replaced with something more meaningful like TPlainSASLProcessor.

          4. Consolidate thread locals used for username, ipaddress

          5. Do not directly use TSetIpAddressProcessor; get it via factory like here:
          https://github.com/apache/hive/blob/trunk/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java#L161
          Vaibhav Gumashta made changes -
          Summary Consolidate HiveServer2 threadlocals Refactor HiveServer2 threadlocals
          Vaibhav Gumashta made changes -
          Summary Refactor HiveServer2 threadlocals Refactor HiveServer2 TSetIpAddressProcessor
          Hide
          Vaibhav Gumashta added a comment -

          Thejas M Nair I am just using this is a placeholder of issues I notice wrt TSetIpAddressProcessor, threadlocals. You might want to take a look. Some of these I'll resolve as part of HIVE-6864.

          Show
          Vaibhav Gumashta added a comment - Thejas M Nair I am just using this is a placeholder of issues I notice wrt TSetIpAddressProcessor, threadlocals. You might want to take a look. Some of these I'll resolve as part of HIVE-6864 .
          Vaibhav Gumashta made changes -
          Description Excerpt HIVE-6837. Issues:
          1. SessionManager#openSession:
          {code}
          public SessionHandle openSession(TProtocolVersion protocol, String username, String password,
                Map<String, String> sessionConf, boolean withImpersonation, String delegationToken)
                    throws HiveSQLException {
              HiveSession session;
              if (withImpersonation) {
                HiveSessionImplwithUGI hiveSessionUgi = new HiveSessionImplwithUGI(protocol, username, password,
                  hiveConf, sessionConf, TSetIpAddressProcessor.getUserIpAddress(), delegationToken);
                session = HiveSessionProxy.getProxy(hiveSessionUgi, hiveSessionUgi.getSessionUgi());
                hiveSessionUgi.setProxySession(session);
              } else {
                session = new HiveSessionImpl(protocol, username, password, hiveConf, sessionConf,
                    TSetIpAddressProcessor.getUserIpAddress());
              }
              session.setSessionManager(this);
              session.setOperationManager(operationManager);
              session.open();
              handleToSession.put(session.getSessionHandle(), session);

              try {
                executeSessionHooks(session);
              } catch (Exception e) {
                throw new HiveSQLException("Failed to execute session hooks", e);
              }
              return session.getSessionHandle();
            }
          {code}
          Notice that if withImpersonation is set to true, we're using TSetIpAddressProcessor.getUserIpAddress() to get the IP address which is wrong for a kerberized setup (should use HiveAuthFactory#getIpAddress).

          2. Also, in case of a kerberized setup, we're wrapping the transport in a doAs (with UGI of the HiveServer2 process) which doesn't make sense to me: https://github.com/apache/hive/blob/trunk/shims/common-secure/src/main/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java#L335.

          3. The name TSetIpAddressProcessor should be replaced with something more meaningful like TPlainSASLProcessor.

          4. Consolidate thread locals used for username, ipaddress

          5. Do not directly use TSetIpAddressProcessor; get it via factory like here:
          https://github.com/apache/hive/blob/trunk/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java#L161
          Excerpt from HIVE-6837 and related issues:
          1. SessionManager#openSession:
          {code}
          public SessionHandle openSession(TProtocolVersion protocol, String username, String password,
                Map<String, String> sessionConf, boolean withImpersonation, String delegationToken)
                    throws HiveSQLException {
              HiveSession session;
              if (withImpersonation) {
                HiveSessionImplwithUGI hiveSessionUgi = new HiveSessionImplwithUGI(protocol, username, password,
                  hiveConf, sessionConf, TSetIpAddressProcessor.getUserIpAddress(), delegationToken);
                session = HiveSessionProxy.getProxy(hiveSessionUgi, hiveSessionUgi.getSessionUgi());
                hiveSessionUgi.setProxySession(session);
              } else {
                session = new HiveSessionImpl(protocol, username, password, hiveConf, sessionConf,
                    TSetIpAddressProcessor.getUserIpAddress());
              }
              session.setSessionManager(this);
              session.setOperationManager(operationManager);
              session.open();
              handleToSession.put(session.getSessionHandle(), session);

              try {
                executeSessionHooks(session);
              } catch (Exception e) {
                throw new HiveSQLException("Failed to execute session hooks", e);
              }
              return session.getSessionHandle();
            }
          {code}
          Notice that if withImpersonation is set to true, we're using TSetIpAddressProcessor.getUserIpAddress() to get the IP address which is wrong for a kerberized setup (should use HiveAuthFactory#getIpAddress).

          2. Also, in case of a kerberized setup, we're wrapping the transport in a doAs (with UGI of the HiveServer2 process) which doesn't make sense to me: https://github.com/apache/hive/blob/trunk/shims/common-secure/src/main/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java#L335.

          3. The name TSetIpAddressProcessor should be replaced with something more meaningful like TPlainSASLProcessor.

          4. Consolidate thread locals used for username, ipaddress

          5. Do not directly use TSetIpAddressProcessor; get it via factory like here:
          https://github.com/apache/hive/blob/trunk/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java#L161
          Vaibhav Gumashta made changes -
          Fix Version/s 0.15.0 [ 12328723 ]
          Brock Noland made changes -
          Fix Version/s 1.2.0 [ 12329345 ]
          Fix Version/s 0.15.0 [ 12328723 ]
          Hide
          Sushanth Sowmyan added a comment -

          Removing fix version of 1.2.0 in preparation of release, since this is not a blocker for 1.2.0.

          Show
          Sushanth Sowmyan added a comment - Removing fix version of 1.2.0 in preparation of release, since this is not a blocker for 1.2.0.
          Sushanth Sowmyan made changes -
          Fix Version/s 1.2.0 [ 12329345 ]

            People

            • Assignee:
              Vaibhav Gumashta
              Reporter:
              Vaibhav Gumashta
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:

                Development