search

nuxsmin / sysPass

Systems Password Manager
https://syspass.org
GNU General Public License v3.0
976 stars 208 forks source link

Passwords inaccessible "integrity check failed" #519

Closed derStephan closed 7 years ago

derStephan commented 7 years ago

Hi,

unfortunately, upgrade did not work as expected.

Starting from 1.2, I upgraded to 2.1.2 which went without errors, so I thought, it was okay. I could add new accounts and everything seemed fine. But is it not.

When I try to show or copy a password which was already present in 1.2, I get eighter the error "integrity check failed" or "Ungültige Option".

My Upgrade is 14 days ago. If I have to switch back to this time point, a couple of new accounts will be lost.

Today, I tried upgrading again to the current release 2.1.3 but the problem is still there.

nuxsmin commented 7 years ago

Hi, could you give me some info about the account password length?, Does the issue occur on other accounts?, Is there any error in the event log during the upgrade process (those messages related to the accounts updated)?

Kind regards

derStephan commented 7 years ago

Hi, could you give me some info about the account password length

The problem is there with all old accounts. Password lengths vary from <6 to >20 characters

Does the issue occur on other accounts?

As far as I can see, it occurs on all old accounts.

Is there any error in the event log during the upgrade process (those messages related to the accounts updated)?

There have been failed database constraints with creation of orphan-items. Everything else says something like this:

"Master-Passwort ändern Accounts aktualisiert : 2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54 ,55,56,57,59,60Fehler : 0"

and "Master-Passwort (H) ändern Accounts aktualisiert : 1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36 Fehler : 0"

and...

"Aktualisierung Version aktualisiert Version : 120 => 21117030601 Typ : db"

Log looks fine for me.

nuxsmin commented 7 years ago

Right, looks fine.

With "old accounts", did you mean to accounts history password or the current password instead?

derStephan commented 7 years ago

With "old accounts", did you mean to accounts history password or the current password instead?

No. With "old accounts" I mean all those accounts which were already there when I did the first upgrade.

nuxsmin commented 7 years ago

Sorry for the late response...a busy week :(

Could you try it out by logging in with another user?

Kind regards

derStephan commented 7 years ago

No, I tried it with 3 different accounts with no success. One of these accounts is admin.

derStephan commented 7 years ago

May I ask, what integrity checks are performed when fetching passwords? Was that changed from 1.2 to 2.1?

nuxsmin commented 7 years ago

Sure!, The encryption schema has totally changed, because now it uses a encryption library that performs several steps to get a more secure encrypted data. It's explained on doc.syspass.org and the library repository.

I haven't missed your issue, but I need some more time to figure out what could be happening. I'll be shortly on the email :)

nuxsmin commented 7 years ago

And that message means that the encrypted data couldn't be decrypted by using the key provided because the HMAC hash does not match, so either the key is wrong or the encrypted data is corrupted.

derStephan commented 7 years ago

Anything, I can do to support your investigations? Currently, syspass is unusable for me. Should I roll back to 1.2? Am I the only one with this problem? If yes, why?

nuxsmin commented 7 years ago

If you know how to access to the database, you could check out if after the upgrade (and before the first login), the user's record field 'isMigrate' in the usrData table is set to 1.

After the first login, it should automatically be set to 0

derStephan commented 7 years ago

Should check this now after a couple of weeks? Or should I roll back to 1.2 and do the upgrade again?

nuxsmin commented 7 years ago

You can check it now, there would be some users with this field set to 1. Check also if yours is set to 0

derStephan commented 7 years ago

I have 3 active users, each with user_isMigrate=0. And there is one "orphan_user" with user_isDisabled=1 and user_isMigrate=1.

nuxsmin commented 7 years ago

