rancher / dashboard

The Rancher UI
https://rancher.com
Apache License 2.0
452 stars 257 forks source link

Allow to change minimum length requirement for user password #6633

Closed cnotv closed 1 year ago

cnotv commented 2 years ago

Detailed Description Allow to change the setting for user password length requirement from default of 12.

Context The password length can be defined exclusively with env var CATTLE_PASSWORD_MIN_LENGTH in setup time and not from the UI as other settings.

As this is a Kube resource, it can be updated using a POST request to settings.

Screenshot 2022-08-10 at 13 49 44

richard-cox commented 2 years ago

@cnotv Double check with the backend team that this is possible at run time, I have a feeling it can only be set when installing.

cnotv commented 2 years ago

@richard-cox Waiting for any answer in the private channels.

cnotv commented 2 years ago

Added extra infos.

nwmac commented 2 years ago

@cnotv Can this be changed after install? If so, what happens when you set the min length to say 20 and users have passwords that are shorter than 20 characters?

cnotv commented 2 years ago

@nwmac seems like it's possible to change it after the install, based to what has been told me, not tested though.

Not sure how is the functionality for existing users, but I imagine that would be a parameter for a new request.

nwmac commented 2 years ago

@cnotv Do you want to go ahead and add support for configuring this setting - code is in shell/list/management.cattle.io.setting.vue - I think you just need to add the setting to ALLOWED_SETTINGS and make sure there is a localised string for the setting.

cnotv commented 2 years ago

Resource value as default:

    {
      "id": "password-min-length",
      "type": "management.cattle.io.setting",
      "links": {
        "remove": "blocked",
        "self": "/v1/management.cattle.io.settings/password-min-length",
        "update": "/v1/management.cattle.io.settings/password-min-length",
        "view": "/apis/management.cattle.io/v3/settings/password-min-length"
      },
      "apiVersion": "management.cattle.io/v3",
      "customized": false,
      "default": "12",
      "kind": "Setting",
      "metadata": {
        "creationTimestamp": "2022-08-04T12:10:19Z",
        "fields": [
          "password-min-length",
          ""
        ],
        "generation": 1,
        "managedFields": [
          {
            "apiVersion": "management.cattle.io/v3",
            "fieldsType": "FieldsV1",
            "fieldsV1": {
              "f:customized": {},
              "f:default": {},
              "f:source": {},
              "f:value": {}
            },
            "manager": "rancher",
            "operation": "Update",
            "time": "2022-08-04T12:10:19Z"
          }
        ],
        "name": "password-min-length",
        "relationships": null,
        "resourceVersion": "511",
        "state": {
          "error": false,
          "message": "Resource is current",
          "name": "active",
          "transitioning": false
        },
        "uid": "7f570f7d-4102-4d84-a0e7-6cf24fc44c1d"
      },
      "source": "",
      "value": "12"
    },
cnotv commented 2 years ago

@nwmac If a password is e.g. length 3 and we increase/restore the value to 12, no message error is provided.

Josh-Diamond commented 2 years ago

Re-opening due to the following concerns:

Suggestion:

Failed:


Ticket #6633 - Test Results

With Docker on a single-node instance:

Reproduced on rancher v2.6.8:

  1. Fresh install of rancher v2.6.8
  2. As admin, navigate to Users & Authentication
  3. Change password to anything less than 12 characters
  4. Reproduced - Password must be at least 12 characters

Screenshot:

Step 4

Screen Shot 2022-09-06 at 3 46 34 PM

Verified on rancher v2.6-400e559407d14ad925caf2f56f5edd8645dbdaba-head:

Scenario 1 - Set pass-min-length to 0; then change password to blank field - ❌

  1. Fresh install of rancher v2.6-head
  2. As admin, navigate to Global Settings > password-min-length
  3. Change min length to 0
  4. Verified - RancherUI successfully sets pass-min-length to ZERO
  5. Navigate to Users & Authentication, and set password to blank field
  6. Veri-FAILED - RancherUI won't allow passwords >1 character to be set; Global Settings allow password-min-length to be set to 0

Scenario 2 - Set pass-min-length to 0; then change password to 1 character - ✅

  1. Fresh install of rancher v2.6-head
  2. As admin, navigate to Global Settings > password-min-length
  3. Change min length to 0
  4. Verified - RancherUI successfully sets pass-min-length to ZERO
  5. Navigate to Users & Authentication, and set password to a single character
  6. Verified - Password successfully set + valid upon login

Scenario 3 - Set pass-min-length to 4; then try to change password to 3 characters; expected fail - ✅

  1. Fresh install of rancher v2.6-head
  2. As admin, navigate to Global Settings > password-min-length
  3. Change min length to 4
  4. Verified - RancherUI successfully sets pass-min-length to FOUR
  5. Navigate to Users & Authentication, and set password to three characters
  6. Verified - Password validation successfully prevents 3 character password and explains "why" w/ message

