Tags:
create new tag
, view all tags

Bug: Password is mangled after user registration (DEVELOP)

From InvalidActionInRegister: From the registration page, a different password is stored than the one entered. When .htpasswd is edited manually and the newly inserted encrypted code is replaced with a different code from another installation, registration works as expected.

Could it be that the password sent from the registration form is a different password that I entered? That somehow the password is mangled before it is stored in .htpasswd? (see also Support.ApachePasswordsUnix2)

Test case

Create a fresh install from the current DEVELOP (SVN 3857). Add a new user. Try to edit a page and log in.

Environment

TWiki version: TWikiAlphaRelease
TWiki plugins: DefaultPlugin, EmptyPlugin, InterwikiPlugin
Server OS:  
Web server:  
Perl version:  
Client OS:  
Web Browser:  

-- ArthurClemens - 23 Mar 2005

Impact and Available Solutions

WhatDoesItAffect: Install, Auth
AffectedExtensions:  
AffectedReleases: latestTWikiAlphaRelease
HaveQuickFixFor:  

Follow up

In ApachePasswordsUnix2, AntonAylward says: I've been doing a lot of testing of Dakar/DEVELOP I-A-A and often work from resetting the password from the command line.

I'm getting to wonder if the code doesn't produce the same results as the Apache utility, since I can always log in after a command line reset but can never log in after I've requested a password reset as an anonymous user (ResetPassword) and when I try resetting my password after logging in successfully from the papche POV (ChangePassword) the "Old Password" is not accepted.

I'd [...] suggest the use of the CPAN module rather than the current code.

-- ArthurClemens - 23 Mar 2005

I'll take a look tomorrow morning. M.

-- MartinCleaver - 24 Mar 2005

I've done a quick instrumentation of the password system by inserting a proxy object in front of it.

-        $this->{users} = new TWiki::Users( $this, 'HtPasswdUser' );
+        $this->{users} = new TWiki::Users( $this, 'ProxyPasswdUser' );

All argument calls being logged in the debug log.

Although I haven't solved the problem I think I've made it much easier to do so.

Committed revision 3869.

-- MartinCleaver - 24 Mar 2005

The debug log currently shows all calls down to the password system. Here's a new registration of account WikiName NewAccount LoginName newaccount. It looks ok to me.