Ok, did you try out to create a new user and log in using it? (you mentioned above you tested some users but I don't know if they were newly created)

derStephan commented 7 years ago

no, all of the users have been there since 1.2.

nuxsmin commented 7 years ago

Ok, could you create a new one and then login view the accounts password?

derStephan commented 7 years ago

Sorry for the delay, but I can not login anymore. Right after #536 is closed, I will do as I am told.

derStephan commented 7 years ago

I now created a new user and logged in as this user. It asked me for the master key, which I entered.

After that, the problem is still there for this user. And it is worse. The passwords, I created after the upgrade are readable for the older users. But for this very new user, even those passwords are inaccessible. The message shows "integrity check failed" again.

nuxsmin commented 7 years ago

Could you send me (email) the table structure for usrData and accounts tables (no data, only structure)?

derStephan commented 7 years ago 

-- 
-- Table USRDATA
-- 
DROP TABLE IF EXISTS `usrData`;

CREATE TABLE `usrData` (
  `user_id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
  `user_name` varchar(80) NOT NULL,
  `user_groupId` smallint(3) unsigned NOT NULL,
  `user_secGroupId` smallint(3) unsigned DEFAULT NULL,
  `user_login` varchar(50) NOT NULL,
  `user_pass` varbinary(255) NOT NULL,
  `user_mPass` varbinary(1000) DEFAULT NULL,
  `user_mKey` varbinary(1000) DEFAULT NULL,
  `user_email` varchar(80) DEFAULT NULL,
  `user_notes` text,
  `user_count` int(10) unsigned NOT NULL DEFAULT '0',
  `user_profileId` smallint(5) unsigned NOT NULL,
  `user_lastLogin` datetime DEFAULT NULL,
  `user_lastUpdate` datetime DEFAULT NULL,
  `user_lastUpdateMPass` int(11) unsigned NOT NULL DEFAULT '0',
  `user_isAdminApp` bit(1) DEFAULT b'0',
  `user_isAdminAcc` bit(1) DEFAULT b'0',
  `user_isLdap` bit(1) DEFAULT b'0',
  `user_isDisabled` bit(1) DEFAULT b'0',
  `user_hashSalt` varbinary(128) NOT NULL,
  `user_isMigrate` bit(1) DEFAULT b'0',
  `user_isChangePass` bit(1) DEFAULT b'0',
  `user_isChangedPass` bit(1) DEFAULT b'0',
  `user_preferences` blob,
  PRIMARY KEY (`user_id`),
  UNIQUE KEY `IDX_login` (`user_login`),
  KEY `IDX_pass` (`user_pass`),
  KEY `fk_usrData_groups_id_idx` (`user_groupId`),
  KEY `fk_usrData_profiles_id_idx` (`user_profileId`),
  CONSTRAINT `fk_usrData_groups_id` FOREIGN KEY (`user_groupId`) REFERENCES `usrGroups` (`usergroup_id`),
  CONSTRAINT `fk_usrData_profiles_id` FOREIGN KEY (`user_profileId`) REFERENCES `usrProfiles` (`userprofile_id`)
) ENGINE=InnoDB AUTO_INCREMENT=6 DEFAULT CHARSET=utf8;

-- 
-- Table ACCOUNTS
-- 
DROP TABLE IF EXISTS `accounts`;

CREATE TABLE `accounts` (
  `account_id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
  `account_userGroupId` smallint(5) unsigned NOT NULL,
  `account_userId` smallint(5) unsigned NOT NULL,
  `account_userEditId` smallint(5) unsigned NOT NULL,
  `account_customerId` int(10) unsigned NOT NULL,
  `account_name` varchar(50) NOT NULL,
  `account_categoryId` smallint(5) unsigned NOT NULL,
  `account_login` varchar(50) DEFAULT NULL,
  `account_url` varchar(255) DEFAULT NULL,
  `account_pass` varbinary(1000) NOT NULL,
  `account_key` varbinary(1000) NOT NULL,
  `account_notes` text,
  `account_countView` int(10) unsigned NOT NULL DEFAULT '0',
  `account_countDecrypt` int(10) unsigned NOT NULL DEFAULT '0',
  `account_dateAdd` datetime NOT NULL,
  `account_dateEdit` datetime DEFAULT NULL,
  `account_otherGroupEdit` bit(1) DEFAULT b'0',
  `account_otherUserEdit` bit(1) DEFAULT b'0',
  `account_isPrivate` bit(1) DEFAULT b'0',
  `account_isPrivateGroup` bit(1) DEFAULT b'0',
  `account_passDate` int(10) unsigned DEFAULT NULL,
  `account_passDateChange` int(10) unsigned DEFAULT NULL,
  `account_parentId` smallint(5) unsigned DEFAULT NULL,
  PRIMARY KEY (`account_id`),
  KEY `IDX_categoryId` (`account_categoryId`),
  KEY `IDX_userId` (`account_userGroupId`,`account_userId`),
  KEY `IDX_customerId` (`account_customerId`),
  KEY `fk_accounts_user_id_idx` (`account_userId`),
  KEY `fk_accounts_user__edit_id_idx` (`account_userEditId`),
  KEY `IDX_parentId` (`account_parentId`) USING BTREE,
  CONSTRAINT `fk_accounts_category_id` FOREIGN KEY (`account_categoryId`) REFERENCES `categories` (`category_id`) ON UPDATE CASCADE,
  CONSTRAINT `fk_accounts_customer_id` FOREIGN KEY (`account_customerId`) REFERENCES `customers` (`customer_id`),
  CONSTRAINT `fk_accounts_userGroup_id` FOREIGN KEY (`account_userGroupId`) REFERENCES `usrGroups` (`usergroup_id`) ON DELETE NO ACTION ON UPDATE NO ACTION,
  CONSTRAINT `fk_accounts_user_edit_id` FOREIGN KEY (`account_userEditId`) REFERENCES `usrData` (`user_id`),
  CONSTRAINT `fk_accounts_user_id` FOREIGN KEY (`account_userId`) REFERENCES `usrData` (`user_id`)
) ENGINE=InnoDB AUTO_INCREMENT=65 DEFAULT CHARSET=utf8;
nuxsmin commented 7 years ago

Please download this gist https://gist.github.com/nuxsmin/7f30c8cdc9fc0833d878df7d4ca6fa2f and replace the file inc/themes/material-blue/views/common/debug.inc. After that enable the debug mode and log in with an "old" user and a "new" one.

You'll see an entry (nearly the bottom of the page) with the current master pass.

nuxsmin commented 7 years ago

The database structure is correct.

derStephan commented 7 years ago

I tried the gist and got the master key for an old user and my new user. They both are equal. BUT The shown master key is NOT the key, I entered for both of the users. The shown key is 80 chars long. My Masterkey is longer. However, these 80 chars are equal to the first 80 chars of my master key.

nuxsmin commented 7 years ago

I will need to do some tests with large keys, because it's weird that the current version is encryting fine (despite it's using a shortened key) and the upgrade process didn't encrypt them right.

