Bug 45640 - Readability of autogenerated passwords
Readability of autogenerated passwords
Status: CLOSED FIXED
Product: UCS@school
Classification: Unclassified
Component: General
UCS@school 4.2
Other Linux
: P5 normal (vote)
: UCS@school 4.2 v9
Assigned To: Daniel Tröder
Jürn Brodersen
:
: 30997 (view as bug list)
Depends on:
Blocks: 46317 46462 46923
  Show dependency treegraph
 
Reported: 2017-11-03 09:26 CET by Sönke Schwardt-Krummrich
Modified: 2018-06-04 15:34 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 5: Major Usability: Impairs usability in key scenarios
Who will be affected by this bug?: 4: Will affect most installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.229
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): External feedback, Usability
Max CVSS v3 score:


Attachments
Example for optical similarities (4.68 KB, image/png)
2017-11-03 09:26 CET, Sönke Schwardt-Krummrich
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Sönke Schwardt-Krummrich univentionstaff 2017-11-03 09:26:45 CET
Created attachment 9270 [details]
Example for optical similarities

A customer complained about the autogenerated passwords. User lists including passwords are handed out to teachers which print them out on their own.

Frequently, the fonts used by teachers differ optically not sufficiently between the characters "|", "I", "i", "l" and "1" and "O" and "0".
(see attached screenshot).

The password generator code should skip these characters and make the passwords a little longer to compensate the smaller character set.
Comment 1 Sönke Schwardt-Krummrich univentionstaff 2017-11-03 09:28:59 CET
The similarities lead to a higher number of support cases.
Comment 2 Michael Grandjean univentionstaff 2018-01-18 14:31:25 CET
*** Bug 30997 has been marked as a duplicate of this bug. ***
Comment 3 Sönke Schwardt-Krummrich univentionstaff 2018-01-22 10:14:30 CET
During last UCS@school meeting with prof services, it has been reported that at least 3-4 customers stumbled upon this issue.
Comment 4 Daniel Tröder univentionstaff 2018-02-14 15:15:27 CET
To make reading autogenerated passwords easier several changes have been done:

* Removed 11 difficult to type or distinguish characters from the default special character list: @ # _ + = : , ; / ( )
The default special character list is now: $ % & * - + = : . ?

* There will now be no more than 20% special characters in a password. In a password of length 4-9 there will be <=1, with 10-14 chars there will be <=2, with 15-19 chars there will be <=3 etc.

* The following characters will not be in any password: i I l L o O 0 1

* A passwords always starts with a letter.


[4.2 53c8ed80] Bug #45640: enhance readability of autogenerated passwords
[4.2 7a2969d0] Bug #45640: changelog, advisory
[4.2 893339dc] Bug #45640: advisory update

ucs-school-lib (10.0.2-10)

[4.3 83d6bb73] Bug #45640: enhance readability of autogenerated passwords
[4.3 d0e1dc96] Bug #45640: changelog

ucs-school-lib (11.0.1-2)
Comment 5 Jürn Brodersen univentionstaff 2018-02-15 11:28:03 CET
If the password length is smaller than four, pw might not have any ascii letters.
'''
# start with a letter
while not pw[0] in string.ascii_letters:
	shuffle(pw)
'''

I suggest to generate pw with length-1 and use something like:
'''
pw = choice(lowercase + uppercase) + shuffle(pw)
'''
Note: that will not work for four letter passwords because of the MS  requirements...

As an alternative we could raise an exception if the password length should be less than four. But I still think we shouldn't depend on shuffle to choose an ascii letter as the first letter.


