From 356764dced7c84a9ba51379cc41c3d8dc7fa2bae Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Thu, 16 Oct 2014 11:06:24 -0700 Subject: [PATCH] Moved the rules into a separate class --- .../security/admin/AdminCallableMonitor.java | 54 +--- .../admin/AdminCallableWhitelist.java | 230 +-------------- .../security/admin/AdminFilePathFilter.java | 14 +- .../security/admin/AdminWhitelistRule.java | 263 ++++++++++++++++++ .../jenkins/security/admin/FilePathRule.java | 22 ++ .../jenkins/security/admin/OpMatcher.java | 14 + .../security/admin/RejectedCallable.java | 22 ++ .../index.jelly | 0 8 files changed, 336 insertions(+), 283 deletions(-) create mode 100644 core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java create mode 100644 core/src/main/java/jenkins/security/admin/FilePathRule.java create mode 100644 core/src/main/java/jenkins/security/admin/OpMatcher.java create mode 100644 core/src/main/java/jenkins/security/admin/RejectedCallable.java rename core/src/main/resources/jenkins/security/admin/{AdminCallableWhitelist => AdminWhitelistRule}/index.jelly (100%) diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java index 4d5b0f34f6..3e535242a4 100644 --- a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java +++ b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java @@ -10,11 +10,7 @@ import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.QueryParameter; import javax.inject.Inject; -import java.io.File; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; -import java.util.regex.Pattern; /** * Report any rejected {@link Callable}s and {@link FilePath} executions and allow @@ -29,51 +25,11 @@ public class AdminCallableMonitor extends AdministrativeMonitor { Jenkins jenkins; @Inject - AdminCallableWhitelist whitelist; - - private interface OpMatcher { - boolean matches(String op); - } - - private class FilePathRule { - final Pattern path; - final OpMatcher op; - final boolean allow; - - private FilePathRule(Pattern path, OpMatcher op, boolean allow) { - this.path = path; - this.op = op; - this.allow = allow; - } - } - - private final List filePathRules = new ArrayList(); - - public boolean checkFileAccess(String op, File path) throws SecurityException { - String pathStr = null; - - for (FilePathRule rule : filePathRules) { - if (rule.op.matches(op)) { - if (pathStr==null) - // do not canonicalize nor absolutize, so that JENKINS_HOME that spans across - // multiple volumes via symlinks can look logically like one unit. - pathStr = path.getPath(); - - if (rule.path.matcher(pathStr).matches()) { - // exclusion rule is only to bypass later path rules within #filePathRules, - // and we still want other FilePathFilters to whitelist/blacklist access. - // therefore I'm not throwing a SecurityException here - return rule.allow; - } - } - } - - return false; - } + AdminWhitelistRule rule; @Override public boolean isActivated() { - return whitelist.hasRejection(); + return rule.hasRejection(); } @Override @@ -82,8 +38,8 @@ public class AdminCallableMonitor extends AdministrativeMonitor { } // bind this to URL - public AdminCallableWhitelist getWhitelist() { - return whitelist; + public AdminWhitelistRule getRule() { + return rule; } /** @@ -94,7 +50,7 @@ public class AdminCallableMonitor extends AdministrativeMonitor { disable(true); return HttpResponses.redirectViaContextPath("/manage"); } else { - return HttpResponses.redirectTo("whitelist/"); + return HttpResponses.redirectTo("rule/"); } } } diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java b/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java index 00fb30312a..22f552b7b7 100644 --- a/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java +++ b/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java @@ -1,39 +1,13 @@ package jenkins.security.admin; -import com.google.common.collect.ImmutableSet; import hudson.Extension; -import hudson.PluginWrapper; -import hudson.Util; -import hudson.init.Initializer; import hudson.remoting.Callable; -import hudson.util.HttpResponses; -import hudson.util.TextFile; -import jenkins.model.Jenkins; import jenkins.security.CallableWhitelist; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleSensitive; -import org.kohsuke.stapler.HttpResponse; -import org.kohsuke.stapler.QueryParameter; -import org.kohsuke.stapler.StaplerProxy; -import org.kohsuke.stapler.StaplerRequest; -import org.kohsuke.stapler.interceptor.RequirePOST; -import javax.annotation.CheckForNull; import javax.inject.Inject; -import java.io.File; -import java.io.IOException; -import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; -import java.util.Enumeration; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Set; -import java.util.logging.Level; -import java.util.logging.Logger; - -import static hudson.init.InitMilestone.*; /** * Whitelists {@link Callable}s that are approved by the admins. @@ -47,210 +21,12 @@ import static hudson.init.InitMilestone.*; * @author Kohsuke Kawaguchi */ @Extension(ordinal=-100) -public class AdminCallableWhitelist extends CallableWhitelist implements StaplerProxy { - /** - * Ones that we rejected but want to run by admins. - */ - private final Set rejected = Collections.synchronizedSet(new LinkedHashSet()); - - // CoW - private volatile Set whitelisted = Collections.emptySet(); - +public class AdminCallableWhitelist extends CallableWhitelist { @Inject - Jenkins jenkins; + AdminWhitelistRule rule; @Override public boolean isWhitelisted(RoleSensitive subject, Collection expected, Object context) { - String name = subject.getClass().getName(); - - if (whitelisted.contains(name)) - return true; // whitelisted by admin - - // otherwise record the problem and refuse to execute that - boolean changed = rejected.add(subject.getClass()); - if (changed) { - saveRejected(); - } - - return false; + return rule.isWhitelisted(subject,expected,context); } - - /** - * Returns the currently rejected callables. - */ - public List getRejectedList() { - List l = new ArrayList(); - synchronized (rejected) { - for (Class c : rejected) { - l.add(new RejectedCallable(c)); - } - } - return l; - } - - public boolean hasRejection() { - return !rejected.isEmpty(); - } - - /** - * Persists {@link #rejected} - */ - private void saveRejected() { - try { - synchronized (rejected) { - StringBuilder buf = new StringBuilder(); - for (Class c : rejected) { - if (buf.length()>0) buf.append('\n'); - buf.append(c.getName()); - } - getRejectedFile().write(buf.toString()); - } - } catch (IOException e) { - LOGGER.log(Level.WARNING, "Failed to persist "+getRejectedFile(), e); - } - } - - public TextFile getRejectedFile() { - // while this file is not a secret, write access to this file is dangerous, - // so put this in the better-protected part of $JENKINS_HOME, which is in secrets/ - return new TextFile(new File(jenkins.getRootDir(),"secrets/rejected-callables.txt")); - } - - private void appendWhiteList(String addition) throws IOException { - String s = getWhitelistFile().read(); - if (!s.endsWith("\n")) - s += "\n"; - s+= addition; - - setWhiteList(s); - } - - private void setWhiteList(String newWhiteList) throws IOException { - jenkins.checkPermission(Jenkins.ADMINISTER); - - getWhitelistFile().write(newWhiteList); - loadWhitelist(); - } - - public TextFile getWhitelistFile() { - return new TextFile(new File(jenkins.getRootDir(),"secrets/whitelisted-callables.txt")); - } - - @RequirePOST - public HttpResponse doSubmit(StaplerRequest req) throws IOException { - jenkins.checkPermission(Jenkins.ADMINISTER); - - String whitelist = Util.fixNull(req.getParameter("whitelist")); - if (!whitelist.endsWith("\n")) - whitelist+="\n"; - - Enumeration e = req.getParameterNames(); - while (e.hasMoreElements()) { - String name = (String) e.nextElement(); - if (name.startsWith("class:")) { - whitelist += name.substring(6)+"\n"; - } - } - - setWhiteList(whitelist); - - return HttpResponses.redirectToDot(); - } - - /** - * Approves all the currently rejected subjects - */ - @RequirePOST - public HttpResponse doApproveAll() throws IOException { - synchronized (rejected) { - StringBuilder buf = new StringBuilder(); - for (Class c : rejected) { - buf.append(c.getName()).append('\n'); - } - appendWhiteList(buf.toString()); - - return HttpResponses.ok(); - } - } - - /** - * Approves specific callables by their names. - */ - @RequirePOST - public HttpResponse doApprove(@QueryParameter String value) throws IOException { - appendWhiteList(value); - return HttpResponses.ok(); - } - - @Initializer(after=EXTENSIONS_AUGMENTED,fatal=false) - public static void init() throws IOException { - CallableWhitelist.all().get(AdminCallableWhitelist.class).load(); - } - - public void load() throws IOException { - loadRejected(); - loadWhitelist(); - } - - private void loadRejected() throws IOException { - if (getRejectedFile().exists()) { - for (String line : getRejectedFile().read().split("\n")) { - try { - Class c = jenkins.pluginManager.uberClassLoader.loadClass(line.trim()); - rejected.add(c); - } catch (ClassNotFoundException e) { - // no longer present in the system? - } - } - } - } - - private void loadWhitelist() throws IOException { - if (getWhitelistFile().exists()) { - List whitelist = new ArrayList(); - for (String line : getWhitelistFile().read().split("\n")) { - whitelist.add(line.trim()); - } - this.whitelisted = ImmutableSet.copyOf(whitelist); - } - - // remove whitelisted ones from the reject list - boolean changed = false; - synchronized (rejected) { - Iterator itr = rejected.iterator(); - while (itr.hasNext()) { - Class next = itr.next(); - if (whitelisted.contains(next.getName())) { - itr.remove(); - changed = true; - } - } - } - - if (changed) - saveRejected(); - } - - /** - * Restricts the access to administrator. - */ - @Override - public Object getTarget() { - jenkins.checkPermission(Jenkins.ADMINISTER); - return this; - } - - public class RejectedCallable { - public final Class clazz; - - public RejectedCallable(Class clazz) { - this.clazz = clazz; - } - - public @CheckForNull PluginWrapper getPlugin() { - return jenkins.pluginManager.whichPlugin(clazz); - } - } - - private static final Logger LOGGER = Logger.getLogger(AdminCallableWhitelist.class.getName()); } diff --git a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java b/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java index e6e52a4cbc..4dfb96bda5 100644 --- a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java +++ b/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java @@ -14,32 +14,32 @@ import java.io.File; * {@link FilePathFilter} that allows admins to whitelist specific file access. * *

- * This class is just a glue, and the real logic happens inside {@link AdminCallableMonitor} + * This class is just a glue, and the real logic happens inside {@link AdminWhitelistRule} * * @author Kohsuke Kawaguchi * @since 1.THU */ public class AdminFilePathFilter extends ReflectiveFilePathFilter { - private final AdminCallableMonitor monitor; + private final AdminWhitelistRule rule; - public AdminFilePathFilter(AdminCallableMonitor monitor) { - this.monitor = monitor; + public AdminFilePathFilter(AdminWhitelistRule rule) { + this.rule = rule; } @Override protected boolean op(String op, File path) throws SecurityException { - return monitor.checkFileAccess(op,path); + return rule.checkFileAccess(op,path); } @Extension public static class ChannelConfiguratorImpl extends ChannelConfigurator { @Inject - AdminCallableMonitor monitor; + AdminWhitelistRule rule; @Override public void onChannelBuilding(ChannelBuilder builder, @Nullable Object context) { - new AdminFilePathFilter(monitor).installTo(builder); + new AdminFilePathFilter(rule).installTo(builder); } } } diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java new file mode 100644 index 0000000000..70c97db6f6 --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -0,0 +1,263 @@ +package jenkins.security.admin; + +import com.google.common.collect.ImmutableSet; +import hudson.CopyOnWrite; +import hudson.Extension; +import hudson.FilePath; +import hudson.Util; +import hudson.init.Initializer; +import hudson.util.HttpResponses; +import hudson.util.TextFile; +import jenkins.model.Jenkins; +import jenkins.security.CallableWhitelist; +import org.jenkinsci.remoting.Role; +import org.jenkinsci.remoting.RoleSensitive; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.StaplerProxy; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.interceptor.RequirePOST; + +import javax.inject.Inject; +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.Enumeration; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; + +import static hudson.init.InitMilestone.*; + +/** + * Rules of whitelisting for {@link RoleSensitive} objects and {@link FilePath}s. + * + * @author Kohsuke Kawaguchi + */ +@Extension +public class AdminWhitelistRule implements StaplerProxy { + @Inject + Jenkins jenkins; + + /** + * Ones that we rejected but want to run by admins. + */ + private final Set rejected = Collections.synchronizedSet(new LinkedHashSet()); + + @CopyOnWrite + private volatile Set whitelisted = Collections.emptySet(); + + @CopyOnWrite + private final List filePathRules = Collections.emptyList(); + + + public boolean checkFileAccess(String op, File path) throws SecurityException { + String pathStr = null; + + for (FilePathRule rule : filePathRules) { + if (rule.op.matches(op)) { + if (pathStr==null) + // do not canonicalize nor absolutize, so that JENKINS_HOME that spans across + // multiple volumes via symlinks can look logically like one unit. + pathStr = path.getPath(); + + if (rule.path.matcher(pathStr).matches()) { + // exclusion rule is only to bypass later path rules within #filePathRules, + // and we still want other FilePathFilters to whitelist/blacklist access. + // therefore I'm not throwing a SecurityException here + return rule.allow; + } + } + } + + return false; + } + + + public boolean isWhitelisted(RoleSensitive subject, Collection expected, Object context) { + String name = subject.getClass().getName(); + + if (whitelisted.contains(name)) + return true; // whitelisted by admin + + // otherwise record the problem and refuse to execute that + boolean changed = rejected.add(subject.getClass()); + if (changed) { + saveRejected(); + } + + return false; + } + + /** + * Returns the currently rejected callables. + */ + public List getRejectedList() { + List l = new ArrayList(); + synchronized (rejected) { + for (Class c : rejected) { + l.add(new RejectedCallable(c)); + } + } + return l; + } + + public boolean hasRejection() { + return !rejected.isEmpty(); + } + + /** + * Persists {@link #rejected} + */ + private void saveRejected() { + try { + synchronized (rejected) { + StringBuilder buf = new StringBuilder(); + for (Class c : rejected) { + if (buf.length()>0) buf.append('\n'); + buf.append(c.getName()); + } + getRejectedFile().write(buf.toString()); + } + } catch (IOException e) { + LOGGER.log(Level.WARNING, "Failed to persist "+getRejectedFile(), e); + } + } + + public TextFile getRejectedFile() { + // while this file is not a secret, write access to this file is dangerous, + // so put this in the better-protected part of $JENKINS_HOME, which is in secrets/ + return new TextFile(new File(jenkins.getRootDir(),"secrets/rejected-callables.txt")); + } + + private void appendWhiteList(String addition) throws IOException { + String s = getWhitelistFile().read(); + if (!s.endsWith("\n")) + s += "\n"; + s+= addition; + + setWhiteList(s); + } + + private void setWhiteList(String newWhiteList) throws IOException { + jenkins.checkPermission(Jenkins.ADMINISTER); + + getWhitelistFile().write(newWhiteList); + loadWhitelist(); + } + + public TextFile getWhitelistFile() { + return new TextFile(new File(jenkins.getRootDir(),"secrets/whitelisted-callables.txt")); + } + + @RequirePOST + public HttpResponse doSubmit(StaplerRequest req) throws IOException { + jenkins.checkPermission(Jenkins.ADMINISTER); + + String whitelist = Util.fixNull(req.getParameter("whitelist")); + if (!whitelist.endsWith("\n")) + whitelist+="\n"; + + Enumeration e = req.getParameterNames(); + while (e.hasMoreElements()) { + String name = (String) e.nextElement(); + if (name.startsWith("class:")) { + whitelist += name.substring(6)+"\n"; + } + } + + setWhiteList(whitelist); + + return HttpResponses.redirectToDot(); + } + + /** + * Approves all the currently rejected subjects + */ + @RequirePOST + public HttpResponse doApproveAll() throws IOException { + synchronized (rejected) { + StringBuilder buf = new StringBuilder(); + for (Class c : rejected) { + buf.append(c.getName()).append('\n'); + } + appendWhiteList(buf.toString()); + + return HttpResponses.ok(); + } + } + + /** + * Approves specific callables by their names. + */ + @RequirePOST + public HttpResponse doApprove(@QueryParameter String value) throws IOException { + appendWhiteList(value); + return HttpResponses.ok(); + } + + @Initializer(after=EXTENSIONS_AUGMENTED,fatal=false) + public static void init() throws IOException { + CallableWhitelist.all().get(AdminCallableWhitelist.class).load(); + } + + public void load() throws IOException { + loadRejected(); + loadWhitelist(); + } + + private void loadRejected() throws IOException { + if (getRejectedFile().exists()) { + for (String line : getRejectedFile().read().split("\n")) { + try { + Class c = jenkins.pluginManager.uberClassLoader.loadClass(line.trim()); + rejected.add(c); + } catch (ClassNotFoundException e) { + // no longer present in the system? + } + } + } + } + + private void loadWhitelist() throws IOException { + if (getWhitelistFile().exists()) { + List whitelist = new ArrayList(); + for (String line : getWhitelistFile().read().split("\n")) { + whitelist.add(line.trim()); + } + this.whitelisted = ImmutableSet.copyOf(whitelist); + } + + // remove whitelisted ones from the reject list + boolean changed = false; + synchronized (rejected) { + Iterator itr = rejected.iterator(); + while (itr.hasNext()) { + Class next = itr.next(); + if (whitelisted.contains(next.getName())) { + itr.remove(); + changed = true; + } + } + } + + if (changed) + saveRejected(); + } + + /** + * Restricts the access to administrator. + */ + @Override + public Object getTarget() { + jenkins.checkPermission(Jenkins.ADMINISTER); + return this; + } + + private static final Logger LOGGER = Logger.getLogger(AdminWhitelistRule.class.getName()); +} diff --git a/core/src/main/java/jenkins/security/admin/FilePathRule.java b/core/src/main/java/jenkins/security/admin/FilePathRule.java new file mode 100644 index 0000000000..2fb82f6661 --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/FilePathRule.java @@ -0,0 +1,22 @@ +package jenkins.security.admin; + +import hudson.FilePath; + +import java.util.regex.Pattern; + +/** + * One entry of the {@link FilePath} access rule. + * + * @author Kohsuke Kawaguchi + */ +class FilePathRule { + final Pattern path; + final OpMatcher op; + final boolean allow; + + FilePathRule(Pattern path, OpMatcher op, boolean allow) { + this.path = path; + this.op = op; + this.allow = allow; + } +} diff --git a/core/src/main/java/jenkins/security/admin/OpMatcher.java b/core/src/main/java/jenkins/security/admin/OpMatcher.java new file mode 100644 index 0000000000..0ea92e65f1 --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/OpMatcher.java @@ -0,0 +1,14 @@ +package jenkins.security.admin; + +import jenkins.ReflectiveFilePathFilter; + +import java.io.File; + +/** + * Tests a match against file operation name of {@link ReflectiveFilePathFilter#op(String, File)}. + * + * @author Kohsuke Kawaguchi + */ +interface OpMatcher { + boolean matches(String op); +} diff --git a/core/src/main/java/jenkins/security/admin/RejectedCallable.java b/core/src/main/java/jenkins/security/admin/RejectedCallable.java new file mode 100644 index 0000000000..363c7e7cfc --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/RejectedCallable.java @@ -0,0 +1,22 @@ +package jenkins.security.admin; + +import hudson.PluginWrapper; +import jenkins.model.Jenkins; + +import javax.annotation.CheckForNull; + +/** +* @author Kohsuke Kawaguchi +*/ +public /*for Jelly*/ class RejectedCallable { + public final Class clazz; + + /*package*/ RejectedCallable(Class clazz) { + this.clazz = clazz; + } + + public @CheckForNull + PluginWrapper getPlugin() { + return Jenkins.getInstance().pluginManager.whichPlugin(clazz); + } +} diff --git a/core/src/main/resources/jenkins/security/admin/AdminCallableWhitelist/index.jelly b/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly similarity index 100% rename from core/src/main/resources/jenkins/security/admin/AdminCallableWhitelist/index.jelly rename to core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly