CASSANDRA-21525 trunk - Verify extension type before initializing reflectively-loaded classes

src/java/org/apache/cassandra/auth/AuthConfig.java

@@ -63,7 +63,7 @@ public static void applyAuth()
 
         /* Authentication, authorization and role management backend, implementing IAuthenticator, I*Authorizer & IRoleManager */
 
-        IAuthenticator authenticator = authInstantiate(conf.authenticator, AllowAllAuthenticator.class);
+        IAuthenticator authenticator = authInstantiate(conf.authenticator, IAuthenticator.class, AllowAllAuthenticator.class);
 
         // the configuration options regarding credentials caching are only guaranteed to
         // work with PasswordAuthenticator, so log a message if some other authenticator
@@ -82,7 +82,7 @@ public static void applyAuth()
 
         // authorizer
 
-        IAuthorizer authorizer = authInstantiate(conf.authorizer, AllowAllAuthorizer.class);
+        IAuthorizer authorizer = authInstantiate(conf.authorizer, IAuthorizer.class, AllowAllAuthorizer.class);
 
         if (!authenticator.requireAuthentication() && authorizer.requireAuthorization())
         {
@@ -94,7 +94,7 @@ public static void applyAuth()
 
         // role manager
 
-        IRoleManager roleManager = authInstantiate(conf.role_manager, CassandraRoleManager.class);
+        IRoleManager roleManager = authInstantiate(conf.role_manager, IRoleManager.class, CassandraRoleManager.class);
 
         if (authenticator instanceof PasswordAuthenticator && !(roleManager instanceof CassandraRoleManager))
             throw new ConfigurationException(authenticator.getClass().getName() + " requires " + CassandraRoleManager.class.getName(), false);
@@ -104,12 +104,15 @@ public static void applyAuth()
         // authenticator
 
         IInternodeAuthenticator internodeAuthenticator = authInstantiate(conf.internode_authenticator,
+                                                                         IInternodeAuthenticator.class,
                                                                          AllowAllInternodeAuthenticator.class);
         DatabaseDescriptor.setInternodeAuthenticator(internodeAuthenticator);
 
         // network authorizer
 
-        INetworkAuthorizer networkAuthorizer = authInstantiate(conf.network_authorizer, AllowAllNetworkAuthorizer.class);
+        INetworkAuthorizer networkAuthorizer = authInstantiate(conf.network_authorizer,
+                                                               INetworkAuthorizer.class,
+                                                               AllowAllNetworkAuthorizer.class);
 
         if (networkAuthorizer.requireAuthorization() && !authenticator.requireAuthentication())
         {
@@ -120,7 +123,9 @@ public static void applyAuth()
 
         // cidr authorizer
 
-        ICIDRAuthorizer cidrAuthorizer = authInstantiate(conf.cidr_authorizer, AllowAllCIDRAuthorizer.class);
+        ICIDRAuthorizer cidrAuthorizer = authInstantiate(conf.cidr_authorizer,
+                                                         ICIDRAuthorizer.class,
+                                                         AllowAllCIDRAuthorizer.class);
 
         if (cidrAuthorizer.requireAuthorization() && !authenticator.requireAuthentication())
         {
@@ -140,11 +145,11 @@ public static void applyAuth()
         DatabaseDescriptor.getInternodeAuthenticator().validateConfiguration();
     }
 
-    private static <T> T authInstantiate(ParameterizedClass authCls, Class<T> defaultCls) {
+    private static <T> T authInstantiate(ParameterizedClass authCls, Class<T> expectedType, Class<? extends T> defaultCls) {
         if (authCls != null && authCls.class_name != null)
         {
             String authPackage = AuthConfig.class.getPackage().getName();
-            return ParameterizedClass.newInstance(authCls, List.of("", authPackage));
+            return ParameterizedClass.newInstance(authCls, List.of("", authPackage), expectedType);
         }
 
         // for now, this has to stay and can not be replaced by ParameterizedClass.newInstance as above

src/java/org/apache/cassandra/auth/MutualTlsAuthenticator.java

@@ -101,7 +101,8 @@ public MutualTlsAuthenticator(Map<String, String> parameters)
             throw new ConfigurationException(message);
         }
         certificateValidator = ParameterizedClass.newInstance(new ParameterizedClass(certificateValidatorClassName),
-                                                              Arrays.asList("", AuthConfig.class.getPackage().getName()));
+                                                              Arrays.asList("", AuthConfig.class.getPackage().getName()),
+                                                              MutualTlsCertificateValidator.class);
 
         Config config = DatabaseDescriptor.getRawConfig();
         certificateValidityPeriodValidator = new MutualTlsCertificateValidityPeriodValidator(config.client_encryption_options.max_certificate_validity_period);

src/java/org/apache/cassandra/auth/MutualTlsInternodeAuthenticator.java

@@ -104,7 +104,8 @@ public MutualTlsInternodeAuthenticator(Map<String, String> parameters)
         }
 
         certificateValidator = ParameterizedClass.newInstance(new ParameterizedClass(certificateValidatorClassName),
-                                                              Arrays.asList("", AuthConfig.class.getPackage().getName()));
+                                                              Arrays.asList("", AuthConfig.class.getPackage().getName()),
+                                                              MutualTlsCertificateValidator.class);
         Config config = DatabaseDescriptor.getRawConfig();
 
         if (parameters.containsKey(TRUSTED_PEER_IDENTITIES))

