opendev/gerrit
Code Issues Proposed changes

Remove the generateHttpPassword capability

Browse Source 
Remove the Generate HTTP Password capability because it exposes a
security vulnerability.  Any user that is granted this capability
can modify an administrator's http password and impersonate the
admin user.  Other reasons for removing this capability are that
the usage of it is inconsistent with the modifyAccount capability
and this capability encourages adding additional capabilities to
restrict permissions, which is not desired.

With this change only administrators are allowed to generate and
delete other users' http passwords.

The motivation behind this change is from comments in changes
Ib1971fad and If8296539.

Change-Id: Id907cc103591eed029fd08af700bb1bb6a618ff8
This commit is contained in:
Khai Do 2014-09-25 13:59:28 -07:00
parent c563e98d9b
commit cf9bce2191
9 changed files with 5 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
Documentation/access-control.txt
View File

@ -1205,13 +1205,6 @@ This capability doesn't imply permissions to the show-caches command. For that
you need the <<capability_viewCaches,view caches capability>>.
[[capability_generateHttpPassword]]
=== Generate HTTP Password
Allow the user to generate HTTP passwords for other users. Typically this would
be assigned to a non-interactive users group.
[[capability_kill]]
=== Kill Task

4
Documentation/cmd-set-account.txt
View File

@ -29,9 +29,7 @@ link:access-control.html#capability_modifyAccount[the 'Modify Account' global ca
To set the HTTP password for the user account (option --http-password) or
to clear the HTTP password (option --clear-http-password) caller must be
a member of the privileged 'Administrators' group, or have been granted
link:access-control.html#capability_generateHttpPassword[the 'Generate HTTP Password' global capability]
in addition to 'Modify Account' global capability.
a member of the privileged 'Administrators' group.
== SCRIPTING
This command is intended to be used in scripts.

4
gerrit-common/src/main/java/com/google/gerrit/common/data/GlobalCapability.java
View File

@ -61,9 +61,6 @@ public class GlobalCapability {
/** Can flush any cache except the active web_sessions cache. */
public static final String FLUSH_CACHES = "flushCaches";
/** Can generate HTTP passwords for user other than self. */
public static final String GENERATE_HTTP_PASSWORD = "generateHttpPassword";
/** Can terminate any task using the kill command. */
public static final String KILL_TASK = "killTask";
@ -112,7 +109,6 @@ public class GlobalCapability {
NAMES_ALL.add(CREATE_PROJECT);
NAMES_ALL.add(EMAIL_REVIEWERS);
NAMES_ALL.add(FLUSH_CACHES);
NAMES_ALL.add(GENERATE_HTTP_PASSWORD);
NAMES_ALL.add(KILL_TASK);
NAMES_ALL.add(MODIFY_ACCOUNT);
NAMES_ALL.add(PRIORITY);

6
gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java
View File

@ -169,12 +169,6 @@ public class CapabilityControl {
|| canAdministrateServer();
}
/** @return true if the user can generate HTTP passwords for users other than self. */
public boolean canGenerateHttpPassword() {
return canPerform(GlobalCapability.GENERATE_HTTP_PASSWORD)
|| canAdministrateServer();
}
/** @return true if the user can impersonate another user. */
public boolean canRunAs() {
return canPerform(GlobalCapability.RUN_AS);

2
gerrit-server/src/main/java/com/google/gerrit/server/account/GetCapabilities.java
View File

@ -20,7 +20,6 @@ import static com.google.gerrit.common.data.GlobalCapability.CREATE_GROUP;
import static com.google.gerrit.common.data.GlobalCapability.CREATE_PROJECT;
import static com.google.gerrit.common.data.GlobalCapability.EMAIL_REVIEWERS;
import static com.google.gerrit.common.data.GlobalCapability.FLUSH_CACHES;
import static com.google.gerrit.common.data.GlobalCapability.GENERATE_HTTP_PASSWORD;
import static com.google.gerrit.common.data.GlobalCapability.KILL_TASK;
import static com.google.gerrit.common.data.GlobalCapability.MODIFY_ACCOUNT;
import static com.google.gerrit.common.data.GlobalCapability.PRIORITY;
@ -115,7 +114,6 @@ class GetCapabilities implements RestReadView<AccountResource> {
have.put(CREATE_PROJECT, cc.canCreateProject());
have.put(EMAIL_REVIEWERS, cc.canEmailReviewers());
have.put(FLUSH_CACHES, cc.canFlushCaches());
have.put(GENERATE_HTTP_PASSWORD, cc.canGenerateHttpPassword());
have.put(KILL_TASK, cc.canKillTask());
have.put(MODIFY_ACCOUNT, cc.canModifyAccount());
have.put(RUN_GC, cc.canRunGC());

2
gerrit-server/src/main/java/com/google/gerrit/server/account/GetHttpPassword.java
View File

@ -36,7 +36,7 @@ public class GetHttpPassword implements RestReadView<AccountResource> {
public String apply(AccountResource rsrc) throws AuthException,
ResourceNotFoundException {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canGenerateHttpPassword()) {
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to get http password");
}
AccountState s = rsrc.getUser().state();

6
gerrit-server/src/main/java/com/google/gerrit/server/account/PutHttpPassword.java
View File

@ -79,19 +79,19 @@ public class PutHttpPassword implements RestModifyView<AccountResource, Input> {
String newPassword;
if (input.generate) {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canGenerateHttpPassword()) {
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to generate HTTP password");
}
newPassword = generate();
} else if (input.httpPassword == null) {
if (self.get() != rsrc.getUser()
&& !self.get().getCapabilities().canGenerateHttpPassword()) {
&& !self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to clear HTTP password");
}
newPassword = null;
} else {
if (!self.get().getCapabilities().canGenerateHttpPassword()) {
if (!self.get().getCapabilities().canAdministrateServer()) {
throw new AuthException("not allowed to set HTTP password directly, "
+ "requires the Generate HTTP Password permission");
}

1
gerrit-server/src/main/java/com/google/gerrit/server/config/CapabilityConstants.java
View File

@ -29,7 +29,6 @@ public class CapabilityConstants extends TranslationBundle {
public String createProject;
public String emailReviewers;
public String flushCaches;
public String generateHttpPassword;
public String killTask;
public String modifyAccount;
public String priority;

1
gerrit-server/src/main/resources/com/google/gerrit/server/config/CapabilityConstants.properties
View File

@ -5,7 +5,6 @@ createGroup = Create Group
createProject = Create Project
emailReviewers = Email Reviewers
flushCaches = Flush Caches
generateHttpPassword = Generate HTTP Password
killTask = Kill Task
modifyAccount = Modify Account
priority = Priority