Also passwords with less than four letters will never have a special character. Because that would be more than 20%. Is that intended?
Comment 6 Daniel Tröder univentionstaff 2018-02-15 15:28:49 CET
(In reply to Jürn Brodersen from comment #5)
> If the password length is smaller than four, pw might not have any ascii
> letters.
> '''
> # start with a letter
> while not pw[0] in string.ascii_letters:
> 	shuffle(pw)
> '''
> 
> I suggest to generate pw with length-1 and use something like:
> '''
> pw = choice(lowercase + uppercase) + shuffle(pw)
> '''
Good catch, suggestion applied.

> Note: that will not work for four letter passwords because of the MS 
> requirements...
Nothing matters with 4 chars. Password is as secure as no password.

Anyway, with 3 chars MS requirements are now still met.

> As an alternative we could raise an exception if the password length should
> be less than four. But I still think we shouldn't depend on shuffle to
> choose an ascii letter as the first letter.
An assertion error is now raised, if the length < 1.

> Also passwords with less than four letters will never have a special
> character. Because that would be more than 20%. Is that intended?
Yes, see above.


[4.2 321ed952] Bug #45640: safer handling of passwords with less than 4 chars
[4.2 71ce17a5] Bug #45640: changelog
[4.3 b0bc4d7f] Bug #45640: safer handling of passwords with less than 4 chars
               (cherry picked from commit 321ed952)
[4.3 9fe183f2] Bug #45640: changelog
[4.2 5d41dff8] Bug #45640: advisory

ucs-school-lib (10.0.2-11)
ucs-school-lib (11.0.1-3)
Comment 7 Jürn Brodersen univentionstaff 2018-02-15 17:21:58 CET
I agree with you on four or less letter passwords.

Notes:
For length=3 you could now get up to two special chars (66%), because "if specials and length:" doesn't check for specials_allowed.
You could specify univentionPWLength=0 as a policy. Which results in two letter passwords. Better than no password I guess...

What I tested so far:
'''
from ucsschool.lib.models import utils
utils.create_passwd()
utils.create_passwd(length=12)
'''
Creates passwords -> OK

I will wait for the jenkins tests tomorrow. But the patch looks good :)
Comment 8 Daniel Tröder univentionstaff 2018-02-16 10:56:04 CET
To many small errors, this needs thorough testing.

[4.2 298736d5] Bug #45640: fix special chars count for length=3
[4.2 5a343696] Bug #45640: add test for password generation
[4.2 3352f77c] Bug #45640: changelog
[4.2 9336414c] Bug #45640: advisory update
[4.3 505da9da] Bug #45640: fix special chars count for length=3
[4.3 8f695060] Bug #45640: add test for password generation
[4.3 5bcdd383] Bug #45640: changelog

ucs-school-lib (10.0.2-12)
ucs-test-ucsschool (4.0.4-60)
ucs-school-lib (11.0.1-4)
ucs-test-ucsschool (5.0.2-7)
Comment 10 Daniel Tröder univentionstaff 2018-02-19 14:23:52 CET
These commits should have been for Bug #46317, to which the failed tests can be attributed:

[4.2 314d3e69] Bug #45640: rewrite self._unique_ids after school move
[4.2 783dd60b] Bug #45640: changelog

[4.2 21473581] Bug #45640: create required attributes
[4.2 046f6454] Bug #45640: fix typo
[4.2 60e1f7a0] Bug #45640: don't fail on slaves
[4.2 28bc2beb] Bug #45640: changelog

& merge to 4.3

ucs-school-import (15.0.3-25)
ucs-test-ucsschool (4.0.4-62)
Comment 11 Jürn Brodersen univentionstaff 2018-02-21 17:51:11 CET
Ok
I changed the test exposure to dangerous because udm objects are created and removed.

-> Verified
Comment 12 Sönke Schwardt-Krummrich univentionstaff 2018-02-27 17:19:04 CET
The test 227_import-users_create_password currently fails on all machines with UCS 4.3-0:

http://jenkins.knut.univention.de:8080/job/UCSschool-4.3/job/Upgrade%20Singleserver/lastCompletedBuild/Config=s4,TestGroup=import2/testReport/90_ucsschool/227_import-users_create_password/test/

Standard Ausgabe (STDOUT)