Screenshot:

Step 6

Screen Shot 2022-09-06 at 4 37 29 PM

Scenario 4 - Set pass-min-length to 1,000,000; set password to 1,000,000 characters - ✅

  1. Fresh install of rancher v2.6-head
  2. As admin, navigate to Global Settings > password-min-length
  3. Change min length to 1,000,000
  4. Verified - RancherUI successfully sets pass-min-length to ONE MILLION
  5. Navigate to Users & Authentication, and set password to ONE MILLION characters
  6. Verified - Password successfully set + valid upon login

Scenario 5 - Set pass-min-length to negative value - ✅ - ❗ PASS NOT EXPECTED ❗

  1. Fresh install of rancher v2.6-head
  2. As admin, navigate to Global Settings > password-min-length
  3. Change min length to -2 - [type input manually]
  4. Verified - RancherUI successfully sets pass-min-length to -2

Screenshot:

Step 4

Screen Shot 2022-09-06 at 4 47 29 PM
cnotv commented 2 years ago

We will have to introduce something like validation to the settings, not sure if it exists yet. Moving back the estimation to 2.

cnotv commented 2 years ago

Created related issue out of this topic, as extension to this adjustment.

cnotv commented 2 years ago

@Josh-Diamond for consistency, is this option validated while installing Rancher as well is we pass something like CATTLE_BOOTSTRAP_PASSWORD=-2?

Josh-Diamond commented 2 years ago

@cnotv Yes, you may set the password minimum length to a negative value upon initial installation:

docker run -d -p 80:80 -p 443:443 --privileged -e CATTLE_PASSWORD_MIN_LENGTH=-2 rancher/rancher:v2.6-head

When doing so, from the UI, you'll notice a banner, Set by Environment Variable:

Screen Shot 2022-09-07 at 8 50 12 AM
Josh-Diamond commented 2 years ago

@cnotv Is there a max value for min_password_length that should be enforced?
With my validation tests, I was able to set min_password_length to 1,000,000.
Although, not an issue, i'm curious if this is ideal/desired.
I can't wrap my head around why someone would use triple digits even for min_password_length

cnotv commented 2 years ago

I think we should set a range but I have no parameters to define a maximum. I guess 32 characters may be enough?

rak-phillip commented 2 years ago

@cnotv it looks like OWASP has some recommendations on password length. This might be a good place to start for considering any limits we might want to set.

Maximum password length should not be set too low, as it will prevent users from creating passphrases. A common maximum length is 64 characters due to limitations in certain hashing algorithms, as discussed in the Password Storage Cheat Sheet. It is important to set a maximum password length to prevent long password Denial of Service attacks.

https://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#implement-proper-password-strength-controls

Josh-Diamond commented 2 years ago

Further reaching out to the team for insights.

From a security perspective, i'm leaning towards no max limit being imposed.

From a performance perspective, I'm leaning towards imposing a max limit. e.g 1GB of data seems like it could consume too much memory

IMO 256 max characters is a fair, secure and practical, but would not like to make this call

Seems to be a case of: How to best save the customer from themselves, if at all

cnotv commented 2 years ago

If we are talking about DDOS attacks at this point the issue should be entirely server side, as from the client this is irrelevant. It is still convenient to have a validation though.

Josh-Diamond commented 2 years ago

Offline conversation with @macedogm has suggested: Yes, it's a good practice to limit the max password length....limiting in 256 is okay

@cnotv Do you have any concerns w/ a 256 character limit? If not, could we try to get that implemented?

cnotv commented 2 years ago

It sounds good to me, thanks for taking care of it.

cnotv commented 2 years ago

It sounds good to me, thanks for taking care of it.

Josh-Diamond commented 2 years ago

@cnotv Sure thing, i will create an issue/enhancement in rancher/rancher to set a max limit of 256 characters for min-password-length

Josh-Diamond commented 2 years ago

@cnotv Could you confirm no further changes are needed from UI end to accommodate the following enhancements:

Josh-Diamond commented 1 year ago

Ticket #6633 - Test Results - REOPENED

With Docker on a single-node instance:

Verified on rancher v2.7-c0f6cdb9a7494cb6eae2e3b341174bd079f13073-head:

  1. Fresh install of rancher v2.7-head
  2. Navigate to Global Settings > password-min-length
  3. Verified - unable to set negative values, zero value, or value of 1
  4. Verified - able to set value from 2-256
  5. Verified - Once value is set from range 2-256, new users are prompted w/ accurate validation during new user creation
  6. Verified - Unable to set value greater than 256
  7. Veri-FAILED: able to set value of 2.5; global settings accept change but it is not respected; validation during new user creation prompts 12 character min requirement;