src/java/org/apache/cassandra/config/DatabaseDescriptor.java

@@ -515,7 +515,7 @@ public static Config loadConfig() throws ConfigurationException
         String loaderClass = CONFIG_LOADER.getString();
         ConfigurationLoader loader = loaderClass == null
                                      ? new YamlConfigurationLoader()
-                                     : FBUtilities.construct(loaderClass, "configuration loading");
+                                     : FBUtilities.construct(loaderClass, "configuration loading", ConfigurationLoader.class);
         Config config = loader.loadConfig();
 
         if (!hasLoggedConfig)
@@ -1655,7 +1655,8 @@ public static void applySeedProvider()
         }
         try
         {
-            Class<?> seedProviderClass = Class.forName(conf.seed_provider.class_name);
+            Class<? extends SeedProvider> seedProviderClass =
+                FBUtilities.classForNameWithoutInitialization(conf.seed_provider.class_name, "seed provider", SeedProvider.class);
             seedProvider = (SeedProvider) seedProviderClass.getConstructor(Map.class).newInstance(conf.seed_provider.parameters);
         }
         // there are about 5 checked exceptions that could be thrown here.
@@ -2032,39 +2033,39 @@ public static IEndpointSnitch createEndpointSnitch(String snitchClassName) throw
     {
         if (!snitchClassName.contains("."))
             snitchClassName = "org.apache.cassandra.locator." + snitchClassName;
-        IEndpointSnitch snitch = FBUtilities.construct(snitchClassName, "snitch");
+        IEndpointSnitch snitch = FBUtilities.construct(snitchClassName, "snitch", IEndpointSnitch.class);
         return snitch;
     }
 
     public static NodeProximity createProximityImpl(String className) throws ConfigurationException
     {
         if (!className.contains("."))
             className = "org.apache.cassandra.locator." + className;
-        NodeProximity sorter = FBUtilities.construct(className, "node proximity measurement");
+        NodeProximity sorter = FBUtilities.construct(className, "node proximity measurement", NodeProximity.class);
         return sorter;
     }
 
     public static InitialLocationProvider createInitialLocationProvider(String className) throws ConfigurationException
     {
         if (!className.contains("."))
             className = "org.apache.cassandra.locator." + className;
-        InitialLocationProvider provider = FBUtilities.construct(className, "initial location provider");
+        InitialLocationProvider provider = FBUtilities.construct(className, "initial location provider", InitialLocationProvider.class);
         return provider;
     }
 
     public static NodeAddressConfig createAddressConfig(String className) throws ConfigurationException
     {
         if (!className.contains("."))
             className = "org.apache.cassandra.locator." + className;
-        NodeAddressConfig config = FBUtilities.construct(className, "node address config");
+        NodeAddressConfig config = FBUtilities.construct(className, "node address config", NodeAddressConfig.class);
         return config;
     }
 
     private static IFailureDetector createFailureDetector(String detectorClassName) throws ConfigurationException
     {
         if (!detectorClassName.contains("."))
             detectorClassName = "org.apache.cassandra.gms." + detectorClassName;
-        IFailureDetector detector = FBUtilities.construct(detectorClassName, "failure detector");
+        IFailureDetector detector = FBUtilities.construct(detectorClassName, "failure detector", IFailureDetector.class);
         return detector;
     }
 

src/java/org/apache/cassandra/config/ParameterizedClass.java

@@ -64,7 +64,17 @@ public ParameterizedClass(Map<String, ?> p)
              p.containsKey(PARAMETERS) ? (Map<String, String>)((List<?>)p.get(PARAMETERS)).get(0) : null);
     }
 
+    /**
+     * Prefer {@link #newInstance(ParameterizedClass, List, Class)}: passing an {@code expectedType} verifies the
+     * resolved class is the intended extension type before it is instantiated. This overload performs no type
+     * check (it still loads without initialization, so a wrong class name cannot run its static initializer here).
+     */
     static public <K> K newInstance(ParameterizedClass parameterizedClass, List<String> searchPackages)
