From 6fb9e91b63521eb8cdcd072cec6610d856aabf34 Mon Sep 17 00:00:00 2001
From: Jesse Glick <jglick@cloudbees.com>
Date: Sun, 12 Feb 2017 05:36:48 -0500
Subject: [PATCH] [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  | 180 +++++++++---------
 .../test/java/hudson/PluginManagerTest.java   |  14 ++
 .../plugins/require-system-during-load.hpi    | Bin 0 -> 5151 bytes
 3 files changed, 105 insertions(+), 89 deletions(-)
 create mode 100644 test/src/test/resources/plugins/require-system-during-load.hpi

diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java
index 8673d99ee4..495ba7ee3f 100644
--- a/core/src/main/java/hudson/PluginManager.java
+++ b/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;
+        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));
+                }
+            }
+            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.
+
+            plugins.add(p);
+            if (p.isActive())
+                activePlugins.add(p);
+            synchronized (((UberClassLoader) uberClassLoader).loaded) {
+                ((UberClassLoader) uberClassLoader).loaded.clear();
+            }
+
+            try {
+                p.resolvePluginDependencies();
+                strategy.load(p);
+
+                Jenkins.getInstance().refreshExtensions();
+
+                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);
+            }
+
+            // 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);
+            }
+
+            // 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;
                     }
                 }
-            } else {
-                throw new RestartRequiredException(Messages._PluginManager_PluginIsAlreadyInstalled_RestartRequired(sn));
             }
+
+            // Redo who depends on who.
+            resolveDependantPlugins();
+
+            LOGGER.info("Plugin " + p.getShortName()+":"+p.getVersion() + " dynamically installed");
         }
-        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.
-
-        plugins.add(p);
-        if (p.isActive())
-            activePlugins.add(p);
-        synchronized (((UberClassLoader) uberClassLoader).loaded) {
-            ((UberClassLoader) uberClassLoader).loaded.clear();
-        }
-
-        try {
-            p.resolvePluginDependencies();
-            strategy.load(p);
-
-            Jenkins.getInstance().refreshExtensions();
-
-            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);
-        }
-
-        // 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);
-        }
-
-        // 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;
-                }
-            }
-        }
-
-        // Redo who depends on who.
-        resolveDependantPlugins();
-
-        LOGGER.info("Plugin " + p.getShortName()+":"+p.getVersion() + " dynamically installed");
     }
 
     @Restricted(NoExternalUse.class)
diff --git a/test/src/test/java/hudson/PluginManagerTest.java b/test/src/test/java/hudson/PluginManagerTest.java
index e7bb69de1b..f68cf85dcb 100644
--- a/test/src/test/java/hudson/PluginManagerTest.java
+++ b/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);
     }
