FreePBX stores administrator passwords in cleartext in the database

Milestone 2.3 deleted

and MD5sums cant be undone? I think the idea here the old saying " you cant secure a box if you cant secure physical access to it". If the box is physically inacssesable then it wouldn't mater if the password is in plaint text.
On a side note, I havnt seen trixbox 2 yet, but i hope that they moved the password up to the "landing" page (http://yourServerIP/)

MD5SUMs can't easily be reversed - which is why they are a secure way of storing passwords. A good number of applications quite happily say "don't forget your password - we can't get it back" and people accept that. Storing passwords in cleartext simply creates an even bigger security headache for any database manager who needs to grant access to tables to other applications for whatever reason.

moving this to 3.0

To store admin passwords in SHA256 format in the db, I did the following:

1. Increase the password field. I increased to varchar(64), 40 bytes long should be sufficient

2. Modify admin/header_auth.php

# diff -u header_auth.php-orig header_auth.php      
--- header_auth.php-orig        2007-09-28 21:58:59.000000000 +0200
+++ header_auth.php     2007-10-08 10:14:28.000000000 +0200
@@ -22,7 +22,7 @@
                                // not logged in, and have provided a user/pass
                                $_SESSION['AMP_user'] = new ampuser($_SERVER['PHP_AUTH_USER']);
 
-                               if (!$_SESSION['AMP_user']->checkPassword($_SERVER['PHP_AUTH_PW'])) {
+                               if (!$_SESSION['AMP_user']->checkPassword(hash("sha256", $_SERVER['PHP_AUTH_PW']))) {
                                        // failed, one last chance -- fallback to amportal.conf db admin user
                                        if ( (count(getAmpAdminUsers()) == 0) && ($_SERVER['PHP_AUTH_USER'] == $amp_conf['AMPDBUSER']) 
                                          && ($_SERVER['PHP_AUTH_PW'] == $amp_conf['AMPDBPASS'])) {

3. Modify admin/modules/core/functions.inc.php

# diff -u functions.inc.php-orig functions.inc.php
--- functions.inc.php-orig      2007-10-08 10:49:58.000000000 +0200
+++ functions.inc.php   2007-10-08 10:07:16.000000000 +0200
@@ -539,9 +539,10 @@
 /* begin page.ampusers.php functions */
 
 function core_ampusers_add($username, $password, $extension_low, $extension_high, $deptname, $sections) {
+       $password_hash = hash("sha256", $password);
        $sql = "INSERT INTO ampusers (username, password, extension_low, extension_high, deptname, sections) VALUES (";
        $sql .= "'".$username."',";
-       $sql .= "'".$password."',";
+       $sql .= "'".$password_hash."',";
        $sql .= "'".$extension_low."',";
        $sql .= "'".$extension_high."',";
        $sql .= "'".$deptname."',";

Patch file for modifying ampusers table to encrypt password with SHA256

I'm not yet convinced that having these in clear text in the db is that big of a deal. For now I'm deferring this to see if there is more feedback for or against.

$me++. I really dont see how encrypting the admin password is going to stop the DHS from eavesdropping on your calls

That's not the issue. The issue is that if the box, or backups are compromised in any way, it becomes ridiculously trivial to extract passwords and have an unauthorised person access the phone system, particularly if the web UI is exposed on the internet. (not something I'd normally recommend, however there have been occasions where clients have demanded it)

Add to this that many people (myself included) will refuse to use one of my standard passwords on any database where the password isn't encrypted - which in many ways compromises security further since you end up having to write down the password you've chosen to use.

Given the fact that there was a patch submitted not quite a year ago to achieve this, I'm amazed that you've taken this attitude now - especially given that I submitted this bug 18 months ago and it was accepted at the time!

hmm - if the box is compromised, it is trivial to get into the system anyhow. But there is the issue of backup files that may be further exposed that is a good point. As far as exposing FreePBX to the web unprotected, that is just plan a really bad idea - as I am sure if ever put to the test, it would fail from a security standpoint.

Anyhow - I'll re-open the ticket to allow for more discussion on this. There were no opinions discussed since last November which is why lazytt closed it.

I'm glad it's reopened. I was not aware that discussion was need for this change to be implemented. For me it's common sense not having plain text passwords in a database. With plain text password, db admins can get passwords by accident. With encrypted passwords you must change it to have access, which will be noticed by the password owner. This is just one of the reasons to encrypt passwords in a db. The problem of backup's with plain text passwords is already mentioned. Next to this, it is fairly easy for a system admin to reset the password when forgotten. Just create an encrypted password on the unix prompt and store it in the database (install hashalot, then 'sha256 -x'. Voila access granted again with the possibility for an audit trail. Plain text password shouldn't be in a database. For many people it could be just fine when it is, but for many it can be a problem. In that case the source must be patched on every update of the core.

ok: im starting to be convinced, and this really isn't such a big deal (to do) anyway. pvanstam: would you re post the changes in diff format please? Also there needs to be some sort of mechanism that will upgrade current db entries and change them to sha

philipple: what kind of collateral damage can we expect from such a change? Maybe it should be an option in amportal.conf?

AMPDBPASSSECURE=true/false

Then we leave it false by default an start spreading FUD until 3.0 where we change it to true by default (that's going to be a one way street though, as there will be no way to automatically go back as we cant reverse the hash)

pvanstam: ping. Any update here, or should I close this ticket?

if we change it then we change it across the board. It means an upgrade script that should:

• check if the field 'password' exists
• if so, then make a new field 'password_sha256'
• pull the old passwords, hash them and store them in 'password_scha256'
• delete the 'password' field

This way it knows the right format and knows if it has been converted. Then the only question, does the code look for the hash and if the field does not exist try the old passowrd field without the hash as a safety measure? Anyhow - it's late, if we do this we do need the upgrade script to go with it that goes in the upgrade code and it should probably be done in table.php in the upgrade directory, I don't want to put it into core's install.php

I have been coming back to this ticket over many, many months and can't believe there is still debate.

If you need another reason to go encrypted let me give you a scenario (can think of many more).

FreePBX is configured in a corporate environment (the kind of environment you want it in.. they have money). You get a tech savvy user that finds a _minor_ exploit or sql_injection possibility in _any_ of the existing code, all this query would require is _read_ access which lets the user dump the passwords in plain text and potentially _escalate_ privilege to that of an administrator with little to no effort.

This isn't this far fetched. http://www.freepbx.org/trac/ticket/2773 http://www.freepbx.org/trac/ticket/2776

Think of the consequences of this, the user then uses that access privilege to listen to the voice mail of other staff, CEO, anyone.. without their knowledge because there is no requirement to modify the password to access it (ie. it's not hashed). If it was hashed with sha256, you'd want to hope that there was a rainbow table with the password in it, otherwise you would need _write_ access to the database to gain access (update password and log in).

I believe it is an absolute liability having this information sitting in plain text when it can be changed to a hash with minor impact. Adding to this, it's not just the security of the FreePBX box you need to worry about, the user's privacy.. and the potential that the users would use the same password for other services on the network is a real risk.

Don't get me wrong, I do appreciate the effort that has gone into this project and it has saved me masses of time, my concerns revolve purely around the attitude towards security.

Will be more than happy to test this patch if it is implemented.

SuperJuice?: There are feature requests open here for more than a few years with comments, pros and cons being added all the time. If this where a commercial product, we would gather in the corporate meeting room and debate it. We would have an resolution in a couple of hours vs. a couple of years. Oh, and you would pay threw your nose for the product.

If you read the ticket carefully, you'll notice that it has already passed the debate stage. At this point the dev's seem to be pro implementing it. Now that that stage has been passed, someone has to step up to the plate and actually write the code. Both you, pvanstam and rjch seem to think this is important. One of you has even started to write a patch for this feature. The dev's ultimately end up soldering the responsibility of supporting the project and its good name. Therefor they filter submitted patches to keep the quality as high as they can, and to make sure that the patch fits in with the overall structure of the program.

While they are pro the idea, there obviously not as concerned about security as you are (maybe because past experience hasn't shown a need for concern?). In order to get the patch rolling you can either: submit one, wait for a dev to critic it, revise and submit another, till there happy with. Option b. is to cau$se a dev to be con$eren for your cau$e and get him to implement it right away. :)

My conern was raised after the comment:

07/08/08 21:13:21 changed by lazytt ¶ pvanstam: ping. Any update here, or should I close this ticket?

I think if nothing else it should stay open.

Is the current plan to implement pvanstam's patch? or are you unsatisfied with this solution? if so, what are the shortfalls?

Option B, how much we talking? ;)

Theoretical this isnt such a difficult thing to do. Personally I hade a couple of questions as to the best way to do it. p_lindheimer was pretty clear in his post as to what the requirements for such a patch would be. (i.e. if we were going to include the current one, we would of done it already :) ) Again: all that's left now is for someone to actually do it.

Philippe: wrt plan b.?

my stance is pretty simple, the patch is not complete. It looks simple and I'm sure it would work once you modify the database, which is easy to do on a new install. So when someone wants to provide the chunk of code that I listed above, in a format that can be dropped into a typical tables.php script used during version upgrades, then we can drop it in to test and probably include. See the upgrades directory for many examples. The code is not rocket science and if it's important enough then someone will do it. (or someone will bribe another dev to do it).

A comment on security. I agree that security is important. However, if in the concerned scenario described, you are best off assuming that there are probably a lot of other issues. FreePBX has not had the staff to harden against security. Also, if worried about the security, do a little research on VoIP in general and don't think that things like VLANs in a corporate environment are really going to do much against a smart person who want to listen in. The strongest thing that was pointed out, by the original poster and that I think has the most merit, is the plane text password present in the backup files as I can see a lot of incidents where this could be an issue. It suggests another thing that someone should consider, which is to provide an encryption key to use to encrypt the the entire database backup set since it includes voicemail.conf and probably other files (pinsets) that might be exposed. (Of course this would then cover the plane test passwords:-) )

pong

it's true the current patch is simple. It's not ment to be included in the upgrade as-is. It's a first step. I'm not yet fully aware of how the upgrade path is working and the possibilities with it. In a few day's I can update the patch, because right now I'm not on the latest FreePBX.

Regard, Pim

pvanstam: ping? At this rate im not sure your going to make it in time for 2.5