derStephan commented 7 years ago

it's weird that the current version is encryting fine (despite it's using a shortened key) and the upgrade process didn't encrypt them right.

But with the new user I could not decrypt one single password, not even those which were added after upgrade. This is not only an upgrading problem.

nuxsmin commented 7 years ago

It's an issue with a long key size that could be truncated in the database record.

Let me think about that and in the meantime please use the last version until the issue is solved. You could also change the master key for a shorter one because there is no need to have such longer​ key and you can safely use a 256 bit key (ie 32 chars long) or even 512, and then upgrade.

I haven't got any issue like this one ever.

derStephan commented 7 years ago

So I just change the master key to regain access to my passwords?

nuxsmin commented 7 years ago

Ummm, not at all, I suggested to switch back to 1.2 release and then (if you want to use the new version) ​upgrade to 2.0 after changing the key for a shorter one, that is still in 1.2, so this will prevent this weird issue.

In the meantime I'm analyzing this behaviour.

nuxsmin commented 7 years ago

Good news!! I've found the issue!, the current RSA encryption (performed in HTML forms) cuts down the encrypted text to 80 bytes though its max length should be 245 bytes (https://security.stackexchange.com/questions/33434/rsa-maximum-bytes-to-encrypt-comparison-to-aes-in-terms-of-security#33445), SO your encrypted passwords should be safe, please, don't do anything (downgrading, change master pass, etc).

I'll let you know any progress

nuxsmin commented 7 years ago

BTW your recently created accounts are encrypted with a 80 bytes key length, so they will need to set again, that is by setting the password again when the issue were solved