Comments: Although, global settings allow the use of decimals within the range of 2-256, this isn't accepted as being valid when creating a new user. We should prevent this usecase from occurring, with further validation, if possible.

Screenshots:

Global Settings > password-min-length

Screenshot 2022-11-17 at 9 48 13 AM

Create New User w/ 2 character password

Screenshot 2022-11-17 at 9 48 43 AM

Create New User w/ 3 character password

Screenshot 2022-11-17 at 10 01 08 AM
cnotv commented 1 year ago

@Josh-Diamond ~looks like Integer validation using decimals is allowed in every input field we have. What would you recommend in this case? I would rather tackle this all at once instead of this very aimed case.~ EDIT: We allow decimals exclusively for this form, which is handled in a single input. We can set a default validation for this case, which does not allow decimals.

cnotv commented 1 year ago

In the current state seems like something has been changed, as I am not able to edit the value anymore.

Screenshot 2022-11-17 at 18 11 00

Josh-Diamond commented 1 year ago

set a default validation for this case, which does not allow decimals - this sounds great!

In the current state seems like something has been changed, as I am not able to edit the value anymore - 😮

cnotv commented 1 year ago

It seems to be solved the issue with the editing capabilities 🤷

cnotv commented 1 year ago

It seems like this option is now flagged as Set by Environment Variable and does not allow us to edit it, unless we enable it. This change seems coming from the server while in the process of QA. I am allowing now to change them in order to make it possible.

Josh-Diamond commented 1 year ago

Ticket #6633 - Test Results [Cont.] - RE-OPENED (again) - ❌

With Docker on a single-node instance:

Verifying on Rancher v2.7-9278cf152f9b98ae3c543c0f956af2ae6ab91368-head:

  1. Fresh install of Rancher v2.7-head
  2. Navigate to Global Settings > password-min-length, and Edit Setting
  3. Verified - password must be between 2-256 characters
  4. Verified - validation prevents submission of non-zero decimal values
  5. Veri-FAILED:
    • 11.0 is accepted as a valid configuration via Global Settings > password-min-length > Edit Setting, but it isn't respected when creating a new user. Validation defaults back to pass-min-length of 12.
    • This usecase should be prevented, if possible.

Screenshots: Passed

Screenshot 2023-01-23 at 11 45 14 AM

Passed

Screenshot 2023-01-23 at 11 45 30 AM

Passed

Screenshot 2023-01-23 at 11 45 47 AM

Questionable

Screenshot 2023-01-23 at 11 46 01 AM

Questionable

Screenshot 2023-01-23 at 12 00 28 PM

Failed

Screenshot 2023-01-23 at 11 47 35 AM
cnotv commented 1 year ago

@Josh-Diamond , as we do not have a clear requirement for the purpose, if you may have any other idea in mind, please share ahead earlier so we can add it directly as case. Thanks for finding out the exceptions so far 😅

Contemplated cases so far have been also:

As mentioned in the chat, the possibility to have this value for new users passwords should be validated by the server and would then require an issue for rancher/rancher.

cnotv commented 1 year ago

@Josh-Diamond waiting for some updates from the rancher/rancher repo to find out why the setting is not honored.

Josh-Diamond commented 1 year ago

@cnotv Are you waiting for some information or input from me? I'm slightly confused as to how this would be related to backend. If setting global setting > password-min-length to 11, this is accepted and valid, however setting this value to 11.0 through dashboard is accepted but invalid and is not truly honored.

I believe the change I'm requesting would be from dashboard side to remove or prevent the use of . in this input field.

Am I off base here? Should this prevention be implemented server-side? Is that what you are suggesting? And to let the dashboard just display the validation error coming from the server?

cnotv commented 1 year ago

The decimal with .0 has been fixed with a newer PR. Sorry for the back and forth, as we did not think about all the possible cases in a first implementation.

The value is still not honored. If you try to set requirement min password to 20, then you update a password with length 3, it will accept. Previously this worked.

I've been asking to the BE information about this but the only answer I received is that it works on CLI.

If there's no other issue UI wise, I would rather close this and create a BE issue on rancher/rancher, what do you think?

Josh-Diamond commented 1 year ago

Ticket #6633 - Test Results - ✅

With Docker on a single-node instance:

Verified on rancher v2.7-3c1e7dbaafe9c38629898273974b97ec6e8fd4da-head:

  1. Fresh install of rancher v2.7-head
  2. Navigate to global settings > password-min-length, and set to 11.0
  3. Verified - validation prevents the use of .; as expected
  4. Sanity checked using negative values, values outside the range of 2-256, and values including e; all failed to be accepted as valid; as expected