#1503 PR merged: Improve encrypted password detection in 500_ssh.sh

Labels: enhancement, fixed / solved / done

N3WWN opened issue at 2017-09-19 13:26:

Currently, usr/share/rear/rescue/default/500_ssh.sh can only detect if SSH_ROOT_PASSWORD is MD5 encrypted. Any string other than those with a '$1$' prefix will be encrypted using MD5. For instance, SHA-512 encrypted passwords are common and are identified with a '$6$' prefix... but would not be detected as an encrypted password and rear would re-encrypt the already encrypted string using MD5, breaking the ability to use intended password.

Per the discussion with @schlomo and @jsmeix in https://github.com/rear/rear/pull/1489#issuecomment-329745987 , this PR improves encrypted password detection.

N3WWN commented at 2017-09-19 13:38:

CI build is failing because bash is not called with -O extglob:

0.69s$ make validate
== Validating scripts and configuration ==
find etc/ usr/share/rear/conf/ -name '.conf' | xargs -n 1 bash -n
bash -n usr/sbin/rear
find . -name '
.sh' | xargs -n 1 bash -n
./usr/share/rear/rescue/default/500_ssh.sh: line 55: syntax error near unexpected token (' ./usr/share/rear/rescue/default/500_ssh.sh: line 55: $[0-9]?([a-z])$*) echo "root:$SSH_ROOT_PASSWORD:::::::" > $ROOTFS_DIR/etc/shadow ;;'
make: *** [validate] Error 123

I can replicate the failure on my dev system:

[root@OLC-1372-centos7 rear]# bash -n usr/share/rear/rescue/default/500_ssh.sh ; echo $?
usr/share/rear/rescue/default/500_ssh.sh: line 55: syntax error near unexpected token (' usr/share/rear/rescue/default/500_ssh.sh: line 55: $[0-9]?([a-z])$*) echo "root:$SSH_ROOT_PASSWORD:::::::" > $ROOTFS_DIR/etc/shadow ;;'
2

By adding -O extglob, the test passes:

[root@OLC-1372-centos7 rear]# bash -O extglob -n usr/share/rear/rescue/default/500_ssh.sh ; echo $?
0
[root@OLC-1372-centos7 rear]#

Is there anything I can do (or need to do) to repair this?

schlomo commented at 2017-09-19 13:39:

Good catch, please add the extglob and the nullglob options to the test. We set them in ReaR so they should be also set for the test.

N3WWN commented at 2017-09-19 13:50:

Thanks @schlomo ! make validate shopt_options updated and CI build is now passing.

jsmeix commented at 2017-09-19 14:11:

@N3WWN
via
https://github.com/rear/rear/pull/1503/commits/f28df4ba41f920fe33f5aaac65ed94323526a22c
I dared to enhance the syntax a bit to better match
https://github.com/rear/rear/wiki/Coding-Style
Please have a look whether it is still o.k. for you.

jsmeix commented at 2017-09-19 14:14:

@N3WWN
If you agree with my syntax enhancements
I would like to merge it soon.

N3WWN commented at 2017-09-19 14:36:

Thanks for the polishing @jsmeix ! 😁 Looks good!

I've read (and reference) the Coding Style document several times, but some of the items fall don't stick in my head. I'll try to adhere better in the future.

jsmeix commented at 2017-09-19 14:58:

@N3WWN
many thanks for this nice enhancement!

Regarding coding syntax style guides:
No need to take coding syntax style guides too serious.
As long as your code is easy to understand everything
is o.k. - this is about coding semantics and that does matter.
All the coding syntax style guides are just nice to have.


[Export of Github issue for rear/rear.]