shssoichiro / zxcvbn-rs Goto Github PK

View Code? Open in Web Editor NEW

198.0 4.0 51.0 623 KB

Port of Dropbox's zxcvbn password strength library for Rust

License: MIT License

Rust 100.00%

rust security password-strength

zxcvbn-rs's Introduction

zxcvbn

Overview

zxcvbn is a password strength estimator based off of Dropbox's zxcvbn library. Through pattern matching and conservative estimation, it recognizes and weighs 30k common passwords, common names and surnames according to US census data, popular English words from Wikipedia and US television and movies, and other common patterns like dates, repeats (aaa), sequences (abcd), keyboard patterns (qwertyuiop), and l33t speak.

Consider using zxcvbn as an algorithmic alternative to password composition policy — it is more secure, flexible, and usable when sites require a minimal complexity score in place of annoying rules like "passwords must contain three of {lower, upper, numbers, symbols}".

More secure: policies often fail both ways, allowing weak passwords (P@ssword1) and disallowing strong passwords.
More flexible: zxcvbn allows many password styles to flourish so long as it detects sufficient complexity — passphrases are rated highly given enough uncommon words, keyboard patterns are ranked based on length and number of turns, and capitalization adds more complexity when it's unpredictable.
More usable: zxcvbn is designed to power simple, rule-free interfaces that give instant feedback. In addition to strength estimation, zxcvbn includes minimal, targeted verbal feedback that can help guide users towards less guessable passwords.

Installing

zxcvbn can be added to your project's Cargo.toml under the [dependencies] section, as such:

[dependencies]
zxcvbn = "2"

zxcvbn has a "ser" feature flag you can enable if you require serialization support via serde. It is disabled by default to reduce bloat.

zxcvbn follows Semantic Versioning.

zxcvbn targets the latest stable Rust compiler. It may compile on earlier versions of the compiler, but is only guaranteed to work on the latest stable. It should also work on the latest beta and nightly, assuming there are no compiler bugs.

Usage

Full API documentation can be found here.

zxcvbn exposes one function called zxcvbn which can be called to calculate a score (0-4) for a password as well as other relevant information. zxcvbn may also take an array of user inputs (e.g. username, email address, city, state) to provide warnings for passwords containing such information.

Usage example:

extern crate zxcvbn;

use zxcvbn::zxcvbn;

fn main() {
    let estimate = zxcvbn("correcthorsebatterystaple", &[]).unwrap();
    println!("{}", estimate.score()); // 3
}

Other fields available on the returned Entropy struct may be viewed in the full documentation.

Contributing

Any contributions are welcome and will be accepted via pull request on GitHub. Bug reports can be filed via GitHub issues. Please include as many details as possible. If you have the capability to submit a fix with the bug report, it is preferred that you do so via pull request, however you do not need to be a Rust developer to contribute. Other contributions (such as improving documentation or translations) are also welcome via GitHub.

License

zxcvbn is open-source software, distributed under the MIT license.

zxcvbn-rs's People

Contributors

Stargazers

Watchers

zxcvbn-rs's Issues

Contact for potential security issue

Hi,

I believe I have found a potential security issue in this library. I would like to know if there is an email or other channel you would prefer for me to use to report this issue and proof of concepts to you.

Thanks,

Evaluate empty passwords

zxcvbn returns None for empty passwords. It could return something more meaningful instead.

Consider making `zxcvbn` return a `Result`

Currently it is not clear what failed when None was returned. Even Result<_, ()> would be semantically more correct IMHO.

`Feedback` for other languages than English

I see two options how this could be made easier:

Make Feedback an enum.
Make it possible to change the feedback strings at compile time.

DOS Exploit

Hey, just wanted to let you know I've gotten reports from users of my library: Nbvcxz that are getting a DOS every so often by specifically crafted passwords.

I even found a tool created by a government contractor used for issuing a DOS against programs using libraries containing the vulnerable (to combination explosion) algorithms from the original zxcvbn implementation:

I've solved this by implementing a maxLength type configuration...but that isn't totally done yet as I feel like I still need to have it do dictionary checks against the full-length password without any transformations. Working on finishing that feature and putting out a release. I just wanted to mention it to you, since this is also often run server-side rather than client-side.

Rework matching

The matching struct currently owns the token it represents. However, the string is always taken (indirectly) from the password. It could borrow from the password inputted by the user and prevent a lot of allocations.

Add support for non-ASCII character sets

