From e943812dc9802d134f2d9627a6c4fc94fe9c26f9 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 30 May 2008 17:35:34 -0400 Subject: [PATCH] hashed password support, and empty password security fix This implements the previously documented hashed password support. While implementing that, I noticed a security hole, which this commit also fixes.. --- IkiWiki/Plugin/passwordauth.pm | 136 ++++++++++++++++++++++++++++----- debian/changelog | 14 ++-- doc/wikitemplates.mdwn | 2 +- ikiwiki-transition | 32 ++++++++ po/ikiwiki.pot | 20 +++-- templates/passwordmail.tmpl | 13 +++- 6 files changed, 180 insertions(+), 37 deletions(-) diff --git a/IkiWiki/Plugin/passwordauth.pm b/IkiWiki/Plugin/passwordauth.pm index af16c2754..f3f1aa4bf 100644 --- a/IkiWiki/Plugin/passwordauth.pm +++ b/IkiWiki/Plugin/passwordauth.pm @@ -11,8 +11,70 @@ sub import { #{{{ call => \&formbuilder_setup); hook(type => "formbuilder", id => "passwordauth", call => \&formbuilder); + hook(type => "sessioncgi", id => "passwordauth", call => \&sessioncgi); } # }}} +# Checks if a string matches a user's password, and returns true or false. +sub checkpassword ($$;$) { #{{{ + my $user=shift; + my $password=shift; + my $field=shift || "password"; + + # It's very important that the user not be allowed to log in with + # an empty password! + if (! length $password) { + return 0; + } + + my $userinfo=IkiWiki::userinfo_retrieve(); + if (! length $user || ! defined $userinfo || + ! exists $userinfo->{$user} || ! ref $userinfo->{$user}) { + return 0; + } + + my $ret=0; + if (exists $userinfo->{$user}->{"crypt".$field}) { + eval q{use Authen::Passphrase}; + error $@ if $@; + my $p = Authen::Passphrase->from_crypt($userinfo->{$user}->{"crypt".$field}); + $ret=$p->match($password); + } + elsif (exists $userinfo->{$user}->{$field}) { + $ret=$password eq $userinfo->{$user}->{$field}; + } + + if ($ret && + (exists $userinfo->{$user}->{resettoken} || + exists $userinfo->{$user}->{cryptresettoken})) { + # Clear reset token since the user has successfully logged in. + delete $userinfo->{$user}->{resettoken}; + delete $userinfo->{$user}->{cryptresettoken}; + IkiWiki::userinfo_store($userinfo); + } + + return $ret; +} #}}} + +sub setpassword ($$;$) { #{{{ + my $user=shift; + my $password=shift; + my $field=shift || "password"; + + eval q{use Authen::Passphrase::BlowfishCrypt}; + if (! $@) { + my $p = Authen::Passphrase::BlowfishCrypt->new( + cost => $config{password_cost} || 8, + salt_random => 1, + passphrase => $password, + ); + IkiWiki::userinfo_set($user, "crypt$field", $p->as_crypt); + IkiWiki::userinfo_set($user, $field, ""); + } + else { + IkiWiki::userinfo_set($user, $field, $password); + } +} #}}} + sub formbuilder_setup (@) { #{{{ my %params=@_; @@ -50,7 +112,7 @@ sub formbuilder_setup (@) { #{{{ "Login" => [qw(name password)], "Register" => [], "Create Account" => [qw(name password confirm_password email)], - "Mail Password" => [qw(name)], + "Reset Password" => [qw(name)], ); foreach my $opt (@{$required{$submittype}}) { $form->field(name => $opt, required => 1); @@ -75,14 +137,13 @@ sub formbuilder_setup (@) { #{{{ $form->field( name => "password", validate => sub { - length $form->field("name") && - shift eq IkiWiki::userinfo_get($form->field("name"), 'password'); + checkpassword($form->field("name"), shift); }, ); } elsif ($submittype eq "Register" || $submittype eq "Create Account" || - $submittype eq "Mail Password") { + $submittype eq "Reset Password") { $form->field(name => "password", validate => 'VALUE'); } @@ -101,7 +162,7 @@ sub formbuilder_setup (@) { #{{{ ); } elsif ($submittype eq "Login" || - $submittype eq "Mail Password") { + $submittype eq "Reset Password") { $form->field( name => "name", validate => sub { @@ -155,8 +216,8 @@ sub formbuilder (@) { #{{{ my $user_name=$form->field('name'); if (IkiWiki::userinfo_setall($user_name, { 'email' => $form->field('email'), - 'password' => $form->field('password'), 'regdate' => time})) { + setpassword($user_name, $form->field('password')); $form->field(name => "confirm_password", type => "hidden"); $form->field(name => "email", type => "hidden"); $form->text(gettext("Account creation successful. Now you can Login.")); @@ -165,17 +226,35 @@ sub formbuilder (@) { #{{{ error(gettext("Error creating account.")); } } - elsif ($form->submitted eq 'Mail Password') { + elsif ($form->submitted eq 'Reset Password') { my $user_name=$form->field("name"); + my $email=IkiWiki::userinfo_get($user_name, "email"); + if (! length $email) { + error(gettext("No email address, so cannot email password reset instructions.")); + } + + # Store a token that can be used once + # to log the user in. This needs to be hard + # to guess. Generating a cgi session id will + # make it as hard to guess as any cgi session. + eval q{use CGI::Session}; + error($@) if $@; + my $token = CGI::Session->new->id; + setpassword($user_name, $token, "resettoken"); + my $template=template("passwordmail.tmpl"); $template->param( user_name => $user_name, - user_password => IkiWiki::userinfo_get($user_name, "password"), + passwordurl => IkiWiki::cgiurl( + 'do' => "reset", + 'name' => $user_name, + 'token' => $token, + ), wikiurl => $config{url}, wikiname => $config{wikiname}, REMOTE_ADDR => $ENV{REMOTE_ADDR}, ); - + eval q{use Mail::Sendmail}; error($@) if $@; sendmail( @@ -184,10 +263,10 @@ sub formbuilder (@) { #{{{ Subject => "$config{wikiname} information", Message => $template->output, ) or error(gettext("Failed to send mail")); - - $form->text(gettext("Your password has been emailed to you.")); + + $form->text(gettext("You have been mailed password reset instructions.")); $form->field(name => "name", required => 0); - push @$buttons, "Mail Password"; + push @$buttons, "Reset Password"; } elsif ($form->submitted eq "Register") { @$buttons="Create Account"; @@ -197,20 +276,39 @@ sub formbuilder (@) { #{{{ @$buttons="Create Account"; } else { - push @$buttons, "Register", "Mail Password"; + push @$buttons, "Register", "Reset Password"; } } elsif ($form->title eq "preferences") { if ($form->submitted eq "Save Preferences" && $form->validate) { my $user_name=$form->field('name'); - foreach my $field (qw(password)) { - if (defined $form->field($field) && length $form->field($field)) { - IkiWiki::userinfo_set($user_name, $field, $form->field($field)) || - error("failed to set $field"); - } - } + if ($form->field("password") && length $form->field("password")) { + setpassword($user_name, $form->field('password')); + } } } } #}}} +sub sessioncgi ($$) { #{{{ + my $q=shift; + my $session=shift; + + if ($q->param('do') eq 'reset') { + my $name=$q->param("name"); + my $token=$q->param("token"); + + if (! defined $name || ! defined $token || + ! length $name || ! length $token) { + error(gettext("incorrect password reset url")); + } + if (! checkpassword($name, $token, "resettoken")) { + error(gettext("password reset denied")); + } + + $session->param("name", $name); + IkiWiki::cgi_prefs($q, $session); + exit; + } +} #}}} + 1 diff --git a/debian/changelog b/debian/changelog index 6012bc3bf..cdd8f8221 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,5 +1,8 @@ -ikiwiki (2.48) UNRELEASED; urgency=low +ikiwiki (2.48) UNRELEASED; urgency=high + * Fix security hole that occurred if openid and passwordauth were both + enabled. passwordauth would allow logging in as a known openid, with an + empty password. * Add rel=nofollow to edit links. This may prevent some spiders from pounding on the cgi following edit links. * When calling decode_utf8 on known-problimatic content in aggregate, @@ -8,12 +11,9 @@ ikiwiki (2.48) UNRELEASED; urgency=low saying it is the default. * passwordauth: If Authen::Passphrase is installed, use it to store password hashes, crypted with Eksblowfish. - * Existing cleartext passwords in the userdb will be automatically hashed - (if Authen::Passphrase is installed) the next time a user logs in. - Or `ikiwiki-transition hashpassword /path/to/srcdir` can be used to force - a conversion. - * Passwords will no longer be mailed, but instead a password reset link - mailed. + * `ikiwiki-transiition hashpassword /path/to/srcdir` can be used to + hash existing plaintext passwords. + * Passwords will no longer be mailed, but instead a password reset link. * The password_cost config setting is provided as a "more security" knob. * teximg: Fix logurl. * teximg: If the log isn't written, avoid ugly error messages. diff --git a/doc/wikitemplates.mdwn b/doc/wikitemplates.mdwn index 389bdbfe9..f095cb035 100644 --- a/doc/wikitemplates.mdwn +++ b/doc/wikitemplates.mdwn @@ -13,7 +13,7 @@ located in /usr/share/ikiwiki/templates by default. * `editpage.tmpl` - Create/edit page. * `change.tmpl` - Used to create a page describing a change made to the wiki. * `passwordmail.tmpl` - Not a html template, this is used to - generate the mail with the user's password in it. + generate a mail with an url the user can use to reset their password. * `rsspage.tmpl` - Used for generating rss feeds for [[blogs|ikiwiki/blog]]. * `rssitem.tmpl` - Used for generating individual items on rss feeds. * `atompage.tmpl` - Used for generating atom feeds for blogs. diff --git a/ikiwiki-transition b/ikiwiki-transition index 0177f98a9..e02c3aaed 100755 --- a/ikiwiki-transition +++ b/ikiwiki-transition @@ -57,6 +57,8 @@ sub indexdb { usage(); } + # Note: No lockwiki here because ikiwiki already locks it + # before calling this. if (! IkiWiki::oldloadindex()) { die "failed to load index\n"; } @@ -71,11 +73,38 @@ sub indexdb { } } +sub hashpassword { + $config{wikistatedir}=shift()."/.ikiwiki"; + + if (! defined $config{wikistatedir}) { + usage(); + } + + eval q{use IkiWiki::UserInfo}; + eval q{use Authen::Passphrase::BlowfishCrypt}; + if ($@) { + error("ikiwiki-transition hashpassword: failed to load Authen::Passphrase, passwords not hashed"); + } + + IkiWiki::lockwiki(); + IkiWiki::loadplugin("passwordauth"); + my $userinfo = IkiWiki::userinfo_retrieve(); + foreach my $user (keys %{$userinfo}) { + if (ref $userinfo->{$user} && + exists $userinfo->{$user}->{password} && + length $userinfo->{$user}->{password} && + ! exists $userinfo->{$user}->{cryptpassword}) { + IkiWiki::Plugin::passwordauth::setpassword($user, $userinfo->{$user}->{password}); + } + } +} + sub usage { print STDERR "Usage: ikiwiki-transition type ...\n"; print STDERR "Currently supported transition subcommands:\n"; print STDERR " prefix_directives file\n"; print STDERR " indexdb srcdir\n"; + print STDERR " hashpassword srcdir\n"; exit 1; } @@ -85,6 +114,9 @@ my $mode=shift; if ($mode eq 'prefix_directives') { prefix_directives(@ARGV); } +if ($mode eq 'hashpassword') { + hashpassword(@ARGV); +} elsif ($mode eq 'indexdb') { indexdb(@ARGV); } diff --git a/po/ikiwiki.pot b/po/ikiwiki.pot index e94c5b8d7..550e3d1e3 100644 --- a/po/ikiwiki.pot +++ b/po/ikiwiki.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: PACKAGE VERSION\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2008-05-25 14:26-0400\n" +"POT-Creation-Date: 2008-05-30 16:47-0400\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: FULL NAME \n" "Language-Team: LANGUAGE \n" @@ -325,20 +325,28 @@ msgstr "" msgid "bad or missing template" msgstr "" -#: ../IkiWiki/Plugin/passwordauth.pm:162 +#: ../IkiWiki/Plugin/passwordauth.pm:223 msgid "Account creation successful. Now you can Login." msgstr "" -#: ../IkiWiki/Plugin/passwordauth.pm:165 +#: ../IkiWiki/Plugin/passwordauth.pm:226 msgid "Error creating account." msgstr "" -#: ../IkiWiki/Plugin/passwordauth.pm:186 +#: ../IkiWiki/Plugin/passwordauth.pm:261 msgid "Failed to send mail" msgstr "" -#: ../IkiWiki/Plugin/passwordauth.pm:188 -msgid "Your password has been emailed to you." +#: ../IkiWiki/Plugin/passwordauth.pm:263 +msgid "You have been mailed password reset instructions." +msgstr "" + +#: ../IkiWiki/Plugin/passwordauth.pm:301 +msgid "incorrect password reset url" +msgstr "" + +#: ../IkiWiki/Plugin/passwordauth.pm:304 +msgid "password reset denied" msgstr "" #: ../IkiWiki/Plugin/pingee.pm:21 diff --git a/templates/passwordmail.tmpl b/templates/passwordmail.tmpl index 8484d39b2..df86be109 100644 --- a/templates/passwordmail.tmpl +++ b/templates/passwordmail.tmpl @@ -1,10 +1,15 @@ -Someone[1], possibly you, requested that you be emailed the password for user - on [2]. +Someone[1], possibly you, requested that the password for + on [2] be reset. -The password is: +To change your password, visit the following url, and enter a new password: + + + +This url can only be used once to change your password, and it will also +stop working the next time you successfully log in. -- ikiwiki -[1] The user requesting the password was at IP address +[1] The IP address was [2] Located at -- 2.26.2