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 abc067abb9ace807f0106d896ce84e48967d3e9c Author: Kevin Risden <kris...@apache.org> AuthorDate: Tue Mar 17 17:01:03 2020 +0000 HIVE-22539: HiveServer2 SPNEGO authentication should skip if authorization header is empty (Kevin Risden via Zoltan Haindrich) Signed-off-by: Zoltan Haindrich <k...@rxd.hu> --- .../hive/service/cli/thrift/ThriftHttpServlet.java | 53 ++++++++-------- .../service/cli/thrift/ThriftHttpServletTest.java | 71 ++++++++++++++++++++++ 2 files changed, 98 insertions(+), 26 deletions(-) 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 e2231c2..6eb2606 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 @@ -36,6 +36,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import javax.ws.rs.core.NewCookie; +import com.google.common.annotations.VisibleForTesting; import com.google.common.io.ByteStreams; import org.apache.commons.codec.binary.Base64; import org.apache.commons.codec.binary.StringUtils; @@ -45,7 +46,6 @@ import org.apache.hadoop.hive.shims.HadoopShims.KerberosNameShim; import org.apache.hadoop.hive.shims.ShimLoader; import org.apache.hadoop.hive.shims.Utils; import org.apache.hadoop.security.UserGroupInformation; -import org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator; import org.apache.hive.service.CookieSigner; import org.apache.hive.service.auth.AuthenticationProviderFactory; import org.apache.hive.service.auth.AuthenticationProviderFactory.AuthMethods; @@ -163,7 +163,7 @@ public class ThriftHttpServlet extends TServlet { List<String> forwardedAddresses = Arrays.asList(forwarded_for.split(",")); SessionManager.setForwardedAddresses(forwardedAddresses); } else { - SessionManager.setForwardedAddresses(Collections.<String>emptyList()); + SessionManager.setForwardedAddresses(Collections.emptyList()); } // If the cookie based authentication is not enabled or the request does not have a valid @@ -196,7 +196,7 @@ public class ThriftHttpServlet extends TServlet { String delegationToken = request.getHeader(HIVE_DELEGATION_TOKEN_HEADER); // Each http request must have an Authorization header if ((delegationToken != null) && (!delegationToken.isEmpty())) { - clientUserName = doTokenAuth(request, response); + clientUserName = doTokenAuth(request); } else { clientUserName = doKerberosAuth(request); } @@ -319,12 +319,12 @@ public class ThriftHttpServlet extends TServlet { * Each cookie is of the format [key]=[value] */ private String toCookieStr(Cookie[] cookies) { - String cookieStr = ""; + StringBuilder cookieStr = new StringBuilder(); - for (Cookie c : cookies) { - cookieStr += c.getName() + "=" + c.getValue() + " ;\n"; + for (Cookie c : cookies) { + cookieStr.append(c.getName()).append('=').append(c.getValue()).append(" ;\n"); } - return cookieStr; + return cookieStr.toString(); } /** @@ -386,9 +386,9 @@ public class ThriftHttpServlet extends TServlet { /** * Do the LDAP/PAM authentication - * @param request - * @param authType - * @throws HttpAuthenticationException + * @param request request to authenticate + * @param authType type of authentication + * @throws HttpAuthenticationException on error authenticating end user */ private String doPasswdAuth(HttpServletRequest request, String authType) throws HttpAuthenticationException { @@ -408,7 +408,7 @@ public class ThriftHttpServlet extends TServlet { return userName; } - private String doTokenAuth(HttpServletRequest request, HttpServletResponse response) + private String doTokenAuth(HttpServletRequest request) throws HttpAuthenticationException { String tokenStr = request.getHeader(HIVE_DELEGATION_TOKEN_HEADER); try { @@ -424,18 +424,23 @@ public class ThriftHttpServlet extends TServlet { * which GSS-API will extract information from. * In case of a SPNego request we use the httpUGI, * for the authenticating service tickets. - * @param request - * @return - * @throws HttpAuthenticationException + * @param request Request to act on + * @return client principal name + * @throws HttpAuthenticationException on error authenticating the user */ - private String doKerberosAuth(HttpServletRequest request) + @VisibleForTesting + String doKerberosAuth(HttpServletRequest request) throws HttpAuthenticationException { - // Try authenticating with the http/_HOST principal + // Each http request must have an Authorization header + // Check before trying to do kerberos authentication twice + getAuthHeader(request, authType); + + // Try authenticating with the HTTP/_HOST principal if (httpUGI != null) { try { return httpUGI.doAs(new HttpKerberosServerAction(request, httpUGI)); } catch (Exception e) { - LOG.info("Failed to authenticate with http/_HOST kerberos principal, " + + LOG.info("Failed to authenticate with HTTP/_HOST kerberos principal, " + "trying with hive/_HOST kerberos principal"); } } @@ -443,13 +448,9 @@ public class ThriftHttpServlet extends TServlet { try { return serviceUGI.doAs(new HttpKerberosServerAction(request, serviceUGI)); } catch (Exception e) { - if (e.getCause() instanceof HttpEmptyAuthenticationException) { - throw (HttpEmptyAuthenticationException)e.getCause(); - } LOG.error("Failed to authenticate with hive/_HOST kerberos principal"); throw new HttpAuthenticationException(e); } - } class HttpKerberosServerAction implements PrivilegedExceptionAction<String> { @@ -578,10 +579,10 @@ public class ThriftHttpServlet extends TServlet { /** * Returns the base64 encoded auth header payload - * @param request - * @param authType - * @return - * @throws HttpAuthenticationException + * @param request request to interrogate + * @param authType Either BASIC or NEGOTIATE + * @return base64 encoded auth header payload + * @throws HttpAuthenticationException exception if header is missing or empty */ private String getAuthHeader(HttpServletRequest request, String authType) throws HttpAuthenticationException { @@ -602,7 +603,7 @@ public class ThriftHttpServlet extends TServlet { } authHeaderBase64String = authHeader.substring(beginIndex); // Authorization header must have a payload - if (authHeaderBase64String == null || authHeaderBase64String.isEmpty()) { + if (authHeaderBase64String.isEmpty()) { throw new HttpAuthenticationException("Authorization header received " + "from the client does not contain any data."); } diff --git a/service/src/test/org/apache/hive/service/cli/thrift/ThriftHttpServletTest.java b/service/src/test/org/apache/hive/service/cli/thrift/ThriftHttpServletTest.java new file mode 100644 index 0000000..948e97f --- /dev/null +++ b/service/src/test/org/apache/hive/service/cli/thrift/ThriftHttpServletTest.java @@ -0,0 +1,71 @@ +/* + * 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 org.apache.hive.service.auth.HiveAuthConstants; +import org.apache.hive.service.auth.HttpAuthUtils; +import org.apache.hive.service.auth.ldap.HttpEmptyAuthenticationException; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +import org.junit.runner.RunWith; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +import javax.servlet.http.HttpServletRequest; + +/** + * ThriftHttpServletTest. + */ +@RunWith(MockitoJUnitRunner.class) +public class ThriftHttpServletTest { + @Rule + public ExpectedException exceptionRule = ExpectedException.none(); + + private ThriftHttpServlet thriftHttpServlet; + + @Before + public void setUp() { + String authType = HiveAuthConstants.AuthTypes.KERBEROS.toString(); + thriftHttpServlet = new ThriftHttpServlet(null, null, authType, null, null, null); + } + + @Test + public void testMissingAuthorizatonHeader() throws Exception { + HttpServletRequest httpServletRequest = Mockito.mock(HttpServletRequest.class); + Mockito.when(httpServletRequest.getHeader(HttpAuthUtils.AUTHORIZATION)).thenReturn(null); + + exceptionRule.expect(HttpEmptyAuthenticationException.class); + exceptionRule.expectMessage("Authorization header received " + + "from the client is empty."); + thriftHttpServlet.doKerberosAuth(httpServletRequest); + } + + @Test + public void testEmptyAuthorizatonHeader() throws Exception { + HttpServletRequest httpServletRequest = Mockito.mock(HttpServletRequest.class); + Mockito.when(httpServletRequest.getHeader(HttpAuthUtils.AUTHORIZATION)).thenReturn(""); + + exceptionRule.expect(HttpEmptyAuthenticationException.class); + exceptionRule.expectMessage("Authorization header received " + + "from the client is empty."); + thriftHttpServlet.doKerberosAuth(httpServletRequest); + } +}