New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[enh] Automatically check for weak password #196

Merged
merged 33 commits into from Nov 4, 2018

Conversation

Projects
None yet
5 participants
@Psycojoker
Member

Psycojoker commented Nov 28, 2016

EDIT: LJF

The problem

Some administrator (like CHATONS) could want to force user to choose good password.

Solution

With this solution administrator can choose the level of password he wants for 'admin' and for 'user' profile.

Level of password:
-1 disabled all check
0 Just give advice if the password is weak
1 Don't accept password with 6 letters or listed in 100000 list of password
2 At least 8 with upper case and numeric
3 With special characters in more
4 12 characters

How to test

apt install python-cracklib
mkdir -p /usr/local/share/dict/cracklib/
cp /vagrant/yunohost/data/other/password/* /usr/local/share/dict/cracklib/

PR Status

Ready to be reviewed.
Related PR YunoHost/SSOwat#104

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :
@julienmalik

This comment has been minimized.

Member

julienmalik commented Nov 28, 2016

I would still like to see something like this in Yunohost.
But this needs thinking at a wider scale. password can be changed from several places (cli, admin, sso), and I would prefer to opt for a common strategy (a single place of code to maintain).

@julienmalik julienmalik force-pushed the cracklib branch from 074870f to 0ec81b4 Nov 28, 2016

@julienmalik

This comment has been minimized.

Member

julienmalik commented Nov 28, 2016

Just rebased on current unstable

@alexAubin alexAubin changed the title from Cracklib to [enh] Automatically check for weak password with cracklib Dec 7, 2016

@M5oul M5oul added this to the Horizon milestone Dec 11, 2016

yunohost-bot pushed a commit to yunohost-bot/yunohost that referenced this pull request Dec 13, 2016

@alexAubin

This comment has been minimized.

Member

alexAubin commented Dec 20, 2016

I think this is an interesting feature that should definitely come sometime soon. I'd vote to put this in milestone 2.6.x.

@zamentur

I have tested the creation of a user it seems working well.
But my be we should implement this feature directly in moulinette for all password ?

@Psycojoker Psycojoker modified the milestones: 2.6.x, Horizon Feb 3, 2017

@M5oul M5oul modified the milestones: 2.7.x, 2.6.x Mar 7, 2017

@julienmalik

This comment has been minimized.

Member

julienmalik commented Mar 1, 2018

Just saw nextcloud implemented something interesting : a check against the HaveIBennPwned db

See https://help.nextcloud.com/t/nextcloud-will-check-passwords-against-database-of-haveibeenpwned/28110 (there's a link to the pr in it)

@zamentur zamentur changed the base branch from unstable to stretch-unstable Aug 26, 2018

@alexAubin alexAubin referenced this pull request Oct 25, 2018

Merged

Cracklib enh #567

@alexAubin alexAubin modified the milestones: Horizon, 3.x Oct 25, 2018

zamentur and others added some commits Oct 25, 2018

alexAubin added some commits Oct 27, 2018

@alexAubin alexAubin changed the title from [enh] Automatically check for weak password with cracklib to [enh] Automatically check for weak password Oct 31, 2018

@alexAubin alexAubin modified the milestones: 3.x, 3.2.x, 3.3.x Oct 31, 2018

@alexAubin

This comment has been minimized.

Member

alexAubin commented Oct 31, 2018

Discussing this with ljf, we ended up concluding that cracklib is too nazi and doesn't offer much flexibility with regard to how the "being in the list of most used password" is done. In particular, it detects passwords that are close to password in the list ... which ends up being annoying because legitime passwords trigger the check.

Simply using a list from https://github.com/danielmiessler/SecLists/tree/master/Passwords/Common-Credentials and grep turns out to be much simpler and at least we have full control on what's done and the outcome is much clearer.

For simplicity and to limit frustration of new users/admin, we also concluded that it was better to (for now) set the default constrains as "at least 8 characters" (because we can't simply add the constrain of numbers+upper/lowercase+special chatacters, some people use 20+ long password which don't necessarily fit those constrains but can be considered secure anyway)

@alexAubin

So that's good for me ... ideally it would be nice to merge this soon

Note that those PR are related to this :
YunoHost/SSOwat#104
YunoHost/yunohost-admin#212
YunoHost/moulinette#175

@alexAubin alexAubin merged commit fe452a6 into stretch-unstable Nov 4, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@alexAubin alexAubin deleted the cracklib branch Nov 4, 2018

Show resolved Hide resolved src/yunohost/tools.py
Show resolved Hide resolved src/yunohost/user.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment