Skip to content
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

ServerDB: use PasswordGenerator class for generating initial SuperUser password. #2891

Merged
merged 1 commit into from Mar 2, 2017

Conversation

@mkrautz
Copy link
Member

commented Feb 28, 2017

This replaces our ad-hoc password generator in ServerDB for generating
the initial SuperUser password.

The original implementation had several problems, including use of a
non-cryptographically secure random number generator, as well as problems
with modulo bias.

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2017

Depends on #2890 and includes its commits for now.
They will need to be removed before merging.

@Kissaki
Copy link
Member

left a comment

Otherwise fine.

while (length--)
pw.append(QChar(qrand() % 94 + 33));

QString pw = PasswordGenerator::generatePassword(12);

This comment has been minimized.

Copy link
@Kissaki

Kissaki Feb 28, 2017

Member

Magic number 12 should be a (class) constant.

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 1, 2017

Author Member

Hmmm. Quite honestly, I think it would hurt readability if we moved the password length too far away from the call site.

I'm OK with making it a constant here though, so it'd be become something like:

const in passwordLength = 12;
QString pw = PasswordGenerator::generatePassword(passwordLength);

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 1, 2017

Author Member

WDYT?

This comment has been minimized.

Copy link
@mkrautz

mkrautz Mar 1, 2017

Author Member

Implemented my example.

@mkrautz mkrautz force-pushed the mkrautz:murmur-use-passwordgenerator branch 2 times, most recently from 40ab090 to fedd1db Mar 1, 2017

@Kissaki
Kissaki approved these changes Mar 1, 2017
ServerDB: use PasswordGenerator class for generating initial SuperUse…
…r password.

This replaces our ad-hoc password generator in ServerDB for generating
the initial SuperUser password.

The original implementation had several problems, including use of a
non-cryptographically secure random number generator, as well as problems
with modulo bias.

@mkrautz mkrautz force-pushed the mkrautz:murmur-use-passwordgenerator branch from fedd1db to 218eb18 Mar 2, 2017

@mkrautz mkrautz merged commit 9ae2a7f into mumble-voip:master Mar 2, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.