| 26 Mar 2005 - 00:23 | TWiki::UI::Register::_addUserToPasswordSystem:655 => TWiki::User::passwordExists:951
| 26 Mar 2005 - 00:23 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:23 |         => Calling TWiki::Users::HtPasswdUser:UserPasswordExists (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:23 | UserPasswordExists returned:
                                    $VAR1 = 0;

| 26 Mar 2005 - 00:23 | TWiki::UI::Register::_addUserToPasswordSystem:655 => TWiki::User::addPassword:966
| 26 Mar 2005 - 00:23 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:23 |         => Calling TWiki::Users::HtPasswdUser:AddUserPassword (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    $VAR3 = 'password';
                                    )
| 26 Mar 2005 - 00:23 | AddUserPassword returned:
                                    $VAR1 = '1';


And now a reset:

| 26 Mar 2005 - 00:27 | TWiki::UI::Register::_resetUsersPassword:444 => TWiki::User::passwordExists:481
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:UserPasswordExists (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:27 | UserPasswordExists returned:
                                    $VAR1 = 1;

| 26 Mar 2005 - 00:27 | TWiki::User::resetPassword:490 => TWiki::User::passwordExists:285
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:UserPasswordExists (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:27 | UserPasswordExists returned:
                                    $VAR1 = 1;

| 26 Mar 2005 - 00:27 | TWiki::User::resetPassword:490 => TWiki::User::removePassword:286
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = , evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:RemoveUser (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:27 | TWiki::Users::ProxyPasswdUser::AUTOLOAD:230 => TWiki::Users::HtPasswdUser::RemoveUser:35
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:htpasswdReadPasswd (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:27 | htpasswdReadPasswd returned:
                                    $VAR1 = 'X/GWqnmr/8ueo';

| 26 Mar 2005 - 00:27 | TWiki::Users::ProxyPasswdUser::AUTOLOAD:230 => TWiki::Users::HtPasswdUser::RemoveUser:35
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:htpasswdUpdateUser (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount:X/GWqnmr/8ueo';
                                    $VAR3 = '';
                                    )
| 26 Mar 2005 - 00:27 | htpasswdUpdateUser returned:
                                    $VAR1 = '1';

| 26 Mar 2005 - 00:27 | RemoveUser returned:
                                    $VAR1 = '1';

| 26 Mar 2005 - 00:27 | TWiki::User::resetPassword:490 => TWiki::User::addPassword:289
| 26 Mar 2005 - 00:27 | [ hasargs = 1, wantarray = , evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:27 |         => Calling TWiki::Users::HtPasswdUser:AddUserPassword (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    $VAR3 = 1672;
                                    )
| 26 Mar 2005 - 00:27 | AddUserPassword returned:
                                    $VAR1 = '1';

And finally an attempted change, which fails because the password '1672' is deemed to be incorrect because the last two crypted strings don't match:

| 26 Mar 2005 - 00:28 | TWiki::UI::Register::changePassword:131 => TWiki::User::checkPassword:567
| 26 Mar 2005 - 00:28 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:28 |         => Calling TWiki::Users::HtPasswdUser:CheckUserPasswd (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    $VAR3 = '1672';
                                    )
| 26 Mar 2005 - 00:28 | TWiki::Users::ProxyPasswdUser::AUTOLOAD:211 => TWiki::Users::HtPasswdUser::CheckUserPasswd:35
| 26 Mar 2005 - 00:28 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:28 |         => Calling TWiki::Users::HtPasswdUser:htpasswdReadPasswd (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:28 | htpasswdReadPasswd returned:
                                    $VAR1 = 'w7ADTSVhNDxz2';

| 26 Mar 2005 - 00:28 | TWiki::Users::ProxyPasswdUser::AUTOLOAD:211 => TWiki::Users::HtPasswdUser::CheckUserPasswd:35
| 26 Mar 2005 - 00:28 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:28 |         => Calling TWiki::Users::HtPasswdUser:htpasswdGeneratePasswd (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    $VAR3 = '1672';
                                    $VAR4 = 1;
                                    )
| 26 Mar 2005 - 00:28 | TWiki::Users::ProxyPasswdUser::AUTOLOAD:247 => TWiki::Users::HtPasswdUser::htpasswdGeneratePasswd:35
| 26 Mar 2005 - 00:28 | [ hasargs = 1, wantarray = 0, evaltext = , is_require = , hints = 0, bitmask = UUUUUUUUUUUU ]
| 26 Mar 2005 - 00:28 |         => Calling TWiki::Users::HtPasswdUser:htpasswdReadPasswd (
                                    $VAR1 = bless( {
                                                     'proxy' => bless( {
                                                                         'session' => 'TWiki=HASH(0x80f58a8)'
                                                                       }, 'TWiki::Users::HtPasswdUser' ),
                                                     'session' => $VAR1->{'proxy'}{'session'}
                                                   }, 'TWiki::Users::ProxyPasswdUser' );
                                    $VAR2 = 'newaccount';
                                    )
| 26 Mar 2005 - 00:28 | htpasswdReadPasswd returned:
                                    $VAR1 = 'w7ADTSVhNDxz2';

| 26 Mar 2005 - 00:28 | htpasswdGeneratePasswd returned:
                                    $VAR1 = 'w7m5c1u6ldE8g';

| 26 Mar 2005 - 00:28 | CheckUserPasswd returned:
                                    $VAR1 = '';

We could debug this but...

Crawford Currie wrote in an email: > It really should be using CPAN:Htpasswd

AntonAylward wrote in response: > Let me put on my professional hat for a moment ...
> From a security POV ... security isn't just about "keeping hackers out".
> Correct and reliable function ("integrity' and 'availability' of
> service) are predicates.

And I wrote: > The CPAN version is undoubtedly better
> tested; it could even be better code. One might argue that the
> security of this module is important so should use better code.
> Another might argue that changing it is fine in principle but they are
> not going to volunteer to change it.

So, three questions:

  1. Do we think our passwd system is robust enough?
  2. Given that PasswordsAreMangled, does anyone care to rewrite it? Even if they did, would a home grown version be of any value to us vis-a-vis a standard one?
  3. Do we have consensus to wire TWiki::Users::HtPasswdUser to use CPAN:Apache::Htpasswd for implementation?

Passwords are very likely to remain managled until we get consensus on this.

-- MartinCleaver - 26 Mar 2005

-- MartinCleaver - 26 Mar 2005

N.B. I read the trace above to mean that its HtPasswdUserDotPm, rather than the code changed by the RegisterCgiScriptRewrite is misbehaving. If someone validates my finding, suggests a reason or disputes my claim I'll use more time to look at this today. Much appreciated, Thanks.

I tried to get onto TWikiIRC to ask about this but it I can't connect frown

-- MartinCleaver - 27 Mar 2005

THis is an important function. It is one of the key components to a professionally functioning system and is a road hurdle to the adoption of the Dakar release.

I agree with Martin about the evidence of the trace. In addition, that HtPasswdDotPm banks out the current password then adds a new one at the end leads to a sloppy looking file. Whatever else, things that aesthetically displeasing probably have other flaws, I've learnt.

In short we have three ways we can go:

  1. Rewrite HttasswdUserDotPm
    It would helpt if we knew what was wrong with it!
  2. In HtPasswdUserDotPm there is a comment that the external program 'htpasswd' cannot be used because it won't accept the new password as STDIN. While true, this is only marginally relevant. The 'htpasswd' program can accept the new password on the command line.
    This is a security risk, and the documentation on 'htpasswd' mentions this. It is a risk becuase someone could run 'ps' at just the right time and see what the password is. This is a small exposure and many may consider it acceptable.
    • In terms of risk, the weakness of the algorithm used by the basic authentication and the fact that the clear-text password is being transmitted in clear over the network and that password resets are being transmitted in clear-text by e-mail over the network to store and forward respositories of questiobnale security may be more significant.
    • If the main threat comes from another user on the host machine, then the protection of the password file is also an issue.
    • In the light of the above I consider the use of 'htpassword' in 'batch mode' (i.e. password on the command line) to be a minor risk.
  3. Use CPAN:Apache::Htpaaswd

My opinion is that the second alternative - use the command line 'htpasswd' is the route we should take first. My reasoning for this is as follows.

  • It is a minimal code change
  • It uses code that is completely compatible - by definition
  • It very quickly and easily proves or disproves the correctness of the existing code in HtPasswdDotPm
  • It elimiates the blank line problem

We will, of course, have to make sure that certain charecters don't appear in passwords smile

-- AntonAylward - 27 Mar 2005

I acknowledge that this is still a problem in DevelopBranchVersion#3985. (I was a bit surprised as I know Crawford checked in a big change that I'd read as fixing this problem and addressing Anton's point).

My password still fails to log me in and hitting escape at the authentication prompt gives me the following error:

Can't call method "param" on an undefined value at /home/.balder/mrjc/testwiki.mrjc.com/twiki/lib/TWiki/UI/Oops.pm line 56.
   TWiki::UI::__ANON__('Can\'t call method "param" on an undefined value at /home/.balde...') called at /home/.balder/mrjc/testwiki.mrjc.com/twiki/lib/TWiki/UI/Oops.pm line 56
   TWiki::UI::Oops::oops_cgi(undef) called at /home/.balder/mrjc/testwiki.mrjc.com/twiki/lib/TWiki/UI.pm line 133
   TWiki::UI::run('CODE(0x85aa0a0)') called
   Error::subs::run_clauses('HASH(0x85bd7fc)', 'Can\'t call method "param" on an undefined value at /home/.balde...', undef, 'ARRAY(0x815f8d8)') called at /home/.balder/mrjc/testwiki.mrjc.com/twiki/lib/Error.pm line 394
   Error::subs::try('CODE(0x80f58a8)', 'HASH(0x85bd7fc)') called at /home/.balder/mrjc/testwiki.mrjc.com/twiki/lib/TWiki/UI.pm line 190
   TWiki::UI::run('CODE(0x85aa0a0)') called at oops line 29

-- MartinCleaver - 19 Apr 2005

Can someone comment whether this issue still exists? Thx.

-- MartinCleaver - 16 May 2005

In my view this bug does still exist. Test case: Reset a password, this will send you a new temporary one. Then try to change the password - TWiki tells you that the old (temporary) password is wrong. However, you can log in with that temporary password.

-- MartinCleaver - 28 Jun 2005

Fix record

I suspect this was cleaned up by Crawford's cleanup of this code. I have therefore closed this bug report.

-- MartinCleaver - 30 May 2005

A scan of a bug - http://develop.twiki.org/~develop/cgi-bin/view/Bugs/Item29 got me digging. It seesm that reset works but that change does not: I've made the failure explicit by dieing as soon as possible: changePassword is complaining that the password assigned by reset is invalid but I disagree as you can actually log in with that password.

HtPasswdUser is so messy; if I could get paid to spend the time I'd ditch it; ApacheHtpasswdUser was an attempt to call the CPAN modules in its place. At least that way we don't spend effort on where well tested functionlity exists elsewhere.

Revision 4430.

-- MartinCleaver - 25 Jun 2005

FYI I ran tests on the apache module versus the TWiki one; they behaved identically, so I never bothered with apache (which requires a CPAN module) i.e. if it ain't broke, why fix it?

-- CrawfordCurrie - 28 Jun 2005

I was working on DB authentication method for Users.pm and I beleive I found the bug at lib/TWiki/UI/Register.pm , line 615:

if ($user->changePassword($passwordB)) {

as you can see, it passes just the new password as an argument. The correct should be:

if ($user->changePassword($oldpassword ,$passwordB)) {

that should fix the problem.

-- GabrielAraujo - 29 Jun 2005

I've added that to the DEVELOP svn. Revision 4465

sigh Why can't perl do checking ....

-- AntonAylward - 29 Jun 2005

Nice one Gabriel! Well spotted! Anton, thanks for putting it into the code; did you add a unit test so we don't risk having the same problem repeated again in the future?

-- CrawfordCurrie - 30 Jun 2005

Discussion

Edit | Attach | Watch | Print version | History: r19 < r18 < r17 < r16 < r15 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r19 - 2005-06-30 - CrawfordCurrie
 
  • Learn about TWiki  
  • Download TWiki
This site is powered by the TWiki collaboration platform Powered by Perl Hosted by OICcam.com Ideas, requests, problems regarding TWiki? Send feedback. Ask community in the support forum.
Copyright © 1999-2015 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.