From cfb59257bf88791292e2a749ee0d0218a0bc24e1 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 10:37:32 -0700 Subject: [PATCH 01/23] Using the wording changes suggested by Daniel --- .../message.properties | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/src/main/resources/jenkins/diagnostics/PinningIsBlockingBundledPluginMonitor/message.properties b/core/src/main/resources/jenkins/diagnostics/PinningIsBlockingBundledPluginMonitor/message.properties index 2236e9cec0..c1fc741c55 100644 --- a/core/src/main/resources/jenkins/diagnostics/PinningIsBlockingBundledPluginMonitor/message.properties +++ b/core/src/main/resources/jenkins/diagnostics/PinningIsBlockingBundledPluginMonitor/message.properties @@ -1,3 +1,4 @@ -blurb=The following plugins are currently pinned, and \ - this prevents Jenkins from using a newer version that''s bundled in the core. \ - We recommend you to unpin these plugins: \ No newline at end of file +blurb=This version of Jenkins comes with new versions of the following plugins that are currently \ + pinned in \ + the plugin manager. \ + It is recommended to upgrade them to at least the version bundled with Jenkins. From da0f7e84e90c00660946167038eb04f0e0a896d8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 17 Oct 2014 16:44:40 -0400 Subject: [PATCH 02/23] Adding a couple of whitelist entries. --- core/src/main/resources/jenkins/security/admin/callable.conf | 3 +++ .../main/resources/jenkins/security/admin/filepath-filter.conf | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/core/src/main/resources/jenkins/security/admin/callable.conf b/core/src/main/resources/jenkins/security/admin/callable.conf index 700b0a3c85..d3c8c67a50 100644 --- a/core/src/main/resources/jenkins/security/admin/callable.conf +++ b/core/src/main/resources/jenkins/security/admin/callable.conf @@ -21,3 +21,6 @@ org.jenkinsci.plugins.gitclient.CliGitAPIImpl$GetPrivateKeys # ssh-credentials plugin com.cloudbees.jenkins.plugins.sshcredentials.SSHAuthenticator$1 + +# cygwin-process-killer plugin +com.synopsys.arc.jenkinsci.plugins.cygwinprocesskiller.CygwinProcessKiller$KillerRemoteCall diff --git a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf index cd9b39bef7..84ebebb3cc 100644 --- a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf +++ b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf @@ -32,7 +32,8 @@ deny all /build.xml # - dependency-check writes archive/artifacts.txt allow all /.+ -# TODO allow /cobertura/ +# cobertura also writes out annotated sources to a dir under the job: +allow all /jobs/.+/cobertura.* # all the other accesses that aren't specified here will be left upto other rules in this directory. # if no rules in those other files matches, then the access will be rejected. From 14f05e91418c6bf94cd67ff18b3c1a628485321a Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 17 Oct 2014 17:20:47 -0400 Subject: [PATCH 03/23] One more entry. --- core/src/main/resources/jenkins/security/admin/callable.conf | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/src/main/resources/jenkins/security/admin/callable.conf b/core/src/main/resources/jenkins/security/admin/callable.conf index d3c8c67a50..070379c61f 100644 --- a/core/src/main/resources/jenkins/security/admin/callable.conf +++ b/core/src/main/resources/jenkins/security/admin/callable.conf @@ -24,3 +24,6 @@ com.cloudbees.jenkins.plugins.sshcredentials.SSHAuthenticator$1 # cygwin-process-killer plugin com.synopsys.arc.jenkinsci.plugins.cygwinprocesskiller.CygwinProcessKiller$KillerRemoteCall + +# selenium-plugin +hudson.plugins.selenium.JenkinsCapabilityMatcher$LabelMatcherCallable From 6ff47b37f667f9e69900cd79b3f96cceb88349d4 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 11:19:32 -0700 Subject: [PATCH 04/23] Need to guide the users to SECURITY-144 landing page --- core/src/main/java/jenkins/SoloFilePathFilter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/SoloFilePathFilter.java b/core/src/main/java/jenkins/SoloFilePathFilter.java index cf9096705d..419b50b0d9 100644 --- a/core/src/main/java/jenkins/SoloFilePathFilter.java +++ b/core/src/main/java/jenkins/SoloFilePathFilter.java @@ -28,7 +28,7 @@ public final class SoloFilePathFilter extends FilePathFilter { private boolean noFalse(String op, File f, boolean b) { if (!b) - throw new SecurityException("slave may not " + op + " " + f); + throw new SecurityException("slave may not " + op + " " + f+"\nSee http://jenkins-ci.org/security-144 for more details"); return true; } From bc4d0e1824f3af4b7f8690bd753068e3a01ec111 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 11:39:27 -0700 Subject: [PATCH 05/23] Adding new line like elsewhere --- .../main/java/jenkins/security/CallableDirectionChecker.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/security/CallableDirectionChecker.java b/core/src/main/java/jenkins/security/CallableDirectionChecker.java index 8b6507eca6..d8910f9c65 100644 --- a/core/src/main/java/jenkins/security/CallableDirectionChecker.java +++ b/core/src/main/java/jenkins/security/CallableDirectionChecker.java @@ -57,7 +57,7 @@ public class CallableDirectionChecker extends RoleChecker { return; } - throw new SecurityException("Sending " + name + " from slave to master is prohibited. See http://jenkins-ci.org/security-144 for more details"); + throw new SecurityException("Sending " + name + " from slave to master is prohibited.\nSee http://jenkins-ci.org/security-144 for more details"); } /** From 050b0e7588520e9e546659d6aefce89c57cf7a22 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 13:13:00 -0700 Subject: [PATCH 06/23] set a shorter ID to make URL bit nicer --- .../java/jenkins/security/admin/AdminCallableMonitor.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java index 7df5a8b31e..c750b62703 100644 --- a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java +++ b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java @@ -27,6 +27,10 @@ public class AdminCallableMonitor extends AdministrativeMonitor { @Inject AdminWhitelistRule rule; + public AdminCallableMonitor() { + super("slaveToMasterAccessControl"); + } + @Override public boolean isActivated() { return !rule.rejected.describe().isEmpty(); From aec88b73b54673f653dd0384a55819297c6f08d5 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 15:35:50 -0700 Subject: [PATCH 07/23] Fixing inconsistency. Daniel Beck pointed out that if we ask people to create files not writable from Jenkins as a means to suppress edit to gui.conf, then we should just stick to the same approach all the way. --- .../jenkins/security/admin/AdminWhitelistRule.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 8848f51996..2ba8fd76fa 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -17,6 +17,7 @@ import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.Enumeration; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -60,7 +61,7 @@ public class AdminWhitelistRule implements StaplerProxy { "filepath-filter.conf"); this.whitelisted = new CallableWhitelistConfig( - new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/gui.conf")); + new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/local.conf")); this.rejected = new CallableRejectionConfig( new File(jenkins.getRootDir(),"secrets/rejected-callables.txt"), whitelisted); @@ -69,8 +70,13 @@ public class AdminWhitelistRule implements StaplerProxy { } private void placeDefaultRule(File f, String resource) throws IOException, InterruptedException { - if (!f.exists() || f.length()>0) { + try { new FilePath(f).copyFrom(getClass().getResource(resource)); + } catch (IOException e) { + // we allow admins to create a read-only file here to block overwrite, + // so this can fail legitimately + if (!f.canWrite()) return; + LOGGER.log(Level.WARNING, "Failed to generate "+f,e); } } From ee16f8b63b1f9abba06307da84ae9e54f442c472 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 15:36:39 -0700 Subject: [PATCH 08/23] Fixing another inconsistency. Most other files are local, so 'gui.conf' better explains what they are. --- .../main/java/jenkins/security/admin/AdminWhitelistRule.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 2ba8fd76fa..9d967e4824 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -61,12 +61,12 @@ public class AdminWhitelistRule implements StaplerProxy { "filepath-filter.conf"); this.whitelisted = new CallableWhitelistConfig( - new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/local.conf")); + new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/gui.conf")); this.rejected = new CallableRejectionConfig( new File(jenkins.getRootDir(),"secrets/rejected-callables.txt"), whitelisted); this.filePathRules = new FilePathRuleConfig( - new File(jenkins.getRootDir(),"secrets/filepath-filters.d/50-local.conf")); + new File(jenkins.getRootDir(),"secrets/filepath-filters.d/50-gui.conf")); } private void placeDefaultRule(File f, String resource) throws IOException, InterruptedException { From 37a3ddaaed29a6c8b7ca394fde000adc28b024df Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 16:07:41 -0700 Subject: [PATCH 09/23] Path has to be in Windows format on Windows --- .../security/admin/AdminWhitelistRule.java | 36 ++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 9d967e4824..09b133bf64 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -2,8 +2,10 @@ package jenkins.security.admin; import hudson.Extension; import hudson.FilePath; +import hudson.Functions; import hudson.Util; import hudson.util.HttpResponses; +import hudson.util.IOUtils; import jenkins.model.Jenkins; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleSensitive; @@ -13,8 +15,17 @@ import org.kohsuke.stapler.StaplerProxy; import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.interceptor.RequirePOST; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.PrintStream; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URL; import java.util.Collection; import java.util.Enumeration; import java.util.logging.Level; @@ -55,10 +66,10 @@ public class AdminWhitelistRule implements StaplerProxy { // 0-byte file is used as a signal from the admin to prevent this overwriting placeDefaultRule( new File(jenkins.getRootDir(), "secrets/whitelisted-callables.d/default.conf"), - "callable.conf"); + getClass().getResourceAsStream("callable.conf")); placeDefaultRule( new File(jenkins.getRootDir(), "secrets/filepath-filters.d/30-default.conf"), - "filepath-filter.conf"); + transformForWindows(getClass().getResourceAsStream("filepath-filter.conf"))); this.whitelisted = new CallableWhitelistConfig( new File(jenkins.getRootDir(),"secrets/whitelisted-callables.d/gui.conf")); @@ -69,9 +80,26 @@ public class AdminWhitelistRule implements StaplerProxy { new File(jenkins.getRootDir(),"secrets/filepath-filters.d/50-gui.conf")); } - private void placeDefaultRule(File f, String resource) throws IOException, InterruptedException { + /** + * Transform path for Windows. + */ + private InputStream transformForWindows(InputStream src) throws IOException { + BufferedReader r = new BufferedReader(new InputStreamReader(src)); + ByteArrayOutputStream out = new ByteArrayOutputStream(); + PrintStream p = new PrintStream(out); + String line; + while ((line=r.readLine())!=null) { + if (!line.startsWith("#") && Functions.isWindows()) + line = line.replace("/","\\\\"); + p.println(line); + } + p.close(); + return new ByteArrayInputStream(out.toByteArray()); + } + + private void placeDefaultRule(File f, InputStream src) throws IOException, InterruptedException { try { - new FilePath(f).copyFrom(getClass().getResource(resource)); + new FilePath(f).copyFrom(src); } catch (IOException e) { // we allow admins to create a read-only file here to block overwrite, // so this can fail legitimately From e8adb10bcba46341db6a882c8533d6bd9be2daf2 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 16:10:01 -0700 Subject: [PATCH 10/23] tightening up the treatment of access to directory itself --- .../resources/jenkins/security/admin/filepath-filter.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf index 84ebebb3cc..7ba39ef0ef 100644 --- a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf +++ b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf @@ -13,12 +13,12 @@ # that slaves have absolutely no business with. # Unless there are rules in other files allowing access to other portions of $JENKINS_HOME, # this rule as it stands here has no effect, because anything left unspecified is rejected. -deny all /secrets/.* +deny all /secrets($|/.*) # User content is publicly readable, so quite safe for slaves to read, too. # (The xunit plugin is known to read from here.) # https://wiki.jenkins-ci.org/display/JENKINS/User+Content -allow read,stat /userContent/.* +allow read,stat /userContent($|/.*) # In the next rule we grant general access under build directories, so first we protect # the actual build record that Jenkins core reads, which nothing should be touching. From 7c9c5c73e87622f268784fd69c46962b9d68c6d0 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 18:08:35 -0700 Subject: [PATCH 11/23] Path related changes & Windows fix - Added based on Daniel's comment - Path macros need to be translated if on Windows - Windows treat '/' as a valid path separator, so it needs to be normalized to '\\'. --- .../security/admin/FilePathRuleConfig.java | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java b/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java index 4bbe3165ac..9508409945 100644 --- a/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java +++ b/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java @@ -12,6 +12,8 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import static hudson.Functions.isWindows; + /** * Config file that lists {@link FilePathRule} rules. * @@ -37,9 +39,10 @@ class FilePathRuleConfig extends ConfigDirectory line = line.trim(); if (line.isEmpty()) return null; - line = line.replace("","/jobs/.+/builds/"); - line = line.replace("",Jenkins.getInstance().getRootDir().getPath()); - line = line.replace("","[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]"); + line = line.replace("",path("/builds/")); + line = line.replace("",path("[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]")); + line = line.replace("",path("/jobs/.+")); + line = line.replace("",path(Jenkins.getInstance().getRootDir().getPath())); Matcher m = PARSER.matcher(line); if (!m.matches()) @@ -55,6 +58,11 @@ class FilePathRuleConfig extends ConfigDirectory } } + private String path(String s) { + if (isWindows()) return s.replace('/','\\'); + return s; + } + private OpMatcher createOpMatcher(String token) { if (token.equals("all")) return OpMatcher.ALL; @@ -73,10 +81,13 @@ class FilePathRuleConfig extends ConfigDirectory for (FilePathRule rule : get()) { if (rule.op.matches(op)) { - if (pathStr==null) + 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 (isWindows()) + pathStr = pathStr.replace('/','\\'); + } if (rule.path.matcher(pathStr).matches()) { // exclusion rule is only to bypass later path rules within #filePathRules, From 9342558ab1959382463d8ba423d4739fc323feb5 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 18:51:57 -0700 Subject: [PATCH 12/23] Based on a discussion with Daniel Beck, implemented a master kill switch as an admin configurable feature. The wiki page already talks about how an admin can do this: https://wiki.jenkins-ci.org/display/JENKINS/Slave+To+Master+Access+Control#SlaveToMasterAccessControl-Disabletheaccesscontrolmechanismaltogether There's a risk in making this too easy, but we keep finding more plugins that need SECURITY-144 adjustments, and I'm really worried about SECURITY-144 fix blowing up in the face of too many people. --- .../security/admin/AdminFilePathFilter.java | 2 +- .../security/admin/AdminWhitelistRule.java | 61 +++++++++++++++++-- .../admin/MasterKillSwitchConfiguration.java | 37 +++++++++++ .../admin/AdminWhitelistRule/index.jelly | 14 ++++- .../config.groovy | 9 +++ .../help-masterToSlaveAccessControl.html | 4 ++ 6 files changed, 118 insertions(+), 9 deletions(-) create mode 100644 core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java create mode 100644 core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy create mode 100644 core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html diff --git a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java b/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java index 6458d9483c..bf5790123a 100644 --- a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java +++ b/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java @@ -29,7 +29,7 @@ public class AdminFilePathFilter extends ReflectiveFilePathFilter { @Override protected boolean op(String op, File path) throws SecurityException { - return rule.filePathRules.checkFileAccess(op,path); + return rule.checkFileAccess(op,path); } @Extension diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 09b133bf64..63b22790d8 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -5,8 +5,9 @@ import hudson.FilePath; import hudson.Functions; import hudson.Util; import hudson.util.HttpResponses; -import hudson.util.IOUtils; import jenkins.model.Jenkins; +import jenkins.util.io.FileBoolean; +import org.apache.commons.io.FileUtils; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleSensitive; import org.kohsuke.stapler.HttpResponse; @@ -23,14 +24,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.PrintStream; -import java.io.PrintWriter; -import java.io.StringWriter; -import java.net.URL; import java.util.Collection; import java.util.Enumeration; -import java.util.logging.Level; import java.util.logging.Logger; +import static java.util.logging.Level.*; + /** * Rules of whitelisting for {@link RoleSensitive} objects and {@link FilePath}s. * @@ -55,6 +54,8 @@ public class AdminWhitelistRule implements StaplerProxy { private final Jenkins jenkins; + private boolean masterKillSwitch; + public AdminWhitelistRule() throws IOException, InterruptedException { this.jenkins = Jenkins.getInstance(); @@ -78,6 +79,28 @@ public class AdminWhitelistRule implements StaplerProxy { whitelisted); this.filePathRules = new FilePathRuleConfig( new File(jenkins.getRootDir(),"secrets/filepath-filters.d/50-gui.conf")); + this.masterKillSwitch = loadMasterKillSwitchFile(); + } + + /** + * Reads the master kill switch. + * + * Instead of {@link FileBoolean}, we use a text file so that the admin can prevent Jenkins from + * writing this to file. + */ + private boolean loadMasterKillSwitchFile() { + File f = getMasterKillSwitchFile(); + try { + if (!f.exists()) return false; + return Boolean.parseBoolean(FileUtils.readFileToString(f).trim()); + } catch (IOException e) { + LOGGER.log(WARNING, "Failed to read "+f, e); + return false; + } + } + + private File getMasterKillSwitchFile() { + return new File(jenkins.getRootDir(),"secrets/master-kill-switch"); } /** @@ -104,11 +127,14 @@ public class AdminWhitelistRule implements StaplerProxy { // we allow admins to create a read-only file here to block overwrite, // so this can fail legitimately if (!f.canWrite()) return; - LOGGER.log(Level.WARNING, "Failed to generate "+f,e); + LOGGER.log(WARNING, "Failed to generate "+f,e); } } public boolean isWhitelisted(RoleSensitive subject, Collection expected, Object context) { + if (masterKillSwitch) + return true; // master kill switch is on. subsystem deactivated + String name = subject.getClass().getName(); if (whitelisted.contains(name)) @@ -119,6 +145,14 @@ public class AdminWhitelistRule implements StaplerProxy { return false; } + public boolean checkFileAccess(String op, File f) { + // if the master kill switch is off, we allow everything + if (masterKillSwitch) + return true; + + return filePathRules.checkFileAccess(op, f); + } + @RequirePOST public HttpResponse doSubmit(StaplerRequest req) throws IOException { jenkins.checkPermission(Jenkins.ADMINISTER); @@ -167,6 +201,21 @@ public class AdminWhitelistRule implements StaplerProxy { return HttpResponses.ok(); } + public boolean getMasterKillSwitch() { + return masterKillSwitch; + } + + public void setMasterKillSwitch(boolean state) { + try { + jenkins.checkPermission(Jenkins.ADMINISTER); + FileUtils.writeStringToFile(getMasterKillSwitchFile(),Boolean.toString(state)); + // treat the file as the canonical source of information in case write fails + masterKillSwitch = loadMasterKillSwitchFile(); + } catch (IOException e) { + LOGGER.log(WARNING, "Failed to write master kill switch", e); + } + } + /** * Restricts the access to administrator. */ diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java b/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java new file mode 100644 index 0000000000..07bef61ca8 --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java @@ -0,0 +1,37 @@ +package jenkins.security.admin; + +import hudson.Extension; +import jenkins.model.GlobalConfiguration; +import jenkins.model.GlobalConfigurationCategory; +import net.sf.json.JSONObject; +import org.kohsuke.stapler.StaplerRequest; + +import javax.inject.Inject; + +/** + * Exposes {@link AdminWhitelistRule#masterKillSwitch} to the admin. + * + * @author Kohsuke Kawaguchi + * @since 1.THU + */ +@Extension +public class MasterKillSwitchConfiguration extends GlobalConfiguration { + @Inject + AdminWhitelistRule rule; + + @Override + public GlobalConfigurationCategory getCategory() { + return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class); + } + + public boolean getMasterToSlaveAccessControl() { + return !rule.getMasterKillSwitch(); + } + + @Override + public boolean configure(StaplerRequest req, JSONObject json) throws FormException { + rule.setMasterKillSwitch(!json.has("masterToSlaveAccessControl")); + return true; + } +} + diff --git a/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly b/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly index 27736403ec..9662491e1c 100644 --- a/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly +++ b/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly @@ -34,10 +34,18 @@ THE SOFTWARE. specify which commands are open for slaves to execute and which ones are not. While plugin developers work on updating this, as an administrator, you can mark commands as OK for slaves to execute (aka "whitelisting".) -