+    {
+        return newInstance(parameterizedClass, searchPackages, null);
+    }
+
+    static public <K> K newInstance(ParameterizedClass parameterizedClass, List<String> searchPackages, Class<K> expectedType)
     {
         Class<?> providerClass = null;
         if (searchPackages == null || searchPackages.isEmpty())
@@ -76,9 +86,12 @@ static public <K> K newInstance(ParameterizedClass parameterizedClass, List<Stri
                 if (!searchPackage.isEmpty() && !searchPackage.endsWith("."))
                     searchPackage = searchPackage + '.';
                 String name = searchPackage + parameterizedClass.class_name;
-                providerClass = Class.forName(name);
+                // Load without initialization so a wrong class name does not run its static initializer here. The
+                // type is verified below (once the search has resolved a class) and the class is only initialized
+                // later, when it is constructed.
+                providerClass = Class.forName(name, false, ParameterizedClass.class.getClassLoader());
             }
-            catch (ClassNotFoundException e)
+            catch (ClassNotFoundException | NoClassDefFoundError e)
             {
                 //no-op
             }
@@ -91,6 +104,13 @@ static public <K> K newInstance(ParameterizedClass parameterizedClass, List<Stri
             throw new ConfigurationException(error);
         }
 
+        // Verify the resolved class is the expected extension type before it is initialized/instantiated. Done once,
+        // after the package search, so a wrong-type match under an earlier search package does not abort the search
+        // before a valid class under a later package is found.
+        if (expectedType != null && !expectedType.isAssignableFrom(providerClass))
+            throw new ConfigurationException("Invalid parameterized class " + providerClass.getName() +
+                                             ": must extend or implement " + expectedType.getName());
+
         try
         {
             Constructor<?> mapConstructor = filterConstructor(providerClass, c -> c.getParameterTypes().length == 1 && c.getParameterTypes()[0].equals(Map.class));

src/java/org/apache/cassandra/db/StorageHook.java

@@ -55,7 +55,7 @@ static StorageHook createHook()
         String className = STORAGE_HOOK.getString();
         if (className != null)
         {
-            return FBUtilities.construct(className, StorageHook.class.getSimpleName());
+            return FBUtilities.construct(className, StorageHook.class.getSimpleName(), StorageHook.class);
         }
 
         return new StorageHook()
@@ -89,4 +89,4 @@ public UnfilteredRowIterator makeRowIterator(ColumnFamilyStore cfs,
             }
         };
     }
-}
+}

src/java/org/apache/cassandra/db/guardrails/GuardrailsConfigProvider.java

