This is an automated email from the ASF dual-hosted git repository.
kgyrtkirk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
commit 94b43f40353216127742eaf1c3604479a36c660f
Author: Kevin Risden <kris...@apache.org>
AuthorDate: Tue Mar 17 17:01:17 2020 +0000
HIVE-22841: ThriftHttpServlet#getClientNameFromCookie should handle
CookieSigner IllegalArgumentException on invalid cookie signature (Kevin Risden
via Zoltan Haindrich)
Signed-off-by: Zoltan Haindrich <k...@rxd.hu>
---
.../auth/TestHttpCookieAuthenticationTest.java | 185 +++++++++++++++++
.../java/org/apache/hive/jdbc/HiveConnection.java | 2 +-
.../hive/jdbc/HttpRequestInterceptorBase.java | 5 +-
.../hive/service/cli/thrift/ThriftHttpServlet.java | 9 +-
.../org/apache/hive/service/TestCookieSigner.java | 53 +++--
.../cli/thrift/ThriftCliServiceTestWithCookie.java | 231 ---------------------
6 files changed, 231 insertions(+), 254 deletions(-)
diff --git
a/itests/hive-unit/src/test/java/org/apache/hive/service/auth/TestHttpCookieAuthenticationTest.java
b/itests/hive-unit/src/test/java/org/apache/hive/service/auth/TestHttpCookieAuthenticationTest.java
new file mode 100644
index 0000000..827cc68
--- /dev/null
+++
b/itests/hive-unit/src/test/java/org/apache/hive/service/auth/TestHttpCookieAuthenticationTest.java
@@ -0,0 +1,185 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hive.service.auth;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
+import java.lang.reflect.Field;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.Statement;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hive.jdbc.HiveConnection;
+import org.apache.hive.jdbc.HiveDriver;
+import org.apache.hive.jdbc.miniHS2.MiniHS2;
+import org.apache.http.client.CookieStore;
+import org.apache.http.client.HttpClient;
+import org.apache.http.cookie.Cookie;
+import org.apache.http.impl.cookie.BasicClientCookie;
+import org.apache.thrift.transport.THttpClient;
+import org.apache.thrift.transport.TTransport;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * TestHttpCookieAuthenticationTest.
+ */
+public class TestHttpCookieAuthenticationTest {
+ private static MiniHS2 miniHS2;
+
+ @BeforeClass
+ public static void startServices() throws Exception {
+ miniHS2 = new MiniHS2.Builder().withHTTPTransport().build();
+
+ Map<String, String> configOverlay = new HashMap<>();
+ configOverlay.put(HiveConf.ConfVars.HIVE_SUPPORT_CONCURRENCY.varname,
Boolean.FALSE.toString());
+
configOverlay.put(HiveConf.ConfVars.HIVE_SERVER2_THRIFT_HTTP_COOKIE_AUTH_ENABLED.varname,
Boolean.TRUE.toString());
+ miniHS2.start(configOverlay);
+ }
+
+ @AfterClass
+ public static void stopServices() throws Exception {
+ if (miniHS2 != null && miniHS2.isStarted()) {
+ miniHS2.stop();
+ miniHS2.cleanup();
+ miniHS2 = null;
+ MiniHS2.cleanupLocalDir();
+ }
+ }
+
+ @Test
+ public void testHttpJdbcCookies() throws Exception {
+ String sqlQuery = "show tables";
+
+ Class.forName(HiveDriver.class.getCanonicalName());
+
+ String username = System.getProperty("user.name");
+ try(Connection connection =
DriverManager.getConnection(miniHS2.getJdbcURL(), username, "bar")) {
+ assertNotNull(connection);
+
+ CookieStore cookieStore = getCookieStoreFromConnection(connection);
+ assertNotNull(cookieStore);
+
+ // Test that basic cookies worked
+ List<Cookie> cookies1 = cookieStore.getCookies();
+ assertEquals(1, cookies1.size());
+
+ try(Statement statement = connection.createStatement()) {
+ assertNotNull(statement);
+ try(ResultSet resultSet = statement.executeQuery(sqlQuery)) {
+ assertNotNull(resultSet);
+ }
+ }
+
+ // Check that cookies worked and still the same after a statement
+ List<Cookie> cookies2 = cookieStore.getCookies();
+ assertEquals(1, cookies2.size());
+ assertEquals(cookies1, cookies2);
+
+ // Empty out cookies to make sure same connection gets new cookies
+ cookieStore.clear();
+ assertTrue(cookieStore.getCookies().isEmpty());
+
+ try(Statement statement = connection.createStatement()) {
+ assertNotNull(statement);
+ try(ResultSet resultSet = statement.executeQuery(sqlQuery)) {
+ assertNotNull(resultSet);
+ }
+ }
+
+ // Check that cookies worked after clearing and got back new cookie
+ List<Cookie> cookies3 = cookieStore.getCookies();
+ assertEquals(1, cookies3.size());
+ assertNotEquals(cookies1, cookies3);
+
+
+ // Get original cookie to copy metadata
+ Cookie originalCookie = cookies3.get(0);
+
+ // Put in a bad client side cookie - ensure HS2 authenticates and
overwrites
+ BasicClientCookie badCookie = new BasicClientCookie("hive.server2.auth",
"bad");
+ badCookie.setDomain(originalCookie.getDomain());
+ badCookie.setPath(originalCookie.getPath());
+ badCookie.setExpiryDate(originalCookie.getExpiryDate());
+ cookieStore.addCookie(badCookie);
+
+ // Check that putting in the bad cookie overrode the original cookie
+ List<Cookie> cookies4 = cookieStore.getCookies();
+ assertEquals(1, cookies4.size());
+ assertTrue(cookies4.contains(badCookie));
+
+ try(Statement statement = connection.createStatement()) {
+ assertNotNull(statement);
+ try(ResultSet resultSet = statement.executeQuery(sqlQuery)) {
+ assertNotNull(resultSet);
+ }
+ }
+
+ // Check that cookies worked and replaced the bad cookie
+ List<Cookie> cookies5 = cookieStore.getCookies();
+ assertEquals(1, cookies5.size());
+ assertNotEquals(cookies4, cookies5);
+
+ try(Statement statement = connection.createStatement()) {
+ assertNotNull(statement);
+ try(ResultSet resultSet = statement.executeQuery(sqlQuery)) {
+ assertNotNull(resultSet);
+ }
+ }
+
+ // Check that cookies worked and didn't get replaced
+ List<Cookie> cookies6 = cookieStore.getCookies();
+ assertEquals(1, cookies6.size());
+ assertEquals(cookies5, cookies6);
+ }
+ }
+
+ // ((InternalHttpClient) ((THttpClient) ((HiveConnection)
connection).transport).client).cookieStore.getCookies()
+ private CookieStore getCookieStoreFromConnection(Connection connection)
throws Exception {
+ CookieStore cookieStore = null;
+ if (connection instanceof HiveConnection) {
+ HiveConnection hiveConnection = (HiveConnection) connection;
+
+ Field transportField =
hiveConnection.getClass().getDeclaredField("transport");
+ transportField.setAccessible(true);
+ TTransport transport = (TTransport) transportField.get(hiveConnection);
+
+ if(transport instanceof THttpClient) {
+ THttpClient httpTransport = (THttpClient) transport;
+ Field clientField =
httpTransport.getClass().getDeclaredField("client");
+ clientField.setAccessible(true);
+ HttpClient httpClient = (HttpClient) clientField.get(httpTransport);
+
+ Field cookieStoreField =
httpClient.getClass().getDeclaredField("cookieStore");
+ cookieStoreField.setAccessible(true);
+ cookieStore = (CookieStore) cookieStoreField.get(httpClient);
+ }
+ }
+ return cookieStore;
+ }
+}
diff --git a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
index d797d40..cbf6632 100644
--- a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
+++ b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
@@ -533,7 +533,7 @@ public class HiveConnection implements java.sql.Connection {
if (isCookieEnabled) {
// Create a http client with a retry mechanism when the server returns a
status code of 401.
httpClientBuilder =
- HttpClients.custom().setServiceUnavailableRetryStrategy(
+
HttpClients.custom().setDefaultCookieStore(cookieStore).setServiceUnavailableRetryStrategy(
new ServiceUnavailableRetryStrategy() {
@Override
public boolean retryRequest(final HttpResponse response, final
int executionCount,
diff --git a/jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java
b/jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java
index 1ef0ab1..bb1abd3 100644
--- a/jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java
+++ b/jdbc/src/java/org/apache/hive/jdbc/HttpRequestInterceptorBase.java
@@ -27,7 +27,6 @@ import org.apache.http.HttpException;
import org.apache.http.HttpRequest;
import org.apache.http.HttpRequestInterceptor;
import org.apache.http.client.CookieStore;
-import org.apache.http.client.protocol.ClientContext;
import org.apache.http.protocol.HttpContext;
public abstract class HttpRequestInterceptorBase implements
HttpRequestInterceptor {
@@ -60,9 +59,7 @@ public abstract class HttpRequestInterceptorBase implements
HttpRequestIntercept
// The necessary condition is either when there are no server side
cookies in the
// cookiestore which can be send back or when the server returns a 401
error code
// indicating that the previous cookie has expired.
- if (isCookieEnabled) {
- httpContext.setAttribute(ClientContext.COOKIE_STORE, cookieStore);
- }
+
// Generate the kerberos ticket under the following scenarios:
// 1. Cookie Authentication is disabled OR
// 2. The first time when the request is sent OR
diff --git
a/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
b/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
index 6eb2606..421aa5a 100644
--- a/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
+++ b/service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpServlet.java
@@ -292,7 +292,14 @@ public class ThriftHttpServlet extends TServlet {
// If we reached here, we have match for HS2 generated cookie
currValue = currCookie.getValue();
// Validate the value.
- currValue = signer.verifyAndExtract(currValue);
+ try {
+ currValue = signer.verifyAndExtract(currValue);
+ } catch (IllegalArgumentException e) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Invalid cookie", e);
+ }
+ currValue = null;
+ }
// Retrieve the user name, do the final validation step.
if (currValue != null) {
String userName = HttpAuthUtils.getUserNameFromCookieToken(currValue);
diff --git a/service/src/test/org/apache/hive/service/TestCookieSigner.java
b/service/src/test/org/apache/hive/service/TestCookieSigner.java
index aec6d47..54801d4 100644
--- a/service/src/test/org/apache/hive/service/TestCookieSigner.java
+++ b/service/src/test/org/apache/hive/service/TestCookieSigner.java
@@ -18,42 +18,61 @@
package org.apache.hive.service;
-import java.util.Random;
-
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import java.util.Random;
-import org.junit.After;
import org.junit.Before;
import org.junit.Test;
/**
- * CLIServiceTest.
+ * TestCookieSigner.
*
*/
public class TestCookieSigner {
-
- protected static CookieSigner cs;
private static final Random RAN = new Random();
- /**
- * @throws java.lang.Exception
- */
+ private CookieSigner cs;
+
@Before
- public void setUp() throws Exception {
+ public void setUp() {
cs = new CookieSigner(Long.toString(RAN.nextLong()).getBytes());
}
- /**
- * @throws java.lang.Exception
- */
- @After
- public void tearDown() throws Exception {
+ @Test
+ public void testVerifyAndExtract() {
+ String originalStr = "cu=scott";
+ String signedStr = cs.signCookie(originalStr);
+ assertEquals(originalStr, cs.verifyAndExtract(signedStr));
+ }
+
+ @Test
+ public void testVerifyAndExtractNoSignature() {
+ String originalStr = "cu=scott";
+ String signedStr = cs.signCookie(originalStr);
+ String modifedSignedStr = signedStr.replace("&s=", "");
+ try {
+ cs.verifyAndExtract(modifedSignedStr);
+ } catch (IllegalArgumentException e) {
+ assertEquals("Invalid input sign: " + modifedSignedStr, e.getMessage());
+ return;
+ }
+ fail("Expected IllegalArgumentException due to no signature");
}
@Test
- public void testVerifyAndExtract() throws Exception {
+ public void testVerifyAndExtractInvalidSignature() {
String originalStr = "cu=scott";
String signedStr = cs.signCookie(originalStr);
- assert(cs.verifyAndExtract(signedStr).equals(originalStr));
+ String modifedSignedStr = signedStr.replace("&s=", "&s=abc");
+ try {
+ cs.verifyAndExtract(modifedSignedStr);
+ } catch (IllegalArgumentException e) {
+ assertTrue(e.getMessage().startsWith("Invalid sign, original = "));
+ return;
+ }
+ fail("Expected IllegalArgumentException checking signature");
}
}
diff --git
a/service/src/test/org/apache/hive/service/cli/thrift/ThriftCliServiceTestWithCookie.java
b/service/src/test/org/apache/hive/service/cli/thrift/ThriftCliServiceTestWithCookie.java
deleted file mode 100644
index cbcaa97..0000000
---
a/service/src/test/org/apache/hive/service/cli/thrift/ThriftCliServiceTestWithCookie.java
+++ /dev/null
@@ -1,231 +0,0 @@
-
-
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.hive.service.cli.thrift;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.fail;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.concurrent.TimeUnit;
-
-import org.apache.hadoop.hive.conf.HiveConf;
-import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
-import org.apache.hadoop.hive.metastore.MetaStoreTestUtils;
-import org.apache.hive.service.Service;
-import org.apache.hive.service.auth.HiveAuthConstants;
-import org.apache.hive.service.cli.OperationHandle;
-import org.apache.hive.service.cli.OperationState;
-import org.apache.hive.service.cli.OperationStatus;
-import org.apache.hive.service.cli.SessionHandle;
-import org.apache.hive.service.server.HiveServer2;
-import org.junit.After;
-import org.junit.AfterClass;
-import org.junit.Before;
-import org.junit.BeforeClass;
-import org.junit.Test;
-
-/**
- * ThriftCLIServiceTestWithCookie.
- */
-public class ThriftCliServiceTestWithCookie {
-
- protected static int port;
- protected static String host = "localhost";
- protected static HiveServer2 hiveServer2;
- protected static ThriftCLIServiceClient client;
- protected static HiveConf hiveConf;
- protected static String USERNAME = "anonymous";
- protected static String PASSWORD = "anonymous";
-
- /**
- * @throws java.lang.Exception
- */
- @BeforeClass
- public static void setUpBeforeClass() throws Exception {
- // Find a free port
- port = MetaStoreTestUtils.findFreePort();
- hiveServer2 = new HiveServer2();
- hiveConf = new HiveConf();
- hiveConf.setBoolVar(ConfVars.HIVE_SERVER2_THRIFT_HTTP_COOKIE_AUTH_ENABLED,
true);
- // Set the cookie max age to a very low value so that
- // the server sends 401 very frequently
- hiveConf.setTimeVar(ConfVars.HIVE_SERVER2_THRIFT_HTTP_COOKIE_MAX_AGE, 1,
TimeUnit.SECONDS);
- hiveConf.setVar(ConfVars.HIVE_SERVER2_TRANSPORT_MODE, "http");
- hiveConf.setVar(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PATH, "cliservice");
-
- assertNotNull(port);
- assertNotNull(hiveServer2);
- assertNotNull(hiveConf);
-
- hiveConf.setBoolVar(ConfVars.HIVE_SERVER2_ENABLE_DOAS, false);
- hiveConf.setVar(ConfVars.HIVE_SERVER2_THRIFT_BIND_HOST, host);
- hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT, port);
- hiveConf.setVar(ConfVars.HIVE_SERVER2_AUTHENTICATION,
HiveAuthConstants.AuthTypes.NOSASL.toString());
-
- startHiveServer2WithConf(hiveConf);
-
- client = getServiceClientInternal();
- }
-
- /**
- * @throws java.lang.Exception
- */
- @AfterClass
- public static void tearDownAfterClass() throws Exception {
- stopHiveServer2();
- }
-
- protected static void startHiveServer2WithConf(HiveConf hiveConf) throws
Exception {
- Exception HS2Exception = null;
- boolean HS2Started = false;
- for (int tryCount = 0; tryCount < MetaStoreTestUtils.RETRY_COUNT;
tryCount++) {
- try {
- hiveServer2.init(hiveConf);
- hiveServer2.start();
- HS2Started = true;
- break;
- } catch (Exception t) {
- HS2Exception = t;
- port = MetaStoreTestUtils.findFreePort();
- hiveConf.setIntVar(ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT, port);
- hiveServer2 = new HiveServer2();
- }
- }
-
- if (!HS2Started) {
- HS2Exception.printStackTrace();
- fail();
- }
-
- // Wait for startup to complete
- Thread.sleep(2000);
- System.out.println("HiveServer2 started on port " + port);
- }
-
- protected static void stopHiveServer2() throws Exception {
- if (hiveServer2 != null) {
- hiveServer2.stop();
- }
- }
-
- protected static ThriftCLIServiceClient getServiceClientInternal() {
- for (Service service : hiveServer2.getServices()) {
- if (service instanceof ThriftBinaryCLIService) {
- return new ThriftCLIServiceClient((ThriftBinaryCLIService) service);
- }
- if (service instanceof ThriftHttpCLIService) {
- return new ThriftCLIServiceClient((ThriftHttpCLIService) service);
- }
- }
- throw new IllegalStateException("HiveServer2 not running Thrift service");
- }
-
- /**
- * @throws java.lang.Exception
- */
- @Before
- public void setUp() throws Exception {
- }
-
- /**
- * @throws java.lang.Exception
- */
- @After
- public void tearDown() throws Exception {
-
- }
-
- @Test
- public void testOpenSession() throws Exception {
- // Open a new client session
- SessionHandle sessHandle = client.openSession(USERNAME,
- PASSWORD, new HashMap<String, String>());
- // Session handle should not be null
- assertNotNull("Session handle should not be null", sessHandle);
- // Close client session
- client.closeSession(sessHandle);
- }
-
- @Test
- public void testGetFunctions() throws Exception {
- SessionHandle sessHandle = client.openSession(USERNAME,
- PASSWORD, new HashMap<String, String>());
- assertNotNull("Session handle should not be null", sessHandle);
-
- String catalogName = null;
- String schemaName = null;
- String functionName = "*";
-
- OperationHandle opHandle = client.getFunctions(sessHandle, catalogName,
- schemaName, functionName);
-
- assertNotNull("Operation handle should not be null", opHandle);
-
- client.closeSession(sessHandle);
- }
-
- /**
- * Test synchronous query execution
- * @throws Exception
- */
- @Test
- public void testExecuteStatement() throws Exception {
- Map<String, String> opConf = new HashMap<String, String>();
- // Open a new client session
- SessionHandle sessHandle = client.openSession(USERNAME,
- PASSWORD, opConf);
- // Session handle should not be null
- assertNotNull("Session handle should not be null", sessHandle);
-
- // Change lock manager to embedded mode
- String queryString = "SET hive.lock.manager=" +
- "org.apache.hadoop.hive.ql.lockmgr.EmbeddedLockManager";
- client.executeStatement(sessHandle, queryString, opConf);
-
- // Drop the table if it exists
- queryString = "DROP TABLE IF EXISTS TEST_EXEC_THRIFT";
- client.executeStatement(sessHandle, queryString, opConf);
-
- // Create a test table
- queryString = "CREATE TABLE TEST_EXEC_THRIFT(ID STRING)";
- client.executeStatement(sessHandle, queryString, opConf);
-
- // Execute another query
- queryString = "SELECT ID+1 FROM TEST_EXEC_THRIFT";
- OperationHandle opHandle = client.executeStatement(sessHandle,
queryString, opConf);
- assertNotNull(opHandle);
-
- OperationStatus opStatus = client.getOperationStatus(opHandle, false);
- assertNotNull(opStatus);
-
- OperationState state = opStatus.getState();
- // Expect query to be completed now
- assertEquals("Query should be finished", OperationState.FINISHED, state);
-
- // Cleanup
- queryString = "DROP TABLE TEST_EXEC_THRIFT";
- client.executeStatement(sessHandle, queryString, opConf);
-
- client.closeSession(sessHandle);
- }
-}
-