[2018-02-26 22:54:34.302663] Checking default password length...
[2018-02-26 22:54:34.302846] ### FAIL ###
[2018-02-26 22:54:34.302910] Password 'LYx;n1Xu' with length 8 contains forbidden character 'L'.
[2018-02-26 22:54:34.302964] ###      ###


Please recheck the UCS@school jenkins results for 4.2 and 4.3 tomorrow!
Comment 13 Daniel Tröder univentionstaff 2018-02-28 11:27:09 CET
The software on the Jenkins machine is not up2date:

Old package from 4.2:
[master206-update-single-s4] 2018-02-27T14:59:05.773000	Installation of UCS@school packages - Entpacken von ucs-school-import (15.0.3-20A~4.2.0.201712181236) ...

When upgrading to 4.3:

[master206-update-single-s4] 2018-02-27T16:10:08.970061	Reading package lists...
[master206-update-single-s4] 2018-02-27T16:10:09.655005	W: The repository 'https://appcenter-test.software-univention.de/univention-repository/4.3/maintained/component ucsschool_20180112151618/all/ Release' does not have a Release file.
[master206-update-single-s4] 2018-02-27T16:10:09.655005	W: The repository 'https://appcenter-test.software-univention.de/univention-repository/4.3/maintained/component ucsschool_20180112151618/amd64/ Release' does not have a Release file.
[master206-update-single-s4] 2018-02-27T16:10:09.655005	W
[master206-update-single-s4] 2018-02-27T16:10:09.670019	: The repository 'https://appcenter-test.software-univention.de/univention-repository/4.2/maintained/component ucsschool_20180112151618/all/ Release' does not have a Release file.
[master206-update-single-s4] 2018-02-27T16:10:09.670019	W: The repository 'https://appcenter-test.software-univention.de/univention-repository/4.2/maintained/component ucsschool_20180112151618/amd64/ Release' does not have a Release file.
[master206-update-single-s4] 2018-02-27T16:10:09.670447	W
[master206-update-single-s4] 2018-02-27T16:10:09.670666	: 
[master206-update-single-s4] 2018-02-27T16:10:09.670926	Conflicting distribution: http://updates-test.software-univention.de/4.3/maintained/component ucsschool_DEVEL/all/ Release (expected ucsschool_DEVEL/all/ but got 4.3/maintained/component/ucsschool_DEVEL/all)
[master206-update-single-s4] 2018-02-27T16:10:09.671299	W
[master206-update-single-s4] 2018-02-27T16:10:09.671510	: 
[master206-update-single-s4] 2018-02-27T16:10:09.671741	Conflicting distribution: http://updates-test.software-univention.de/4.3/maintained/component ucsschool_DEVEL/amd64/ Release (expected ucsschool_DEVEL/amd64/ but got 4.3/maintained/component/ucsschool_DEVEL/amd64)


And thus the UCS@school packages from 4.3 are not installed/upgraded.
Comment 14 Jürn Brodersen univentionstaff 2018-02-28 11:32:24 CET
Afaik that's bug 45950.

The workaround I'm currently using, is adding "[trusted=yes]" between deb and url in the appropriate apt source file.
Comment 15 Sönke Schwardt-Krummrich univentionstaff 2018-03-02 13:43:27 CET
Daniel, Jürn: please update the advisory and give the customer a short summary what has changed in password generation (some characters are skipped, ...).
Comment 16 Daniel Tröder univentionstaff 2018-03-02 14:49:16 CET
[4.2 ff9391124] Bug #45640: improve advisory
Comment 17 Jürn Brodersen univentionstaff 2018-03-02 15:31:47 CET
OK
[4.2 fb16555c] Bug #45640: fix typo in yaml
-> Verified
Comment 18 Sönke Schwardt-Krummrich univentionstaff 2018-05-02 17:52:52 CEST
UCS@school 4.2 v9 has been released.

https://docs.software-univention.de/changelog-ucsschool-4.2v9-de.html

If this error occurs again, please clone this bug.