+

+
Please see the discussion of this feature to understand the security implication of this.
+ +
+ The slave → master access control subsystem is currently disabled. + This is unsafe if you have slaves from other less trusted people. + You can put it back on from Global Security Configuration UI +
+

Currently Whitelisted Commands

@@ -97,7 +105,9 @@ allow read,stat <JENKINS_HOME>/userContent/.* # GENERAL SYNTAX: (allow|deny) Op,Op,Op,... PathRegExp - +
+ +
diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy new file mode 100644 index 0000000000..1bb7db71c1 --- /dev/null +++ b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy @@ -0,0 +1,9 @@ +package jenkins.security.admin.MasterKillSwitchConfiguration + +def f=namespace(lib.FormTagLib) + +f.optionalBlock(field:"masterToSlaveAccessControl", title:_("Enable Slave \u2192 Master Access Control")) { + f.nested() { + raw _("Rules can be tweaked here") + } +} diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html new file mode 100644 index 0000000000..e05b9581df --- /dev/null +++ b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html @@ -0,0 +1,4 @@ +
+ See Jenkins project website for discussion of this feature. + We do not recommend you disable this. +
\ No newline at end of file From ec1e47c05343a466827c739512c4aecaa17ac900 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 18:55:05 -0700 Subject: [PATCH 13/23] Change the name because this directory is shared by all secrets --- .../main/java/jenkins/security/admin/AdminWhitelistRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 63b22790d8..55a1046f6e 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -100,7 +100,7 @@ public class AdminWhitelistRule implements StaplerProxy { } private File getMasterKillSwitchFile() { - return new File(jenkins.getRootDir(),"secrets/master-kill-switch"); + return new File(jenkins.getRootDir(),"secrets/slave-to-master-security-kill-switch"); } /** From 8a105ce57c1d8efe5ca48138e74c755c5a86e464 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Fri, 17 Oct 2014 19:03:55 -0700 Subject: [PATCH 14/23] Adding separate check for symlink to prevent jailbreak Otherwise a symlink can be created to point to a file outside $JENKINS_HOME and be read --- core/src/main/java/hudson/FilePath.java | 13 ++++++++++++- core/src/main/java/jenkins/FilePathFilter.java | 7 +++++++ .../main/java/jenkins/FilePathFilterAggregator.java | 9 +++++++++ .../main/java/jenkins/ReflectiveFilePathFilter.java | 5 +++++ core/src/main/java/jenkins/SoloFilePathFilter.java | 5 +++++ .../jenkins/security/admin/filepath-filter.conf | 5 +++-- 6 files changed, 41 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java index f367f4964c..4b2622e6ec 100644 --- a/core/src/main/java/hudson/FilePath.java +++ b/core/src/main/java/hudson/FilePath.java @@ -617,7 +617,7 @@ public final class FilePath implements Serializable { act(new SecureFileCallable() { private static final long serialVersionUID = 1L; public Void invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { - writing(f); + symlinking(f); Util.createSymlink(f.getParentFile(),target,f.getName(),listener); return null; } @@ -2692,6 +2692,17 @@ public final class FilePath implements Serializable { return f; } + /** + * Pass through 'f' after ensuring that we can create that symlink. + */ + private File symlinking(File f) { + FilePathFilter filter = filterNonNull(); + if (!f.exists()) + filter.create(f); + filter.symlink(f); + return f; + } + /** * Pass through 'f' after ensuring that we can delete that file. */ diff --git a/core/src/main/java/jenkins/FilePathFilter.java b/core/src/main/java/jenkins/FilePathFilter.java index 8d008e446a..4a1ed5337c 100644 --- a/core/src/main/java/jenkins/FilePathFilter.java +++ b/core/src/main/java/jenkins/FilePathFilter.java @@ -40,6 +40,13 @@ public abstract class FilePathFilter { */ public boolean write(File f) throws SecurityException { return false; } + /** + * Checks if a symlink can be created at 'f' + * + * On POSIX, this corresponds to the 'w' permission of the file itself. + */ + public boolean symlink(File f) throws SecurityException { return false; } + /** * Checks if the given directory can be created. * diff --git a/core/src/main/java/jenkins/FilePathFilterAggregator.java b/core/src/main/java/jenkins/FilePathFilterAggregator.java index 859d36d3a3..8bf4949e17 100644 --- a/core/src/main/java/jenkins/FilePathFilterAggregator.java +++ b/core/src/main/java/jenkins/FilePathFilterAggregator.java @@ -99,6 +99,15 @@ class FilePathFilterAggregator extends FilePathFilter { return defaultAction(); } + @Override + public boolean symlink(File f) throws SecurityException { + for (Entry e : all) { + if (e.filter.symlink(f)) + return true; + } + return defaultAction(); + } + @Override public boolean create(File f) throws SecurityException { for (Entry e : all) { diff --git a/core/src/main/java/jenkins/ReflectiveFilePathFilter.java b/core/src/main/java/jenkins/ReflectiveFilePathFilter.java index 9905524431..ce963943cd 100644 --- a/core/src/main/java/jenkins/ReflectiveFilePathFilter.java +++ b/core/src/main/java/jenkins/ReflectiveFilePathFilter.java @@ -26,6 +26,11 @@ public abstract class ReflectiveFilePathFilter extends FilePathFilter { return op("write", f); } + @Override + public boolean symlink(File f) throws SecurityException { + return op("symlink",f); + } + @Override public boolean mkdirs(File f) throws SecurityException { return op("mkdirs", f); diff --git a/core/src/main/java/jenkins/SoloFilePathFilter.java b/core/src/main/java/jenkins/SoloFilePathFilter.java index 419b50b0d9..c2cfa58ef0 100644 --- a/core/src/main/java/jenkins/SoloFilePathFilter.java +++ b/core/src/main/java/jenkins/SoloFilePathFilter.java @@ -42,6 +42,11 @@ public final class SoloFilePathFilter extends FilePathFilter { return noFalse("write",f,base.write(f)); } + @Override + public boolean symlink(File f) throws SecurityException { + return noFalse("symlink",f,base.write(f)); + } + @Override public boolean mkdirs(File f) throws SecurityException { return noFalse("mkdirs",f,base.mkdirs(f)); diff --git a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf index 7ba39ef0ef..faa7cffa16 100644 --- a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf +++ b/core/src/main/resources/jenkins/security/admin/filepath-filter.conf @@ -30,10 +30,11 @@ deny all /build.xml # - cobertura writes coverage.xml # - violations writes violations.xml and other content under violations/ # - dependency-check writes archive/artifacts.txt -allow all /.+ +# But not allowing deletion to prevent data loss and symlink to prevent jailbreaking. +allow create,mkdirs,read,stat,write /.+ # cobertura also writes out annotated sources to a dir under the job: -allow all /jobs/.+/cobertura.* +allow create,mkdirs,read,stat,write /jobs/.+/cobertura.* # all the other accesses that aren't specified here will be left upto other rules in this directory. # if no rules in those other files matches, then the access will be rejected. From e24ceef22af26061b772bfe4d158a37c6fa943dd Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 18:20:09 -0700 Subject: [PATCH 15/23] Disabling this subsystem by default. As much as we'd like to enable this by default, what we've learned over the past few days indicate that this is just too disruptive a change. In a nutshell, - The change is just too big. It touches more than 4000 lines just in the core and remoting, which makes this by far the biggest change we've done in secret. The chance of something going wrong is just too high. As recently as yesterday we are still finding significant problems with this subsystem. - We've learnt that Subversion 1.x contains Callables that we cannot just whitelist, but that version is used by roughly a third of users. If we put this on by default, they will see their builds break. - Every time we come up with a new way to scan plugins to find impacted code, we always find several plugins that are affected, indicating that we probably have missed more plugins that will be impacted. - We haven't really analyzed past versions of any plugins at all, but as the Subversion version distribution indicates, considerable portion of people run old plugins. As we see in the Wiki page (http://jenkins-ci.org/security-144), vast majority of Jenkins installations will fit in the category of "I have set up Jenkins master and all slaves by myself", for whom this subsystem is entirely redundant. Based on the knowledge we now have about the impact of this addition, and considering people who benefit from this vs people who don't benefit from this, I'm not enabling this feature out of the box, just yet. Our plan is to advertise this feature, then get necesary plugin updates in place over time. At some later point, we can make this feature on by default, which will only impact new installations. Further down the road, we can inch up the warning given to people who run without this feature, or remove the GUI configuration and force this feature on, to get close to the ideal state of having everyone run with this feature on. Jesse doesn't like this, but I think he understands where I'm coming from. Sorry Jesse, this time I'm having this my way. --- .../main/java/jenkins/security/admin/AdminWhitelistRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java index 55a1046f6e..dbecc6447f 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java @@ -91,7 +91,7 @@ public class AdminWhitelistRule implements StaplerProxy { private boolean loadMasterKillSwitchFile() { File f = getMasterKillSwitchFile(); try { - if (!f.exists()) return false; + if (!f.exists()) return true; return Boolean.parseBoolean(FileUtils.readFileToString(f).trim()); } catch (IOException e) { LOGGER.log(WARNING, "Failed to read "+f, e); From 5db8cecae3e995eeaa95beebc81296a94f4774ec Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 18:51:25 -0700 Subject: [PATCH 16/23] Added a AdminMonitor to to warn when this subsystem is disabled ... unless a few conditions indicate that it is likely irrelevant. --- .../admin/MasterKillSwitchWarning.java | 43 +++++++++++++++++++ .../MasterKillSwitchWarning/message.jelly | 36 ++++++++++++++++ .../message.properties | 2 + 3 files changed, 81 insertions(+) create mode 100644 core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java create mode 100644 core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly create mode 100644 core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java b/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java new file mode 100644 index 0000000000..182de24685 --- /dev/null +++ b/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java @@ -0,0 +1,43 @@ +package jenkins.security.admin; + +import hudson.Extension; +import hudson.model.AdministrativeMonitor; +import jenkins.model.Jenkins; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; +import org.kohsuke.stapler.QueryParameter; + +import javax.inject.Inject; +import java.io.IOException; + +/** + * If {@link AdminWhitelistRule#masterKillSwitch} is on, warn the user. + * + * @author Kohsuke Kawaguchi + * @since 1.THU + */ +@Extension +public class MasterKillSwitchWarning extends AdministrativeMonitor { + @Inject + AdminWhitelistRule rule; + + @Inject + Jenkins jenkins; + + @Override + public boolean isActivated() { + return rule.getMasterKillSwitch() + && jenkins.getComputers().length>1 // if there's no slave, there's no point + && jenkins.isUseSecurity() // if security is off, likewise this is pointless + ; + } + + public HttpResponse doAct(@QueryParameter String dismiss) throws IOException { + if(dismiss!=null) { + disable(true); + return HttpResponses.redirectViaContextPath("/manage"); + } else { + return HttpResponses.redirectViaContextPath("configureSecurity"); + } + } +} diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly new file mode 100644 index 0000000000..6192f8b897 --- /dev/null +++ b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly @@ -0,0 +1,36 @@ + + + + +
+
+
+ + +
+ ${%blurb} +
+
+
\ No newline at end of file diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties new file mode 100644 index 0000000000..7c847a2111 --- /dev/null +++ b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties @@ -0,0 +1,2 @@ +blurb=Slave to master security subsystem is currently off. \ + Please read the documentation and consider turning it on. \ No newline at end of file From 062f9d48455f6f9740b86c51647382fa0f727afa Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 19:34:57 -0700 Subject: [PATCH 17/23] Show the warning & config only when it's relevant --- .../admin/MasterKillSwitchConfiguration.java | 20 ++++++++++++++++++- .../admin/MasterKillSwitchWarning.java | 8 ++------ .../config.groovy | 8 +++++--- 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java b/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java index 07bef61ca8..5102ce57f2 100644 --- a/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java +++ b/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java @@ -3,6 +3,7 @@ package jenkins.security.admin; import hudson.Extension; import jenkins.model.GlobalConfiguration; import jenkins.model.GlobalConfigurationCategory; +import jenkins.model.Jenkins; import net.sf.json.JSONObject; import org.kohsuke.stapler.StaplerRequest; @@ -19,6 +20,9 @@ public class MasterKillSwitchConfiguration extends GlobalConfiguration { @Inject AdminWhitelistRule rule; + @Inject + Jenkins jenkins; + @Override public GlobalConfigurationCategory getCategory() { return GlobalConfigurationCategory.get(GlobalConfigurationCategory.Security.class); @@ -30,8 +34,22 @@ public class MasterKillSwitchConfiguration extends GlobalConfiguration { @Override public boolean configure(StaplerRequest req, JSONObject json) throws FormException { - rule.setMasterKillSwitch(!json.has("masterToSlaveAccessControl")); + if (isRelevant()) { + // don't record on/off unless this becomes relevant, so that we can differentiate + // those who have disabled vs those who haven't cared. + rule.setMasterKillSwitch(!json.has("masterToSlaveAccessControl")); + } return true; } + + /** + * Returns true if the configuration of this subsystem becomes relevant. + * Unless this option is relevant, we don't let users choose this. + */ + public boolean isRelevant() { + return jenkins.getComputers().length>1 // if there's no slave, there's no point + && jenkins.isUseSecurity() // if security is off, likewise this is pointless + ; + } } diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java b/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java index 182de24685..897c4f038a 100644 --- a/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java +++ b/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java @@ -2,7 +2,6 @@ package jenkins.security.admin; import hudson.Extension; import hudson.model.AdministrativeMonitor; -import jenkins.model.Jenkins; import org.kohsuke.stapler.HttpResponse; import org.kohsuke.stapler.HttpResponses; import org.kohsuke.stapler.QueryParameter; @@ -22,14 +21,11 @@ public class MasterKillSwitchWarning extends AdministrativeMonitor { AdminWhitelistRule rule; @Inject - Jenkins jenkins; + MasterKillSwitchConfiguration config; @Override public boolean isActivated() { - return rule.getMasterKillSwitch() - && jenkins.getComputers().length>1 // if there's no slave, there's no point - && jenkins.isUseSecurity() // if security is off, likewise this is pointless - ; + return rule.getMasterKillSwitch() && config.isRelevant(); } public HttpResponse doAct(@QueryParameter String dismiss) throws IOException { diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy index 1bb7db71c1..8bd7be1173 100644 --- a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy +++ b/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy @@ -2,8 +2,10 @@ package jenkins.security.admin.MasterKillSwitchConfiguration def f=namespace(lib.FormTagLib) -f.optionalBlock(field:"masterToSlaveAccessControl", title:_("Enable Slave \u2192 Master Access Control")) { - f.nested() { - raw _("Rules can be tweaked here") +if (instance.isRelevant()) { + f.optionalBlock(field:"masterToSlaveAccessControl", title:_("Enable Slave \u2192 Master Access Control")) { + f.nested() { + raw _("Rules can be tweaked here") + } } } From 7376aa0e292c5df6ec7dd46bf0e399d93911348e Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 19:39:21 -0700 Subject: [PATCH 18/23] Renamed package. --- .../src/main/java/jenkins/security/DefaultFilePathFilter.java | 2 +- core/src/main/java/jenkins/security/admin/package-info.java | 4 ---- .../jenkins/security/{admin => s2m}/AdminCallableMonitor.java | 2 +- .../security/{admin => s2m}/AdminCallableWhitelist.java | 2 +- .../jenkins/security/{admin => s2m}/AdminFilePathFilter.java | 2 +- .../jenkins/security/{admin => s2m}/AdminWhitelistRule.java | 2 +- .../security/{admin => s2m}/CallableRejectionConfig.java | 2 +- .../security/{admin => s2m}/CallableWhitelistConfig.java | 2 +- .../java/jenkins/security/{admin => s2m}/ConfigDirectory.java | 2 +- .../main/java/jenkins/security/{admin => s2m}/ConfigFile.java | 2 +- .../java/jenkins/security/{admin => s2m}/FilePathRule.java | 2 +- .../jenkins/security/{admin => s2m}/FilePathRuleConfig.java | 2 +- .../{admin => s2m}/MasterKillSwitchConfiguration.java | 2 +- .../security/{admin => s2m}/MasterKillSwitchWarning.java | 2 +- .../main/java/jenkins/security/{admin => s2m}/OpMatcher.java | 2 +- .../jenkins/security/{admin => s2m}/RejectedCallable.java | 2 +- core/src/main/java/jenkins/security/s2m/package-info.java | 4 ++++ .../{admin => s2m}/AdminCallableMonitor/message.jelly | 0 .../{admin => s2m}/AdminCallableMonitor/message.properties | 0 .../security/{admin => s2m}/AdminWhitelistRule/index.jelly | 0 .../MasterKillSwitchConfiguration/config.groovy | 2 +- .../help-masterToSlaveAccessControl.html | 0 .../{admin => s2m}/MasterKillSwitchWarning/message.jelly | 0 .../{admin => s2m}/MasterKillSwitchWarning/message.properties | 0 .../resources/jenkins/security/{admin => s2m}/callable.conf | 0 .../jenkins/security/{admin => s2m}/filepath-filter.conf | 0 26 files changed, 20 insertions(+), 20 deletions(-) delete mode 100644 core/src/main/java/jenkins/security/admin/package-info.java rename core/src/main/java/jenkins/security/{admin => s2m}/AdminCallableMonitor.java (97%) rename core/src/main/java/jenkins/security/{admin => s2m}/AdminCallableWhitelist.java (96%) rename core/src/main/java/jenkins/security/{admin => s2m}/AdminFilePathFilter.java (97%) rename core/src/main/java/jenkins/security/{admin => s2m}/AdminWhitelistRule.java (99%) rename core/src/main/java/jenkins/security/{admin => s2m}/CallableRejectionConfig.java (98%) rename core/src/main/java/jenkins/security/{admin => s2m}/CallableWhitelistConfig.java (96%) rename core/src/main/java/jenkins/security/{admin => s2m}/ConfigDirectory.java (98%) rename core/src/main/java/jenkins/security/{admin => s2m}/ConfigFile.java (98%) rename core/src/main/java/jenkins/security/{admin => s2m}/FilePathRule.java (92%) rename core/src/main/java/jenkins/security/{admin => s2m}/FilePathRuleConfig.java (99%) rename core/src/main/java/jenkins/security/{admin => s2m}/MasterKillSwitchConfiguration.java (98%) rename core/src/main/java/jenkins/security/{admin => s2m}/MasterKillSwitchWarning.java (96%) rename core/src/main/java/jenkins/security/{admin => s2m}/OpMatcher.java (92%) rename core/src/main/java/jenkins/security/{admin => s2m}/RejectedCallable.java (93%) create mode 100644 core/src/main/java/jenkins/security/s2m/package-info.java rename core/src/main/resources/jenkins/security/{admin => s2m}/AdminCallableMonitor/message.jelly (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/AdminCallableMonitor/message.properties (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/AdminWhitelistRule/index.jelly (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/MasterKillSwitchConfiguration/config.groovy (84%) rename core/src/main/resources/jenkins/security/{admin => s2m}/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/MasterKillSwitchWarning/message.jelly (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/MasterKillSwitchWarning/message.properties (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/callable.conf (100%) rename core/src/main/resources/jenkins/security/{admin => s2m}/filepath-filter.conf (100%) diff --git a/core/src/main/java/jenkins/security/DefaultFilePathFilter.java b/core/src/main/java/jenkins/security/DefaultFilePathFilter.java index 4696e25cf3..69f85b74e0 100644 --- a/core/src/main/java/jenkins/security/DefaultFilePathFilter.java +++ b/core/src/main/java/jenkins/security/DefaultFilePathFilter.java @@ -27,7 +27,7 @@ package jenkins.security; import hudson.Extension; import hudson.remoting.ChannelBuilder; import jenkins.ReflectiveFilePathFilter; -import jenkins.security.admin.AdminFilePathFilter; +import jenkins.security.s2m.AdminFilePathFilter; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; diff --git a/core/src/main/java/jenkins/security/admin/package-info.java b/core/src/main/java/jenkins/security/admin/package-info.java deleted file mode 100644 index e3607c45bd..0000000000 --- a/core/src/main/java/jenkins/security/admin/package-info.java +++ /dev/null @@ -1,4 +0,0 @@ -/** - * Series of extensions that provide a hook for administrator to whitelist access from slave to master. - */ -package jenkins.security.admin; diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java b/core/src/main/java/jenkins/security/s2m/AdminCallableMonitor.java similarity index 97% rename from core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java rename to core/src/main/java/jenkins/security/s2m/AdminCallableMonitor.java index c750b62703..d6a483af76 100644 --- a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java +++ b/core/src/main/java/jenkins/security/s2m/AdminCallableMonitor.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import hudson.FilePath; diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java b/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java similarity index 96% rename from core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java rename to core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java index 22f552b7b7..7c57360834 100644 --- a/core/src/main/java/jenkins/security/admin/AdminCallableWhitelist.java +++ b/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import hudson.remoting.Callable; diff --git a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java b/core/src/main/java/jenkins/security/s2m/AdminFilePathFilter.java similarity index 97% rename from core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java rename to core/src/main/java/jenkins/security/s2m/AdminFilePathFilter.java index bf5790123a..7832f82bbb 100644 --- a/core/src/main/java/jenkins/security/admin/AdminFilePathFilter.java +++ b/core/src/main/java/jenkins/security/s2m/AdminFilePathFilter.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import hudson.remoting.ChannelBuilder; diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/s2m/AdminWhitelistRule.java similarity index 99% rename from core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java rename to core/src/main/java/jenkins/security/s2m/AdminWhitelistRule.java index dbecc6447f..880a76a388 100644 --- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java +++ b/core/src/main/java/jenkins/security/s2m/AdminWhitelistRule.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import hudson.FilePath; diff --git a/core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java b/core/src/main/java/jenkins/security/s2m/CallableRejectionConfig.java similarity index 98% rename from core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java rename to core/src/main/java/jenkins/security/s2m/CallableRejectionConfig.java index 0ce0e09948..fecdfb7a1f 100644 --- a/core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java +++ b/core/src/main/java/jenkins/security/s2m/CallableRejectionConfig.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import com.google.common.collect.ImmutableSet; import jenkins.model.Jenkins; diff --git a/core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java b/core/src/main/java/jenkins/security/s2m/CallableWhitelistConfig.java similarity index 96% rename from core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java rename to core/src/main/java/jenkins/security/s2m/CallableWhitelistConfig.java index 063a5ebc37..a56d5f8b33 100644 --- a/core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java +++ b/core/src/main/java/jenkins/security/s2m/CallableWhitelistConfig.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import com.google.common.collect.ImmutableSet; import hudson.Util; diff --git a/core/src/main/java/jenkins/security/admin/ConfigDirectory.java b/core/src/main/java/jenkins/security/s2m/ConfigDirectory.java similarity index 98% rename from core/src/main/java/jenkins/security/admin/ConfigDirectory.java rename to core/src/main/java/jenkins/security/s2m/ConfigDirectory.java index 7b2356265b..6356cc038e 100644 --- a/core/src/main/java/jenkins/security/admin/ConfigDirectory.java +++ b/core/src/main/java/jenkins/security/s2m/ConfigDirectory.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import java.io.BufferedReader; import java.io.File; diff --git a/core/src/main/java/jenkins/security/admin/ConfigFile.java b/core/src/main/java/jenkins/security/s2m/ConfigFile.java similarity index 98% rename from core/src/main/java/jenkins/security/admin/ConfigFile.java rename to core/src/main/java/jenkins/security/s2m/ConfigFile.java index 5face75819..69d9358eed 100644 --- a/core/src/main/java/jenkins/security/admin/ConfigFile.java +++ b/core/src/main/java/jenkins/security/s2m/ConfigFile.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.CopyOnWrite; import hudson.util.TextFile; diff --git a/core/src/main/java/jenkins/security/admin/FilePathRule.java b/core/src/main/java/jenkins/security/s2m/FilePathRule.java similarity index 92% rename from core/src/main/java/jenkins/security/admin/FilePathRule.java rename to core/src/main/java/jenkins/security/s2m/FilePathRule.java index 2fb82f6661..c247769e1f 100644 --- a/core/src/main/java/jenkins/security/admin/FilePathRule.java +++ b/core/src/main/java/jenkins/security/s2m/FilePathRule.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.FilePath; diff --git a/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java similarity index 99% rename from core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java rename to core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java index 9508409945..13f8cefeb9 100644 --- a/core/src/main/java/jenkins/security/admin/FilePathRuleConfig.java +++ b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java b/core/src/main/java/jenkins/security/s2m/MasterKillSwitchConfiguration.java similarity index 98% rename from core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java rename to core/src/main/java/jenkins/security/s2m/MasterKillSwitchConfiguration.java index 5102ce57f2..d83d436f4b 100644 --- a/core/src/main/java/jenkins/security/admin/MasterKillSwitchConfiguration.java +++ b/core/src/main/java/jenkins/security/s2m/MasterKillSwitchConfiguration.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import jenkins.model.GlobalConfiguration; diff --git a/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java b/core/src/main/java/jenkins/security/s2m/MasterKillSwitchWarning.java similarity index 96% rename from core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java rename to core/src/main/java/jenkins/security/s2m/MasterKillSwitchWarning.java index 897c4f038a..7a5df576f1 100644 --- a/core/src/main/java/jenkins/security/admin/MasterKillSwitchWarning.java +++ b/core/src/main/java/jenkins/security/s2m/MasterKillSwitchWarning.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.Extension; import hudson.model.AdministrativeMonitor; diff --git a/core/src/main/java/jenkins/security/admin/OpMatcher.java b/core/src/main/java/jenkins/security/s2m/OpMatcher.java similarity index 92% rename from core/src/main/java/jenkins/security/admin/OpMatcher.java rename to core/src/main/java/jenkins/security/s2m/OpMatcher.java index cfebb38b88..7cb83623a2 100644 --- a/core/src/main/java/jenkins/security/admin/OpMatcher.java +++ b/core/src/main/java/jenkins/security/s2m/OpMatcher.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import jenkins.ReflectiveFilePathFilter; diff --git a/core/src/main/java/jenkins/security/admin/RejectedCallable.java b/core/src/main/java/jenkins/security/s2m/RejectedCallable.java similarity index 93% rename from core/src/main/java/jenkins/security/admin/RejectedCallable.java rename to core/src/main/java/jenkins/security/s2m/RejectedCallable.java index 363c7e7cfc..4cfd55c6f8 100644 --- a/core/src/main/java/jenkins/security/admin/RejectedCallable.java +++ b/core/src/main/java/jenkins/security/s2m/RejectedCallable.java @@ -1,4 +1,4 @@ -package jenkins.security.admin; +package jenkins.security.s2m; import hudson.PluginWrapper; import jenkins.model.Jenkins; diff --git a/core/src/main/java/jenkins/security/s2m/package-info.java b/core/src/main/java/jenkins/security/s2m/package-info.java new file mode 100644 index 0000000000..84875777f6 --- /dev/null +++ b/core/src/main/java/jenkins/security/s2m/package-info.java @@ -0,0 +1,4 @@ +/** + * Slave -> master security. + */ +package jenkins.security.s2m; diff --git a/core/src/main/resources/jenkins/security/admin/AdminCallableMonitor/message.jelly b/core/src/main/resources/jenkins/security/s2m/AdminCallableMonitor/message.jelly similarity index 100% rename from core/src/main/resources/jenkins/security/admin/AdminCallableMonitor/message.jelly rename to core/src/main/resources/jenkins/security/s2m/AdminCallableMonitor/message.jelly diff --git a/core/src/main/resources/jenkins/security/admin/AdminCallableMonitor/message.properties b/core/src/main/resources/jenkins/security/s2m/AdminCallableMonitor/message.properties similarity index 100% rename from core/src/main/resources/jenkins/security/admin/AdminCallableMonitor/message.properties rename to core/src/main/resources/jenkins/security/s2m/AdminCallableMonitor/message.properties diff --git a/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly b/core/src/main/resources/jenkins/security/s2m/AdminWhitelistRule/index.jelly similarity index 100% rename from core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly rename to core/src/main/resources/jenkins/security/s2m/AdminWhitelistRule/index.jelly diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy b/core/src/main/resources/jenkins/security/s2m/MasterKillSwitchConfiguration/config.groovy similarity index 84% rename from core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy rename to core/src/main/resources/jenkins/security/s2m/MasterKillSwitchConfiguration/config.groovy index 8bd7be1173..bc95f575f7 100644 --- a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/config.groovy +++ b/core/src/main/resources/jenkins/security/s2m/MasterKillSwitchConfiguration/config.groovy @@ -1,4 +1,4 @@ -package jenkins.security.admin.MasterKillSwitchConfiguration +package jenkins.security.s2m.MasterKillSwitchConfiguration def f=namespace(lib.FormTagLib) diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html b/core/src/main/resources/jenkins/security/s2m/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html similarity index 100% rename from core/src/main/resources/jenkins/security/admin/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html rename to core/src/main/resources/jenkins/security/s2m/MasterKillSwitchConfiguration/help-masterToSlaveAccessControl.html diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly b/core/src/main/resources/jenkins/security/s2m/MasterKillSwitchWarning/message.jelly similarity index 100% rename from core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.jelly rename to core/src/main/resources/jenkins/security/s2m/MasterKillSwitchWarning/message.jelly diff --git a/core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties b/core/src/main/resources/jenkins/security/s2m/MasterKillSwitchWarning/message.properties similarity index 100% rename from core/src/main/resources/jenkins/security/admin/MasterKillSwitchWarning/message.properties rename to core/src/main/resources/jenkins/security/s2m/MasterKillSwitchWarning/message.properties diff --git a/core/src/main/resources/jenkins/security/admin/callable.conf b/core/src/main/resources/jenkins/security/s2m/callable.conf similarity index 100% rename from core/src/main/resources/jenkins/security/admin/callable.conf rename to core/src/main/resources/jenkins/security/s2m/callable.conf diff --git a/core/src/main/resources/jenkins/security/admin/filepath-filter.conf b/core/src/main/resources/jenkins/security/s2m/filepath-filter.conf similarity index 100% rename from core/src/main/resources/jenkins/security/admin/filepath-filter.conf rename to core/src/main/resources/jenkins/security/s2m/filepath-filter.conf From b4ffb2d01be9ed5ea2a2fbafbcb9a4d6593b522c Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 19:47:19 -0700 Subject: [PATCH 19/23] More follow-up renames --- .../java/jenkins/security/s2m/AdminCallableWhitelist.java | 1 - .../jenkins/security/{ => s2m}/CallableDirectionChecker.java | 4 +++- .../java/jenkins/security/{ => s2m}/CallableWhitelist.java | 3 ++- .../jenkins/security/{ => s2m}/DefaultFilePathFilter.java | 4 ++-- .../test/java/jenkins/security/DefaultFilePathFilterTest.java | 3 +-- 5 files changed, 8 insertions(+), 7 deletions(-) rename core/src/main/java/jenkins/security/{ => s2m}/CallableDirectionChecker.java (97%) rename core/src/main/java/jenkins/security/{ => s2m}/CallableWhitelist.java (96%) rename core/src/main/java/jenkins/security/{ => s2m}/DefaultFilePathFilter.java (97%) diff --git a/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java b/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java index 7c57360834..a6bc100d31 100644 --- a/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java +++ b/core/src/main/java/jenkins/security/s2m/AdminCallableWhitelist.java @@ -2,7 +2,6 @@ package jenkins.security.s2m; import hudson.Extension; import hudson.remoting.Callable; -import jenkins.security.CallableWhitelist; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleSensitive; diff --git a/core/src/main/java/jenkins/security/CallableDirectionChecker.java b/core/src/main/java/jenkins/security/s2m/CallableDirectionChecker.java similarity index 97% rename from core/src/main/java/jenkins/security/CallableDirectionChecker.java rename to core/src/main/java/jenkins/security/s2m/CallableDirectionChecker.java index d8910f9c65..706d17c0be 100644 --- a/core/src/main/java/jenkins/security/CallableDirectionChecker.java +++ b/core/src/main/java/jenkins/security/s2m/CallableDirectionChecker.java @@ -1,8 +1,10 @@ -package jenkins.security; +package jenkins.security.s2m; import hudson.Extension; import hudson.remoting.Callable; import hudson.remoting.ChannelBuilder; +import jenkins.security.ChannelConfigurator; +import jenkins.security.Roles; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleChecker; import org.jenkinsci.remoting.RoleSensitive; diff --git a/core/src/main/java/jenkins/security/CallableWhitelist.java b/core/src/main/java/jenkins/security/s2m/CallableWhitelist.java similarity index 96% rename from core/src/main/java/jenkins/security/CallableWhitelist.java rename to core/src/main/java/jenkins/security/s2m/CallableWhitelist.java index 7d69d7f886..50023a0cd0 100644 --- a/core/src/main/java/jenkins/security/CallableWhitelist.java +++ b/core/src/main/java/jenkins/security/s2m/CallableWhitelist.java @@ -1,10 +1,11 @@ -package jenkins.security; +package jenkins.security.s2m; import hudson.ExtensionList; import hudson.ExtensionPoint; import hudson.remoting.Callable; import hudson.remoting.ChannelBuilder; import jenkins.model.Jenkins; +import jenkins.security.ChannelConfigurator; import org.jenkinsci.remoting.Role; import org.jenkinsci.remoting.RoleChecker; import org.jenkinsci.remoting.RoleSensitive; diff --git a/core/src/main/java/jenkins/security/DefaultFilePathFilter.java b/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java similarity index 97% rename from core/src/main/java/jenkins/security/DefaultFilePathFilter.java rename to core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java index 69f85b74e0..563d19269e 100644 --- a/core/src/main/java/jenkins/security/DefaultFilePathFilter.java +++ b/core/src/main/java/jenkins/security/s2m/DefaultFilePathFilter.java @@ -22,12 +22,12 @@ * THE SOFTWARE. */ -package jenkins.security; +package jenkins.security.s2m; import hudson.Extension; import hudson.remoting.ChannelBuilder; import jenkins.ReflectiveFilePathFilter; -import jenkins.security.s2m.AdminFilePathFilter; +import jenkins.security.ChannelConfigurator; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.DoNotUse; diff --git a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java index 0b1a40b680..83ce1b15c1 100644 --- a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java +++ b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java @@ -30,9 +30,8 @@ import hudson.remoting.Callable; import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Collection; -import org.jenkinsci.remoting.Role; +import jenkins.security.s2m.DefaultFilePathFilter; import org.jenkinsci.remoting.RoleChecker; import org.junit.Test; import static org.junit.Assert.*; From ef2dd2473bf9a56e6c9e34300193d85a7fb94454 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 19:55:28 -0700 Subject: [PATCH 20/23] $JENKINS_HOME might contain symbols that are interpreted as regular expression --- core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java index 13f8cefeb9..9d7d7bfcc6 100644 --- a/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java +++ b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java @@ -42,7 +42,7 @@ class FilePathRuleConfig extends ConfigDirectory line = line.replace("",path("/builds/")); line = line.replace("",path("[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]")); line = line.replace("",path("/jobs/.+")); - line = line.replace("",path(Jenkins.getInstance().getRootDir().getPath())); + line = line.replace("","\\Q"+path(Jenkins.getInstance().getRootDir().getPath())) + "\\E"; Matcher m = PARSER.matcher(line); if (!m.matches()) From 8632efc35f5a466c1d96d767ebbfc08369f436a7 Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 20:03:38 -0700 Subject: [PATCH 21/23] Allowing (and encouraging) rules in config file to use '/' as separator. Based on comments from Daniel Beck and Jesse Glick in 37a3dda --- .../security/s2m/FilePathRuleConfig.java | 21 +++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java index 9d7d7bfcc6..76289186bd 100644 --- a/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java +++ b/core/src/main/java/jenkins/security/s2m/FilePathRuleConfig.java @@ -39,10 +39,14 @@ class FilePathRuleConfig extends ConfigDirectory line = line.trim(); if (line.isEmpty()) return null; - line = line.replace("",path("/builds/")); - line = line.replace("",path("[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]")); - line = line.replace("",path("/jobs/.+")); - line = line.replace("","\\Q"+path(Jenkins.getInstance().getRootDir().getPath())) + "\\E"; + line = line.replace("","/builds/"); + line = line.replace("","[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]_[0-9][0-9]-[0-9][0-9]-[0-9][0-9]"); + line = line.replace("","/jobs/.+"); + line = line.replace("","\\Q"+Jenkins.getInstance().getRootDir().getPath()+"\\E"); + + // config file is always /-separated even on Windows, so bring it back to \-separation. + // This is done in the context of regex, so it has to be \\, which means in the source code it is \\\\ + if (isWindows()) line = line.replace("/","\\\\"); Matcher m = PARSER.matcher(line); if (!m.matches()) @@ -58,11 +62,6 @@ class FilePathRuleConfig extends ConfigDirectory } } - private String path(String s) { - if (isWindows()) return s.replace('/','\\'); - return s; - } - private OpMatcher createOpMatcher(String token) { if (token.equals("all")) return OpMatcher.ALL; @@ -82,10 +81,10 @@ class FilePathRuleConfig extends ConfigDirectory for (FilePathRule rule : get()) { if (rule.op.matches(op)) { if (pathStr==null) { - // do not canonicalize nor absolutize, so that JENKINS_HOME that spans across + // do not canonicalize, so that JENKINS_HOME that spans across // multiple volumes via symlinks can look logically like one unit. pathStr = path.getPath(); - if (isWindows()) + if (isWindows()) // Windows accepts '/' as separator, but for rule matching we want to normalize for consistent comparison pathStr = pathStr.replace('/','\\'); } From b4c2a1392b1cb00579c2a2f3779e29892fafc42f Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 22:42:08 -0700 Subject: [PATCH 22/23] Now that the default is off, test has to activate this explicitly --- .../security/DefaultFilePathFilterTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java index 83ce1b15c1..9143042fcf 100644 --- a/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java +++ b/test/src/test/java/jenkins/security/DefaultFilePathFilterTest.java @@ -31,17 +31,31 @@ import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; +import jenkins.security.s2m.AdminWhitelistRule; import jenkins.security.s2m.DefaultFilePathFilter; +import jenkins.security.s2m.MasterKillSwitchConfiguration; import org.jenkinsci.remoting.RoleChecker; +import org.junit.Before; import org.junit.Test; import static org.junit.Assert.*; import org.junit.Rule; import org.jvnet.hudson.test.JenkinsRule; +import javax.inject.Inject; + public class DefaultFilePathFilterTest { @Rule public JenkinsRule r = new JenkinsRule(); + @Inject + AdminWhitelistRule rule; + + @Before + public void setUp() { + r.jenkins.getInjector().injectMembers(this); + rule.setMasterKillSwitch(false); + } + @Test public void remotePath() throws Exception { Slave s = r.createOnlineSlave(); FilePath forward = s.getRootPath().child("forward"); From b2d5cd5162667d2ce559852115dea502a4009f3e Mon Sep 17 00:00:00 2001 From: Kohsuke Kawaguchi Date: Sat, 18 Oct 2014 22:42:28 -0700 Subject: [PATCH 23/23] Integtrating a new version of credentials plugin. This version fixes a test failure in 1.580.1 --- war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/war/pom.xml b/war/pom.xml index 0dbdec1df1..16149eae89 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -275,7 +275,7 @@ THE SOFTWARE. org.jenkins-ci.plugins credentials - 1.17 + 1.18 hpi