XWiki Platform

Safe password storage

Details

  • Type: Bug Bug
  • Status: Open Open
  • Priority: Critical Critical
  • Resolution: Unresolved
  • Affects Version/s: 0.9.793
  • Fix Version/s: None
  • Component/s: Model
  • Labels:
    None
  • keywords:
    security
  • Development Priority:
    Medium
  • Similar issues:
    XWIKI-582Automatic password update when changing the storage type
    XWIKI-433A means to get password should be implemented
    XWIKI-3719Cache the database name in the Hibernate-based storage
    XWIKI-2727Html Cleaner is not thread safe
    XWIKI-2728DefaultObservationManager is not thread safe
    XWIKI-2941Extend XMLRPC API with methods: getModifiedPagesHistory, safe storePage and safe storeObject
    XWIKI-5433Change SMTP password property from String to Password
    XWIKI-1982Attachments should be removed from the document in the document class, not in storage
    XWIKI-7139Changing password form fills the current password field

Description

Since XWiki uses a simple/single view on all the data, password fields cannot be treated in a special way, so they cannot be excluded from search queries, scripted data access, or search engine indexing. Thus, the passwords should be stored in such a manner that the stored value cannot be used in any way, like the values from /etc/shadow cannot be used.

So, this meas that password fields should be stored as:

  • plain text (no security)
  • hash (safe, but cannot be reversed) with an optional salt
  • encrypted (safe, as long as the encryption key is safe) with an optional salt
  1. File
    crypt.groovy
    11/Sep/06 21:24
    2 kB
    Sergiu Dumitriu
  2. Text File
    PasswordCrypt.patch
    10/Sep/06 20:08
    5 kB
    Sergiu Dumitriu

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ludovic Dubost added a comment -

Indeed.. thanks for reporting this issue. Fix will not be easy because I thing we will need to move password to a different table.

Show
Ludovic Dubost added a comment - Indeed.. thanks for reporting this issue. Fix will not be easy because I thing we will need to move password to a different table.
Ludovic Dubost made changes -
Field Original Value New Value
Fix Version/s 1.0 RC1 [ 10001 ]
Hide
Permalink
solo turn added a comment -

you might consider just scrambling or encrypting it.

Show
solo turn added a comment - you might consider just scrambling or encrypting it.
Hide
Permalink
Cody Burleson added a comment -

MySQL has several different encryption functions and one decryption function built into the software. Both the PASSWORD() and ENCRYPT() functions of MySQL create an encrypted string that cannot be decrypted. This means that stored passwords can never be retrieved in readable form. So, this would be an easy fix, perhaps for MySQL. Unfortunately, such a fix would bind the password encryption capability to the use of MySQL as the database for XWiki. This might be a decent short-term plan to fix the issue, but it is not a good long-term plan because you ultimately want XWiki to work on any database and possibly eventually enetrprise-class database systems such as Microsoft SQL Server, IBM DB2, or Oracle.

Show
Cody Burleson added a comment - MySQL has several different encryption functions and one decryption function built into the software. Both the PASSWORD() and ENCRYPT() functions of MySQL create an encrypted string that cannot be decrypted. This means that stored passwords can never be retrieved in readable form. So, this would be an easy fix, perhaps for MySQL. Unfortunately, such a fix would bind the password encryption capability to the use of MySQL as the database for XWiki. This might be a decent short-term plan to fix the issue, but it is not a good long-term plan because you ultimately want XWiki to work on any database and possibly eventually enetrprise-class database systems such as Microsoft SQL Server, IBM DB2, or Oracle.
Hide
Permalink
Cody Burleson added a comment -

Also to consider - what is the strategy for a migration from one version of XWiki to the next if there is a change to the database schema? Once we have a base of users who begin to create Wikis it would seem that a database migration method or utility would be necessary so that you can upgrade non-destructively. Do we have a strategy for that?