@@ -65,7 +65,7 @@ public interface GuardrailsConfigProvider
      */
     static GuardrailsConfigProvider build(String customImpl)
     {
-        return FBUtilities.construct(customImpl, "custom guardrails config provider");
+        return FBUtilities.construct(customImpl, "custom guardrails config provider", GuardrailsConfigProvider.class);
     }
 
     /**

src/java/org/apache/cassandra/db/guardrails/ValueGenerator.java

@@ -143,8 +143,11 @@ public static <VALUE> ValueGenerator<VALUE> getGenerator(String name, @Nonnull C
 
         try
         {
+            Class<? extends ValueGenerator> rawGeneratorClass =
+                FBUtilities.classForNameWithoutInitialization(className, "generator", ValueGenerator.class);
+            @SuppressWarnings("unchecked")
             Class<? extends ValueGenerator<VALUE>> generatorClass =
-            FBUtilities.classForName(className, "generator");
+                (Class<? extends ValueGenerator<VALUE>>) rawGeneratorClass;
 
             @SuppressWarnings("unchecked")
             ValueGenerator<VALUE> generator = generatorClass.getConstructor(CustomGuardrailConfig.class)
@@ -165,4 +168,4 @@ public static <VALUE> ValueGenerator<VALUE> getGenerator(String name, @Nonnull C
                                                     className, message), ex);
         }
     }
-}
+}

src/java/org/apache/cassandra/db/guardrails/ValueValidator.java

@@ -127,8 +127,11 @@ public static <VALUE> ValueValidator<VALUE> getValidator(String name, @Nonnull C
 
         try
         {
+            Class<? extends ValueValidator> rawValidatorClass =
+                FBUtilities.classForNameWithoutInitialization(className, "validator", ValueValidator.class);
+            @SuppressWarnings("unchecked")
             Class<? extends ValueValidator<VALUE>> validatorClass =
-            FBUtilities.classForName(className, "validator");
+                (Class<? extends ValueValidator<VALUE>>) rawValidatorClass;
 
             @SuppressWarnings("unchecked")
             ValueValidator<VALUE> validator = validatorClass.getConstructor(CustomGuardrailConfig.class)

src/java/org/apache/cassandra/db/marshal/TypeParser.java

@@ -449,8 +449,7 @@ public Pair<Pair<String, ByteBuffer>, List<Pair<ByteBuffer, AbstractType>>> getU
 
     private static AbstractType<?> getAbstractType(String compareWith) throws ConfigurationException
     {
-        String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith;
-        Class<? extends AbstractType<?>> typeClass = FBUtilities.<AbstractType<?>>classForName(className, "abstract-type");
+        Class<? extends AbstractType<?>> typeClass = getAbstractTypeClass(compareWith);
         try
         {
             Field field = typeClass.getDeclaredField("instance");
@@ -465,8 +464,7 @@ private static AbstractType<?> getAbstractType(String compareWith) throws Config
 
     private static AbstractType<?> getAbstractType(String compareWith, TypeParser parser) throws SyntaxException, ConfigurationException
     {
-        String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith;
-        Class<? extends AbstractType<?>> typeClass = FBUtilities.<AbstractType<?>>classForName(className, "abstract-type");
+        Class<? extends AbstractType<?>> typeClass = getAbstractTypeClass(compareWith);
         if (PseudoUtf8Type.class.isAssignableFrom(typeClass))
         {
             if (StorageService.instance.isDaemonSetupCompleted())
@@ -491,6 +489,19 @@ private static AbstractType<?> getAbstractType(String compareWith, TypeParser pa
         }
     }
 
+    private static Class<? extends AbstractType<?>> getAbstractTypeClass(String compareWith) throws ConfigurationException
+    {
+        String className = compareWith.contains(".") ? compareWith : "org.apache.cassandra.db.marshal." + compareWith;
+        // Defer class initialization until after confirming this is an AbstractType. The static instance field
+        // access or getInstance(TypeParser) invocation below performs the initialization for valid types.
+        @SuppressWarnings("unchecked")
+        Class<? extends AbstractType<?>> typeClass =
+            (Class<? extends AbstractType<?>>) FBUtilities.classForNameWithoutInitialization(className,
+                                                                                             "abstract-type",
+                                                                                             AbstractType.class);
+        return typeClass;
+    }
+
     private static AbstractType<?> getRawAbstractType(Class<? extends AbstractType<?>> typeClass) throws ConfigurationException
     {
         try

src/java/org/apache/cassandra/diag/DiagnosticEventPersistence.java

@@ -128,19 +128,20 @@ private void onEvent(DiagnosticEvent event)
         LastEventIdBroadcaster.instance().setLastEventId(event.getClass().getName(), store.getLastEventId());
     }
 
+    @SuppressWarnings("unchecked")
     private Class<DiagnosticEvent> getEventClass(String eventClazz) throws ClassNotFoundException, InvalidClassException
     {
         // get class by eventClazz argument name
         // restrict class loading for security reasons
         if (!eventClazz.startsWith("org.apache.cassandra."))
             throw new RuntimeException("Not a Cassandra event class: " + eventClazz);
 
-        Class<DiagnosticEvent> clazz = (Class<DiagnosticEvent>) Class.forName(eventClazz);
+        Class<?> clazz = Class.forName(eventClazz, false, DiagnosticEventPersistence.class.getClassLoader());
 
         if (!(DiagnosticEvent.class.isAssignableFrom(clazz)))
             throw new InvalidClassException("Event class must be of type DiagnosticEvent");
 
-        return clazz;
+        return (Class<DiagnosticEvent>) clazz.asSubclass(DiagnosticEvent.class);
     }
 
     private DiagnosticEventStore<Long> getStore(Class cls)

src/java/org/apache/cassandra/index/SecondaryIndexManager.java

@@ -921,6 +921,11 @@ public Index getIndexByName(String indexName)
         return indexes.get(indexName);
     }
 
+    static Class<? extends Index> loadIndexClass(String className)
+    {
+        return FBUtilities.classForNameWithoutInitialization(className, "Index", Index.class);
+    }
+
     private Index createInstance(IndexMetadata indexDef)
     {
         Index newIndex;
@@ -933,7 +938,7 @@ private Index createInstance(IndexMetadata indexDef)
 
             try
             {
-                Class<? extends Index> indexClass = FBUtilities.classForName(className, "Index");
+                Class<? extends Index> indexClass = loadIndexClass(className);
                 Constructor<? extends Index> ctor = indexClass.getConstructor(ColumnFamilyStore.class, IndexMetadata.class);
                 newIndex = ctor.newInstance(baseCfs, indexDef);
             }