diff --git a/ReleaseNotes/ReleaseNotes-2.10.txt b/ReleaseNotes/ReleaseNotes-2.10.txt index f356cd3d19..10eef4b2be 100644 --- a/ReleaseNotes/ReleaseNotes-2.10.txt +++ b/ReleaseNotes/ReleaseNotes-2.10.txt @@ -442,6 +442,23 @@ explicitly named on the command line (which may be empty) should be notified of the change. Users watching the project should not be notified, as the change has not yet been published. +* Fix resource exhaustion due to unclosed LDAP connection. ++ +When `auth.type` is set to `LDAP` (not `LDAP_BIND`), two LDAP connections are +made, but one was not being closed. This eventually caused resource exhaustion +and LDAP authentications failed. + +Access Permissions +~~~~~~~~~~~~~~~~~~ + +* link:https://code.google.com/p/gerrit/issues/detail?id=2995[Issue 2995]: +Fix faulty behaviour in `BLOCK` permission. ++ +`BLOCK` can be overruled with `ALLOW` on the same project, however there was a +bug when a child of the above project duplicates the `ALLOW` permission. In this +case the `BLOCK` would always win for the child, even though the `BLOCK` was +overruled in the parent. + Web UI ~~~~~~ diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java index cf68a8bc39..eb6249c3bf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapAuthBackend.java @@ -89,7 +89,7 @@ public class LdapAuthBackend implements AuthBackend { // We found the user account, but we need to verify // the password matches it before we can continue. // - helper.authenticate(m.getDN(), req.getPassword()); + helper.authenticate(m.getDN(), req.getPassword()).close(); } return new AuthUser(new AuthUser.UUID(username), username); } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java index 66da633598..2e216d4a57 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapRealm.java @@ -221,7 +221,7 @@ public class LdapRealm extends AbstractRealm { // We found the user account, but we need to verify // the password matches it before we can continue. // - helper.authenticate(m.getDN(), who.getPassword()); + helper.authenticate(m.getDN(), who.getPassword()).close(); } who.setDisplayName(apply(schema.accountFullName, m)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java index 53fc8d207a..e7a6ff9d79 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/PermissionCollection.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.project; +import static com.google.common.base.Objects.firstNonNull; import static com.google.gerrit.server.project.RefControl.isRE; import com.google.common.collect.Lists; @@ -117,6 +118,7 @@ public class PermissionCollection { Set exclusiveGroupPermissions = new HashSet<>(); HashMap> permissions = new HashMap<>(); + HashMap> overridden = new HashMap<>(); Map ruleProps = Maps.newIdentityHashMap(); for (AccessSection section : sections) { Project.NameKey project = sectionToProject.get(section); @@ -132,11 +134,19 @@ public class PermissionCollection { } else { addRule = seen.add(s) && !rule.isDeny() && !exclusivePermissionExists; } + + HashMap> p = null; if (addRule) { - List r = permissions.get(permission.getName()); + p = permissions; + } else if (!rule.isDeny() && !exclusivePermissionExists) { + p = overridden; + } + + if (p != null) { + List r = p.get(permission.getName()); if (r == null) { r = new ArrayList<>(2); - permissions.put(permission.getName(), r); + p.put(permission.getName(), r); } r.add(rule); ruleProps.put(rule, new ProjectRef(project, section.getName())); @@ -149,18 +159,22 @@ public class PermissionCollection { } } - return new PermissionCollection(permissions, ruleProps, perUser); + return new PermissionCollection(permissions, overridden, ruleProps, + perUser); } } private final Map> rules; + private final Map> overridden; private final Map ruleProps; private final boolean perUser; private PermissionCollection(Map> rules, + Map> overridden, Map ruleProps, boolean perUser) { this.rules = rules; + this.overridden = overridden; this.ruleProps = ruleProps; this.perUser = perUser; } @@ -186,6 +200,11 @@ public class PermissionCollection { return r != null ? r : Collections. emptyList(); } + List getOverridden(String permissionName) { + return firstNonNull( + overridden.get(permissionName), Collections. emptyList()); + } + ProjectRef getRuleProps(PermissionRule rule) { return ruleProps.get(rule); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java index 669afc6b88..ea3cbe65e3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java @@ -127,6 +127,7 @@ public class RefControl { */ public boolean isVisibleByRegisteredUsers() { List access = relevant.getPermission(Permission.READ); + List overridden = relevant.getOverridden(Permission.READ); Set allows = Sets.newHashSet(); Set blocks = Sets.newHashSet(); for (PermissionRule rule : access) { @@ -136,6 +137,11 @@ public class RefControl { allows.add(relevant.getRuleProps(rule)); } } + for (PermissionRule rule : overridden) { + if (SystemGroupBackend.isAnonymousOrRegistered(rule.getGroup())) { + blocks.remove(relevant.getRuleProps(rule)); + } + } blocks.removeAll(allows); return blocks.isEmpty() && !allows.isEmpty(); } @@ -541,6 +547,7 @@ public class RefControl { private boolean doCanPerform(String permissionName, boolean blockOnly) { List access = access(permissionName); + List overridden = relevant.getOverridden(permissionName); Set allows = Sets.newHashSet(); Set blocks = Sets.newHashSet(); for (PermissionRule rule : access) { @@ -550,6 +557,9 @@ public class RefControl { allows.add(relevant.getRuleProps(rule)); } } + for (PermissionRule rule : overridden) { + blocks.remove(relevant.getRuleProps(rule)); + } blocks.removeAll(allows); return blocks.isEmpty() && (!allows.isEmpty() || blockOnly); } @@ -557,6 +567,7 @@ public class RefControl { /** True if the user has force this permission. Works only for non labels. */ private boolean canForcePerform(String permissionName) { List access = access(permissionName); + List overridden = relevant.getOverridden(permissionName); Set allows = Sets.newHashSet(); Set blocks = Sets.newHashSet(); for (PermissionRule rule : access) { @@ -566,6 +577,11 @@ public class RefControl { allows.add(relevant.getRuleProps(rule)); } } + for (PermissionRule rule : overridden) { + if (rule.getForce()) { + blocks.remove(relevant.getRuleProps(rule)); + } + } blocks.removeAll(allows); return blocks.isEmpty() && !allows.isEmpty(); } @@ -573,6 +589,7 @@ public class RefControl { /** True if for this permission force is blocked for the user. Works only for non labels. */ private boolean isForceBlocked(String permissionName) { List access = access(permissionName); + List overridden = relevant.getOverridden(permissionName); Set allows = Sets.newHashSet(); Set blocks = Sets.newHashSet(); for (PermissionRule rule : access) { @@ -582,6 +599,11 @@ public class RefControl { allows.add(relevant.getRuleProps(rule)); } } + for (PermissionRule rule : overridden) { + if (rule.getForce()) { + blocks.remove(relevant.getRuleProps(rule)); + } + } blocks.removeAll(allows); return !blocks.isEmpty(); } diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java index 6f159fccca..4c123fde22 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java @@ -370,6 +370,17 @@ public class RefControlTest { assertFalse("u can't vote 2", range.contains(2)); } + @Test + public void testInheritSubmit_AllowInChildDoesntAffectUnblockInParent() { + block(parent, SUBMIT, ANONYMOUS_USERS, "refs/heads/*"); + allow(parent, SUBMIT, REGISTERED_USERS, "refs/heads/*"); + allow(local, SUBMIT, REGISTERED_USERS, "refs/heads/*"); + + ProjectControl u = util.user(local); + assertFalse("not blocked from submitting", u.controlForRef( + "refs/heads/master").isBlocked(SUBMIT)); + } + @Test public void testUnblockNoForce() { block(local, PUSH, ANONYMOUS_USERS, "refs/heads/*");