Show
Cody Burleson added a comment - Also to consider - what is the strategy for a migration from one version of XWiki to the next if there is a change to the database schema? Once we have a base of users who begin to create Wikis it would seem that a database migration method or utility would be necessary so that you can upgrade non-destructively. Do we have a strategy for that?
Hide
Permalink
Ludovic Dubost added a comment -

I'm not so fan of a mysql specific function for password. I was more thinking of having an option in the Password class to only store it as a hash (MD5 or SHA) and then modify the verification system to verify hashes.

Concerning database migration, it is possible to add migration sql to updateSchema which can be called if the option xwiki.store.hibernate.updateschema=1

Ludovic

Show
Ludovic Dubost added a comment - I'm not so fan of a mysql specific function for password. I was more thinking of having an option in the Password class to only store it as a hash (MD5 or SHA) and then modify the verification system to verify hashes. Concerning database migration, it is possible to add migration sql to updateSchema which can be called if the option xwiki.store.hibernate.updateschema=1 Ludovic
jeremi Joslin made changes -
Assignee Ludovic Dubost [ ludovic ] Unassigned Task [ unasssigned ]
jeremi Joslin made changes -
Description if you search, the user having your search string as password is displayed in the search result.
 if you search, the user having your search string as password is displayed in the search result.
Component/s Core [ 10116 ]
Environment
jeremi Joslin made changes -
Fix Version/s 1.0 B2 [ 10081 ]
Fix Version/s 1.0 RC1 [ 10001 ]
Hide
Permalink
Sergiu Dumitriu added a comment -

Created a patch.

How things work:

  • PasswordClass always stores an MD5 hash of the password string
  • XWikiAuthServiceImpl computes the MD5 hash of the entered password and compares it to the stored hash

It breakes all pre-existing passwords, i.e. only superadmin can login, but updating all passwords can be easily done from a script.

Show
Sergiu Dumitriu added a comment - Created a patch. How things work: PasswordClass always stores an MD5 hash of the password string XWikiAuthServiceImpl computes the MD5 hash of the entered password and compares it to the stored hash It breakes all pre-existing passwords, i.e. only superadmin can login, but updating all passwords can be easily done from a script.
Sergiu Dumitriu made changes -
Attachment PasswordCrypt.patch [ 10490 ]
Hide
Permalink
Sergiu Dumitriu added a comment -

Added the crypt passwords script.

Show
Sergiu Dumitriu added a comment - Added the crypt passwords script.
Sergiu Dumitriu made changes -
Attachment crypt.groovy [ 10491 ]
Vincent Massol made changes -
Workflow jira [ 10114 ] No Resolved state workflow [ 13267 ]
Vincent Massol made changes -
Workflow jira [ 13267 ] No Resolved state workflow [ 13650 ]
Sergiu Dumitriu made changes -
Assignee Unassigned Task [ unasssigned ] Sergiu Dumitriu [ sdumitriu ]
Sergiu Dumitriu made changes -
Fix Version/s 1.0 B3 [ 10001 ]
Fix Version/s 1.0 B2 [ 10081 ]
Vincent Massol made changes -
Fix Version/s 1.0 B4 [ 10174 ]
Fix Version/s 1.0 B3 [ 10001 ]
Hide
Permalink
Vincent Massol added a comment -

Moved to post 1.0

Show
Vincent Massol added a comment - Moved to post 1.0
Vincent Massol made changes -
Fix Version/s 1.1 [ 10013 ]
Fix Version/s 1.0 B4 [ 10174 ]
Hide
Permalink
David Ward added a comment -

You could avoid breaking all pre-existing passwords by using the same type of technique that some LDAP and other tools use where the encryption type is embedded with the encrypted string. Such as '$apr1$8qggK/..$cab3gSSRgKqVjgvSFTRpM', '

{SHA}

qUqP5cyxm6YcTAhz05Hph5gvu9M=', '

{crypt}

