Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dictionary word not always recognized #7

Closed
jdhoek opened this issue Feb 17, 2017 · 5 comments
Closed

Dictionary word not always recognized #7

jdhoek opened this issue Feb 17, 2017 · 5 comments
Assignees

Comments

@jdhoek
Copy link
Contributor

jdhoek commented Feb 17, 2017

In this example I have configured nbvcxz with an additional single-entry dictionary to test how a common password occurrence (surname or given name plus a few numbers) is handled:

// Just a random string for testing. Please assume for the sake of the example that
// 'Gohklhiepu' is the user's surname.
Map<String, Integer> excludeMap = new HashMap<>();
excludeMap.put("Gohklhiepu", 0);

// Just like the README.
List<Dictionary> dictionaryList = ConfigurationBuilder.getDefaultDictionaries();
dictionaryList.add(new Dictionary("exclude", excludeMap, true));

Configuration configuration = new ConfigurationBuilder()
        .setDictionaries(dictionaryList)
        .setMinimumEntropy(30d)
        .createConfiguration();

Nbvcxz nbvcxz = new Nbvcxz(configuration);


// Test A.

Result result = nbvcxz.estimate("Gohklhiepu");

System.out.println(result.getEntropy());
// 0.0

for (Match match : result.getMatches()) {
    System.out.println(match.getDetails());
    // DictionaryMatch
}


// Test B.

result = nbvcxz.estimate("Gohklhiepu3425");

System.out.println(result.getEntropy());
// 60.29210956096036

for (Match match : result.getMatches()) {
    System.out.println(match.getDetails());
    // A series of BruteForceMatch
}

As expected, using the fictional dictionary word as password gives 0.0 entropy.

Surprisingly, word + 3425 (which is a rather weak password if we assume that word is the user's surname and the number his postal code or house number) results in a rather high entropy of 60.3. It looks like the word is not recognized as a dictionary word — all matches are BruteForceMatch.

Could this be a bug?

@Tostino
Copy link
Collaborator

Tostino commented Feb 17, 2017

Thanks for bringing this to my attention. Looking into it and will get back to you. I appreciate the reproducible test.

@Tostino
Copy link
Collaborator

Tostino commented Feb 17, 2017

Okay, I found the offending code. I am not sure why I thought that I should do this at the time.

// Only match exclude dictionaries on full passwords
if (dictionary.isExclusion() && (start != 0 || end != password.length()))
{
    continue;
}

I had explicitly stopped matching on an exclude dictionary if the whole password wasn't in it. That obviously reduces the usefulness of an exclude dictionary and is not how I want it to work.

I will fix this and push a new version out shortly.

@Tostino Tostino self-assigned this Feb 17, 2017
@jdhoek
Copy link
Contributor Author

jdhoek commented Feb 17, 2017

Interesting. Glad that this is something you could find easily. I'll test this code again when you push a fix.

Thanks for looking into this.

@Tostino
Copy link
Collaborator

Tostino commented Feb 17, 2017

Alright, I have committed a fix in: 0bcab0c

There was one extra problem I had to tackle, because it wasn't obvious but was causing another issue after the first I found. And that problem is that dictionaries need to only contain lower case words for the matching to work properly. We do case insensitive matching by just making sure everything is being lower cased to speed things up. That was not at all obvious with the previous API. I created a builder class for creating dictionaries, which will help with that. The readme is updated with an example of using that builder.

If you want to use the constructor directly, I made sure to update the javadoc to note that words had to be lower cased to work correctly.

Also created some tests for catching issues with exclusion dictionaries in the future.

I pushed out 1.3.3 to Maven, so so that should be out there shortly. Will create another release asap.

Please let me know if you find anything else working not as you'd expect.

Thanks,
-Adam

@Tostino Tostino closed this as completed Feb 17, 2017
@jdhoek
Copy link
Contributor Author

jdhoek commented Feb 20, 2017

The second example from my test now gives a much lower 16.421653954955087 as entropy. Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants