CVE-2021-21643

@@ -1,10 +1,13 @@
 package org.jenkinsci.plugins.configfiles.maven.security;
 
 import java.io.Serializable;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
+import com.cloudbees.plugins.credentials.CredentialsProvider;
 import hudson.model.*;
+import hudson.security.Permission;
 import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.AncestorInPath;
 import org.kohsuke.stapler.DataBoundConstructor;
@@ -48,12 +51,16 @@ public Descriptor<ServerCredentialMapping> getDescriptor() {
     @Extension
     public static class DescriptorImpl extends Descriptor<ServerCredentialMapping> {
 
-        public ListBoxModel doFillCredentialsIdItems(@AncestorInPath ItemGroup context, @QueryParameter String serverId) {
-            AccessControlled _context = (context instanceof AccessControlled ? (AccessControlled) context : Jenkins.get());
-            if (_context == null || !_context.hasPermission(Item.CONFIGURE)) {
+        public ListBoxModel doFillCredentialsIdItems(@AncestorInPath ItemGroup context, @AncestorInPath Item projectOrFolder, @QueryParameter String serverId) {
+            List<Permission> permsToCheck = projectOrFolder == null ? Arrays.asList(Jenkins.ADMINISTER) : Arrays.asList(Item.EXTENDED_READ, CredentialsProvider.USE_ITEM);
+            AccessControlled contextToCheck = projectOrFolder == null ? Jenkins.get() : projectOrFolder;
+            
+            // If we're on the global page and we don't have administer permission or if we're in a project or folder 
+            // and we don't have permission to use credentials and extended read in the item
+            if (permsToCheck.stream().anyMatch( per -> !contextToCheck.hasPermission(per))) {
                 return new StandardUsernameListBoxModel().includeCurrentValue(serverId);
             }
-
+            
             List<DomainRequirement> domainRequirements = Collections.emptyList();
             if (StringUtils.isNotBlank(serverId)) {
                 domainRequirements = Collections.singletonList(new MavenServerIdRequirement(serverId));

@@ -11,6 +11,7 @@
 import hudson.model.Queue;
 import hudson.security.ACL;
 import hudson.security.AccessControlled;
+import hudson.security.Permission;
 import hudson.util.ListBoxModel;
 import jenkins.model.Jenkins;
 import org.apache.commons.lang.StringUtils;
@@ -50,12 +51,16 @@ public Descriptor<PropertiesCredentialMapping> getDescriptor() {
     @Extension
     public static class DescriptorImpl extends Descriptor<PropertiesCredentialMapping> {
 
-        public ListBoxModel doFillCredentialsIdItems(@AncestorInPath ItemGroup context, @QueryParameter String propertyKey) {
-            AccessControlled _context = (context instanceof AccessControlled ? (AccessControlled) context : Jenkins.get());
-            if (_context == null || !_context.hasPermission(Item.CONFIGURE)) {
+        public ListBoxModel doFillCredentialsIdItems(@AncestorInPath ItemGroup context, @AncestorInPath Item projectOrFolder, @QueryParameter String propertyKey) {
+            Permission permToCheck = projectOrFolder == null ? Jenkins.ADMINISTER : Item.CONFIGURE;
+            AccessControlled contextToCheck = projectOrFolder == null ? Jenkins.get() : projectOrFolder;
+
+            // If we're on the global page and we don't have administer permission or if we're in a project or folder 
+            // and we don't have configure permission there
+            if (!contextToCheck.hasPermission(permToCheck)) {
                 return new StandardUsernameListBoxModel().includeCurrentValue(propertyKey);
             }
-
+            
             List<DomainRequirement> domainRequirements = Collections.emptyList();
             if (StringUtils.isNotBlank(propertyKey)) {
                 domainRequirements = Collections.singletonList(new PropertyKeyRequirement(propertyKey));

@@ -0,0 +1,71 @@
+package org.jenkinsci.plugins.configfiles.sec;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.security.AccessDeniedException2;
+import hudson.security.Permission;
+import org.hamcrest.core.IsEqual;
+
+import java.util.function.Supplier;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.fail;
+
+/**
+ * A class to run pieces of code with a certain user and assert either the code runs successfully, without even worrying
+ * about the result, or the code fails with an {@link AccessDeniedException2} with the specified {@link Permission}.
+ */
+public class PermissionChecker extends ProtectedCodeRunner<Void> {
+    /**
+     * Build a checker to run a specific code with this user and assert it runs successfully or it fails because certain
+     * permission.
+     * @param code The code to execute.
+     * @param user The user to execute with.
+     */
+    public PermissionChecker(@NonNull Runnable code, @NonNull String user) {
+        super(getSupplier(code), user);
+    }
+
+    /**
+     * Assert the execution of the code by this user fails with this permission. The code throws an {@link AccessDeniedException2}
+     * with the permission field being permission. Otherwise it fails.
+     * @param permission The permission thrown by the code.
+     */
+    public void assertFailWithPermission(Permission permission) {
+        Throwable t = getThrowable();
+        if (t instanceof AccessDeniedException2) {
+            assertThat(((AccessDeniedException2) t).permission, IsEqual.equalTo(permission));
+        } else {
+            fail(String.format("The code run by %s didn't throw an AccessDeniedException2 with %s. If failed with the unexpected throwable: %s", getUser(), permission, t));
+        }
+    }
+
+    /**
+     * Assert the execution is done without any exception. The result doesn't matter.
+     */
+    public void assertPass() {
+        getResult(); // The result doesn't matter
+    }
+
+    /**
+     * Change the user to run the code with.
+     * @param user The user.
+     * @return This object.
+     */
+    @Override
+    public PermissionChecker withUser(String user) {
+        super.withUser(user);
+        return this;
+    }
+
+    /**
+     * Get a supplier from a runnable.
+     * @param code The runnable to run.
+     * @return A supplier executing the runnable code and returning just null.
+     */
+    private static Supplier<Void> getSupplier(Runnable code) {
+        return () -> {
+            code.run();
+            return null;
+        };
+    }
+}

@@ -0,0 +1,34 @@
+package org.jenkinsci.plugins.configfiles.sec;
+
+import jenkins.model.Jenkins;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+public class PermissionCheckerTests {
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Before
+    public void setUpAuthorizationAndProject() {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
+                grant(Jenkins.READ).everywhere().to("reader").
+                grant(Jenkins.ADMINISTER).everywhere().to("administer")
+        );
+    }
+    
+    @Test
+    public void protectedCodeCheckerTest() {
+        Runnable run = () -> r.jenkins.checkPermission(Jenkins.ADMINISTER);
+
+        // The administer passes
+        PermissionChecker checker = new PermissionChecker(run, "administer");
+        checker.assertPass();
+
+        // The reader fails
+        checker.withUser("reader").assertFailWithPermission(Jenkins.ADMINISTER);
+    }
+}

@@ -0,0 +1,79 @@
+package org.jenkinsci.plugins.configfiles.sec;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+
+import java.util.function.Supplier;
+
+import static org.junit.Assert.fail;
+
+/**
+ * Class to run a code returning something with a specific user. You can get the result or the {@link Throwable} thrown
+ * by the code executed. Afterwards you can assert the result or the exception thrown.
+ * @param <Result>
+ */
+public class ProtectedCodeRunner<Result> {
+    @NonNull
+    private final Supplier<Result> code;
+    @NonNull
+    private String user;
+
+    /**
+     * Create an object to run this piece of code with this Jenkins user.
+     * @param code The code to be executed.
+     * @param user The user executing this code.
+     */
+    public ProtectedCodeRunner(@NonNull Supplier<Result> code, @NonNull String user) {
+        this.code = code;
+        this.user = user;
+    }
+
+    /**
+     * We run the code expecting to get a result. If the execution throws an exception (Throwable), the test fails with
+     * a descriptive message.
+     * @return The result of the execution.
+     */
+    public Result getResult() {
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName(user))) {
+            return code.get();
+        } catch (Throwable t) {
+            fail(String.format("The code executed by %s didn't run successfully. The throwable thrown is: %s", user, t));
+            return null;
+        }
+    }
+
+    /**
+     * We run the code expecting an exception to be thrown. If it's not the case, the test fails with a descriptive 
+     * message.
+     * @return The {@link Throwable} thrown by the execution.
+     */
+    public Throwable getThrowable() {
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName(user))) {
+            Result result = code.get();
+            fail(String.format("The code executed by %s was successful but we were expecting it to fail. The result of the execution was: %s", user, result));
+            return null;
+        } catch (Throwable t) {
+            return t;
+        }
+    }
+
+    /**
+     * Get the user.
+     * @return The user.
+     */
+    public String getUser() {
+        return user;
+    }
+
+    /**
+     * Use a different user.
+     * @param user The user.
+     * @return This object.
+     */
+    public ProtectedCodeRunner<Result> withUser(String user) {
+        this.user = user;
+        return this;
+    }
+}

@@ -0,0 +1,48 @@
+package org.jenkinsci.plugins.configfiles.sec;
+
+import hudson.security.AccessDeniedException2;
+import jenkins.model.Jenkins;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+import java.util.function.Supplier;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+
+/**
+ * Check the {@link ProtectedCodeRunner} class works correctly.
+ */
+public class ProtectedCodeRunnerTests {
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+
+    @Before
+    public void setUpAuthorizationAndProject() {
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy().
+                grant(Jenkins.READ).everywhere().to("reader").
+                grant(Jenkins.ADMINISTER).everywhere().to("administer")
+        );
+    }
+    
+    @Test
+    public void protectedCodeCheckerTest() {
+        Supplier<String> supplier = () -> {
+            r.jenkins.checkPermission(Jenkins.ADMINISTER);
+            return "allowed";
+        };
+        
+        ProtectedCodeRunner<String> checker = new ProtectedCodeRunner<>(supplier, "administer");
+        assertThat(checker.getResult(), is("allowed"));
+
+        Throwable t = checker.withUser("reader").getThrowable(); 
+        assertThat(t, instanceOf(AccessDeniedException2.class));
+        assertThat(((AccessDeniedException2) t).permission, equalTo(Jenkins.ADMINISTER));
+    }
+}

@@ -0,0 +1,177 @@
+package org.jenkinsci.plugins.configfiles.sec;
+
+import com.cloudbees.hudson.plugins.folder.Folder;
+import com.cloudbees.hudson.plugins.folder.properties.FolderCredentialsProvider;
+import com.cloudbees.plugins.credentials.CredentialsProvider;
+import com.cloudbees.plugins.credentials.CredentialsScope;
+import com.cloudbees.plugins.credentials.CredentialsStore;
+import com.cloudbees.plugins.credentials.SystemCredentialsProvider;
+import com.cloudbees.plugins.credentials.UserCredentialsProvider;
+import com.cloudbees.plugins.credentials.domains.Domain;
+import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl;
+import hudson.model.Item;
+import hudson.model.ModelObject;
+import hudson.model.User;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+import hudson.util.ListBoxModel;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.configfiles.maven.security.ServerCredentialMapping;
+import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.junit.Before;
+import org.junit.ClassRule;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.BuildWatcher;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+
+import java.io.IOException;
+import java.util.function.Supplier;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasSize;
+
+public class Security2254Test {
+    @ClassRule
+    public static BuildWatcher buildWatcher = new BuildWatcher();
+    
+    @Rule
+    public JenkinsRule r = new JenkinsRule();
+    
+    private WorkflowJob project;
+    private Folder folder;
+    
+    @Before
+    public void setUpAuthorizationAndProject() throws IOException {
+        // A folder and a project inside
+        folder = r.jenkins.createProject(Folder.class, "f");
+        project = folder.createProject(WorkflowJob.class, "p");
+        
+        // The permissions
+        r.jenkins.setSecurityRealm(r.createDummySecurityRealm());
+        MockAuthorizationStrategy strategy = new MockAuthorizationStrategy();
+        
+        // Everyone can read
+        strategy.grant(Jenkins.READ).everywhere().toEveryone();
+        
+        // Reader. No permissions to manage credentials
+        strategy.grant(Item.READ).everywhere().to("reader");
+        
+        // accredited with own credentials store and able to use them in project
+        strategy.grant(Item.EXTENDED_READ).onItems(project).to("accredited");
+        strategy.grant(CredentialsProvider.USE_ITEM).onItems(project).to("accredited");
+        
+        // Project configurer on project
+        strategy.grant(Item.CONFIGURE).onItems(project).to("projectConfigurer");
+        
+        // Folder configurer
+        strategy.grant(Item.CONFIGURE).onItems(folder).to("folderConfigurer");
+        
+        // Administer
+        strategy.grant((Jenkins.ADMINISTER)).everywhere().to("administer");
+        
+        r.jenkins.setAuthorizationStrategy(strategy);
+        
+        // A system global credential
+        SystemCredentialsProvider.getInstance().getCredentials().add(new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "systemCred", "", "systemUser", "systemPassword"));
+        
+        // The credentials in folder and for accredited
+        CredentialsStore folderStore = getFolderStore(folder);
+        UsernamePasswordCredentialsImpl folderCredential =
+                new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "folderCred", "description", "folderUser",
+                        "folderPassword");
+        folderStore.addCredentials(Domain.global(), folderCredential);
+
+        // A credential for accredited
+        CredentialsStore userStore = getUserStore(User.getOrCreateByIdOrFullName("accredited"));
+        UsernamePasswordCredentialsImpl userCredential =
+                new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "userCred", "description", "accreditedUser",
+                        "accreditedPassword");
+        // SYSTEM cannot add credentials to the user store
+        try (ACLContext ctx = ACL.as(User.getOrCreateByIdOrFullName("accredited"))) {
+            userStore.addCredentials(Domain.global(), userCredential);
+        }
+    }
+    
+    @Test
+    @Issue("SECURITY-2254")
+    public void fillCredentialIdItemsForServer() throws Exception {
+        final String CURRENT = "current-value";
+        
+        // Code called from global pages
+        Supplier<ListBoxModel> systemCredentialListSupplier = () -> {
+            ServerCredentialMapping.DescriptorImpl descriptor = (ServerCredentialMapping.DescriptorImpl) Jenkins.get().getDescriptorOrDie(ServerCredentialMapping.class);
+            return descriptor.doFillCredentialsIdItems(r.jenkins, null, CURRENT);
+        };
+        ProtectedCodeRunner<ListBoxModel> globalChecker = new ProtectedCodeRunner<>(systemCredentialListSupplier, "user-will-be-replaced");
+
+        // Code called from a project
+        Supplier<ListBoxModel> projectCredentialListSupplier = () -> {
+            ServerCredentialMapping.DescriptorImpl descriptor = (ServerCredentialMapping.DescriptorImpl) Jenkins.get().getDescriptorOrDie(ServerCredentialMapping.class);
+            return descriptor.doFillCredentialsIdItems(folder, project, CURRENT);
+        };
+        ProtectedCodeRunner<ListBoxModel> projectChecker = new ProtectedCodeRunner<>(projectCredentialListSupplier, "user-will-be-replaced");
+        
+        // Code called from a folder
+        Supplier<ListBoxModel> folderCredentialListSupplier = () -> {
+            ServerCredentialMapping.DescriptorImpl descriptor = (ServerCredentialMapping.DescriptorImpl) Jenkins.get().getDescriptorOrDie(ServerCredentialMapping.class);
+            return descriptor.doFillCredentialsIdItems(r.jenkins, folder, CURRENT);
+        };
+        ProtectedCodeRunner<ListBoxModel> folderChecker = new ProtectedCodeRunner<>(folderCredentialListSupplier, "user-will-be-replaced");
+
+        ListBoxModel result;
+        
+        // Reader doesn't get the list of credentials
+        result = globalChecker.withUser("reader").getResult();
+        assertThat(result, hasSize(1));
+        assertThat(result.get(0).value, equalTo(CURRENT));
+
+        // Administer has access to the one stored
+        result = globalChecker.withUser("administer").getResult();
+        assertThat(result, hasSize(2));
+        assertThat(result.get(1).value, equalTo("systemCred"));
+
+        // accredited see system global and folder ones. Their own credentials are not available because the 
+        // gathering of the credentials is used by ACL.SYSTEM. See: https://github.com/jenkinsci/config-file-provider-plugin/blob/master/src/main/java/org/jenkinsci/plugins/configfiles/maven/security/ServerCredentialMapping.java#L64
+        // So there is no visibility of their own credentials while configuring the project.
+        result = projectChecker.withUser("accredited").getResult();
+        assertThat(result, hasSize(3));
+        assertThat(result.get(1).value, equalTo("folderCred")); // system, folder
+        assertThat(result.get(2).value, equalTo("systemCred")); // project
+
+        // Project configurer see system and folder ones because CONFIGURE implies USE_ITEMS of credentials
+        result = projectChecker.withUser("projectConfigurer").getResult();
+        assertThat(result, hasSize(3));
+        assertThat(result.get(1).value, equalTo("folderCred")); // system, folder
+        assertThat(result.get(2).value, equalTo("systemCred")); // project
+        
+        // Folder configurer, without access to the project cannot get them
+        result = globalChecker.withUser("folderConfigurer").getResult();
+        assertThat(result, hasSize(1));
+        assertThat(result.get(0).value, equalTo(CURRENT));
+    }
+    
+    private CredentialsStore getFolderStore(Folder f) {
+        return getCredentialStore(f, FolderCredentialsProvider.class);
+    }
+
+    private CredentialsStore getUserStore(User u) {
+        return getCredentialStore(u, UserCredentialsProvider.class);
+    }
+    
+    private CredentialsStore getCredentialStore(ModelObject object, Class<? extends CredentialsProvider> clazz) {
+        Iterable<CredentialsStore> stores = CredentialsProvider.lookupStores(object);
+        CredentialsStore folderStore = null;
+        for (CredentialsStore s : stores) {
+            if (clazz.isInstance(s.getProvider()) && s.getContext() == object) {
+                folderStore = s;
+                break;
+            }
+        }
+        return folderStore;
+    }
+    
+}