TdCqUztn3MHfI'.

Show
David Ward added a comment - You could avoid breaking all pre-existing passwords by using the same type of technique that some LDAP and other tools use where the encryption type is embedded with the encrypted string. Such as '$apr1$8qggK/..$cab3gSSRgKqVjgvSFTRpM', ' {SHA} qUqP5cyxm6YcTAhz05Hph5gvu9M=', ' {crypt} TdCqUztn3MHfI'.
Hide
Permalink
Catalin Hritcu added a comment -

David, while your suggestion seems to allow multiple encryption algorithms to be used at the same time, wouldn't it protect only new passwords from search attacks while all old passwords would still be stored in clear ?

Show
Catalin Hritcu added a comment - David, while your suggestion seems to allow multiple encryption algorithms to be used at the same time, wouldn't it protect only new passwords from search attacks while all old passwords would still be stored in clear ?
Hide
Permalink
Catalin Hritcu added a comment -

To protect against many types of attacks consider adding salt or/and pepper to each password before hashing.
http://en.wikipedia.org/wiki/Salt_(cryptography)

Show
Catalin Hritcu added a comment - To protect against many types of attacks consider adding salt or/and pepper to each password before hashing. http://en.wikipedia.org/wiki/Salt_(cryptography )
Hide
Permalink
Sergiu Dumitriu added a comment -

@ward: In a discussion on the mailing list, it was already decided to prepend the encryption/hashing identifier.
@Catalin: Yes, good idea.

Show
Sergiu Dumitriu added a comment - @ward: In a discussion on the mailing list, it was already decided to prepend the encryption/hashing identifier. @Catalin: Yes, good idea.
Sergiu Dumitriu made changes -
keywords security
Vincent Massol made changes -
Fix Version/s Future [ 10210 ]
Fix Version/s 1.1 [ 10013 ]
Vincent Massol made changes -
Workflow No Resolved state workflow [ 13650 ] XWiki Workflow [ 15531 ]
Hide
Permalink
Jérôme Delacroix added a comment -

Do you think that issue can be closed since passwords are now encrypted ?

Show
Jérôme Delacroix added a comment - Do you think that issue can be closed since passwords are now encrypted ?
Vincent Massol made changes -
Summary password can be searched Passwords can be searched
Hide
Permalink
Catalin Hritcu added a comment -

Is this kept open only because of the subtasks ?

Show
Catalin Hritcu added a comment - Is this kept open only because of the subtasks ?
Hide
Permalink
Sergiu Dumitriu added a comment -

Yes

Show
Sergiu Dumitriu added a comment - Yes
Hide
Permalink
Vincent Massol added a comment -

Sergiu, could we close this issue and open other ones or change the title since this is misleading?

Thanks

Show
Vincent Massol added a comment - Sergiu, could we close this issue and open other ones or change the title since this is misleading? Thanks
Vincent Massol made changes -
Development Priority Medium
Sergiu Dumitriu made changes -
Summary Passwords can be searched Safe password storage
Description if you search, the user having your search string as password is displayed in the search result.
 Since XWiki uses a simple/single view on all the data, password fields cannot be treated in a special way, so they cannot be excluded from search queries, scripted data access, or search engine indexing. Thus, the passwords should be stored in such a manner that the stored value cannot be used in any way, like the values from /etc/shadow cannot be used.

So, this meas that password fields should be stored as:
- plain text (no security)
- hash (safe, but cannot be reversed) with an optional salt
- encrypted (safe, as long as the encryption key is safe) with an optional salt
Vincent Massol made changes -
Component/s Model [ 11198 ]
Component/s {Unused} Core [ 10116 ]
Vincent Massol made changes -
Fix Version/s Future [ 10210 ]

People

  • Assignee:
    Sergiu Dumitriu
    Reporter:
    solo turn
Vote (4)
Watch (3)

Dates

  • Created:
    Updated:
    Date of First Response: