admin password is limited to 126 characters

[dismantl]

When setting a new admin password in the Basic Config menu, the password is trunctated to 126 characters. There should be no maximum password length.

[gradyoti]

Update: Adding password string longer than 126 chars does not present an error to the user. Passwords above 126 chars are rejected (old, valid password remains active).

[gradyoti]

Update #2: It looks like this is actually a limit for the passwd utility on the nodes, which the setup wizard calls directly. Unfortunately, passwd does not pass any errors, it just truncates the password and accepts the first 127 chars. This means that setup wizard assumes the operation was successful, even though the full password was not accepted.

My suggestion is to validate the password field before calling passwd, and pass an error back to the user if the password is too long.

[gradyoti]

Update #3: Added issue to commotion-router, tagged upstream: https://github.com/opentechinstitute/commotion-router/issues/146

[seamustuohy]

Thought we should add notes from the back and forth Grady and I had on this issue so that the exploration of this issue was captured for any future validation issues.

Questions about this bug.

Q: Is the password truncated in the text field? So that you can't add any more chars? A: Because then we should be looking at the js running on the page and not at the back end.

Q: Is the password being rejected before the page is submitted to the back end? A: Because then we are again exploring the Javascript

You can eliminate possibility of front-end issues by simply turning off javascript in your browser and testing if it is still rejected.

I didn't explore front end impact because I don't have a node to do the easy tests.

Q: Is an error issues to the user about a failed password entry when the password is above 126 chars? A: Then we are looking at the back end code. (See below, that is what I explored a little bit.)

Well, I don't have a router to test here, but it looks like it is a limit by the command line passwd setter.

Here are the commands used to come to that conclusion, and none of the outputs.

NOTE: my current shell looks like this ([GIT-BRANCH]) [DEVICE NAME]:[FOLDER-NAME] $

(master) computer:luci-commotion $ git log -S "admin password"

(master) computer:luci-commotion $ git show --oneline
bdc1b72bab7e0fa7b2460d51d51c9a7b0c01926c

(master) computer:luci-commotion $ cat -n
luasrc/model/cbi/commotion/basic_ns.lua |grep -i admin

(master) WORKGROUP:luci-commotion $ cat -n
luasrc/model/cbi/commotion/basic_ns.lua |grep -A 40

(master) computer:luci-commotion $ cat -n
luasrc/model/cbi/commotion/basic_ns.lua |grep root_pass_check

(master) computer:luci-commotion $ cat -n
luasrc/model/cbi/commotion/basic_ns.lua |grep -A 40 143

***switched over to the luci repo here***

computer:luci-0.11 $ cat -n libs/sys/luasrc/sys.lua |grep -B 5 -A 40
setpasswd

While the older libraries (apps) use just views for user facing content and controllers for logic, all the interfaces that are in the setup-wizard process use the traditional structure of model (controls data manipulation and logic), view (shows pretty), controller (just loads models). So when you are looking for the logic behind things that are done in setup wizard you should start with the controller to identify the model using the url you are exploring, the model to identify the logic being used on the back-end, and any views that are explicitly loaded by the model.

The luci libraries /libs/web/luasrc/cbi.lua file has all the default views model load. You can look in there to see the views used by default by model objects and therefore the javascript that those views use on the page.

[dismantl]

Thanks for the additional info, @elationfoundation. To answer the questions:

1. The password is not truncated in the input field. You can put a password in there as long as you want.
2. The password is not rejected by the javascript before submission.
3. No error is returned by the back-end for overly long passwords.

Basically, you can input a password as long as you want, the LuCI page accepts the submission, and no errors are ever given, giving the user reason to think everything went fine. Yet the actual password stored by the system has been truncated silently.

[dismantl]

My proposal to @gradyoti was to add a maxlength property to the input field on the front-end, and then do back-end validation, returning an error if the password is too long.

[gradyoti]

Addressed by https://github.com/opentechinstitute/luci-commotion/pull/438

[jheretic]

Closed as per #438