SecurityFix: enforce HTTP password checking on gitBasicAuth

When using auth.gitBasicAuth option in gerrit.config, the git
over HTTP authentication was using *ONLY* the Gerrit realm
for validating the user's credentials.

However only the LDAP realm did a real full checking of the
user's password, whilst with other realms (i.e. HTTP and HTTP_LDAP)
there was only a check on the existence of the user's account.

This created a potential security hole when a non-LDAP auth realm
was used in conjunction with gitBasicAuth.

The fix is to check, for non-LDAP authentication realms, the
HTTP Password in the Gerrit external access ids and deny
unauthenticated access if password do not match or has not
been generated in the user's settings HTTP tab.

Change-Id: I620eb780e6d77b45f6bc8a3af42f8b7b1caf821d

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;
+  }
 }