Skip to content

Commit

Permalink
Merge branch 'master' into JENKINS-34616
Browse files Browse the repository at this point in the history
  • Loading branch information
daniel-beck committed Sep 19, 2017
2 parents f498441 + 9306d66 commit cfe4dd4
Show file tree
Hide file tree
Showing 18 changed files with 769 additions and 679 deletions.
Expand Up @@ -34,18 +34,18 @@
import hudson.model.listeners.ItemListener;
import hudson.security.AuthorizationStrategy;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.matrixauth.AuthorizationMatrixPropertyDescriptor;
import hudson.security.GlobalMatrixAuthorizationStrategy;
import org.jenkinsci.plugins.matrixauth.AuthorizationPropertyDescriptor;
import hudson.security.Permission;
import hudson.security.PermissionScope;
import hudson.security.ProjectMatrixAuthorizationStrategy;
import hudson.security.SidACL;
import org.jenkinsci.plugins.matrixauth.AbstractMatrixPropertyConverter;
import org.jenkinsci.plugins.matrixauth.AbstractAuthorizationPropertyConverter;
import org.jenkinsci.plugins.matrixauth.AuthorizationProperty;
import hudson.util.FormValidation;
import net.sf.json.JSONObject;
import org.acegisecurity.acls.sid.Sid;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.jenkinsci.plugins.matrixauth.inheritance.InheritParentStrategy;
import org.jenkinsci.plugins.matrixauth.inheritance.InheritanceStrategy;
Expand Down Expand Up @@ -96,8 +96,9 @@ public AuthorizationMatrixProperty(Map<Permission,? extends Set<String>> granted
this.grantedPermissions.put(e.getKey(),new HashSet<>(e.getValue()));
}

@Restricted(NoExternalUse.class)
public Set<String> getGroups() {
return sids;
return new HashSet<>(sids);
}