nuxsmin commented 7 years ago

Finally, it was an issue on the login form, which cut down the text entered to 80 character length. So for testing purposes you could force a password change on your recently created user and after entering the master password again (the app will ask for it on every user password change), you would be able to view the old passwords

derStephan commented 7 years ago

you would be able to view the old passwords

Which one of those? All of them or only those which have been created after the upgrade?

nuxsmin commented 7 years ago

The old ones before upgrading.

nuxsmin commented 7 years ago

The new ones have been encrypted using a 80 byte key length, so they would need to set again, that is changing the accounts password

derStephan commented 7 years ago

ah. I see.

So my next steps will be?

  1. create backup
  2. copy the passwords after upgrade to a textfile
  3. upgrade my current installation with complete master-sources
  4. create a new user with the original (long) masterkey
  5. check if the old passwords are accessible again
  6. copy the passwords from the textfile to syspass

correct?

nuxsmin commented 7 years ago

Not at all:

  1. Create a backup (not really needed but for security)
  2. Change the password of an existing user (Did you create it yesterday?)
  3. Log in again and enter the master key (the long one)
  4. Try to view the accounts password (the ones before upgrading)
  5. If passwords are displayed fine, the recently created passwords need to to be resetting by an user that has the long master key (eg. the one used on the step 2)
derStephan commented 7 years ago

just to be sure: Do I have to replace files in my installation or not?

nuxsmin commented 7 years ago

Yes, with the downloaded from the master branch: https://github.com/nuxsmin/sysPass/archive/master.zip

The sysPass version will appear as 2.1.6

derStephan commented 7 years ago

Alright.

Changing the password (klicking upper right corner) gives me "ungültige Aktion" (=invalid action). When changing that password from the user-management, and logging in again, then I am not asked for the master key but for the old password.

Creating a new user with the master key works as you discribed. I can access the old passwords (prior to upgrade) but not the new ones.

nuxsmin commented 7 years ago

Ok, if you were entered a wrong "old/previous password" it would ask for the master key (I forgot to mention...)

BTW, it's nice that it worked fine and passwords are readable again..

derStephan commented 7 years ago

Ok, if you were entered a wrong "old/previous password" it would ask for the master key (I forgot to mention...)

Ah, I see. Good to know.

BTW, it's nice that it worked fine and passwords are readable again..

yeah!! Thanks for that.

But can you take a look at

Changing the password (klicking upper right corner) gives me "ungültige Aktion" (=invalid action).

please?

nuxsmin commented 7 years ago

Sure!

nuxsmin commented 7 years ago

Done!

And you may be wondering why the app has let you logging in with a wrong master key: because the hashing algorithm used to store the hashed master key (only for comparison/checking), which is bcrypt, only hashes the first 72 bytes, so if the string to hash (ie the master key) is longer than 72 bytes it always will return true if the first 72 bytes match.

I could do some hacks to get a shorter string, BUT it would decrease the overall password entropy (http://stackoverflow.com/questions/16594613/how-to-hash-long-passwords-72-characters-with-blowfish/16597402#16597402) and it will hash a hash and not the real password, so be aware of that and I would suggest not to use such long passwords, because hashing them (for checking purposes) will be useless.

And...why to use Bcrypt?: because, nowadays, it's the best hashing algorithm (https://stackoverflow.com/questions/1561174/sha512-vs-blowfish-and-bcrypt) and many more sources..

nuxsmin commented 7 years ago

A really "must read" source: https://paragonie.com/blog/2016/02/how-safely-store-password-in-2016

nuxsmin commented 7 years ago

@derStephan

In the latest release, the master key hash is enforced to have less than 72 characters, by hashing with SHA256 and then Bcrypt, and a message is logged in syspass.log

nuxsmin commented 7 years ago

And many thanks for your feedback and testing!!

derStephan commented 7 years ago

Hey Ruben, I wanted to tell you, that I successfully fixed my installation in the way, you described. Thanks again.

nuxsmin commented 7 years ago

Hey @derStephan it's nice to hear that your sysPass instance is working fine, I'm glad to know it :)

Thanks again for the feedback!