Detect tampering with signed plugins more reliably and stop plugin classes from loading in the case of suspected tampering

gameyfin/src/main/frontend/components/general/cards/PluginManagementCard.tsx

@@ -4,13 +4,15 @@ import {
     PauseCircle,
     PlayCircle,
     Power,
+    Question,
     QuestionMark,
     SealCheck,
     SealQuestion,
     SealWarning,
     SlidersHorizontal,
     StopCircle,
-    WarningCircle
+    WarningCircle,
+    XCircle
 } from "@phosphor-icons/react";
 import {PluginManagementEndpoint} from "Frontend/generated/endpoints";
 import PluginDto from "Frontend/generated/de/grimsi/gameyfin/core/plugins/management/PluginDto";
@@ -19,25 +21,29 @@ import React, {ReactNode, useEffect, useState} from "react";
 import PluginDetailsModal from "Frontend/components/general/modals/PluginDetailsModal";
 import PluginLogo from "Frontend/components/general/PluginLogo";
 import PluginTrustLevel from "Frontend/generated/de/grimsi/gameyfin/core/plugins/management/PluginTrustLevel";
+import PluginConfigValidationResult
+    from "Frontend/generated/de/grimsi/gameyfin/core/plugins/config/PluginConfigValidationResult";
 
 export function PluginManagementCard({plugin, updatePlugin}: {
     plugin: PluginDto,
     updatePlugin: (plugin: PluginDto) => void
 }) {
     const pluginDetailsModal = useDisclosure();
-    const [configValid, setConfigValid] = useState<boolean | undefined>(undefined);
+    const [configValidationResult, setConfigValidationResult] = useState<PluginConfigValidationResult | undefined>(undefined);
 
     useEffect(() => {
-        PluginManagementEndpoint.validatePluginConfig(plugin.id).then((response: boolean) => {
+        PluginManagementEndpoint.validatePluginConfig(plugin.id).then((response: PluginConfigValidationResult | undefined) => {
             if (response === undefined) return;
-            setConfigValid(response);
+            setConfigValidationResult(response);
         });
     }, [pluginDetailsModal.isOpen]);
 
-    function borderColor(state: PluginState | undefined): "success" | "warning" | "danger" | "default" {
+    function borderColor(state: PluginState | undefined, trustLevel: PluginTrustLevel | undefined): "success" | "warning" | "danger" | "default" {
+        if (trustLevel === PluginTrustLevel.UNTRUSTED) return "danger";
+
         if (isDisabled(state)) return "warning";
-        if (configValid === undefined) return "default";
+        if (configValidationResult === undefined) return "default";
-        if (!configValid) return "danger";
+        if (!configValidationResult) return "danger";
         return stateToColor(state);
     }
 
@@ -62,11 +68,37 @@ export function PluginManagementCard({plugin, updatePlugin}: {
                 return <PauseCircle/>;
             case PluginState.FAILED:
                 return <StopCircle/>;
+            case PluginState.UNLOADED:
+            case PluginState.RESOLVED:
+                return <XCircle/>;
             default:
                 return <QuestionMark/>;
         }
     }
 
+    function configValidationResultToChip(validationResult: PluginConfigValidationResult | undefined): ReactNode {
+        switch (validationResult) {
+            case PluginConfigValidationResult.VALID:
+                return <Tooltip content="Config valid" placement="bottom" color="foreground">
+                    <Chip size="sm" radius="sm" className="text-xs" color="success">
+                        <CheckCircle/>
+                    </Chip>
+                </Tooltip>
+            case PluginConfigValidationResult.INVALID:
+                return <Tooltip content="Config invalid" placement="bottom" color="foreground">
+                    <Chip size="sm" radius="sm" className="text-xs" color="danger">
+                        <WarningCircle/>
+                    </Chip>
+                </Tooltip>;
+            default:
+                return <Tooltip content="Config could not be validated" placement="bottom" color="foreground">
+                    <Chip size="sm" radius="sm" className="text-xs">
+                        <Question/>
+                    </Chip>
+                </Tooltip>
+        }
+    }
+
     function trustLevelToBadge(trustLevel: PluginTrustLevel | undefined): React.ReactNode {
         switch (trustLevel) {
             case PluginTrustLevel.OFFICIAL:
@@ -82,7 +114,7 @@ export function PluginManagementCard({plugin, updatePlugin}: {
                     <SealWarning/>
                 </Tooltip>;
             case PluginTrustLevel.UNTRUSTED:
-                return <Tooltip color="foreground" placement="bottom" content="Invlalid plugin signature">
+                return <Tooltip color="foreground" placement="bottom" content="Invalid plugin signature">
                     <SealWarning weight="fill" className="fill-danger"/>
                 </Tooltip>;
             default:
@@ -117,11 +149,16 @@ export function PluginManagementCard({plugin, updatePlugin}: {
     // @ts-ignore
     return (
         <>
-            <Card className={`flex flex-row justify-between p-2 border-2 border-${borderColor(plugin.state)}`}>
+            <Card
+                className={`flex flex-row justify-between p-2 border-2 border-${borderColor(plugin.state, plugin.trustLevel)}`}>
                 <div className="absolute right-0 top-0 flex flex-row">
                     <Tooltip content={`${isDisabled(plugin.state) ? "Enable" : "Disable"} plugin`} placement="bottom"
                              color="foreground">
-                        <Button isIconOnly variant="light" onPress={() => togglePluginEnabled()}>
+                        <Button isIconOnly
+                                variant="light"
+                                onPress={() => togglePluginEnabled()}
+                                isDisabled={plugin.state == PluginState.UNLOADED || plugin.state == PluginState.RESOLVED}
+                        >
                             <Power/>
                         </Button>
                     </Tooltip>
@@ -145,19 +182,9 @@ export function PluginManagementCard({plugin, updatePlugin}: {
                                 {stateToIcon(plugin.state)}
                             </Tooltip>
                         </Chip>
-                        {configValid === undefined ?
+                        {configValidationResult === undefined ?
-                            <Skeleton className="rounded-md h-6 w-9"/>
+                            <Skeleton className="rounded-md h-6 w-9"/> :
-                            : configValid ?
+                            configValidationResultToChip(configValidationResult)
-                                <Tooltip content="Config valid" placement="bottom" color="foreground">
-                                    <Chip size="sm" radius="sm" className="text-xs" color="success">
-                                        <CheckCircle/>
-                                    </Chip>
-                                </Tooltip> :
-                                <Tooltip content="Config invalid" placement="bottom" color="foreground">
-                                    <Chip size="sm" radius="sm" className="text-xs" color="danger">
-                                        <WarningCircle/>
-                                    </Chip>
-                                </Tooltip>
                         }
                     </div>
                 </div>

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/config/PluginConfigService.kt

@@ -16,7 +16,15 @@ class PluginConfigService(
 
     fun getConfigMetadata(pluginId: String): List<PluginConfigElement> {
         log.info { "Getting config metadata for plugin $pluginId" }
-        val plugin = pluginManager.getPlugin(pluginId).plugin as GameyfinPlugin
+
+        val plugin = try {
+            pluginManager.getPlugin(pluginId).plugin
+        } catch (_: NoClassDefFoundError) {
+            return emptyList()
+        }
+
+        if (plugin !is GameyfinPlugin) return emptyList()
+
         return plugin.configMetadata
     }
 

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/config/PluginConfigValidationResult.kt

@@ -0,0 +1,7 @@
+package de.grimsi.gameyfin.core.plugins.config
+
+enum class PluginConfigValidationResult {
+    VALID,
+    INVALID,
+    UNKNWOWN,
+}

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/GameyfinDevelopmentPluginLoader.kt

@@ -9,7 +9,7 @@ import java.nio.file.Path
 /**
  * @see https://stackoverflow.com/questions/73654174/my-application-cant-find-the-extension-with-pf4j
  */
-class GameyfinPluginLoader(
+class GameyfinDevelopmentPluginLoader(
     pluginManager: PluginManager,
     private val parentClassLoader: ClassLoader
 ) : DevelopmentPluginLoader(pluginManager) {

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/GameyfinJarPluginLoader.kt

@@ -0,0 +1,31 @@
+package de.grimsi.gameyfin.core.plugins.management
+
+import org.pf4j.DevelopmentPluginLoader
+import org.pf4j.PluginDescriptor
+import org.pf4j.PluginManager
+import org.pf4j.util.FileUtils
+import java.nio.file.Files
+import java.nio.file.Path
+
+/**
+ * JAR plugin loader using a [GameyfinPluginClassLoader]
+ */
+class GameyfinJarPluginLoader(
+    pluginManager: PluginManager
+) : DevelopmentPluginLoader(pluginManager) {
+
+    override fun isApplicable(pluginPath: Path): Boolean {
+        return Files.exists(pluginPath) && FileUtils.isJarFile(pluginPath)
+    }
+
+    override fun loadPlugin(pluginPath: Path, pluginDescriptor: PluginDescriptor?): ClassLoader {
+        if (pluginDescriptor == null) {
+            throw IllegalArgumentException("Plugin descriptor cannot be null")
+        }
+
+        val pluginClassLoader = GameyfinPluginClassLoader(pluginManager, pluginDescriptor, javaClass.getClassLoader())
+        pluginClassLoader.addFile(pluginPath.toFile())
+
+        return pluginClassLoader
+    }
+}

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/GameyfinPluginClassLoader.kt

@@ -0,0 +1,24 @@
+package de.grimsi.gameyfin.core.plugins.management
+
+import org.pf4j.PluginClassLoader
+import org.pf4j.PluginDescriptor
+import org.pf4j.PluginManager
+
+/**
+ * Adds custom functionality to the [PluginClassLoader] for Gameyfin (mostly related to JAR signature validation).
+ */
+class GameyfinPluginClassLoader(
+    pluginManager: PluginManager,
+    pluginDescriptor: PluginDescriptor,
+    parentClassLoader: ClassLoader,
+) : PluginClassLoader(pluginManager, pluginDescriptor, parentClassLoader) {
+
+    override fun loadClass(className: String?): Class<*>? {
+        try {
+            return super.loadClass(className)
+        } catch (_: SecurityException) {
+        }
+
+        return null
+    }
+}

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/GameyfinPluginManager.kt

@@ -1,6 +1,7 @@
 package de.grimsi.gameyfin.core.plugins.management
 
 import de.grimsi.gameyfin.core.plugins.config.PluginConfigRepository
+import de.grimsi.gameyfin.core.plugins.config.PluginConfigValidationResult
 import de.grimsi.gameyfin.pluginapi.core.GameyfinPlugin
 import io.github.oshai.kotlinlogging.KotlinLogging
 import org.pf4j.*
@@ -51,13 +52,10 @@ class GameyfinPluginManager(
     }
 
     override fun createPluginLoader(): PluginLoader {
-        val compoundPluginLoader = CompoundPluginLoader()
+        return when (this.isDevelopment) {
-        val developmentPluginLoader = GameyfinPluginLoader(this, javaClass.classLoader)
+            true -> GameyfinDevelopmentPluginLoader(this, javaClass.classLoader)
-        val jarPluginLoader = JarPluginLoader(this)
+            false -> GameyfinJarPluginLoader(this)
-
+        }
-        return compoundPluginLoader
-            .add(developmentPluginLoader, this::isDevelopment)
-            .add(jarPluginLoader, this::isNotDevelopment)
     }
 
     override fun createPluginStatusProvider(): PluginStatusProvider {
@@ -69,9 +67,6 @@ class GameyfinPluginManager(
 
         if (pluginWrapper == null || pluginPath == null) return null
 
-        // Inject config after loading, before starting
-        configurePlugin(pluginWrapper)
-
         var pluginManagementEntry = pluginManagementRepository.findByIdOrNull(pluginWrapper.pluginId)
 
         if (pluginManagementEntry == null) {
@@ -104,6 +99,16 @@ class GameyfinPluginManager(
             }
         }
 
+        // If the plugin is untrusted, we disable it regardless of the previous state
+        if (pluginManagementEntry.trustLevel == PluginTrustLevel.UNTRUSTED) {
+            log.warn { "Plugin ${pluginWrapper.pluginId} is untrusted, disabling" }
+            pluginManagementEntry.enabled = false
+        }
+
+        // Inject config after loading and verification, before starting
+        // Note: If the plugin is untrusted, we don't want to inject the config
+        if (pluginManagementEntry.trustLevel != PluginTrustLevel.UNTRUSTED) configurePlugin(pluginWrapper)
+
         log.debug { "Plugin ${pluginWrapper.pluginId} verification status: ${pluginManagementEntry.trustLevel}" }
         pluginManagementRepository.save(pluginManagementEntry)
 
@@ -113,8 +118,17 @@ class GameyfinPluginManager(
     override fun startPlugin(pluginId: String?): PluginState? {
         if (pluginId == null) return PluginState.FAILED
 
+        val trustLevel = pluginManagementRepository.findByIdOrNull(pluginId)?.trustLevel ?: PluginTrustLevel.UNKNOWN
+        if (trustLevel == PluginTrustLevel.UNTRUSTED) {
+            val pluginWrapper = getPlugin(pluginId)
+            val pluginState = PluginState.UNLOADED
+
+            this.firePluginStateEvent(PluginStateEvent(this, pluginWrapper, pluginState))
+            return pluginState
+        }
+
         // Validate config before starting the plugin
-        if (!validatePluginConfig(pluginId)) {
+        if (validatePluginConfig(pluginId) == PluginConfigValidationResult.INVALID) {
             log.warn { "Plugin $pluginId has invalid configuration" }
 
             val pluginWrapper = getPlugin(pluginId)
@@ -127,34 +141,8 @@ class GameyfinPluginManager(
     }
 
     override fun startPlugins() {
-        for (pluginWrapper in resolvedPlugins) {
+        val pluginsToStart = resolvedPlugins.filter { !it.pluginState.isDisabled && !it.pluginState.isStarted }
-            val pluginState = pluginWrapper.pluginState
+        pluginsToStart.forEach { startPlugin(it.pluginId) }
-            if (!pluginState.isDisabled && !pluginState.isStarted) {
-
-                // Validate config before starting the plugin
-                if (!validatePluginConfig(pluginWrapper.pluginId)) {
-                    log.error { "Plugin ${pluginWrapper.pluginId} has invalid configuration" }
-                    pluginWrapper.pluginState = PluginState.FAILED
-
-                    firePluginStateEvent(PluginStateEvent(this, pluginWrapper, pluginState))
-                    return
-                }
-
-                try {
-                    log.info { "Start plugin '${getPluginLabel(pluginWrapper.descriptor)}'" }
-                    pluginWrapper.plugin.start()
-                    pluginWrapper.pluginState = PluginState.STARTED
-                    pluginWrapper.failedException = null
-                    startedPlugins.add(pluginWrapper)
-                } catch (e: Exception) {
-                    pluginWrapper.pluginState = PluginState.FAILED
-                    pluginWrapper.failedException = e
-                    log.error { "Unable to start plugin '${getPluginLabel(pluginWrapper.descriptor)}': $e" }
-                } finally {
-                    firePluginStateEvent(PluginStateEvent(this, pluginWrapper, pluginState))
-                }
-            }
-        }
     }
 
     fun restart(pluginId: String) {
@@ -164,12 +152,18 @@ class GameyfinPluginManager(
         startPlugin(pluginId)
     }
 
-    fun validatePluginConfig(pluginId: String): Boolean {
+    fun validatePluginConfig(pluginId: String): PluginConfigValidationResult {
-        val plugin = getPlugin(pluginId)?.plugin ?: return false
+        val plugin = try {
-        if (plugin is GameyfinPlugin) {
+            getPlugin(pluginId)?.plugin
-            return plugin.validateConfig()
+        } catch (_: NoClassDefFoundError) {
+            return PluginConfigValidationResult.UNKNWOWN
         }
-        return false
+
+        if (plugin is GameyfinPlugin && plugin.validateConfig()) {
+            return PluginConfigValidationResult.VALID
+        }
+
+        return PluginConfigValidationResult.INVALID
     }
 
     private fun configurePlugin(pluginWrapper: PluginWrapper) {

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/PluginManagementEndpoint.kt

@@ -2,6 +2,7 @@ package de.grimsi.gameyfin.core.plugins.management
 
 import com.vaadin.hilla.Endpoint
 import de.grimsi.gameyfin.core.Role
+import de.grimsi.gameyfin.core.plugins.config.PluginConfigValidationResult
 import jakarta.annotation.security.RolesAllowed
 
 @Endpoint
@@ -23,7 +24,8 @@ class PluginManagementEndpoint(
 
     fun disablePlugin(pluginId: String) = pluginManagementService.disablePlugin(pluginId)
 
-    fun validatePluginConfig(pluginId: String): Boolean = pluginManagementService.validatePluginConfig(pluginId)
+    fun validatePluginConfig(pluginId: String): PluginConfigValidationResult =
+        pluginManagementService.validatePluginConfig(pluginId)
 
     fun setPluginPriority(pluginId: String, priority: Int) =
         pluginManagementService.setPluginPriority(pluginId, priority)

gameyfin/src/main/kotlin/de/grimsi/gameyfin/core/plugins/management/PluginManagementService.kt

@@ -1,7 +1,9 @@
 package de.grimsi.gameyfin.core.plugins.management
 
+import de.grimsi.gameyfin.core.plugins.config.PluginConfigValidationResult
 import de.grimsi.gameyfin.pluginapi.core.GameyfinPlugin
 import org.pf4j.ExtensionPoint
+import org.pf4j.PluginWrapper
 import org.springframework.data.repository.findByIdOrNull
 import org.springframework.stereotype.Service
 import java.io.InputStream
@@ -9,37 +11,15 @@ import java.io.InputStream
 @Service
 class PluginManagementService(
     private val pluginManager: GameyfinPluginManager,
-    private val pluginManagementRepository: PluginManagementRepository
+    private val pluginManagementRepository: PluginManagementRepository,
 ) {
     fun getPluginDtos(): List<PluginDto> {
-        return pluginManager.plugins.map {
+        return pluginManager.plugins.map { toDto(it) }
-            val pluginManagementEntry = getPluginManagementEntry(it.pluginId)
-            PluginDto(
-                it.pluginId,
-                it.descriptor.pluginDescription,
-                it.descriptor.version,
-                it.descriptor.provider,
-                (it.plugin as GameyfinPlugin).hasLogo(),
-                it.pluginState,
-                pluginManagementEntry.priority,
-                pluginManagementEntry.trustLevel
-            )
-        }
     }
 
     fun getPluginDto(pluginId: String): PluginDto {
         val plugin = pluginManager.getPlugin(pluginId)
-        val pluginManagementEntry = getPluginManagementEntry(pluginId)
+        return toDto(plugin)
-        return PluginDto(
-            plugin.pluginId,
-            plugin.descriptor.pluginDescription,
-            plugin.descriptor.version,
-            plugin.descriptor.provider,
-            (plugin.plugin as GameyfinPlugin).hasLogo(),
-            plugin.pluginState,
-            pluginManagementEntry.priority,
-            pluginManagementEntry.trustLevel
-        )
     }
 
     fun getPluginManagementEntry(pluginId: String): PluginManagementEntry {
@@ -73,7 +53,7 @@ class PluginManagementService(
         pluginManager.disablePlugin(pluginId)
     }
 
-    fun validatePluginConfig(pluginId: String): Boolean {
+    fun validatePluginConfig(pluginId: String): PluginConfigValidationResult {
         return pluginManager.validatePluginConfig(pluginId)
     }
 
@@ -95,4 +75,28 @@ class PluginManagementService(
         val plugin = pluginManager.getPlugin(pluginId).plugin as GameyfinPlugin
         return plugin.getLogo()
     }
+
+    private fun toDto(pluginWrapper: PluginWrapper): PluginDto {
+        val pluginManagementEntry = getPluginManagementEntry(pluginWrapper.pluginId)
+
+        val hasLogo = try {
+            when (pluginWrapper.plugin is GameyfinPlugin) {
+                true -> (pluginWrapper.plugin as GameyfinPlugin).hasLogo()
+                false -> false
+            }
+        } catch (_: NoClassDefFoundError) {
+            false
+        }
+
+        return PluginDto(
+            pluginWrapper.pluginId,
+            pluginWrapper.descriptor.pluginDescription,
+            pluginWrapper.descriptor.version,
+            pluginWrapper.descriptor.provider,
+            hasLogo,
+            pluginWrapper.pluginState,
+            pluginManagementEntry.priority,
+            pluginManagementEntry.trustLevel
+        )
+    }
 }

gameyfin/vite.generated.ts

@@ -274,7 +274,7 @@ function statsExtracterPlugin(): PluginOption {
       const frontendFiles: Record<string, string> = {};
       frontendFiles['index.html'] = createHash('sha256').update(customIndexData.replace(/\r\n/g, '\n'), 'utf8').digest('hex');
 
-      const projectFileExtensions = ['.js', '.js.map', '.ts', '.ts.map', '.tsx', '.tsx.map', '.css', '.css.map', '.'];
+      const projectFileExtensions = ['.js', '.js.map', '.ts', '.ts.map', '.tsx', '.tsx.map', '.css', '.css.map'];
 
       const isThemeComponentsResource = (id: string) =>
           id.startsWith(themeOptions.frontendGeneratedFolder.replace(/\\/g, '/'))