Currently all of zxcvbn's operations are byte-based rather than character based, which makes operations on unicode strings likely to panic. The Repeat matcher also relies on having the ASCII character code of each character.

`Match` should be an enum

I believe the way it is currently implemented is not very idiomatic. It should be an enum instead of a flat struct with lots of optional fields, like so:

pub struct Dict {
    pub matched_word: String,                                                      
    pub rank: usize,                                                               
    pub name: &'static str,                                             
    pub reversed: bool,                                                                    
    pub l33t: bool,                                                                        
    pub sub: Option<HashMap<char, char>>,                                                  
    pub sub_display: Option<String>
}

pub enum Pattern {
    Dict(Dict),
    ...
}

pub struct Match {                                                                         
    pub i: usize,                                                                          
    pub j: usize,                                                                          
    pub token: String,
    pub pattern: Pattern,
}

Some of the Output doesn't match the original library

I noticed that using some of the input doesn't give the same results as the original javascript library.

Examples:

Input	Feedback	Original feedback
password	This is similar to a commonly used password	This is a top-10 common password
test	This is similar to a commonly used password	This is a top-100 common password

Is this a bug in your implementation or is this intended?

Reevaluate using builders

I think in all places a builder is used, the spread syntax can be used instead:

Struct {
  a: 0,
  b: 1,
  ..Struct::default()
}

I think this is cleaner, it eliminates a dependency and doesn't expose builders in the API.

Performance and ideomatic code

I have some ideas that would improve performance and ideomaticy (is that a word?).

I think the properties of Entropy are better private and can then be coverted to methods. The crack times and score can be computed lazily if needed by the user.

Feeback should probably use enums Warning and Suggestion that implement Display. This would close #16.

I think some allocations can be prevented. It could be difficult, but I think the field token in Match could be a &str that borrows from password. Match could also have a reference to password and compute the correct slice in a method when needed.

Does not build with Rust 1.36.0

error[E0277]: `(dyn matching::Matcher + 'static)` cannot be sent between threads safely
  --> C:\Users\Geob\.cargo\registry\src\github.com-1ecc6299db9ec823\zxcvbn-1.0.1\src\matching\mod.rs:70:1
   |
