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
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 27c046f..1a8ca54 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -422,8 +422,9 @@
[[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
-method configured for Gerrit Web UI.
+standard BasicAuth and credentials validated against the randomly
+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
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 17ca968..b892c4a 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -20,7 +20,8 @@
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
diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java
index 38afaab..e33989b 100644
--- a/gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java
+++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/GerritConfig.java
@@ -33,7 +33,7 @@
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 @@
reportBugText = t;
}
- public boolean isGitBasicAuth() {
- return gitBasicAuth;
+ public boolean isHttpPasswordSettingsEnabled() {
+ return httpPasswordSettingsEnabled;
}
- public void setGitBasicAuth(boolean gba) {
- gitBasicAuth = gba;
+ public void setHttpPasswordSettingsEnabled(boolean httpPasswordSettingsEnabled) {
+ this.httpPasswordSettingsEnabled = httpPasswordSettingsEnabled;
}
public String getEditFullNameUrl() {
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SettingsScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SettingsScreen.java
index 2c29ab2..c689b49 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SettingsScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/SettingsScreen.java
@@ -29,7 +29,7 @@
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);
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java
index 7f6a2df..a37bea6 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/GerritConfigProvider.java
@@ -18,6 +18,7 @@
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 @@
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 @@
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)) {
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index 763075e..d299f54 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -137,6 +137,14 @@
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 @@
}
}
+ 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");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java
index 6d01e7c5..a633acd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/AuthConfig.java
@@ -277,4 +277,9 @@
public String getRegisterPageUrl() {
return registerPageUrl;
}
+
+ public boolean isLdapAuthType() {
+ return authType == AuthType.LDAP ||
+ authType == AuthType.LDAP_BIND;
+ }
}
