Bug 25622

Created attachment 105436 [details] [review]
Bug 25622: Use special chars in DB password (koha-create)

On bug 23250 we decided to generate a password without special chars then add a
'@' at the end to comply with MySQL policy.
That is wrong, we should handle correctly the special chars we don't want to be
part of the password.

Test plan:
1.
mysqlpwd="$(pwgen -s -y 32 1)"
echo $mysqlpwd
mysqlpwd="$(echo $mysqlpwd | tr -d :\'\&\\\\\<\>\/ | cut -c1-16)"
echo $mysqlpwd

Confirm that in the second line you don't see one of the following chars : ' & \ < > /

2.
Copy from src and edit /usr/sbin/koha-create to add an echo $mysqlpwd
Create several instances, like:
koha-create --create-db x
koha-create --create-db xx
koha-create --create-db xxx
...

When you see a password with a special chars, do:
koha-shell xxx
grep '<pass>' $KOHA_CONF
And make sure the password does not contain "__DB_PASS__"

Created attachment 105504 [details] [review]
Bug 25622: Use special chars in DB password (koha-create)

On bug 23250 we decided to generate a password without special chars then add a
'@' at the end to comply with MySQL policy.
That is wrong, we should handle correctly the special chars we don't want to be
part of the password.

Test plan:
1.
mysqlpwd="$(pwgen -s -y 32 1)"
echo $mysqlpwd
mysqlpwd="$(echo $mysqlpwd | tr -d :\'\&\\\\\<\>\/ | cut -c1-16)"
echo $mysqlpwd

Confirm that in the second line you don't see one of the following chars : ' & \ < > /

2.
Copy from src and edit /usr/sbin/koha-create to add an echo $mysqlpwd
Create several instances, like:
koha-create --create-db x
koha-create --create-db xx
koha-create --create-db xxx
...

When you see a password with a special chars, do:
koha-shell xxx
grep '<pass>' $KOHA_CONF
And make sure the password does not contain "__DB_PASS__"

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

pwgen has a --remove-chars option that seems to do exactly what you want to do with the tr command.

Try this:
    pwgen -s -y -r ":'&\\<>/" 16 1

Created attachment 105626 [details] [review]
Bug 25622: use -r paramter to blacklist some special chars

(In reply to Julian Maurice from comment #3)
> pwgen has a --remove-chars option that seems to do exactly what you want to
> do with the tr command.
> 
> Try this:
>     pwgen -s -y -r ":'&\\<>/" 16 1

Thanks Julian, that's definitely the way to go!

Created attachment 105779 [details] [review]
Bug 25622: Use special chars in DB password (koha-create)

On bug 23250 we decided to generate a password without special chars
then add a '@' at the end to comply with MySQL policy.
That is wrong, we should handle correctly the special chars we don't
want to be part of the password.

Test plan:
1.
mysqlpwd=$(pwgen -s -y -r ":'&\\<>/" 16 1)
echo $mysqlpwd

Confirm that you don't see one of the following chars : ' & \ < > /

2.
Copy from src and edit /usr/sbin/koha-create to add an echo $mysqlpwd
Create several instances, like:
koha-create --create-db x
koha-create --create-db xx
koha-create --create-db xxx
...

When you see a password with a special chars, do:
koha-shell xxx
grep '<pass>' $KOHA_CONF
And make sure the password does not contain "__DB_PASS__"

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

I squashed the patches, and removed the comment about the previous method (pwgen 32 | tr | cut)

Hey team, this doesn't work for me.

-r is not a valid option for pwgen on stretch, jessie or Ubuntu 16.04.

pwgen (2.08-1)
  * Add new option --remove-chars which removes chars from the set of
    characters used to generate passwords (Closes: #836334)

Buster has it, Stretch has 2.07-1

Good catch Aleisha!

We are postponing this one.

We don't care about Stretch anymore. Can we give it a new QA round please?

> Insecure MySQL password generated by koha-create

How is that insecure currently? It's 89 bit strong.
python: import math ; math.log((26+26+10)**15, 2)
It's totally impractical for online bruteforcing (even from localhost). And for offline attack on the file where the password is stored, assuming it's hashed (otherwise the complexity of the password doesn't matter) it's totally impractical also.
Digits + lowercase + uppercase on 15 chars (length matters the most) is super strong.

Assuming the special symbol pool is 20 (it looks less than that), we go to 95 bits.
And we go from 15 chars to 16. Which ups from 95 to 102 bits. We see that +1 length had at least the same impact as using specials chars.
Changing only the length from 15 to 20 gives us 119 bit strength.

I can still QA that[1] because it doesn't hurt.
Even though it's overkill and doesn't change much the strength. It's an example of how password strength is unintuitive.

Here is an online calculator, it's in french but guessing the meaning should be doable (Longueur = length)
https://www.ssi.gouv.fr/administration/precautions-elementaires/calculer-la-force-dun-mot-de-passe/
It's the national IT security agency, on the same page they recommend 82 bits for secure passwords. And currently we are 128 times (7 bits) above that.

[1] Available versions are good, no issue in sight https://repology.org/project/pwgen/versions

(In reply to Victor Grousset/tuxayo from comment #11)

Do you think adding the same special character at the end of a generated password is the way to go? We want to generate a 16 characters length password, this is what this patch is doing.
If you think the bug title is misleading, feel free to adjust it.

(In reply to Jonathan Druart from comment #12)
> Do you think adding the same special character at the end of a generated
> password is the way to go? 

That's a valid way to deal with mandatory rules such as the one of mysql.

> We want to generate a 16 characters length
> password, this is what this patch is doing.

Indeed it's also a valid to do it now that we have the right pwgen version in the relevant versions of distros.

> If you think the bug title is misleading, feel free to adjust it.

Let's move on with that. I'm not much inspired but at least that removes the idea that it's a needed security change.

Created attachment 136559 [details] [review]
Bug 25622: Use special chars in DB password (koha-create)

On bug 23250 we decided to generate a password without special chars
then add a '@' at the end to comply with MySQL policy.
That is wrong, we should handle correctly the special chars we don't
want to be part of the password.

Test plan:
1.
mysqlpwd=$(pwgen -s -y -r ":'&\\<>/" 16 1)
echo $mysqlpwd

Confirm that you don't see one of the following chars : ' & \ < > /

2.
Copy from src and edit /usr/sbin/koha-create to add an echo $mysqlpwd
Create several instances, like:
koha-create --create-db x
koha-create --create-db xx
koha-create --create-db xxx
...

When you see a password with a special chars, do:
koha-shell xxx
grep '<pass>' $KOHA_CONF
And make sure the password does not contain "__DB_PASS__"

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Code looks good, passing QA :)

This should be moved out of the security project.
There is no actual security concern (comment 11) but we didn't notice it earlier. This doesn't make any harm and replaces a hardcoded "@" concatenation in the generated password which could look like a hacky workaround. So let's go with this.

Not worth backporting IMHO. And certainly not to 21.05 since Ubuntu 16.04 is supported by it.

Is this a security bug or just a security enhancement?

I would say it's an enhancement really myself

Pushed to master for 22.11.

Nice work everyone, thanks!

Backported to 22.05.x for 22.05.04

pushed to 21.11.x for 21.11.11

Not backported to oldoldstable (21.05.x). See comment 15

Nothing to document, marking resolved.