[JENKINS-41684] Ensure that PluginManager.dynamicLoad runs as SYSTEM (#2732)

* [FIXED JENKINS-41684] Ensure that PluginManager.dynamicLoad runs as SYSTEM.
Test plugin source:
package test;
import hudson.Plugin;
import jenkins.model.Jenkins;
public class ThePlugin extends Plugin {
    @Override
    public void postInitialize() throws Exception {
        Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
    }
}

* @daniel-beck wants this all reindented.

core/src/main/java/hudson/PluginManager.java

@@ -819,100 +819,102 @@ public abstract class PluginManager extends AbstractModelObject implements OnMas
      */
     @Restricted(NoExternalUse.class)
     public void dynamicLoad(File arc, boolean removeExisting) throws IOException, InterruptedException, RestartRequiredException {
-        LOGGER.info("Attempting to dynamic load "+arc);
-        PluginWrapper p = null;
-        String sn;
-        try {
-            sn = strategy.getShortName(arc);
-        } catch (AbstractMethodError x) {
-            LOGGER.log(WARNING, "JENKINS-12753 fix not active: {0}", x.getMessage());
-            p = strategy.createPluginWrapper(arc);
-            sn = p.getShortName();
-        }
-        PluginWrapper pw = getPlugin(sn);
-        if (pw!=null) {
-            if (removeExisting) { // try to load disabled plugins
-                for (Iterator<PluginWrapper> i = plugins.iterator(); i.hasNext();) {
-                    pw = i.next();
-                    if(sn.equals(pw.getShortName())) {
-                        i.remove();
-                        pw = null;
-                        break;
+        try (ACLContext context = ACL.as(ACL.SYSTEM)) {
+            LOGGER.info("Attempting to dynamic load "+arc);
+            PluginWrapper p = null;
+            String sn;
+            try {
+                sn = strategy.getShortName(arc);
+            } catch (AbstractMethodError x) {
+                LOGGER.log(WARNING, "JENKINS-12753 fix not active: {0}", x.getMessage());
+                p = strategy.createPluginWrapper(arc);
+                sn = p.getShortName();
+            }
+            PluginWrapper pw = getPlugin(sn);
+            if (pw!=null) {
+                if (removeExisting) { // try to load disabled plugins
+                    for (Iterator<PluginWrapper> i = plugins.iterator(); i.hasNext();) {
+                        pw = i.next();
+                        if(sn.equals(pw.getShortName())) {
+                            i.remove();
+                            pw = null;
+                            break;
+                        }
                     }
+                } else {
+                    throw new RestartRequiredException(Messages._PluginManager_PluginIsAlreadyInstalled_RestartRequired(sn));
                 }
-            } else {
-                throw new RestartRequiredException(Messages._PluginManager_PluginIsAlreadyInstalled_RestartRequired(sn));
             }
-        }
-        if (p == null) {
-            p = strategy.createPluginWrapper(arc);
-        }
-        if (p.supportsDynamicLoad()== YesNoMaybe.NO)
-            throw new RestartRequiredException(Messages._PluginManager_PluginDoesntSupportDynamicLoad_RestartRequired(sn));
+            if (p == null) {
+                p = strategy.createPluginWrapper(arc);
+            }
+            if (p.supportsDynamicLoad()== YesNoMaybe.NO)
+                throw new RestartRequiredException(Messages._PluginManager_PluginDoesntSupportDynamicLoad_RestartRequired(sn));
 
-        // there's no need to do cyclic dependency check, because we are deploying one at a time,
-        // so existing plugins can't be depending on this newly deployed one.
+            // there's no need to do cyclic dependency check, because we are deploying one at a time,
+            // so existing plugins can't be depending on this newly deployed one.
 
-        plugins.add(p);
-        if (p.isActive())
-            activePlugins.add(p);
-        synchronized (((UberClassLoader) uberClassLoader).loaded) {
-            ((UberClassLoader) uberClassLoader).loaded.clear();
-        }
-
-        try {
-            p.resolvePluginDependencies();
-            strategy.load(p);
+            plugins.add(p);
+            if (p.isActive())
+                activePlugins.add(p);
+            synchronized (((UberClassLoader) uberClassLoader).loaded) {
+                ((UberClassLoader) uberClassLoader).loaded.clear();
+            }
 
-            Jenkins.getInstance().refreshExtensions();
+            try {
+                p.resolvePluginDependencies();
+                strategy.load(p);
 
-            p.getPlugin().postInitialize();
-        } catch (Exception e) {
-            failedPlugins.add(new FailedPlugin(sn, e));
-            activePlugins.remove(p);
-            plugins.remove(p);
-            throw new IOException("Failed to install "+ sn +" plugin",e);
-        }
+                Jenkins.getInstance().refreshExtensions();
 
-        // run initializers in the added plugin
-        Reactor r = new Reactor(InitMilestone.ordering());
-        final ClassLoader loader = p.classLoader;
-        r.addAll(new InitializerFinder(loader) {
-            @Override
-            protected boolean filter(Method e) {
-                return e.getDeclaringClass().getClassLoader() != loader || super.filter(e);
+                p.getPlugin().postInitialize();
+            } catch (Exception e) {
+                failedPlugins.add(new FailedPlugin(sn, e));
+                activePlugins.remove(p);
+                plugins.remove(p);
+                throw new IOException("Failed to install "+ sn +" plugin",e);
             }
-        }.discoverTasks(r));
-        try {
-            new InitReactorRunner().run(r);
-        } catch (ReactorException e) {
-            throw new IOException("Failed to initialize "+ sn +" plugin",e);
-        }
 
-        // recalculate dependencies of plugins optionally depending the newly deployed one.
-        for (PluginWrapper depender: plugins) {
-            if (depender.equals(p)) {
-                // skip itself.
-                continue;
+            // run initializers in the added plugin
+            Reactor r = new Reactor(InitMilestone.ordering());
+            final ClassLoader loader = p.classLoader;
+            r.addAll(new InitializerFinder(loader) {
+                @Override
+                protected boolean filter(Method e) {
+                    return e.getDeclaringClass().getClassLoader() != loader || super.filter(e);
+                }
+            }.discoverTasks(r));
+            try {
+                new InitReactorRunner().run(r);
+            } catch (ReactorException e) {
+                throw new IOException("Failed to initialize "+ sn +" plugin",e);
             }
-            for (Dependency d: depender.getOptionalDependencies()) {
-                if (d.shortName.equals(p.getShortName())) {
-                    // this plugin depends on the newly loaded one!
-                    // recalculate dependencies!
-                    try {
-                        getPluginStrategy().updateDependency(depender, p);
-                    } catch (AbstractMethodError x) {
-                        LOGGER.log(WARNING, "{0} does not yet implement updateDependency", getPluginStrategy().getClass());
+
+            // recalculate dependencies of plugins optionally depending the newly deployed one.
+            for (PluginWrapper depender: plugins) {
+                if (depender.equals(p)) {
+                    // skip itself.
+                    continue;
+                }
+                for (Dependency d: depender.getOptionalDependencies()) {
+                    if (d.shortName.equals(p.getShortName())) {
+                        // this plugin depends on the newly loaded one!
+                        // recalculate dependencies!
+                        try {
+                            getPluginStrategy().updateDependency(depender, p);
+                        } catch (AbstractMethodError x) {
+                            LOGGER.log(WARNING, "{0} does not yet implement updateDependency", getPluginStrategy().getClass());
+                        }
+                        break;
                     }
-                    break;
                 }
             }
-        }
 
-        // Redo who depends on who.
-        resolveDependantPlugins();
+            // Redo who depends on who.
+            resolveDependantPlugins();
 
-        LOGGER.info("Plugin " + p.getShortName()+":"+p.getVersion() + " dynamically installed");
+            LOGGER.info("Plugin " + p.getShortName()+":"+p.getVersion() + " dynamically installed");
+        }
     }
 
     @Restricted(NoExternalUse.class)

test/src/test/java/hudson/PluginManagerTest.java

@@ -30,7 +30,10 @@ import hudson.model.Hudson;
 import hudson.model.UpdateCenter;
 import hudson.model.UpdateCenter.UpdateCenterJob;
 import hudson.model.UpdateSite;
+import hudson.model.User;
 import hudson.scm.SubversionSCM;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
 import hudson.util.FormValidation;
 import hudson.util.PersistedList;
 import java.io.File;
@@ -53,6 +56,7 @@ import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
 import org.jvnet.hudson.test.Url;
 import org.jvnet.hudson.test.recipes.WithPlugin;
 import org.jvnet.hudson.test.recipes.WithPluginManager;
@@ -444,6 +448,16 @@ public class PluginManagerTest {
         assertTrue(pluginInfo.getString("dependencies") != null);
     }
 
+    @Issue("JENKINS-41684")
+    @Test
+    public void requireSystemDuringLoad() throws Exception {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy());
+        try (ACLContext context = ACL.as(User.get("underprivileged").impersonate())) {
+            dynamicLoad("require-system-during-load.hpi");
+        }
+    }
+
     private void dynamicLoad(String plugin) throws IOException, InterruptedException, RestartRequiredException {
         PluginManagerUtil.dynamicLoad(plugin, r.jenkins);
     }