Merge "SecurityFix: enforce HTTP password checking on gitBasicAuth"

Documentation/config-gerrit.txt

@@ -422,8 +422,9 @@ By default this is set to false.
 [[auth.gitBasicAuth]]auth.gitBasicAuth::
 +
 If true then Git over HTTP and HTTP/S traffic is authenticated using
-standard BasicAuth and credentials validated using the same auth
+standard BasicAuth and credentials validated against the randomly
-method configured for Gerrit Web UI.
+generated HTTP password or against LDAP when it is configured as
+Gerrit Web UI authentication method.
 +
 This parameter only affects git over http traffic. If set to false
 then Gerrit will authenticate through DIGEST authentication and

Documentation/user-upload.txt

@@ -20,7 +20,8 @@ user must authenticate via HTTP/HTTPS.
 
 When link:config-gerrit.html#auth.gitBasicAuth[gitBasicAuth] is enabled,
 the user is authenticated using standard BasicAuth and credentials validated
-using the same authentication method configured for the Gerrit Web UI.
+using the randomly generated HTTP password on the `HTTP Password` tab
+in the user settings page or against LDAP when configured for the Gerrit Web UI.
 
 When gitBasicAuth is not configured, the user's HTTP credentials can be
 accessed within Gerrit by going to `Settings`, and then accessing the `HTTP

gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java

@@ -33,7 +33,7 @@ public class GerritConfig implements Cloneable {
   protected String httpPasswordUrl;
   protected String reportBugUrl;
   protected String reportBugText;
-  protected boolean gitBasicAuth;
+  protected boolean httpPasswordSettingsEnabled = true;
 
   protected GitwebConfig gitweb;
   protected boolean useContributorAgreements;
@@ -112,12 +112,12 @@ public class GerritConfig implements Cloneable {
     reportBugText = t;
   }
 
-  public boolean isGitBasicAuth() {
+  public boolean isHttpPasswordSettingsEnabled() {
-    return gitBasicAuth;
+    return httpPasswordSettingsEnabled;
   }
 
-  public void setGitBasicAuth(boolean gba) {
+  public void setHttpPasswordSettingsEnabled(boolean httpPasswordSettingsEnabled) {
-    gitBasicAuth = gba;
+    this.httpPasswordSettingsEnabled = httpPasswordSettingsEnabled;
   }
 
   public String getEditFullNameUrl() {

gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SettingsScreen.java

@@ -29,7 +29,7 @@ public abstract class SettingsScreen extends MenuScreen {
     if (Gerrit.getConfig().getSshdAddress() != null) {
       link(Util.C.tabSshKeys(), PageLinks.SETTINGS_SSHKEYS);
     }
-    if (!Gerrit.getConfig().isGitBasicAuth()) {
+    if (Gerrit.getConfig().isHttpPasswordSettingsEnabled()) {
       link(Util.C.tabHttpAccess(), PageLinks.SETTINGS_HTTP_PASSWORD);
     }
     link(Util.C.tabWebIdentities(), PageLinks.SETTINGS_WEBIDENT);

gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java

@@ -18,6 +18,7 @@ import com.google.gerrit.common.data.GerritConfig;
 import com.google.gerrit.common.data.GitwebConfig;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.AccountGeneralPreferences;
+import com.google.gerrit.reviewdb.client.AuthType;
 import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.config.AnonymousCowardName;
@@ -86,6 +87,7 @@ class GerritConfigProvider implements Provider<GerritConfig> {
         config.setRegisterUrl(cfg.getString("auth", null, "registerurl"));
         config.setRegisterText(cfg.getString("auth", null, "registertext"));
         config.setEditFullNameUrl(cfg.getString("auth", null, "editFullNameUrl"));
+        config.setHttpPasswordSettingsEnabled(!authConfig.isGitBasicAuth());
         break;
 
       case CUSTOM_EXTENSION:
@@ -134,8 +136,6 @@ class GerritConfigProvider implements Provider<GerritConfig> {
         reportBugUrl : "http://code.google.com/p/gerrit/issues/list");
     config.setReportBugText(cfg.getString("gerrit", null, "reportBugText"));
 
-    config.setGitBasicAuth(authConfig.isGitBasicAuth());
-
     final Set<Account.FieldName> fields = new HashSet<Account.FieldName>();
     for (final Account.FieldName n : Account.FieldName.values()) {
       if (realm.allowsEdit(n)) {

gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java

@@ -137,6 +137,14 @@ class ProjectBasicAuthFilter implements Filter {
       return false;
     }
 
+    if (!authConfig.isLdapAuthType()
+        && !passwordMatchesTheUserGeneratedOne(who, username, password)) {
+      log.warn("Authentication failed for " + username
+          + ": password does not match the one stored in Gerrit");
+      rsp.sendError(SC_UNAUTHORIZED);
+      return false;
+    }
+
     AuthRequest whoAuth = AuthRequest.forUser(username);
     whoAuth.setPassword(password);
 
@@ -154,6 +162,13 @@ class ProjectBasicAuthFilter implements Filter {
     }
   }
 
+  private boolean passwordMatchesTheUserGeneratedOne(AccountState who,
+      String username, String password) {
+    String accountPassword = who.getPassword(username);
+    return accountPassword != null && password != null
+        && accountPassword.equals(password);
+  }
+
   private String encoding(HttpServletRequest req) {
     return Objects.firstNonNull(req.getCharacterEncoding(), "UTF-8");
   }

gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java

@@ -277,4 +277,9 @@ public class AuthConfig {
   public String getRegisterPageUrl() {
     return registerPageUrl;
   }
+
+  public boolean isLdapAuthType() {
+    return authType == AuthType.LDAP ||
+        authType == AuthType.LDAP_BIND;
+  }
 }