diff --git a/test/src/test/resources/plugins/require-system-during-load.hpi b/test/src/test/resources/plugins/require-system-during-load.hpi
new file mode 100644
index 0000000000000000000000000000000000000000..a0b717afcd90d9625bf50c0e14797c8e128d7073
GIT binary patch
literal 5151
zcmb_g2{@E%8y=+WG)A&!jVxo02pL-jO=B5@%045Ebqum3$(k*qNMx5SWDi+VNT?x^
znk?C}8%&n`<D8>9f9IU*T-X1<bItdCb6wAKKl9$t_rA|_zi@r319X7BVHT~S_uIoC
z7h3ZDoVE!>5(?9ig8yMg13;3^KrO3w1{45*CnW#?`q}Ip1P0a7HZ}pD(}^*B4{Mb^
z+T`}eXFAu?%T=>|E;i&aO}H?N+d2GBw8mf$FHJ)uPn&310W?v85JY0SFC+ZcnkSIj
zF4U9*nsqcgeUz*G&?@y5km+MWFQpC@RW}5(^&*<+^juLLjbWzCZeo9Pf63(pHK9ru
zBGO)Jz6)x1gC^J#1Kb~^&U^KUbwmXFNHM_uE1ASBd<9LQxoB#gmz=d`^gS2xPl2$o
zV8o}iYs0jt5H+iGSG2lPf)rXv(T{$_M!wXE7I94~FDQFUW?7ZHK5b*=^X2l^g3mcr
zoyD&r=w);uMmKGVwv~@&YC9qnP?at?BXCxp%&Fna<KIRD3foo}s~;y9>2;oSd8W2~
zBnJP){mrSFgq(Z01;;JTnxioRHd6Q1@|X<|Wn-I>Vh&yWbC6l(+j2n6*Lv4z#<(cN
zb-W{--6YI6`cSm=CSafaUz0>bW*@1o@uwX9YQ^xA6~@u_r)=&e-Q_TS3Z887fSl32
zZ0_S>9PLo9SQHlQ>w>9*Ltt&vEDfts2(X$`Ga~{j24>@f2%c1FuDH|RZXSN=iBnKq
z=C$ciuKa<^4&LZ$bKX3Fzy0#9;o178LO`d^lq05S$|l#X5asjwMra#HXVU$%!2$y2
zH%zJBZXM8W>5%E&C`H_Q;TKMvg;uWNG@x`0BD=PjO+M)+RByp&QCg9CS#~*|ERR`{
zNHHjJa;BOvQgUcuXZ;Li3hl1iYK9FT_Qba&sKk~Mn~@^sFfR<&7p`&@)}~9lxh6>D
z_PK0~y;7<?x`>`CQ>phqv`;F{Z*RU|Tga14GMh|{0D#z^NU)XiM0wyGJyDX_D_Ad-
zi=;iy)6vyI660oL4|cNgEHS-us^6den0!TO-U;W6`W@0Cel9YD33V|j1`fd*jGgq!
zo9Jj=vB~4dE3^d#oI$AglDP{)ky-G!6*&WbiS_u#56+Prff@P!S9`yXZ>dbJDiH|g
zO;ZmfeHCZ`O;MV)kxJGnOW6Ld?jAFqNR8$`LN^Iu7RynKrgP||2Egkm085^7;&vdQ
zO*MhZDKw&NMti_10%@ci8-r5*8h?)Kc(0to>irM=dgZeh85Cz+6GxvImp8(Mg}6A1
zJ_I>gk}4)+!FT3^)kiq|Y3);e{Z<};{KrA=AfauQNead5;rRpad^1l3Y`SO1-a2Hc
z#n)(5Jic=J4a|DCB=C6`qasrPg(5s&#uYEp@aUr7JU-7)QGWWU0SymasCafd-#WaT
z2oP9+CM(la^UM`fW@=oQrh?}Cla?83v9d#Lx>5%xGYha6%X)DeF>Tv-@nyxvGxVv8
zjJYopC)FC3YzNU#J3cw~G#(Qxh?K5Yol=-L^};p~T?-VNBD88k^H7en>%NRZyeb|g
zuCp#4Hru#Ww78M#HoiKd=lw^X`f(YKNTT8mYCLc`g0-~l<&wnVdoeSWPQ`@|H%D@o
zZ+r{A#5cD1d1C3@Edv905ei8(`xtwZ%*Am*z|Q#u>7s-K7QBI+>9hLHqmE^3^TFI}
zE%<<BVPa8aeNgUI(J7NwLr5&JW5M^#w2jKG>D>%dad=r6i#uPRpPa&-6KLYZx`dCs
z3PQHZSfQ|{=F8=@GsSiOiMRry8z*(N-cWil=L8&mR0T0c8h@$1?ysM|vR?4DdX6$m
zj{loeL#L(&^^r^`8H%nO$<mI_k7`Q~FcOefoF~GY%?CgzHWu#O_o|4i<?`xc787A1
z_;KgAS6E#<j@2P8eRZIv7j9#LlLd3BGD}%c@S^sjsk$3N@%*~DY0Z|&z<Ub|m41C)
zRAx~0%QC%{soReSYCONmse4<Jo|eJ6Unl9%SC&_lo&++2qUCz&d0Fi8gpqO#JMzaW
zo^dJ6#g#oQi6GqehXghpEss!*<t@F%6Va~$F_36u6_GsaAm+6`l6)?+ftobQdet_E
zdZx;6!KBDXE&tde7IM+K<;K^A%8|^53uD@Jhz;R_cEb#Y<7xS&(Z?7_j0SW0gh&F=
zLX35eWG`+U>$(1kW2@D_?DP<wpe(LrzznJLCS|lN#xX(89~^a;!NPtFc%1JHJ-5n<
z#pDE*`Nw@`IyKyOs<uyZL+z@$JJCw=%S~sLrn>w>_!`T6acsl7jc2iL3280mg$L<x
z%h9PVz*ZUpfZ~-wr@QcA#Ay7tkK8X>rJ{K1V?q|083;<7rv}Zxv^pa(ZE|d{Tj=~4
zvw^P~D$O1GYl@LwCZeumAiN<w3a6bC$p%k^df!B<<|nx}r;nzi0^gV}101EjWvylp
z1ip)rx^w6fb)(3W<tW04gJ_MYR;m{y<3&4{yav~MpY4^R>sh#K>zeMLD~7Fh)FUN&
zPj?QmpARc>D_&}p37zdc;jwa<G9@ooP#sy>zHwk_H||>089fpB@~nJNDsy_y0xTcc
zkv^*xRkT%+oH*%zTD2nuHU+vUAqB8rZyWgt0`|vgHH4ODtzC{VHGo8IdAsvV(T*n2
z2mobtJBvj@+F32>jIBxSl?o_RgnH^FVD@ut!Z5*WJ=si?*p)$epiGk>Ym#*2bM<+y
zkhbpSxty8XF3N&~FqaJd$BnTu-qI@`2n)=7X0@8lM|rt*x7aEz2TP%z9%M}n^6`$&
zc5JjeLPa>X5yPV+?UZ%o{OvpjMOOs4FM^$(>mK&25qmHfIL(-6uD<D0WXcrRga`9B
z=936MJ^W%{rNeNM>yB!Ls&#@@lQ>3ab2viK%)eSt63;uH@vgRaWN0o>m)<Bn{#q4D
zsGaGC)d}%$eCU$rFC`a~!%MFc8=w#?Afy=BXPghD8c^L-v5-^|0Bavnss#oOZ8&#S
zi|I1KvCO5xDTfC;;9^{y4(u}W38bo~?y)N4Pw>Ena9Y8`r{+{I>7p^^a&AGl#QIFF
zcgr72zy6vW@x-@|LX@zon*u#;J%YYd;xkuS?5t87Li@=+Q=UEg#sjU2G5w9!odZdJ
zH;}Xkxe9Mn;B@7oS}lT^c86wr-69J{jK99&dMg~%eO6dLC-6mfb@%pUBLtXuK|Q5;
zQ&LAWWawzQ-F)EdHFh3P=8;riBIi1okQ)jfjJmN_w>dd`J+5#@#oYSr>8g_|YLb|D
z#O`zInQ2Vz#;hdvjF(I@+5H-iMig0+>!W0oZr?O1m&s*Bqz?^Su?m>Z>Cc}gO-|ue
z@>rL{Z=M`jS&TdHnmE&z0rlm>+W{H8SA<&{fZC|+%4cd~v0TyIykcdiGT$>wa<S<!
z*5Bdl(jkmtTW@ybW+}f~GAj*>t7U6S(@*dl9C&FY)D>`>2QGTR$E2tC6)x-eMEGo7
zLI0f>N==@X2fp+Md2?2C6kb1S`;m9}RjM=*hfeB=l*F|RGOcDQ4IRM3Y`Ckok@d~@
z^p>9+2h#Ws=b`B-!oox!sIufLJy=ZMVm($F5WNDwAzbT&)XK*lqBFr?&@i`N;r5Ah
zc_dj9Sg3$bRu%*C-S>M%di%I|S^s?i2mj~lD@=xT&ELLI%iu_WhpT3|na3H#^ww4W
z3cC}6Gti;rv2sqR-9)jYo}e}@|La$N`LGqzwL%6ovy0XRcaM-OZ>=7E0~$6v+ckHg
zzPP;`{_>WPa5$%-XQgUn*+RT6Vcej!iala&xtWdh;^M9ZNdJzdbFNuEQDrcS+U}(L
z+GQ)sN%!4e$kS8|`B~uJXG{OS-I}z>V$xbaqjPu0S__?cvkM_)F#YTo)>b4-6d))-
zO|sp!gtiLwMDWbqm-h`83!WuY)}5aXn}Rkx)Gq+8Je8&QXwu>ZeCo^CA$@Oje(ejH
z$&JqU7T?9j8|C`FHT>0B@K<9uPY19Q%GKG?6)S1y2=+o@z5W3}?cV|XeYf=;n+Cv5
z?prN;9}=_43MBC$0HFOpW4pV#{MP6q&S8h8SsGWxt4D+>ps5UYX^zYs2~f_wc$gbU
zyJ<Plt}dbbMefr^@lNY|@iPz8`WEo-YtD-+)qgDYG^2OP5j(!a&qIWOBK1(^X~c{p
ziW7xFJR4a?MaWxb>RvX-ce;l?yWd4U(uQ2=Of-P!-odPsntNuN%Y7$&ohK+7hcMHV
z-QzkTE%7@6wHf2j*+eR&li4GuH>`KtV(i8rZ-|Y)E!uKGkZ4A!>;u24`11CZK(}N}
zqfEuvzQ*<W&(R9Jja=g{oUi~XtiQ?1HAP&x_gq)cN_#o13o784g4EyUDi2h9P*=Ue
zeqQ1rrGRY-yt)753qfU09n4cYB}q?Apd@+(-K~#~NsX&Cv%#Fzy%2WfsrW-`nbUUd
zQ8`J^Lyp&t&6HP$NT<=t?yjR0GStAzl&@1JV(gGFPl@LyR$Cn;REkj>(HZ-!MzWB)
zVmeU#pTIO9<9O#xsFMy`vo)B=o^L@@UUvkgGh|sKIOsXjg(mGE8#{_V)45!OKt>$T
z0h`^^xCc{<15cYji)D>zI^8tsF7;Nx$4fLKtWUVVC%3FqE0|-B6_`u?j8%PXv;e{G
z(>HSK*&VZD^QzhYd8wr`>n<FBH&klG#=H0w$U{Q$-B$T6%bxauYfP#yKG@O;*5kk^
z&lK97kH^NM;w2&;yi#G}%7<xk^2do9YGTKlLfHr{VeHl3o+B3*&RnC?oK1GBmXRoX
zH8XdFcLWuh(#t!naUEpo?ILyg9?P+KS@un#8I=jAu?qcwQ-k2TDJIyceT`1t)FSF(
zbfunRTOiH+n1!j(2b?%!%XJJ9bJu`5UT>=0|1pD_g-BU(v8889S%i|c=(GAHi!T-i
zAZ5}w%W=c8r{b!J#Y^&cXH~te?$&Ow|5><P##$O)C%^rX<gda1Wfi!4y1An~y&O?k
z#!h{@5NQ@bUm#58ld^3w9}T#E+U+&-U3x9K82tG+Fks938n>+|DnI+gzSi2-A&F`2
z%qa$c?#x0+)rY*9)Fa*@Vf98OX7Pg5grbUsHk3bCEK-s?d|PQqtxWvbbjCz>T7kjQ
zMCr6?*F2)zbGPF<9D{e4Bwf%eu%_$fW8}Syf)Yga_XiZ@36lp4U>>q}|Gt;~aQo{6
zi#_ALkrOOJzJ7Q6>G<LY!mm5wp6LF4>;=vx%jSK_{7+tcBK(Kf%b&b{m*o2s{aug#
zw<zED%?V!nljjeK{v-BZE7~6jo8(P%{~zzg{`(zwPv8E){gsKm0`mLVV`7~CCocEP
z$^Nu{b>1r|Kb%>}{CrQu@6LZ+Uj80%ufY5OH2imfe?NJ0x!I5P*9B)kT264u{~YU&
x^7C`Zy|bDBUfn-JJ|=VjFZlY0>FtI5vDV@G)Z{t}0ML^^PE-Iu2Fu>B{{Xx9Q6&HX

literal 0
HcmV?d00001