Flyspray - The bug killer!

  • Status Unconfirmed
  • Percent Complete
    0%
  • Task Type Bug Report
  • Category Backend/Core
  • Assigned To No-one
  • Operating System Linux
  • Severity Medium
  • Priority Low
  • Reported Version 1.0-rc
  • Due in Version Undecided
  • Due Date Undecided
  • Votes 1
  • Private
Attached to Project: Flyspray - The bug killer!
Opened by Dominik Meyer - 07.12.2016
Last edited by peterdd - 23.01.2017

FS#2320 - crypt() Raise E_NOTICE security warning if salt is omitted.

Hello together,

in php >= 5.6.0 you get a notice if you dont use a salt param.
(see: http://php.net/manual/en/function.crypt.php)

we need to fix the else case in the function Flyspray::cryptPassword()

see the attached image for the error in the frontend.

Dominik Meyer commented on 07.12.2016 12:31

hm i can´t edit my own created tasks?

PS i my happy to found flyspray. it´s the only nearly good ticket system in pure php that i found. i hope i can help here in development and maybe more.

Admin
peterdd commented on 07.12.2016 21:01
  1. Currently Modify own tasks and Edit own comments is switched off for group *Reporters* on bugs.flyspray.org
  2. Glad to hear you like Flyspray and new people trying to help are very welcome.

Damn, even the PHP makers have it as example in their documentation:
http://php.net/manual/en/function.crypt.php

// Get the hash, letting the salt be automatically generated
$hash = crypt($password);

:-(

Project Manager
peterdd commented on 23.01.2017 21:17

Notes:

  • Currently Flyspray supports PHP5.3 and newer. password_hash() and password_verify() are available since PHP5.5.0
  • For older versions (PHP5.3-PHP5.4.*) Flyspray has includes/password_compat.php , but it is not used yet.
  • crypt($password) lets the PHP configuration of the webserver decide which algorithm it uses, so server admins can configure the best/most secure parameters.
  • bad that php.net documentation comes with an example that triggers a warning in PHP5.6

Patches welcome.

Arthmoor commented on 15.02.2017 06:02

I've submitted a patch for this (and one other unrelated thing): https://github.com/Flyspray/flyspray/pull/627

One problem, which IMO ought to be a forced issue anyway, is that anyone with an unsalted password will be required to ask for a password reset before logging in via the login form will work. That seems like a relatively small price to pay for ensuring that passwords for the system are in fact as secure as they can be.

Project Manager
peterdd commented on 18.02.2017 08:55

That pull request breaks compatibility, ignoring the passwdcrypt setting.
Not between 1.0-rc4 → rcx releases.

It is the task of admin to decide/communicate a full password reset for everyone.
Suddenly a forced password reset would make me suspiciuos, someone tries to phish me?

How about showing a notice in the admin area about possible pwhash improvements if passwdcrypt setting is md5/sha1/sha512?

What I could imagine is when the admin raises the bars from old md5/sha1 to bcrypt/modern pw hashing algorithms by modifying passwdcrypt setting, the users password hash is upgraded silently at login. (only this direction: bad hashes → better hash)

ps: I'm away for 1 week (skiing)

Project Manager
peterdd commented on 03.03.2017 19:10

I tried your pull request.

  • It seems on one of my linux devices (php5.6.1 / a standard linux distribution) that PASSWORD_DEFAULT uses salted MD5 instead bcrypt. (hash starts with $1$... instead $2y$... ) So force bcrypt if passwdcrypt config setting is empty?

But instead of breaking compatibility between rc4→rc5, lets:

  1. Upgrade the password hash when a user logs in if the passwdcrypt config changed to a better choice (not md5,sha1,sha256) and user has old unsalted hash. (done by me, needs a bit testing before commmit)
  2. Show a warning in admin area if the installation is using deprecated algorithms.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing