diff --git a/service-rpc/pom.xml b/service-rpc/pom.xml index 6f3da75a21a8..9a171dbdd6b2 100644 --- a/service-rpc/pom.xml +++ b/service-rpc/pom.xml @@ -158,6 +158,23 @@ + + mask-delegation-token + process-sources + + replace + + + ${basedir}/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/ + TCancelDelegationTokenReq.java,TRenewDelegationTokenReq.java + + + if \(this.delegationToken \=\= null\) \{\n sb.append\(\"null"\)\;\n \} else \{\n sb.append\(this.delegationToken\)\;\n \} + sb.append("*** REDACTED ***"); + + + + diff --git a/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenReq.java b/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenReq.java index 396b1fa23cb8..1f2941b0ef9b 100644 --- a/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenReq.java +++ b/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCancelDelegationTokenReq.java @@ -333,11 +333,7 @@ public java.lang.String toString() { first = false; if (!first) sb.append(", "); sb.append("delegationToken:"); - if (this.delegationToken == null) { - sb.append("null"); - } else { - sb.append(this.delegationToken); - } + sb.append("*** REDACTED ***"); first = false; sb.append(")"); return sb.toString(); diff --git a/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TRenewDelegationTokenReq.java b/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TRenewDelegationTokenReq.java index 57ab7481b3ff..7457b2d170d8 100644 --- a/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TRenewDelegationTokenReq.java +++ b/service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TRenewDelegationTokenReq.java @@ -333,11 +333,7 @@ public java.lang.String toString() { first = false; if (!first) sb.append(", "); sb.append("delegationToken:"); - if (this.delegationToken == null) { - sb.append("null"); - } else { - sb.append(this.delegationToken); - } + sb.append("*** REDACTED ***"); first = false; sb.append(")"); return sb.toString(); diff --git a/service-rpc/src/test/org/apache/hive/service/rpc/thrift/TestDelegationTokenRequestRedaction.java b/service-rpc/src/test/org/apache/hive/service/rpc/thrift/TestDelegationTokenRequestRedaction.java new file mode 100644 index 000000000000..f6f48442dc60 --- /dev/null +++ b/service-rpc/src/test/org/apache/hive/service/rpc/thrift/TestDelegationTokenRequestRedaction.java @@ -0,0 +1,47 @@ +/* + * 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.rpc.thrift; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class TestDelegationTokenRequestRedaction { + private static final String LIVE_TOKEN = "LIVE_DELEGATION_TOKEN_ABC123"; + + @Test + public void testCancelReqToStringRedactsDelegationToken() { + TCancelDelegationTokenReq req = new TCancelDelegationTokenReq(); + req.setDelegationToken(LIVE_TOKEN); + + String reqString = req.toString(); + assertTrue(reqString.contains("delegationToken:")); + assertFalse(reqString.contains(LIVE_TOKEN)); + } + + @Test + public void testRenewReqToStringRedactsDelegationToken() { + TRenewDelegationTokenReq req = new TRenewDelegationTokenReq(); + req.setDelegationToken(LIVE_TOKEN); + + String reqString = req.toString(); + assertTrue(reqString.contains("delegationToken:")); + assertFalse(reqString.contains(LIVE_TOKEN)); + } +} diff --git a/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java b/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java index 9e9d47837fd4..532e7982b679 100644 --- a/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java +++ b/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java @@ -257,8 +257,7 @@ public void cancelDelegationToken(String delegationToken) throws HiveSQLExceptio try { delegationTokenManager.cancelDelegationToken(delegationToken); } catch (IOException e) { - throw new HiveSQLException( - "Error canceling delegation token " + delegationToken, "08S01", e); + throw new HiveSQLException(delegationTokenErrorMessage("canceling"), "08S01", e); } } @@ -270,8 +269,7 @@ public void renewDelegationToken(String delegationToken) throws HiveSQLException try { delegationTokenManager.renewDelegationToken(delegationToken); } catch (IOException e) { - throw new HiveSQLException( - "Error renewing delegation token " + delegationToken, "08S01", e); + throw new HiveSQLException(delegationTokenErrorMessage("renewing"), "08S01", e); } } @@ -283,7 +281,7 @@ public String verifyDelegationToken(String delegationToken) throws HiveSQLExcept try { return delegationTokenManager.verifyDelegationToken(delegationToken); } catch (IOException e) { - String msg = "Error verifying delegation token " + delegationToken; + String msg = delegationTokenErrorMessage("verifying"); LOG.error(msg, e); throw new HiveSQLException(msg, "08S01", e); } @@ -297,11 +295,14 @@ public String getUserFromToken(String delegationToken) throws HiveSQLException { try { return delegationTokenManager.getUserFromToken(delegationToken); } catch (IOException e) { - throw new HiveSQLException( - "Error extracting user from delegation token " + delegationToken, "08S01", e); + throw new HiveSQLException(delegationTokenErrorMessage("extracting user from"), "08S01", e); } } + private static String delegationTokenErrorMessage(String operation) { + return "Error " + operation + " delegation token"; + } + public static void verifyProxyAccess(String realUser, String proxyUser, String ipAddress, HiveConf hiveConf) throws HiveSQLException { try { diff --git a/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java b/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java index ccf576fe50d3..a2e5f4357769 100644 --- a/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java +++ b/service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java @@ -362,7 +362,7 @@ public TCancelDelegationTokenResp CancelDelegationToken(TCancelDelegationTokenRe hiveAuthFactory, req.getDelegationToken()); resp.setStatus(OK_STATUS); } catch (HiveSQLException e) { - LOG.error("Failed to cancel delegation token [request: {}]", req, e); + LOG.error("Failed to cancel delegation token for session {}", req.getSessionHandle(), e); resp.setStatus(HiveSQLException.toTStatus(e)); } } @@ -381,7 +381,7 @@ public TRenewDelegationTokenResp RenewDelegationToken(TRenewDelegationTokenReq r hiveAuthFactory, req.getDelegationToken()); resp.setStatus(OK_STATUS); } catch (HiveSQLException e) { - LOG.error("Failed to renew delegation token [request: {}]", e); + LOG.error("Failed to renew delegation token for session {}", req.getSessionHandle(), e); resp.setStatus(HiveSQLException.toTStatus(e)); } } diff --git a/service/src/test/org/apache/hive/service/auth/TestDelegationTokenLeakPrevention.java b/service/src/test/org/apache/hive/service/auth/TestDelegationTokenLeakPrevention.java new file mode 100644 index 000000000000..aa4c92d55e9b --- /dev/null +++ b/service/src/test/org/apache/hive/service/auth/TestDelegationTokenLeakPrevention.java @@ -0,0 +1,109 @@ +/* + * 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.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; + +import java.io.IOException; +import java.lang.reflect.Field; + +import org.apache.hadoop.hive.conf.HiveConf; +import org.apache.hadoop.hive.metastore.security.MetastoreDelegationTokenManager; +import org.apache.hive.service.cli.HiveSQLException; +import org.apache.hive.service.rpc.thrift.TStatus; +import org.junit.Test; +import org.apache.thrift.transport.TTransportException; + +public class TestDelegationTokenLeakPrevention { + private static final String LIVE_TOKEN = "LIVE_DELEGATION_TOKEN_ABC123"; + + @Test + public void testCancelDelegationTokenExceptionDoesNotContainToken() throws Exception { + MetastoreDelegationTokenManager tokenManager = mock(MetastoreDelegationTokenManager.class); + doThrow(new IOException("boom")).when(tokenManager).cancelDelegationToken(LIVE_TOKEN); + + HiveAuthFactory authFactory = createAuthFactoryWithTokenManager(tokenManager); + assertTokenIsNotExposed(() -> authFactory.cancelDelegationToken(LIVE_TOKEN), + "Error canceling delegation token"); + } + + @Test + public void testRenewDelegationTokenExceptionDoesNotContainToken() throws Exception { + MetastoreDelegationTokenManager tokenManager = mock(MetastoreDelegationTokenManager.class); + doThrow(new IOException("boom")).when(tokenManager).renewDelegationToken(LIVE_TOKEN); + + HiveAuthFactory authFactory = createAuthFactoryWithTokenManager(tokenManager); + assertTokenIsNotExposed(() -> authFactory.renewDelegationToken(LIVE_TOKEN), + "Error renewing delegation token"); + } + + @Test + public void testVerifyDelegationTokenExceptionDoesNotContainToken() throws Exception { + MetastoreDelegationTokenManager tokenManager = mock(MetastoreDelegationTokenManager.class); + doThrow(new IOException("boom")).when(tokenManager).verifyDelegationToken(LIVE_TOKEN); + + HiveAuthFactory authFactory = createAuthFactoryWithTokenManager(tokenManager); + assertTokenIsNotExposed(() -> authFactory.verifyDelegationToken(LIVE_TOKEN), + "Error verifying delegation token"); + } + + @Test + public void testGetUserFromTokenExceptionDoesNotContainToken() throws Exception { + MetastoreDelegationTokenManager tokenManager = mock(MetastoreDelegationTokenManager.class); + doThrow(new IOException("boom")).when(tokenManager).getUserFromToken(LIVE_TOKEN); + + HiveAuthFactory authFactory = createAuthFactoryWithTokenManager(tokenManager); + assertTokenIsNotExposed(() -> authFactory.getUserFromToken(LIVE_TOKEN), + "Error extracting user from delegation token"); + } + + private HiveAuthFactory createAuthFactoryWithTokenManager(MetastoreDelegationTokenManager tokenManager) + throws NoSuchFieldException, IllegalAccessException, TTransportException { + HiveAuthFactory authFactory = new HiveAuthFactory(new HiveConf(), false); + Field delegationTokenManagerField = HiveAuthFactory.class.getDeclaredField("delegationTokenManager"); + delegationTokenManagerField.setAccessible(true); + delegationTokenManagerField.set(authFactory, tokenManager); + return authFactory; + } + + private void assertTokenIsNotExposed(ThrowingRunnable callback, String expectedMessagePrefix) + throws Exception { + try { + callback.run(); + fail("Expected HiveSQLException to be thrown"); + } catch (HiveSQLException e) { + assertMessageIsSanitized(e.getMessage(), expectedMessagePrefix); + TStatus status = HiveSQLException.toTStatus(e); + assertMessageIsSanitized(status.getErrorMessage(), expectedMessagePrefix); + } + } + + private void assertMessageIsSanitized(String message, String expectedMessagePrefix) { + assertTrue(message.contains(expectedMessagePrefix)); + assertFalse(message.contains(LIVE_TOKEN)); + } + + @FunctionalInterface + private interface ThrowingRunnable { + void run() throws Exception; + } +}