Place sanity checks on Username and Password length on RPC keystore.createUser method

[swdee]

With reference to #6 this PR checks the Username and Password RPC Args length before creating a new user via the RPC keystore.createUser method.

Currently it limits the maximum length for both of these fields to 1024 characters.

Sample RPC rejection response;

{
  "jsonrpc": "2.0",
  "error": {
    "code": -32000,
    "message": "CreateUser call rejected due to username or password exceeding maximum length of 1024 chars",
    "data": null
  },
  "id": 1
}

Furthermore should the presence of nil/empty password be checked, or is it a feature to not require one?

[codecov]

Codecov Report

Merging #13 into master will increase coverage by 0.01144%.
The diff coverage is 100.00000%.

@@                 Coverage Diff                 @@
##              master         #13         +/-   ##
===================================================
+ Coverage   63.70802%   63.71946%   +0.01144%     
===================================================
  Files            191         191                 
  Lines          12686       12690          +4     
===================================================
+ Hits            8082        8086          +4     
  Misses          3972        3972                 
  Partials         632         632                 

[danlaine]

@swdee Thanks for this! Good contribution and much appreciated.

[StephenButtolph]

It might make sense for us to place some restrictions on the password. If we are going to disallow them from having an empty password, we may want to enforce that they use a strong password. I.E. length >= 8 with at least 1 special character (or something along those lines).

[swdee]

For password checking, what approach do you prefer? There are a few packages out there that cover parsing of given passwords to determine the strength using a score, eg: https://github.com/nbutton23/zxcvbn-go

Or others with simple regex checking for the presence of specific chars (upper/lowercase, numbers, special char), such as https://github.com/hasanaliqureshi/Go-Validation

[StephenButtolph]

I'd say that https://github.com/nbutton23/zxcvbn-go is probably the way to go. It seems to give us the most optionality for strength requirements, while also not require too much thought. What do you think?

[swdee]

zxcvbn is the better package as it allows for a greater range of stronger passwords without requiring use of specific characters. The only cost of the package is a bigger Go binary so unless one is conscious of size due to wanting to limit themselves to Raspberry Pi's and dialup modems, then the point is moot ;)

Note that with this change of checking for password strength, the documentation https://docs.ava.network/v1.0/en/quickstart/ava-getting-started/ should be updated with a note about password strength.

Sample interaction with RPC;

$ curl -X POST --data '{
>      "jsonrpc": "2.0",
>      "id": 1,
>      "method": "keystore.createUser",
>      "params": {
>          "username": "test",
>          "password": "test"
>      }
> }' -H 'content-type:application/json;' 127.0.0.1:9650/ext/keystore

{"jsonrpc":"2.0","error":{"code":-32000,"message":"Failed to create user as the given password is to weak. Passwords must be 8 or more characters and contain a combination of UPPER and lowercase letters, numbers, and special characters","data":null},"id":1}

The variable requiredPassScore is set to a score of two which allows for 8 character passwords consisting of a combination of upper/lower/numbers/special characters. Personally I think that is too weak but I do realise that not everyone uses random passwords of 24+ characters which gets a score of four, but some allowances probably need to be made here for convenience.

There is an online tester of this library here https://lowe.github.io/tryzxcvbn/ which you can test out password combinations with which gives a score metric between 0 and 4.

[danlaine]

Hey @swdee thanks again for your contribution.

Two suggestions:

1. The error message returned when the password is too weak says Passwords must be 8 or more characters and contain a combination of UPPER and lowercase letters, numbers, and special characters. However, this isn't enforced; one can create a password that is accepted as sufficiently strong but doesn't have the above attributes. Let's say instead that a password should have those characteristics.
2. In the event that the username/password is rejected for being too long, the call to CreateUser is never logged.

Can you please address these? Thanks

[swdee]

Hi @danlaine

I have made the changes per your suggestion.

[danlaine]

to should be too

[danlaine]

Other than the one typo this looks great. Will approve once the typo is corrected. Thanks again for your contribution @swdee !