Improve Security / UX of private pages using a shared password

[RealOrangeOne]

Issue Summary

Wagtail's page privacy feature could be improved to communicate better to the user and provide ways for Wagtail installations to fine tune how the 'shared password' is created.

Is your proposal related to a problem?

Wagtail allows pages to be password protected as part of a feature called 'private pages'.

An admin can specify a password, which an end user must enter when visiting a page to view the page. The same functionality exists for documents. This feature was only really intended for low-security applications.

Whilst the implementation is fundamentally secure, is has a few small weaknesses:

1. The password is stored in the database in plain text. This allows admin users to see what the password is set to, but also means a database leak also exposes the password's values.
2. There are no strength checks for the password, meaning a user may use a weak password to protect sensitive content.
3. It is not explicitly clear to the user that this is not a secure option, it is presented to be similar to the more secure auth (login) based private access.
4. The term password can be be inferred to be 'secure' when it is not.

Aside: CVE-2020-11037 ensures that verification of the password is not vulnerable to a timing attack.

Because the password validation implementation is secure, it's not possible to exploit any of the above weaknesses to bypass authentication, however people may be using this feature incorrectly, and so improvements should be made.

Describe the solution you'd like

The following items may be split to standalone issues for easier collaboration.

• Refine the UX so it can be better communicated that the 'shared password' is not the most secure option. Update wording and help text for private pages using shared password #11535
• Add the ability to opt out of the ability for shared password access to be used across the admin. Add the ability to disable usage of shared password for page privacy #11536
• Maybe. Add the ability to remove other options in the privacy modals and disable all together. Add the option to disable all page-view restriction options, sitewide #11640
• Add the ability to generate a randomised/unique value for the shared password. Add ability to auto-generate a random/unique value for Private access shared passwords #11537
• Add internal code comments with clarity that the 'password' is not the same as a full security password, maybe even updating internal field names to shared_password to alleviate future concerns.
• Add the ability to have a password validators configured (e.g. min length) leveraging the Django password validation system.

Additional context

• Thanks to @joliveira98 for bring this issue to our attention.
• Rename settings PASSWORD_REQUIRED_TEMPLATE & DOCUMENT_PASSWORD_REQUIRED_TEMPLATE #11368 covers the change of the password form for password protected pages to have a WAGTAIL_ prefixed setting name.
• Documentation https://docs.wagtail.org/en/stable/advanced_topics/privacy.html & https://docs.wagtail.org/en/stable/topics/permissions.html#image-document-permissions

[RealOrangeOne]

My suggestions for fixing:

1. Add opt-out functionality for validating the strength of the passwords used to protect pages, using Django's existing password validators. This helps protect users from using weak passwords to protect their pages.
2. Add a warning to users that password protection is only designed for low-security applications, and that authenticated user permissions are far more secure.

If we want a more thorough fix, we could:

3. Store the password hashed, along with the name of the person who set the password. This doesn't allow password retrieval, but at least shows an admin who to ask for its current value.

Personally, I think it's worth doing the full solution, as adding point 3 would remove the need for point 2. If we're going to ship a feature, we should make sure it's as secure as possible by default, whilst protecting users from a potential foot-gun.

Happy to create a patch if that makes sense to others.

[zerolab]

Store the password hashed, along with the name of the person who set the password. This doesn't allow password retrieval, but at least shows an admin who to ask for its current value.

We already capture who updates the privacy options in the audit log. In the screenshot, I added a password, then updated it.

[allcaps]

The feature isn't very secure and not intended to be used in situations that should be very secure. Therefore, the help text warning (point 2) is the most important. We should add it to the user interface (for content editors) and to the Wagtail documentation (for developers).

Hashing the password could contribute to the false impression that this feature is secure. Do we want to go down this road?

Maybe we should not hash the password, but make the intended use-case even more clear. Maybe:

• Rename 'Password' to 'Shared secret'
• Have a reveal option, and display the shared secret in the interface.
• Something else?

This way, everybody could have the correct understanding of the intended use-case and implications. And don't miss use the feature for situations it isn't designed for.

[lb-]

Great idea Coen, I think that clear communication and terminology could really help here.

[RealOrangeOne]

The feature isn't very secure

Why? It follows Wagtail's existing permissions model for pages (which ought to be secure), collects and compares passwords in a secure way, and correctly restricts access. Besides how the password is stored (and the lack of strength requirements), I'm not sure I see how it "isn't very secure"?

[allcaps]

Why?

AFAIK, the intended use case is to share the password between multiple people. I've never encountered anyone using this feature in any other way. And: shared passwords are insecure.

I'm sure Python/Django/Wagtail code doing the permission checks is fine. To me the plain password storage fits the use-case. Password is just the wrong name for the field. Maybe: shared secret?

This shared password feature is the lightest form of 'security'. Hashing the password is easy, but I'm against it, as it doesn't fit the intended use-case.

It would be better to reword and show the password in the admin interface. This would make it clear that, if a developer needs something better, they have to use one of the other options, or roll their own.

[lb-]

I agree with @allcaps - I think we should opt for a different name for the field and maybe add some documentation.

If we want to enhance this further, we could add a button that generates a really long unique string. It's still a shared secret, insecure, but it makes it slightly easier for users to make this secret more complex.

This could make the behaviour a bit closer to something like a public link for a Google Document.

We could even use some of their wording about how 'anyone with this link can access this page'.

[thibaudcolas]

Same as @allcaps and @lb-.

• As a baseline – "Shared secret" as a name, and the "Anyone with this secret can access the page. " as a user-friendly explanation of the behavior that matches what other tools do.
• If we want to increase the likelihood of good secrets being used, a "[Button]\ Generate a pass-phrase" as discussed
• And also having a way to disable this option for sites that don’t want to allow this approach.

[lb-]

Discussed this today at core team. LB to update the issue description & feedback on the PR accordingly.

1. 'Shared password' should be the field label
2. re-order the options so that it goes from least secure to most secure (move logged in users after the Shared password)
3. Better help text to advise that this is not secure
4. Add a comment in the code that this is called password but is not the same as a user password (to avoid concerns in the future)
5. De-scope the code generator & setting to disable this feature for now, can be done as a future enhancement (I will create two separate issues for that).

[lb-]

OK - I have updated this issue's description to hopefully align with the agreed items.

I have created three new issues:

• Refine the UX so it can be better communicated that the 'shared password' is not the most secure option. Update wording and help text for private pages using shared password #11535
• Add the ability to opt out of the ability for shared password access to be used across the admin. Add the ability to disable usage of shared password for page privacy #11536
• Add the ability to generate a randomised/unique value for the shared password. Add ability to auto-generate a random/unique value for Private access shared passwords #11537

And left two tasks as standalone on this issue (we may opt to do/not do these or split these later).

• Add internal code comments with clarity that the 'password' is not the same as a full security password, maybe even updating internal field names to shared_password to alleviate future concerns.
• Add the ability to have a password validators configured (e.g. min length) leveraging the Django password validation system.

[lb-]

Just added #11640 to the main issue description, a new enhancement suggestion has come up to allow either disabling privacy features completely or having even more control over them.

Input welcome on that issue. cc @RealOrangeOne