I’ve been blogging about some of the security fixes we’ve made in the CodeRanch fork of JForum such as XSS with quotes and CSRF. Today it is time to write about Clickjacking and preventing brute force logins.
Clickjacking
Clickjacking is an attack where someone includes your site in transparent frames and the attacker intercepts anything typed in/clicked. We had originally decided not to do anything about ClickJacking because we want our site to be available in frames. For example, DZone serves the main page in a frame when asking people to vote on it.
However, that doesn’t mean that we shouldn’t protect anything. We decided to protect the most critical parts of the site – the admin screens and the login form. That leaves the read only pages (which we don’t mind framed) and editing posts/user info (which is displayed publicly anyway).
The implementation turned out to be simple. We added this header to the relevant pages. (the admin screens are always within a frame on the site and the login screen is when an admin’s session times out).
response.setHeader("X-Frame-Options", "SAMEORIGIN");
It turned out the method only needed to be called from two places: UserAction (for login) and the AdminCommand (for the admin console).
While this isn’t complete protection, it is certainly better than the nothing we had before. And it is easy to add more screens in the future if we need to. We also didn’t include the JavaScript framebusting logic for older browsers because Google Analytics says hardly of our users are on such browsers.
Preventing Brute Force Logins
Another security issue that we knew about but didn’t fix because “it wasn’t a problem” was preventing brute force logins. It seemed like high time to do something about that one too. While this was more work than fixing Clickjacking, it wasn’t a huge deal either. The logic is mostly in one (well unit tested) class that gets called from the method with the login check. There is also a property file to externalize the number of attempts/wait. And a job to clear out the Map once a day for attempts where the IP gives up and goes away.
The idea is that an IP gets a number of “free” attempts. After that more login attempts from that IP must wait longer and longer. Once a user logs in successfully from that IP or there are no attempts for a certain amount of time,t he clock starts over and the “free” attempts are back.
This approach doesn’t protect against a distributed attack because the IPs are different. (I choose the IP based approach so someone can’t lock out a user’s account specifically by targeting them and entering invalid passwords. That said, we aren’t running a bank so it seems unlikely for someone to go through that much effort to attack us.
Note: I got a private comment to be careful about IP blocking due to companies using a single public IP for thousands of users. This approach is NOT using IP blocking. Let’s say have 1000 users from InfoSys. (We probably have more, but that’s not the point.) Most of those users will either know their password or use the “remember password” functionality. If a few forget their password and type in a bunch of wrong attempts, the time throttling kicks in. It seems unlikely for multiple users to having this problem at the same time. And as soon as anyone from the company (IP) has a correct password, the “penalty” count resets.
Code for the curious:
package net.jforum.repository;
import java.util.*;
import net.jforum.util.preferences.*;
/**
* We prevent brute force login attacks by throttling how often we will check
* credential after three failed logins from the same IP since the last success.
* If there are no failures within an hour, the clock will restart on this
* throttling.
*
* @author Jeanne
* @version $Id: $
*/
public class FailedLoginRepository {
private static final int NUM_SECONDS_IN_MINUTE = 1000 * 60;
private static FailedLoginRepository cache = new FailedLoginRepository();
private Map<String, List<Date>> failedLogins;
private int maxLoginsBeforeThrottling;
private int numMinutesForReset;
private int[] throttlingDelay;
// ----------------------------------------------------------------
public FailedLoginRepository() {
failedLogins = new HashMap<String, List<Date>>();
numMinutesForReset = SystemGlobals.getIntValue(ConfigKeys.BRUTE_FORCE_PREVENTION_NUM_MINUTES_FOR_RESET);
String waitTimes = SystemGlobals.getValue(ConfigKeys.BRUTE_FORCE_PREVENTION_WAIT_TIMES);
setWaitsAsIntArray(waitTimes);
setMaxLoginsBeforeThrottling();
}
private void setMaxLoginsBeforeThrottling() {
for (int i = 0; i < throttlingDelay.length; i++) {
if (throttlingDelay[i] == 0) {
maxLoginsBeforeThrottling++;
}
}
}
private void setWaitsAsIntArray(String waitTimes) {
String[] stringArray = waitTimes.split(",");
throttlingDelay = new int[stringArray.length];
for (int i = 0; i < throttlingDelay.length; i++) {
throttlingDelay[i] = Integer.parseInt(stringArray[i].trim());
}
}
// ----------------------------------------------------------------
public static FailedLoginRepository getInstance() {
return cache;
}
// ----------------------------------------------------------------
/**
* After a successful login from the API, remove the blocks. This indicates
* the user just mistyped and not a hacker.
*
* @param ip
*/
public void successfulAttempt(String ip) {
failedLogins.remove(ip);
}
// ----------------------------------------------------------------
/**
* Keep track of time of login failure
*
* @param ip
* @param now
*/
public void failedAttempt(String ip, Date now) {
removeOldAttempts(ip);
List<Date> fails = failedLogins.get(ip);
if (fails == null) {
fails = new ArrayList<Date>();
failedLogins.put(ip, fails);
}
fails.add(now);
}
public void failedAttempt(String ip) {
failedAttempt(ip, new Date());
}
// ----------------------------------------------------------------
/**
* If there haven't been any failed attempts from that IP in the last hour,
* start over.
*
* @param ip
*/
private void removeOldAttempts(String ip) {
List<Date> fails = failedLogins.get(ip);
if (fails != null) {
Date lastFail = getLastFail(fails);
Calendar cal = Calendar.getInstance();
cal.add(Calendar.MINUTE, -numMinutesForReset);
if (lastFail.before(cal.getTime())) {
failedLogins.remove(ip);
}
}
}
private Date getLastFail(List<Date> fails) {
Date lastFail = fails.get(fails.size() - 1);
return lastFail;
}
public void removeAllOldAttempts() {
for (String ip : failedLogins.keySet()) {
removeOldAttempts(ip);
}
}
// ----------------------------------------------------------------
public boolean shouldCheckPassword(String ip) {
removeOldAttempts(ip);
int waitRemaining = numberMinutesUntilNextAllowedLogin(ip, new Date());
return waitRemaining == 0;
}
public boolean shouldCheckPassword(String ip, Date now) {
removeOldAttempts(ip);
List<Date> fails = failedLogins.get(ip);
return fails == null || fails.size() <= maxLoginsBeforeThrottling;
}
// ----------------------------------------------------------------
public int numberMinutesUntilNextAllowedLogin(String ip) {
return numberMinutesUntilNextAllowedLogin(ip, new Date());
}
/**
* Contains algorithm for delays: see if already waited number of minutes
* need to wai
*
* @param ip
* @return
*/
public int numberMinutesUntilNextAllowedLogin(String ip, Date now) {
List<Date> fails = failedLogins.get(ip);
if (fails == null) {
return 0;
}
int index = fails.size() - 1;
if (index >= throttlingDelay.length) {
index = throttlingDelay.length - 1;
}
int delay = throttlingDelay[index];
Date lastFailure = getLastFail(fails);
int numMinutesWaited = (int) (now.getTime() - lastFailure.getTime()) / NUM_SECONDS_IN_MINUTE;
int result = delay - numMinutesWaited;
// if delay is over, do need to wait longer
if (result < 0) {
result = 0;
}
return result;
}
}
