diff --git a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java
index 3e535242a4..15b1934db7 100644
--- a/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java
+++ b/core/src/main/java/jenkins/security/admin/AdminCallableMonitor.java
@@ -29,12 +29,12 @@ public class AdminCallableMonitor extends AdministrativeMonitor {
@Override
public boolean isActivated() {
- return rule.hasRejection();
+ return !rule.rejected.get().isEmpty();
}
@Override
public String getDisplayName() {
- return "Slave \u2192 Master Command Whitelisting";
+ return "Slave \u2192 Master Access Control";
}
// bind this to URL
diff --git a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java
index 70c97db6f6..49464ee019 100644
--- a/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java
+++ b/core/src/main/java/jenkins/security/admin/AdminWhitelistRule.java
@@ -1,15 +1,11 @@
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;
@@ -21,19 +17,12 @@ 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.
*
@@ -47,14 +36,22 @@ public class AdminWhitelistRule implements StaplerProxy {
/**
* Ones that we rejected but want to run by admins.
*/
- private final Set rejected = Collections.synchronizedSet(new LinkedHashSet());
+ public final CallableRejectionConfig rejected;
- @CopyOnWrite
- private volatile Set whitelisted = Collections.emptySet();
+ public final CallableWhitelistConfig whitelisted;
@CopyOnWrite
private final List filePathRules = Collections.emptyList();
+ public AdminWhitelistRule() {
+ // 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/
+ this.whitelisted = new CallableWhitelistConfig(
+ new File(jenkins.getRootDir(),"secrets/whitelisted-callables.txt"));
+ this.rejected = new CallableRejectionConfig(
+ new File(jenkins.getRootDir(),"secrets/rejected-callables.txt"),
+ whitelisted);
+ }
public boolean checkFileAccess(String op, File path) throws SecurityException {
String pathStr = null;
@@ -86,75 +83,10 @@ public class AdminWhitelistRule implements StaplerProxy {
return true; // whitelisted by admin
// otherwise record the problem and refuse to execute that
- boolean changed = rejected.add(subject.getClass());
- if (changed) {
- saveRejected();
- }
-
+ rejected.report(subject.getClass());
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);
@@ -171,7 +103,7 @@ public class AdminWhitelistRule implements StaplerProxy {
}
}
- setWhiteList(whitelist);
+ whitelisted.set(whitelist);
return HttpResponses.redirectToDot();
}
@@ -181,15 +113,13 @@ public class AdminWhitelistRule implements StaplerProxy {
*/
@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();
+ StringBuilder buf = new StringBuilder();
+ for (Class c : rejected.get()) {
+ buf.append(c.getName()).append('\n');
}
+ whitelisted.append(buf.toString());
+
+ return HttpResponses.ok();
}
/**
@@ -197,59 +127,10 @@ public class AdminWhitelistRule implements StaplerProxy {
*/
@RequirePOST
public HttpResponse doApprove(@QueryParameter String value) throws IOException {
- appendWhiteList(value);
+ whitelisted.append(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.
*/
diff --git a/core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java b/core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java
new file mode 100644
index 0000000000..48a83d06e1
--- /dev/null
+++ b/core/src/main/java/jenkins/security/admin/CallableRejectionConfig.java
@@ -0,0 +1,78 @@
+package jenkins.security.admin;
+
+import com.google.common.collect.ImmutableSet;
+import jenkins.model.Jenkins;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+/**
+ * Text file that lists whitelisted callables.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class CallableRejectionConfig extends ConfigFile> {
+ private final CallableWhitelistConfig whitelist;
+
+ CallableRejectionConfig(File file, CallableWhitelistConfig whitelist) {
+ super(file);
+ this.whitelist = whitelist;
+ }
+
+ @Override
+ protected Set create() {
+ return new HashSet();
+ }
+
+ @Override
+ protected Set readOnly(Set base) {
+ return ImmutableSet.copyOf(base);
+ }
+
+ @Override
+ protected Class parse(String line) {
+ try {
+ line = line.trim();
+ if (whitelist.contains(line)) return null; // already whitelisted
+
+ return Jenkins.getInstance().pluginManager.uberClassLoader.loadClass(line);
+ } catch (ClassNotFoundException e) {
+ // no longer present in the system?
+ return null;
+ }
+ }
+
+ /**
+ * This method gets called every time we see a new type of callable that we reject,
+ * so that we can persist the list.
+ */
+ void report(Class c) {
+ if (!get().contains(c)) {
+ try {
+ append(c.getName());
+ } catch (IOException e) {
+ LOGGER.log(Level.WARNING, "Failed to persist " + file, e);
+ }
+ }
+ }
+
+ /**
+ * Return the object that helps the UI rendering by providing the details.
+ */
+ public List describe() {
+ List l = new ArrayList();
+ for (Class c : get()) {
+ l.add(new RejectedCallable(c));
+ }
+ return l;
+ }
+
+
+ private static final Logger LOGGER = Logger.getLogger(CallableRejectionConfig.class.getName());
+}
diff --git a/core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java b/core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java
new file mode 100644
index 0000000000..6fe5fde4a2
--- /dev/null
+++ b/core/src/main/java/jenkins/security/admin/CallableWhitelistConfig.java
@@ -0,0 +1,39 @@
+package jenkins.security.admin;
+
+import com.google.common.collect.ImmutableSet;
+import hudson.Util;
+import org.jenkinsci.remoting.RoleSensitive;
+
+import java.io.File;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Set of fully-qualified {@link RoleSensitive} (mostly Callable) class names that are whitelisted by admin.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+class CallableWhitelistConfig extends ConfigFile> {
+ CallableWhitelistConfig(File file) {
+ super(file);
+ }
+
+ @Override
+ protected Set create() {
+ return new HashSet();
+ }
+
+ @Override
+ protected Set readOnly(Set base) {
+ return ImmutableSet.copyOf(base);
+ }
+
+ @Override
+ protected String parse(String line) {
+ return Util.fixEmptyAndTrim(line);
+ }
+
+ public boolean contains(String name) {
+ return get().contains(name);
+ }
+}
diff --git a/core/src/main/java/jenkins/security/admin/ConfigFile.java b/core/src/main/java/jenkins/security/admin/ConfigFile.java
new file mode 100644
index 0000000000..fb40bd08e2
--- /dev/null
+++ b/core/src/main/java/jenkins/security/admin/ConfigFile.java
@@ -0,0 +1,73 @@
+package jenkins.security.admin;
+
+import hudson.CopyOnWrite;
+import hudson.util.TextFile;
+import jenkins.model.Jenkins;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.Collection;
+
+/**
+ * Abstraction of a line-by-line configuration text file that gets parsed into some in-memory data form.
+ *
+ * @author Kohsuke Kawaguchi
+ */
+abstract class ConfigFile> extends TextFile {
+ @CopyOnWrite
+ private volatile COL parsed;
+
+ public ConfigFile(File file) {
+ super(file);
+ load();
+ }
+
+ protected abstract COL create();
+ protected abstract COL readOnly(COL base);
+
+ public synchronized void load() {
+ COL result = create();
+
+ if (exists()) {
+ for (String line : lines()) {
+ if (line.startsWith("#")) continue; // comment
+ T r = parse(line);
+ if (r != null)
+ result.add(r);
+ }
+ }
+
+ parsed = readOnly(result);
+ }
+
+ protected abstract T parse(String line);
+
+ public synchronized void set(String newContent) throws IOException {
+ Jenkins.getInstance().checkPermission(Jenkins.ADMINISTER);
+
+ write(newContent);
+ load();
+ }
+
+ public synchronized void append(String additional) throws IOException {
+ String s = read();
+ if (!s.endsWith("\n"))
+ s += "\n";
+ s+= additional;
+
+ set(s);
+ }
+
+ public COL get() {
+ // load upon the first use
+ if (parsed==null) {
+ synchronized (this) {
+ if (parsed==null)
+ load();
+ }
+ }
+ return parsed;
+ }
+
+
+}
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 e78d4e8328..5b6871eed4 100644
--- a/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly
+++ b/core/src/main/resources/jenkins/security/admin/AdminWhitelistRule/index.jelly
@@ -26,7 +26,7 @@ THE SOFTWARE.
- ${%Slave → Master Command Whitelisting}
+ ${%Slave → Master Access Control}
Jenkins master is now more strict about what commands its slaves can send to the master.
@@ -40,14 +40,14 @@ THE SOFTWARE.