Password Validation Practices

Posted on

It is usually quite rare for me to stumble upon some really bad code, but to stumble upon some really stupid code… well that happens almost everyday!

So while auditing someone else’s source code at work I found something very odd in the utility that validates passwords against our policy. We have a standard password policy that only allows passwords to be at least 8 characters in length, 1 upper case, 1 lower case, 1 digit or 1 special character and no spaces. The code that is used to validate this policy is what really blew my mind. I mean, it’s not that it does not work, it does. It’s that I could not comprehend how the code was brought to life by a "senior" developer with a university degree in computer engineering.

The source code is in Java so there are many options to use like regular expressions. So the the function to validate a password against the password policy would look something like this:

/**
 * <p>Standard Password Policy
 * <ul>
 * 	<li>Minimum 8 characters</li>
 * 	<li>At least 1 upper case character</li>
 * 	<li>At least 1 lower case character</li>
 * 	<li>At least 1 digit or 1 special character</li>
 * 	<li>No spaces allowed</li>
 * 	<li>List of special characters allowed are (!@#$%^&+=.,_)</li>
 * </ul>
 * </p>
 */
private static final String PASSWORD_REGEX = "^(?=.*[a-z])(?=.*[A-Z])((?=.*[!@#$%^&+=.,_])|(?=.*[0-9]))(?=[^ ]+$).{8,}$";
private static final Pattern passPattern = Pattern.compile(PASSWORD_REGEX);
	
private boolean validatePasswordRegex(String password)
{
	return passPattern.matcher(password).matches();
}

It’s lean, clean and simple. Then I started to think that what if someone did not know anything about regular expressions (which if they did not they should have found out about by googling "password validation") So I felt compelled to re-write a function that validates the password without using regular expressions:

private boolean validatePasswordFast(String password)
{
	boolean hasUpper = false;
	boolean hasLower = false;
	boolean hasSpecial = false;
	boolean hasDigit = false;
	boolean valid = false;
		
	if(password.length() > 7) {
		char[] cPassword = password.toCharArray();
		for(int i = 0; i < cPassword.length; i++) {
			int c = cPassword[i];
				
			if(c == 32) {
				valid = false;
				break;
			}
				
			if(!hasUpper) {
				if(c >= 65 && c <= 90) {
					hasUpper = true;
				}
			}
				
			if(!hasLower) {
				if(c >= 97 && c <= 122) {
					hasLower = true;
				}
			}
				
			if(!hasDigit) {
				if(c >= 48 && c <= 57) {
					hasDigit = true;
				}
			}
				
			if(!hasSpecial) {
				switch(c)
				{
					case 33:
					case 35:
					case 36:
					case 37:
					case 38:
					case 43:
					case 44:
					case 46:
					case 61:
					case 94:
					case 95:
					{
						hasSpecial = true;
						break;
					}
				}
			}
				
			if(hasUpper && hasLower && (hasSpecial || hasDigit)) {
				valid = true;
				break;
			}
		}
	}
	
	return valid;
}

The above is relatively longer than the regular expression function. Knowing that each character in a String has an ordinal number and knowing of the ASCII table it was intuitive on how apply the password policy. Also I tried to make it as efficient as possible so that there are no wasted iterations.

But, dear readers, what I found was worse. I saw something so wasteful and so … "un-computer science like" that it baffled me:

private boolean validatePasswordNotSmart(String password)
{
	boolean passwdLength =true;
	boolean passwdSpace = true;
	boolean passwdSpecialChar = false;
	boolean passwdLowerCase = false;
	boolean passwdUpperCase = false;
	boolean passwdDigit = false;
		
	if(password == null || password.length() == 0) {
		return false;
	}
		
	if(password.length() < 8) {
		passwdLength = false;
	}
			
	for(int i = 0; i < password.length(); i++) {
		if (password.contains(" ")) {
			passwdSpace = false;
			break;
		}
			
		if(password.contains("$")
		   || password.contains("+")
		   || password.contains("=")
		   || password.contains("!")
		   || password.contains("#")
		   || password.contains("%")
		   || password.contains("&")
		   || password.contains("^")
		   || password.contains("@")
		   || password.contains(".")
		   || password.contains(",")
		   || password.contains("_")) {
			passwdSpecialChar = true;
		}
			
		if(password.contains("a")
		   || password.contains("b")
		   || password.contains("c")
		   || password.contains("d")
		   || password.contains("e")
		   || password.contains("f")
		   || password.contains("g")
		   || password.contains("h")
		   || password.contains("i")
		   || password.contains("j")
		   || password.contains("k")
		   || password.contains("l")
		   || password.contains("m")
		   || password.contains("n")
		   || password.contains("o")
		   || password.contains("p")
		   || password.contains("q")
		   || password.contains("r")
		   || password.contains("s")
		   || password.contains("t")
		   || password.contains("u")
		   || password.contains("v")
		   || password.contains("w")
		   || password.contains("x")
		   || password.contains("y")
		   || password.contains("z")) {
			passwdLowerCase = true;
		}
			
		if(password.contains("A")
		   || password.contains("B")
		   || password.contains("C")
		   || password.contains("D")
		   || password.contains("E")
		   || password.contains("F")
		   || password.contains("G")
		   || password.contains("H")
		   || password.contains("I")
		   || password.contains("J")
		   || password.contains("K")
		   || password.contains("L")
		   || password.contains("M")
		   || password.contains("N")
		   || password.contains("O")
		   || password.contains("P")
		   || password.contains("Q")
		   || password.contains("R")
		   || password.contains("S")
		   || password.contains("T")
		   || password.contains("U")
		   || password.contains("V")
		   || password.contains("W")
		   || password.contains("X")
		   || password.contains("Y")
		   || password.contains("Z")) {
			passwdUpperCase = true;
		}
			
		if(password.contains("0")
		   || password.contains("1")
		   || password.contains("2")
		   || password.contains("3")
		   || password.contains("4")
		   || password.contains("5")
		   || password.contains("6")
		   || password.contains("7")
		   || password.contains("8")
		   || password.contains("9")) {
			passwdDigit = true;
		}
	}

	return passwdLength && passwdSpace && (passwdSpecialChar || passwdDigit) && passwdLowerCase && passwdUpperCase;
}

How does one go through all the trouble to write that function and roll it out into production believing that what they did was good? The problems with this are that

  • Long to write
  • Uses many internal methods that affect performance
  • Shows the lack of research into the topic
  • Waste of time

I even tried bench-marking the three functions to see which was fastest:

Password to benchmark: ThisIs_A_validPassword123!
Iterations: 500,000
Validate Password Not Smart: 5491 ms
Validate Password Fast: 63 ms
Validate Password Regex: 2340 ms

Go for another round...

Password to benchmark: ThisIs_A_validPassword123!
Iterations: 500,000
Validate Password Not Smart: 5461 ms
Validate Password Fast: 48 ms
Validate Password Regex: 2356 ms

The non-regex but fast function triumphed at less than 100ms to validate a password 500,000 times. Second place was the regex function averaging at 2,300ms and last place was the not smart function which took a whopping 5,400ms.

The advantages of the regex function is that it is short to write and easy to read, but you have to have intermediate knowledge of regular expressions. The fast function is fast, but it’s a bit long and you have to scan the code a bit to figure out what it is doing. Not to leave anything out, the not smart function is the easiest to read, but sadly causing you to come to the conclusion that this is the wrong way to code it.

Advertisements

2 thoughts on “Password Validation Practices

    zuzZ said:
    October 25, 2012 at 11:30 am

    How can a developer with such BAD coding practices gain a “Senior Developer” title?! :/

      geodma responded:
      November 4, 2012 at 2:00 am

      I does makes us wonder doesn’t it 😦

Leave a Comment

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s