/**
Expand All @@ -124,10 +125,10 @@ public void add(Permission p, String sid) {
}

@Extension(optional = true)
public static class DescriptorImpl extends AbstractFolderPropertyDescriptor implements AuthorizationMatrixPropertyDescriptor<AuthorizationMatrixProperty> {
public static class DescriptorImpl extends AbstractFolderPropertyDescriptor implements AuthorizationPropertyDescriptor<AuthorizationMatrixProperty> {

@Override
public AuthorizationMatrixProperty createProperty() {
public AuthorizationMatrixProperty create() {
return new AuthorizationMatrixProperty();
}

Expand All @@ -147,7 +148,7 @@ public boolean isApplicable(Class<? extends AbstractFolder> folder) {
}

public FormValidation doCheckName(@AncestorInPath AbstractFolder<?> folder, @QueryParameter String value) {
return GlobalMatrixAuthorizationStrategy.DESCRIPTOR.doCheckName_(value, folder, Item.CONFIGURE);
return doCheckName_(value, folder, Item.CONFIGURE);
}
}

Expand Down Expand Up @@ -177,13 +178,14 @@ public InheritanceStrategy getInheritanceStrategy() {
* Persist {@link ProjectMatrixAuthorizationStrategy} as a list of IDs that
* represent {@link ProjectMatrixAuthorizationStrategy#grantedPermissions}.
*/
public static final class ConverterImpl extends AbstractMatrixPropertyConverter {
@Restricted(DoNotUse.class)
public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixProperty> {
public boolean canConvert(Class type) {
return type == AuthorizationMatrixProperty.class;
}

@Override
public AuthorizationProperty createSubject() {
public AuthorizationMatrixProperty create() {
return new AuthorizationMatrixProperty();
}
}
Expand Down
241 changes: 121 additions & 120 deletions src/main/java/hudson/security/AuthorizationMatrixProperty.java
Expand Up @@ -53,10 +53,11 @@
import org.acegisecurity.acls.sid.PrincipalSid;
import org.acegisecurity.acls.sid.Sid;
import org.jenkinsci.Symbol;
import org.jenkinsci.plugins.matrixauth.AbstractMatrixPropertyConverter;
import org.jenkinsci.plugins.matrixauth.AuthorizationMatrixPropertyDescriptor;
import org.jenkinsci.plugins.matrixauth.AbstractAuthorizationPropertyConverter;
import org.jenkinsci.plugins.matrixauth.AuthorizationPropertyDescriptor;
import org.jenkinsci.plugins.matrixauth.AuthorizationProperty;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.jenkinsci.plugins.matrixauth.inheritance.InheritParentStrategy;
import org.jenkinsci.plugins.matrixauth.inheritance.InheritanceStrategy;
Expand All @@ -74,33 +75,33 @@
*/
public class AuthorizationMatrixProperty extends JobProperty<Job<?, ?>> implements AuthorizationProperty {

private final transient SidACL acl = new AclImpl();
private final transient SidACL acl = new AclImpl();

/**
* List up all permissions that are granted.
*
* Strings are either the granted authority or the principal, which is not
* distinguished.
*/
private final Map<Permission, Set<String>> grantedPermissions = new HashMap<>();
/**
* List up all permissions that are granted.
* <p>
* Strings are either the granted authority or the principal, which is not
* distinguished.
*/
private final Map<Permission, Set<String>> grantedPermissions = new HashMap<>();

private final Set<String> sids = new HashSet<>();
private final Set<String> sids = new HashSet<>();

/**
* @deprecated unused, use {@link #setInheritanceStrategy(InheritanceStrategy)} instead.
*/
@Deprecated
/**
* @deprecated unused, use {@link #setInheritanceStrategy(InheritanceStrategy)} instead.
*/
@Deprecated
private transient Boolean blocksInheritance;

private InheritanceStrategy inheritanceStrategy = new InheritParentStrategy();
private InheritanceStrategy inheritanceStrategy = new InheritParentStrategy();

private AuthorizationMatrixProperty() {
}

public AuthorizationMatrixProperty(Map<Permission, Set<String>> grantedPermissions) {
// do a deep copy to be safe
for (Entry<Permission,Set<String>> e : grantedPermissions.entrySet())
this.grantedPermissions.put(e.getKey(),new HashSet<>(e.getValue()));
for (Entry<Permission, Set<String>> e : grantedPermissions.entrySet())
this.grantedPermissions.put(e.getKey(), new HashSet<>(e.getValue()));
}

@DataBoundConstructor
Expand All @@ -127,138 +128,138 @@ public List<String> getPermissions() {
return permissions;
}

public Set<String> getGroups() {
return sids;
}
public Set<String> getGroups() {
return new HashSet<>(sids);
}

/**
* Returns all the (Permission,sid) pairs that are granted, in the multi-map form.
*
* @return
* read-only. never null.
* @return read-only. never null.
*/
public Map<Permission,Set<String>> getGrantedPermissions() {
public Map<Permission, Set<String>> getGrantedPermissions() {
return Collections.unmodifiableMap(grantedPermissions);
}

/**
* Adds to {@link #grantedPermissions}. Use of this method should be limited
* during construction, as this object itself is considered immutable once
* populated.
*/
public void add(Permission p, String sid) {
Set<String> set = grantedPermissions.get(p);
if (set == null)
grantedPermissions.put(p, set = new HashSet<>());
set.add(sid);
sids.add(sid);
}
* Adds to {@link #grantedPermissions}. Use of this method should be limited
* during construction, as this object itself is considered immutable once
* populated.
*/
public void add(Permission p, String sid) {
Set<String> set = grantedPermissions.get(p);
if (set == null)
grantedPermissions.put(p, set = new HashSet<>());
set.add(sid);
sids.add(sid);
}

@Extension
@Symbol("authorizationMatrix")
public static class DescriptorImpl extends JobPropertyDescriptor implements AuthorizationMatrixPropertyDescriptor<AuthorizationMatrixProperty> {
public static class DescriptorImpl extends JobPropertyDescriptor implements AuthorizationPropertyDescriptor<AuthorizationMatrixProperty> {

@Override
public AuthorizationMatrixProperty createProperty() {
public AuthorizationMatrixProperty create() {
return new AuthorizationMatrixProperty();
}

@Override
public PermissionScope getPermissionScope() {
return PermissionScope.ITEM;
}
@Override
public JobProperty<?> newInstance(StaplerRequest req, JSONObject formData) throws FormException {

@Override
public JobProperty<?> newInstance(StaplerRequest req, JSONObject formData) throws FormException {
return createNewInstance(req, formData, true);
}
}

@Override
public boolean isApplicable(Class<? extends Job> jobType) {
@Override
public boolean isApplicable(Class<? extends Job> jobType) {
return isApplicable();
}
}

public FormValidation doCheckName(@AncestorInPath Job project, @QueryParameter String value) {
return GlobalMatrixAuthorizationStrategy.DESCRIPTOR.doCheckName_(value, project, Item.CONFIGURE);
return doCheckName_(value, project, Item.CONFIGURE);
}
}

private final class AclImpl extends SidACL {
@CheckForNull
@SuppressFBWarnings(value = "NP_BOOLEAN_RETURN_NULL",
justification = "As designed, implements a third state for the ternary logic")
protected Boolean hasPermission(Sid sid, Permission p) {
if (AuthorizationMatrixProperty.this.hasPermission(toString(sid),p,sid instanceof PrincipalSid)) {
return true;
}
return null;
}
}

public SidACL getACL() {
return acl;
}

@DataBoundSetter
public void setInheritanceStrategy(InheritanceStrategy inheritanceStrategy) {
this.inheritanceStrategy = inheritanceStrategy;
}

public InheritanceStrategy getInheritanceStrategy() {
return inheritanceStrategy;
}

/**
* Persist {@link ProjectMatrixAuthorizationStrategy} as a list of IDs that
* represent {@link ProjectMatrixAuthorizationStrategy#grantedPermissions}.
*/
public static final class ConverterImpl extends AbstractMatrixPropertyConverter {
public boolean canConvert(Class type) {
return type == AuthorizationMatrixProperty.class;
}

public AuthorizationProperty createSubject() {
return new AuthorizationMatrixProperty();
private final class AclImpl extends SidACL {
@CheckForNull
@SuppressFBWarnings(value = "NP_BOOLEAN_RETURN_NULL",
justification = "As designed, implements a third state for the ternary logic")
protected Boolean hasPermission(Sid sid, Permission p) {
if (AuthorizationMatrixProperty.this.hasPermission(toString(sid), p, sid instanceof PrincipalSid)) {
return true;
}
return null;
}
}

public SidACL getACL() {
return acl;
}

@DataBoundSetter
public void setInheritanceStrategy(InheritanceStrategy inheritanceStrategy) {
this.inheritanceStrategy = inheritanceStrategy;
}

public InheritanceStrategy getInheritanceStrategy() {
return inheritanceStrategy;
}

/**
* Persist {@link ProjectMatrixAuthorizationStrategy} as a list of IDs that
* represent {@link ProjectMatrixAuthorizationStrategy#grantedPermissions}.
*/
@Restricted(DoNotUse.class)
public static final class ConverterImpl extends AbstractAuthorizationPropertyConverter<AuthorizationMatrixProperty> {
public boolean canConvert(Class type) {
return type == AuthorizationMatrixProperty.class;
}

public AuthorizationMatrixProperty create() {
return new AuthorizationMatrixProperty();
}
}

/**
* Ensure that the user creating a job has Read and Configure permissions
*/
@Extension
@Restricted(NoExternalUse.class)
public static class ItemListenerImpl extends ItemListener {
@Override
public void onCreated(Item item) {
AuthorizationStrategy authorizationStrategy = Jenkins.getInstance().getAuthorizationStrategy();
if (authorizationStrategy instanceof ProjectMatrixAuthorizationStrategy) {
ProjectMatrixAuthorizationStrategy strategy = (ProjectMatrixAuthorizationStrategy) authorizationStrategy;

if (item instanceof Job) {
Job<?, ?> job = (Job<?, ?>) item;
AuthorizationMatrixProperty prop = job.getProperty(AuthorizationMatrixProperty.class);
if (prop == null) {
prop = new AuthorizationMatrixProperty();
}

User current = User.current();
String sid = current == null ? "anonymous" : current.getId();

if (!strategy.getACL(job).hasPermission(Jenkins.getAuthentication(), Item.READ)) {
prop.add(Item.READ, sid);
}
if (!strategy.getACL(job).hasPermission(Jenkins.getAuthentication(), Item.CONFIGURE)) {
prop.add(Item.CONFIGURE, sid);
}
if (prop.getGrantedPermissions().size() > 0) {
try {
job.addProperty(prop);
} catch (IOException ex) {
// TODO LOGGER
}
}
}
}
}
}
/**
* Ensure that the user creating a job has Read and Configure permissions
*/
@Extension
@Restricted(NoExternalUse.class)
public static class ItemListenerImpl extends ItemListener {
@Override
public void onCreated(Item item) {
AuthorizationStrategy authorizationStrategy = Jenkins.getInstance().getAuthorizationStrategy();
if (authorizationStrategy instanceof ProjectMatrixAuthorizationStrategy) {
ProjectMatrixAuthorizationStrategy strategy = (ProjectMatrixAuthorizationStrategy) authorizationStrategy;

if (item instanceof Job) {
Job<?, ?> job = (Job<?, ?>) item;
AuthorizationMatrixProperty prop = job.getProperty(AuthorizationMatrixProperty.class);
if (prop == null) {
prop = new AuthorizationMatrixProperty();
}

User current = User.current();
String sid = current == null ? "anonymous" : current.getId();

if (!strategy.getACL(job).hasPermission(Jenkins.getAuthentication(), Item.READ)) {
prop.add(Item.READ, sid);
}
if (!strategy.getACL(job).hasPermission(Jenkins.getAuthentication(), Item.CONFIGURE)) {
prop.add(Item.CONFIGURE, sid);
}
if (prop.getGrantedPermissions().size() > 0) {
try {
job.addProperty(prop);
} catch (IOException ex) {
// TODO LOGGER
}
}
}
}
}
}
}
Expand Up @@ -37,6 +37,12 @@
import java.util.Collections;
import java.util.List;

/**
* Administrative monitor that shows up when 'dangerous' permissions are granted to non-admin users.
* Those are permissions that could be used to grant themselves administer permissions.
*
* See also https://jenkins.io/security/advisory/2017-04-10/#matrix-authorization-strategy-plugin-allowed-configuring-dangerous-permissions
*/
@Extension
@Restricted(DoNotUse.class)
public class DangerousMatrixPermissionsAdministrativeMonitor extends AdministrativeMonitor {
Expand Down

0 comments on commit cfe4dd4

Please sign in to comment.