Add a new rule for repetitive or sequential characters.

[DR-DinoMight]

As the title says, do you think it's a good idea to add a new rule that checks against the NIST guideline of

Repetitive or sequential characters (e.g. ‘aaaaaa’, ‘1234abcd’).

Source: https://pages.nist.gov/800-63-3/sp800-63b.html#sec5

[DivineOmega]

Definitely a good idea. :+1:

[DR-DinoMight]

I'm happy to take a stab at this, I already have an idea of how to do the repetitive characters.

The only current struggle I'll have is how to do the check for sequential characters.

[DivineOmega]

Sounds good!

It might be worth having a generator that builds a text file containing a list of repetitive and sequential character sequences, which can then be checked against.

[AlexCatch]

Would a regex be a good candidate here? https://stackoverflow.com/questions/7392717/checking-for-repeated-letters-in-php

[DR-DinoMight]

Yeah, I agree, I already have a regex pattern to figure out repeating characters.

I just don't see how to sequential characters and numbers could be caught with a regex.

[AlexCatch]

The solution @DivineOmega proposed is a possiblity, if you want to avoid reading from the filesystem you could whip up an algorithm to check the string for sequential characters and numbers without too much strain I think.

[DivineOmega]

Something similar to the following will give you a reasonable list of sequential character sequences. We can then simply check if the provided password string contains any of these sequences.

$sequentialCharSequences = [];

for ($start = 48; $start <= 120; $start++) {
    $sequence = '';
    for ($charCode = $start; $charCode < $start+3; $charCode++) {
        $sequence .= chr($charCode);
    }
    $sequentialCharSequences[] = $sequence;
}

[DR-DinoMight]

Brilliant, set me on the right track! The only problem I can see with above is that symbols and such will be included in the check, and I think personally that symbols/special characters shouldn't be included in this check as if this occurs check fails for an 'everyday' user are they going to truly understand why it happens?

I've altered (if you can call it that, I like to say butchered 😆) your above code into the following

  $sequentialCharSequences = [];

  // number check
  for ($numericStart = 0; $numericStart <= 9; $numericStart++) {
      $sequence = '';
      for ($number = $numericStart; $number < $numericStart+3; $number++) {
          $sequence .= $number;
      }
      $sequentialCharSequences[] = $sequence;
  }

  // Character check
  $containOnlyCharacters = true;
  for ($alphaStart = 97; $alphaStart <= 122; $alphaStart++) {
      $sequence = '';
      for ($charCode = $alphaStart; $charCode < $alphaStart+3; $charCode++) {
          if($charCode > 122){
            $containOnlyCharacters = false;
            break;
          }
          $sequence .= chr($charCode);
      }
      if(!$containOnlyCharacters) break;
      $sequentialCharSequences[] = $sequence;
  }

This should cover everything if we convert the password into lowercase before validating.

[DivineOmega]

I definitely agree with not including symbols in the sequential character sequences.

What do you think of the following implementation? It's a little simpler.

$sequentialCharSequences = [];

for ($start = 48; $start <= 88; $start++) {
    $sequence = '';
    for ($charCode = $start; $charCode < $start+3; $charCode++) {
    if ($charCode >= 58 && $charCode <= 64) {
        continue 2;
    }
        $sequence .= chr($charCode);
    }
    $sequentialCharSequences[] = strtolower($sequence);
}

[dextermb]

Why not use regex to search for repeated characters? Here's an example regex:

/([a-zA-Z0-9])\1{2,}/

This'll check if a character is repeated 3 times.

---

Example test code:

// * matched
$tests = [
    'a',
    'aa',
    'aaa', // *
    'ab',
    'abc',
    'aaabbb' // *
];

echo '<pre>';

foreach ($tests as $test) {
    preg_match_all('/([a-zA-Z0-9])\1{2,}/', $test, $matches);

    var_dump($test);
    var_dump($matches);
}

[DR-DinoMight]

@dextermb I currently have something very similar for the repeatable check, the current problem I'm trying to solve is the sequential alphanumeric character check.

and @DivineOmega that's working a treat just doing some more tests and I should be able to commit and PR the code for both repeatable characters and now sequential.

[dextermb]

@dextermb I currently have something very similar for the repeatable check, the current problem I'm trying to solve is the sequential alphanumeric character check.

Unfortunately there isn't a nice regex solution for this, but perhaps something like:

$test = str_split('abc123');
$last = count($test) - 1;

echo '<pre>';

foreach ($test as $key => $t) {
     if (!is_numeric($t)) {
        continue;
    }

    $prev = $key > 0 && $test[$key - 1] == ($t - 1);
    $next = $key < $last && $test[$key + 1] == ($t + 1);

    if ($prev || $next) {
        throw new \Exception('SEQUENCE');
    }
}

So all together:

$test = 'abc123';
$invalid = false;

preg_match_all('/([a-zA-Z0-9])\1{2,}/', $test, $matches);

if (!empty($matches[1])) {
    $invalid = true;
} else {
    $arr = str_split($test);
    $last = count($test) - 1;

    foreach ($arr as $key => $t) {
        if (!is_numeric($t)) {
            continue;
        }

        $prev = $key > 0 && $arr[$key - 1] == ($t - 1);
        $next = $key < $last && $arr[$key + 1] == ($t + 1);

        if ($prev && $next) {
            $invalid = true;

            break;
        }
    }
}

echo 'Is invalid: '.$invalid;

---

Edit: As discussed with @DivineOmega, it's probably worth changing the if-statement when comparing prev and next to && this way it'll check for a sequence of 3 numbers rather than 2.

[AlexCatch]

@DivineOmega Can this be closed?

[DR-DinoMight]

Yeah, I'll close this now.

[DR-DinoMight]

Sorry I didn't realise you were talking to me 😆 all new to this open source contributing. Apologies for jumping the gun, if it needs reopening please do so I'll not jump the gun any more..

[dextermb]

Sorry I didn't realise you were talking to me 😆 all new to this open source contributing. Apologies for jumping the gun, if it needs reopening please do so I'll not jump the gun any more..

No problem. It's also worth noting that you can get the issue to auto-close when a PR is merged by adding:

Closes #16

to the description of the pull request. 🎉

[DivineOmega]

Released as v4.0.0.