From fa36fd82d1aefa874fb70645655e87c374fb6b5c Mon Sep 17 00:00:00 2001 From: Vyom Mani Tiwari Date: Fri, 18 Sep 2026 16:00:15 +0530 Subject: [PATCH] RANGER-5799: Validate class assignability before instantiating configured service-def classes --- .../RangerAbstractGeolocationProvider.java | 12 ++- .../contextenricher/RangerGdsEnricher.java | 12 ++- .../contextenricher/RangerTagEnricher.java | 13 +++- .../RangerUserStoreEnricher.java | 13 +++- .../policyengine/RangerPolicyRepository.java | 10 ++- .../TestRangerGdsEnricher.java | 23 ++++++ ...RangerPolicyRepositoryContextEnricher.java | 73 +++++++++++++++++++ .../org/apache/ranger/biz/ServiceMgr.java | 5 +- .../org/apache/ranger/biz/TestServiceMgr.java | 38 ++++++++++ 9 files changed, 184 insertions(+), 15 deletions(-) create mode 100644 agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerPolicyRepositoryContextEnricher.java diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java index 1c9e0a64c3..9340d9e8f1 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerAbstractGeolocationProvider.java @@ -27,6 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.lang.reflect.InvocationTargetException; import java.util.Map; public abstract class RangerAbstractGeolocationProvider extends RangerAbstractContextEnricher { @@ -60,15 +61,20 @@ public void init() { try { // Get the class definition and ensure it is of the correct type + Class cls = Class.forName(geoSourceLoader, false, RangerAbstractGeolocationProvider.class.getClassLoader()); + if (!GeolocationStore.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + geoSourceLoader + " is not assignable to " + GeolocationStore.class.getName()); + } + @SuppressWarnings("unchecked") - Class geoSourceLoaderClass = (Class) Class.forName(geoSourceLoader); + Class geoSourceLoaderClass = (Class) cls; // instantiate the loader class and initialize it with options - geoStore = geoSourceLoaderClass.newInstance(); + geoStore = geoSourceLoaderClass.getDeclaredConstructor().newInstance(); } catch (ClassNotFoundException exception) { LOG.error("RangerAbstractGeolocationProvider.init() - Class {} not found, exception={}", geoSourceLoader, exception.toString()); } catch (ClassCastException exception) { LOG.error("RangerAbstractGeolocationProvider.init() - Class {} is not a type of GeolocationStore, exception={}", geoSourceLoader, exception.toString()); - } catch (IllegalAccessException | InstantiationException exception) { + } catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException exception) { LOG.error("RangerAbstractGeolocationProvider.init() - Class {} could not be instantiated, exception={}", geoSourceLoader, exception.toString()); } diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java index 14b68e265b..b9aebe1580 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerGdsEnricher.java @@ -38,6 +38,7 @@ import java.io.FileWriter; import java.io.Reader; import java.io.Writer; +import java.lang.reflect.InvocationTargetException; import java.util.Timer; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -68,11 +69,16 @@ public void init() { if (StringUtils.isNotBlank(retrieverClassName)) { try { + Class cls = Class.forName(retrieverClassName, false, RangerGdsEnricher.class.getClassLoader()); + if (!RangerGdsInfoRetriever.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + retrieverClassName + " is not assignable to " + RangerGdsInfoRetriever.class.getName()); + } + @SuppressWarnings("unchecked") - Class retriverClass = (Class) Class.forName(retrieverClassName); + Class retriverClass = (Class) cls; - gdsInfoRetriever = retriverClass.newInstance(); - } catch (ClassNotFoundException | ClassCastException | IllegalAccessException | InstantiationException excp) { + gdsInfoRetriever = retriverClass.getDeclaredConstructor().newInstance(); + } catch (ClassNotFoundException | ClassCastException | IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException excp) { LOG.error("Failed to instantiate retriever (className={})", retrieverClassName, excp); } } diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java index 056574ca59..16665ed7ef 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerTagEnricher.java @@ -58,6 +58,7 @@ import java.io.FileWriter; import java.io.Reader; import java.io.Writer; +import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -157,17 +158,23 @@ public void init() { if (StringUtils.isNotBlank(tagRetrieverClassName)) { try { + Class cls = Class.forName(tagRetrieverClassName, false, RangerTagEnricher.class.getClassLoader()); + + if (!RangerTagRetriever.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + tagRetrieverClassName + " is not assignable to " + RangerTagRetriever.class.getName()); + } + @SuppressWarnings("unchecked") - Class tagRetriverClass = (Class) Class.forName(tagRetrieverClassName); + Class tagRetriverClass = (Class) cls; - tagRetriever = tagRetriverClass.newInstance(); + tagRetriever = tagRetriverClass.getDeclaredConstructor().newInstance(); } catch (ClassNotFoundException exception) { LOG.error("Class {} not found, exception={}", tagRetrieverClassName, exception); } catch (ClassCastException exception) { LOG.error("Class {} is not a type of RangerTagRetriever, exception={}", tagRetrieverClassName, exception); } catch (IllegalAccessException exception) { LOG.error("Class {} illegally accessed, exception={}", tagRetrieverClassName, exception); - } catch (InstantiationException exception) { + } catch (InstantiationException | NoSuchMethodException | InvocationTargetException exception) { LOG.error("Class {} could not be instantiated, exception={}", tagRetrieverClassName, exception); } diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java index 1b65134608..13daa472bf 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/RangerUserStoreEnricher.java @@ -31,6 +31,7 @@ import org.slf4j.LoggerFactory; import java.io.File; +import java.lang.reflect.InvocationTargetException; import java.util.Timer; import java.util.concurrent.BlockingQueue; import java.util.concurrent.LinkedBlockingQueue; @@ -64,17 +65,23 @@ public void init() { if (StringUtils.isNotBlank(userStoreRetrieverClassName)) { try { + Class cls = Class.forName(userStoreRetrieverClassName, false, RangerUserStoreEnricher.class.getClassLoader()); + + if (!RangerUserStoreRetriever.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + userStoreRetrieverClassName + " is not assignable to " + RangerUserStoreRetriever.class.getName()); + } + @SuppressWarnings("unchecked") - Class userStoreRetriverClass = (Class) Class.forName(userStoreRetrieverClassName); + Class userStoreRetriverClass = (Class) cls; - userStoreRetriever = userStoreRetriverClass.newInstance(); + userStoreRetriever = userStoreRetriverClass.getDeclaredConstructor().newInstance(); } catch (ClassNotFoundException exception) { LOG.error("Class {} not found, exception={}", userStoreRetrieverClassName, exception); } catch (ClassCastException exception) { LOG.error("Class {} is not a type of RangerUserStoreRetriever, exception={}", userStoreRetrieverClassName, exception); } catch (IllegalAccessException exception) { LOG.error("Class {} illegally accessed, exception={}", userStoreRetrieverClassName, exception); - } catch (InstantiationException exception) { + } catch (InstantiationException | NoSuchMethodException | InvocationTargetException exception) { LOG.error("Class {} could not be instantiated, exception={}", userStoreRetrieverClassName, exception); } diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java index 314941c184..1f208a0e9d 100644 --- a/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java +++ b/agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java @@ -1077,10 +1077,16 @@ private RangerContextEnricher buildContextEnricher(RangerServiceDef.RangerContex if (!StringUtils.isEmpty(clsName)) { try { + Class cls = Class.forName(clsName, false, RangerPolicyRepository.class.getClassLoader()); + + if (!RangerContextEnricher.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + clsName + " is not assignable to " + RangerContextEnricher.class.getName()); + } + @SuppressWarnings("unchecked") - Class enricherClass = (Class) Class.forName(clsName); + Class enricherClass = (Class) cls; - ret = enricherClass.newInstance(); + ret = enricherClass.getDeclaredConstructor().newInstance(); } catch (Exception excp) { LOG.error("failed to instantiate context enricher '{}' for '{}'", clsName, name, excp); } diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/contextenricher/TestRangerGdsEnricher.java b/agents-common/src/test/java/org/apache/ranger/plugin/contextenricher/TestRangerGdsEnricher.java index 843918767d..443669893e 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/contextenricher/TestRangerGdsEnricher.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/contextenricher/TestRangerGdsEnricher.java @@ -21,6 +21,7 @@ import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig; import org.apache.ranger.plugin.model.RangerServiceDef; +import org.apache.ranger.plugin.model.RangerServiceDef.RangerContextEnricherDef; import org.apache.ranger.plugin.policyengine.RangerAccessRequestImpl; import org.apache.ranger.plugin.policyengine.RangerPluginContext; import org.apache.ranger.plugin.policyengine.gds.GdsAccessResult; @@ -33,6 +34,9 @@ import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; +import java.lang.reflect.Field; +import java.util.Collections; + import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -85,4 +89,23 @@ public void test02_SetGdsInfo_SetsEngine_And_PreCleanup() { // No need to construct a real GdsPolicyEngine here; verify preCleanup path executes safely assertTrue(enricher.preCleanup()); } + + @Test + public void test03_init_rejectsRetrieverClassNameNotAssignableToRangerGdsInfoRetriever() throws Exception { + RangerGdsEnricher enricher = new RangerGdsEnricher(); + RangerServiceDef serviceDef = new RangerServiceDef(); + serviceDef.setName("hive"); + enricher.setServiceDef(serviceDef); + enricher.setEnricherDef(new RangerContextEnricherDef(1L, "gds", RangerGdsEnricher.class.getName(), + Collections.singletonMap(RangerGdsEnricher.RETRIEVER_CLASSNAME_OPTION, Thread.class.getName()))); + + RangerPluginConfig cfg = new RangerPluginConfig("hive", "svc", "appid", null, null, null); + RangerPluginContext ctx = new RangerPluginContext(cfg); + enricher.setPluginContext(ctx); + enricher.init(); + + Field f = RangerGdsEnricher.class.getDeclaredField("gdsInfoRetriever"); + f.setAccessible(true); + assertNull(f.get(enricher)); + } } diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerPolicyRepositoryContextEnricher.java b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerPolicyRepositoryContextEnricher.java new file mode 100644 index 0000000000..c7dc86764c --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/policyengine/TestRangerPolicyRepositoryContextEnricher.java @@ -0,0 +1,73 @@ +/* + * 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.ranger.plugin.policyengine; + +import org.apache.ranger.authorization.hadoop.config.RangerPluginConfig; +import org.apache.ranger.plugin.contextenricher.RangerContextEnricher; +import org.apache.ranger.plugin.contextenricher.RangerTagEnricher; +import org.apache.ranger.plugin.model.RangerServiceDef; +import org.apache.ranger.plugin.model.RangerServiceDef.RangerContextEnricherDef; +import org.apache.ranger.plugin.util.ServicePolicies; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; + +public class TestRangerPolicyRepositoryContextEnricher { + private static RangerPolicyRepository buildRepository(String enricherClassName) { + RangerServiceDef serviceDef = new RangerServiceDef(); + + serviceDef.setName("test-enricher-svc"); + serviceDef.setContextEnrichers(Collections.singletonList( + new RangerContextEnricherDef(1L, "testEnricher", enricherClassName, Collections.emptyMap()))); + + ServicePolicies servicePolicies = new ServicePolicies(); + + servicePolicies.setServiceName("test-enricher-svc-instance"); + servicePolicies.setServiceDef(serviceDef); + servicePolicies.setPolicies(Collections.emptyList()); + + RangerPluginContext pluginContext = new RangerPluginContext(new RangerPluginConfig("test-enricher-svc", "test-enricher-svc-instance", "test-enricher-svc", "cl1", "on-prem", null)); + + return new RangerPolicyRepository(servicePolicies, pluginContext); + } + + @Test + public void testMaliciousEnricherClassIsNotInstantiated() { + // java.lang.Thread has a public no-arg constructor and is on the classpath, + // but does not implement RangerContextEnricher. + RangerPolicyRepository repository = buildRepository(Thread.class.getName()); + + List enrichers = repository.getContextEnrichers(); + + Assertions.assertTrue(enrichers == null || enrichers.isEmpty(), + "a class not assignable to RangerContextEnricher must not be instantiated as one"); + } + + @Test + public void testLegitimateEnricherClassIsStillInstantiated() { + RangerPolicyRepository repository = buildRepository(RangerTagEnricher.class.getName()); + + List enrichers = repository.getContextEnrichers(); + + Assertions.assertNotNull(enrichers); + Assertions.assertEquals(1, enrichers.size()); + Assertions.assertEquals(RangerTagEnricher.class, enrichers.get(0).getClass()); + } +} diff --git a/security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java b/security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java index e90c58a418..c5f929cb9d 100755 --- a/security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java +++ b/security-admin/src/main/java/org/apache/ranger/biz/ServiceMgr.java @@ -481,7 +481,10 @@ private Class getClassForServiceType(RangerServiceD } else { URL[] pluginFiles = getPluginFilesForServiceType(serviceType); URLClassLoader clsLoader = new URLClassLoader(pluginFiles, Thread.currentThread().getContextClassLoader()); - Class cls = Class.forName(clsName, true, clsLoader); + Class cls = Class.forName(clsName, false, clsLoader); + if (!RangerBaseService.class.isAssignableFrom(cls)) { + throw new ClassCastException("class " + clsName + " is not assignable to " + RangerBaseService.class.getName()); + } ret = (Class) cls; } diff --git a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java index 30de32adca..362b0cbabc 100644 --- a/security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java +++ b/security-admin/src/test/java/org/apache/ranger/biz/TestServiceMgr.java @@ -289,4 +289,42 @@ public void test10_lookupResource_tagServiceDirectCall() throws Exception { mgr.lookupResource("s", ctx, store); Assertions.assertTrue(true); } + + @Test + public void test11_getRangerServiceByService_rejectsImplClassNotAssignableToRangerBaseService() throws Exception { + ServiceMgr mgr = new ServiceMgr(); + ServiceStore store = mock(ServiceStore.class); + RangerService svc = new RangerService(); + svc.setType("evil"); + svc.setName("s"); + RangerServiceDef def = new RangerServiceDef(); + def.setName("evil"); + def.setImplClass(Thread.class.getName()); + when(store.getServiceDefByName("evil")).thenReturn(def); + + RangerBaseService built = mgr.getRangerServiceByService(svc, store); + + Assertions.assertNotNull(built); + Assertions.assertEquals(RangerDefaultService.class, built.getClass()); + Assertions.assertEquals("s", built.getServiceName()); + } + + @Test + public void test12_getRangerServiceByService_acceptsImplClassAssignableToRangerBaseService() throws Exception { + // Sanity check: a legitimate implClass that does extend RangerBaseService still works. + ServiceMgr mgr = new ServiceMgr(); + ServiceStore store = mock(ServiceStore.class); + RangerService svc = new RangerService(); + svc.setType("legit"); + svc.setName("s2"); + RangerServiceDef def = new RangerServiceDef(); + def.setName("legit"); + def.setImplClass(RangerDefaultService.class.getName()); + when(store.getServiceDefByName("legit")).thenReturn(def); + + RangerBaseService built = mgr.getRangerServiceByService(svc, store); + + Assertions.assertNotNull(built); + Assertions.assertEquals(RangerDefaultService.class, built.getClass()); + } }