70 | / lazy_static! {
71 | |     static ref MATCHERS: [Box<Matcher>; 8] = [
72 | |         Box::new(DictionaryMatch {}),
73 | |         Box::new(ReverseDictionaryMatch {}),
...  |
80 | |     ];
81 | | }
   | |_^ `(dyn matching::Matcher + 'static)` cannot be sent between threads safely
   |
   = help: the trait `std::marker::Send` is not implemented for `(dyn matching::Matcher + 'static)`
   = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<(dyn matching::Matcher + 'static)>`
   = note: required because it appears within the type `std::boxed::Box<(dyn matching::Matcher + 'static)>`
   = note: required because it appears within the type `[std::boxed::Box<(dyn matching::Matcher + 'static)>; 8]`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `spin::once::Once<[std::boxed::Box<(dyn matching::Matcher + 'static)>; 8]>`
   = note: required because it appears within the type `lazy_static::lazy::Lazy<[std::boxed::Box<(dyn matching::Matcher + 'static)>; 8]>`
   = note: shared static variables must have a type that implements `Sync`
   = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

Allow building with frequency lists outside of the source code.

Hello,
I'm building a web app (https://github.com/PaulGrandperrin/cachou) using your very cool library, here is my use-case:

the app is distributed as a wasm module (built with https://github.com/yewstack/yew)
the authentication is done with https://github.com/novifinancial/opaque-ke : meaning that my server-side code never ever sees the user's passwords
this means that user password checking needs to be done client-side
but I can't really use zxcvbn client-side because of its prohibitive compiled size:
- my app's release mode wasm without zxcvbn: 1.4M uncompressed, 338K compressed with brotli
- my app's release mode wasm with zxcvbn: 2.5M uncompressed, 753K compressed with brotli
Even though my app is using 264 other crates, zxcvbn alone doubles its download time and probably wasm compilation time too.. But is only useful when a user is signing up or changing its password.

The best solution I see to this problem would be to allow building a version of this create (behind a feature flag) without the frequency lists in the source code, and then allow loading them at runtime.

This way, the ~400K of compressed lists would only be downloaded and processed when the client code needs them without affecting loading time and "time to interactivity" :-)

I'm sure it could even be done in a non-api breaking way.

I'm willing to investigate and propose a PR if this feels reasonable to you

Unit test failure

There is currently a unit test failure.

failures:
---- tests::test_zxcvbn stdout ----
thread 'tests::test_zxcvbn' panicked at 'assertion failed: `(left == right)`
  left: `490970150656000`,
 right: `473471216704000`', src/lib.rs:153:9
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
failures:
    tests::test_zxcvbn

Is it enough to change the expected number to pass the test or was this number manually computed somehow? This is happening on the Travis CI build system as well.

Wasm

Add a `de` feature to complement `ser`

I can serialize the Entropy struct, but have no way to deserialize it without making my own struct.

I propose adding

#[cfg_attr(feature = "de", derive(Deserialize))]

Add optional support for rustc-serialize and serde

Change the structure of Match

I think Match could be improved in the following way:

i and j currently indicate character positions. Instead, I think it would be better to replace them by a range that instead can be used to slice the string, i.e. changing them to byte indexes.
guesses is always set in the public API. For that reason I think the publicly exposed type should not use an Option for it.

`Match` should be public

I'm surprised that it is legal that it is private, since the public sequence field of Entropy is a Vec<Match>.

Please consider publishing a new release

Hey there,

We were wondering if it would be possible to publish a new patch release sometime if you're available. This would let the itertools and chrono dependency improvements make their way into downstream crates.

Thanks!

Drop rustc-serialize support

According to Rust release milestone predictions, rustc-serialize is scheduled to be deprecated in next month's beta release of Rust.

This would be a good time to let us know if there is any functionality you require from Serde before that happens.

Build broken on wasm targets

See the logs of our recent CI runs here. I guess the issue is there's no use wasm_bindgen_test::. A way it's handled in our downstream crate is by having some thing along the lines of

#[cfg(all(target_arch = "wasm32", test))]
use wasm_bindgen_test::{wasm_bindgen_test as test};

#[test] // Always refers to the correct test attribute :)
fn the_test_case() { }

Note that wasm_bindgen_test is a dev-dependency only, so the attribute only exists in dev builds anyway.

Always adds wasm dependencies

I think the wasm support pulls a bunch of crates into the Cargo.lock. Looks like other crates have solved this by using a feature flag chronotope/chrono#334

$ cargo update
    Updating crates.io index
...
      Adding js-sys v0.3.46
      Adding wasm-bindgen v0.2.69
      Adding wasm-bindgen-backend v0.2.69
      Adding wasm-bindgen-macro v0.2.69
      Adding wasm-bindgen-macro-support v0.2.69
      Adding wasm-bindgen-shared v0.2.69
    Updating zxcvbn v2.0.1 -> v2.1.0

Consider a simpler signature for `zxcvbn`

user_inputs: Option<&[&str]> is redundant, it could be &[&str] instead. If there are no user inputs, you would just pass &[] instead of None.

Panic - attempt to multiple with overflow

I was recently testing something with zxcvbn and hit the following panic:

panicked at 'attempt to multiply with overflow', /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:364:21

  16:        0x103257ef0 - <zxcvbn::matching::patterns::SpatialPattern as zxcvbn::scoring::Estimator>::estimate::h93e72d2a26156230
                               at /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:364:21
  17:        0x10325787c - <zxcvbn::matching::patterns::MatchPattern as zxcvbn::scoring::Estimator>::estimate::h185ae12d4df3b913
                               at /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:242:49
  18:        0x10323094c - zxcvbn::scoring::estimate_guesses::h8d187b85a94d83c2
                               at /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:229:19
  19:        0x10322fb34 - zxcvbn::scoring::most_guessable_match_sequence::update::h4424cab6ef6613c0
                               at /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:83:22
  20:        0x10322f938 - zxcvbn::scoring::most_guessable_match_sequence::h53ab778e7167a99e
                               at /Users/william/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-2.2.1/src/scoring.rs:189:17
  21:        0x10322510c - zxcvbn::zxcvbn::hfedca045584cf211

Sadly I don't know what the input was that caused this, but I assume that https://github.com/shssoichiro/zxcvbn-rs/blob/master/src/scoring.rs#L364 likel needs checked mul :)

Massively incorrect scores with some passwords

E.g. Imaginative-Say-Shoulder-Dish-0 produces a score of 0 when I expect 3 or 4. Removing hyphens still results in a score of 0.

Migrate to fancy-regex

Currently this library is using oniguruma for regexes that require backreferences. I would like to move to fancy-regex, a native Rust crate that supports backreferences, to reduce the number of FFI dependencies.

Feedback and related subtypes don't derive deserialise

Hi!

I was trying to use this library and noticed that when I add it to a type that has Serialise and Deserialise an error is raised:

#[derive(Serialize, Deserialize)]
struct MyStruct {
    fb: Feedback
}

...

error[E0277]: the trait bound `zxcvbn::feedback::Feedback: v1::_IMPL_DESERIALIZE_FOR_SchemaError::_serde::Deserialize<'_>` is not satisfied
  --> kanidm_proto/src/v1.rs:61:21
   |
61 |     InvalidPassword(feedback::Feedback),
   |                     ^^^^^^^^ the trait `v1::_IMPL_DESERIALIZE_FOR_SchemaError::_serde::Deserialize<'_>` is not implemented for `zxcvbn::feedback::Feedback`
   |
   = note: required by `v1::_IMPL_DESERIALIZE_FOR_SchemaError::_serde::de::VariantAccess::newtype_variant`

I'm wondering if perhaps you are missing:

#[cfg_attr(feature = "ser", derive(Serialize, Deserialize))]

On the Feedback and related child types.

Thanks!

release wasm + asm.js version on npm

build error

Getting the following with 0.4.3

$ cargo --version
cargo 0.16.0-dev (6e0c18c 2017-01-27)
$ rustc --version
rustc 1.15.1
$ cargo run --verbose
       Fresh quote v0.3.15
       Fresh utf8-ranges v1.0.0
       Fresh log v0.3.7
       Fresh getopts v0.2.14
       Fresh slab v0.3.0
       Fresh take v0.1.0
       Fresh unicode-normalization v0.1.4
       Fresh cfg-if v0.1.0
       Fresh semver v0.1.20
       Fresh language-tags v0.2.2
       Fresh byteorder v1.0.0
       Fresh smallvec v0.2.1
       Fresh lazycell v0.4.0
       Fresh libc v0.2.21
       Fresh bit-vec v0.4.3
       Fresh bitflags v0.5.0
       Fresh rustc_version v0.1.7
       Fresh scoped-tls v0.1.0
       Fresh regex-syntax v0.4.0
       Fresh either v1.1.0
       Fresh void v1.0.2
       Fresh iovec v0.1.0
       Fresh net2 v0.2.27
       Fresh thread-id v3.0.0
       Fresh num_cpus v1.3.0
       Fresh itertools v0.5.10
       Fresh bit-set v0.4.0
       Fresh bytes v0.4.2
       Fresh unreachable v0.1.1
       Fresh memchr v1.0.1
       Fresh time v0.1.36
       Fresh lazy_static v0.2.6
       Fresh matches v0.1.4
       Fresh rustc-serialize v0.3.23
       Fresh thread_local v0.3.3
       Fresh aho-corasick v0.6.3
       Fresh mime v0.2.3
       Fresh futures v0.1.13
       Fresh unicode-xid v0.0.4
       Fresh unicode-bidi v0.2.5
       Fresh mio v0.6.6
       Fresh rand v0.3.15
       Fresh pulldown-cmark v0.0.8
       Fresh regex v0.2.1
       Fresh tokio-service v0.1.0
       Fresh synom v0.11.3
       Fresh unicase v1.4.0
       Fresh futures-cpupool v0.1.5
       Fresh idna v0.1.1
       Fresh tempdir v0.3.5
       Fresh tokio-io v0.1.1
       Fresh fancy-regex v0.1.0
       Fresh httparse v1.2.1
       Fresh syn v0.11.10
       Fresh base64 v0.4.1
       Fresh gcc v0.3.45
       Fresh url v1.4.0
       Fresh skeptic v0.7.1
       Fresh tokio-core v0.1.6
       Fresh derive_builder_core v0.1.0
       Fresh tokio-proto v0.1.1
       Fresh hyper v0.11.0-a.0 (https://github.com/hyperium/hyper.git#5c1cfa2b)
       Fresh rust-crypto v0.2.36
       Fresh derive_builder v0.4.0
   Compiling zxcvbn v0.4.3
     Running `rustc --crate-name zxcvbn /Users/connortaffe/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-0.4.3/src/lib.rs --crate-type lib -g --cfg feature=\"default\" -C metadata=7b4b8e321443c5bc -C extra-filename=-7b4b8e321443c5bc --out-dir /Users/connortaffe/src/api/target/debug/deps --emit=dep-info,link -L dependency=/Users/connortaffe/src/api/target/debug/deps --extern fancy_regex=/Users/connortaffe/src/api/target/debug/deps/libfancy_regex-61503bc2c195e065.rlib --extern time=/Users/connortaffe/src/api/target/debug/deps/libtime-53238cb92943b947.rlib --extern lazy_static=/Users/connortaffe/src/api/target/debug/deps/liblazy_static-c06652d9f423bc2f.rlib --extern itertools=/Users/connortaffe/src/api/target/debug/deps/libitertools-6a04d0d668aebda3.rlib --extern regex=/Users/connortaffe/src/api/target/debug/deps/libregex-85104e2a1249fae2.rlib --extern derive_builder=/Users/connortaffe/src/api/target/debug/deps/libderive_builder-e2aa5f8eedaf6307.dylib --cap-lints allow`
error: custom derive attribute panicked
 --> /Users/connortaffe/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-0.4.3/src/matching.rs:6:44
  |
6 | #[derive(Debug, Clone, Default, PartialEq, Builder)]
  |                                            ^^^^^^^
  |
  = help: message: assertion failed: !p.is_null()

error: Could not compile `zxcvbn`.

Caused by:
  process didn't exit successfully: `rustc --crate-name zxcvbn /Users/connortaffe/.cargo/registry/src/github.com-1ecc6299db9ec823/zxcvbn-0.4.3/src/lib.rs --crate-type lib -g --cfg feature="default" -C metadata=7b4b8e321443c5bc -C extra-filename=-7b4b8e321443c5bc --out-dir /Users/connortaffe/src/api/target/debug/deps --emit=dep-info,link -L dependency=/Users/connortaffe/src/api/target/debug/deps --extern fancy_regex=/Users/connortaffe/src/api/target/debug/deps/libfancy_regex-61503bc2c195e065.rlib --extern time=/Users/connortaffe/src/api/target/debug/deps/libtime-53238cb92943b947.rlib --extern lazy_static=/Users/connortaffe/src/api/target/debug/deps/liblazy_static-c06652d9f423bc2f.rlib --extern itertools=/Users/connortaffe/src/api/target/debug/deps/libitertools-6a04d0d668aebda3.rlib --extern regex=/Users/connortaffe/src/api/target/debug/deps/libregex-85104e2a1249fae2.rlib --extern derive_builder=/Users/connortaffe/src/api/target/debug/deps/libderive_builder-e2aa5f8eedaf6307.dylib --cap-lints allow` (exit code: 101)

Is it still true that only ASCII is supported?

The docs for function zxcvbn::zxcvbn say:

Currently zxcvbn only supports ASCII input.

I'm curious if this still the case after #4, #31 and perhaps #58? It looks like perhaps all the known issues are resolved?

Inconsistent scoring between JS and Rust

Some passwords are showing inconsistent scores between the rust version and the javascript version. These should be the same in both versions.

"TestMeNow!" scores: 2 (rust), 3 (js)
"hey<123" scores 1 (rust), 2 (js)

Export two functions

To make the interface clearer, the zxcvbn function could be split into two:

pub fn zxcvbn(password: &str) -> Result<Entropy, ZxcvbnError>;
pub fn zxcvbn_with_user_inputs(
    password: &str, 
    user_inputs: &[&str]
) -> Result<Entropy, ZxcvbnError>;

Internally, the former could just call the other, but this does make the meaning of the extra parameter clearer, as well as cleaning up usages without it.

Ensure docs.rs link becomes available

Currently the documentation on docs.rs has not yet been generated. Not sure if this is because of a delay in docs.rs. Created this issue as a reminder to check back and ensure the documentation becomes available.

breaking change to error type

Just an FYI:

The error type changed in e48a226 to include a date conversion error. Adding a variant to a public enum is a breaking change. If your MSRV allows it use #[non_exhaustive] so that further variant additions are not breaking.

Please consider allowing developers to choose a list of matchers

Right now it is only possible to get a feedback for a password based on all possible factors (matchers). It is essentially all or nothing.

This behaviour may not be desirable for the case when only certain matchers matter. In our case we'd like to use zxcvbn-rs for our wasm project. We found that regex dependency is just too heavy and makes our wasm file huge for no reason (it adds ~1.5MB).

I can see why certain structs like SequenceMatch are private. But I'd like to explore any reasonable ways to configure a list of matchers.

One option could be an exposure of additional API that will enable more granular control.
Another idea is to put certain matchers behind feature flags.
I'm happy to help with experiments and an implementation.

Thoughts?