[aur-dev] [PATCH] Remove maxlength on password fields
From c5798e0914075de8eba05ba57992ff23252bd491 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal <stein.magnus@jodal.no> Date: Thu, 24 Nov 2011 22:51:05 +0100 Subject: [PATCH] Remove maxlength on password fields
The password field for login had a maxlength of 128 chars, while the password fields for account creation and modification, and for password resetting, had a maxlength of 32 chars. After this change, they will all behave in the same way. Users with password managers will often use passwords longer than 32 characters, which was the previous maxlength when creating an account. Because of the password length, they will copy-paste the password into the browser. Most browsers does not notify the user that the text he pasted was cut of after a limit. If the login form also cuts of the password at the same length, the user will not notice anything until the site is changed to accept longer passwords, and his full password no longer matches the password the site has stored. In AUR's case, the login form accepted passwords of 128 characters length, so users trying to use longer passwords than 32 characters will notice the problem when first trying to log in. Setting a maximum length on input fields makes sense where the input is stored directly to a database where the database field got a limited length. In the case of password fields, limiting the length make little sense as the password is hashed before being stored. The hash function's output has constant length, irrespective of the length of the input. --- web/html/passreset.php | 4 ++-- web/lib/acctfuncs.inc.php | 4 ++-- web/lib/config.inc.php.proto | 1 - web/template/login_form.php | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/web/html/passreset.php b/web/html/passreset.php index 82be3ef..0792f72 100644 --- a/web/html/passreset.php +++ b/web/html/passreset.php @@ -109,11 +109,11 @@ html_header(__("Password Reset")); </tr> <tr> <td><?php echo __("Enter your new password:"); ?></td> - <td><input type="password" name="password" size="30" maxlength="32" /></td> + <td><input type="password" name="password" size="30" /></td> </tr> <tr> <td><?php echo __("Confirm your new password:"); ?></td> - <td><input type="password" name="confirm" size="30" maxlength="32" /></td> + <td><input type="password" name="confirm" size="30" /></td> </tr> </table> <br /> diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index 512e66c..b724580 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -84,7 +84,7 @@ function display_account_form($UTYPE,$A,$U="",$T="",$S="", print "<tr>"; print "<td align='left'>".__("Password").":</td>"; - print "<td align='left'><input type='password' size='30' maxlength='32'"; + print "<td align='left'><input type='password' size='30'"; print " name='P' value='".$P."' />"; if ($A != "UpdateAccount") { print " (".__("required").")"; @@ -93,7 +93,7 @@ function display_account_form($UTYPE,$A,$U="",$T="",$S="", print "<tr>"; print "<td align='left'>".__("Re-type password").":</td>"; - print "<td align='left'><input type='password' size='30' maxlength='32'"; + print "<td align='left'><input type='password' size='30'"; print " name='C' value='".$C."' />"; if ($A != "UpdateAccount") { print " (".__("required").")"; diff --git a/web/lib/config.inc.php.proto b/web/lib/config.inc.php.proto index 1f19651..aef2e34 100644 --- a/web/lib/config.inc.php.proto +++ b/web/lib/config.inc.php.proto @@ -15,7 +15,6 @@ define( "URL_DIR", "/packages/" ); define( "USERNAME_MIN_LEN", 3 ); define( "USERNAME_MAX_LEN", 16 ); define( "PASSWD_MIN_LEN", 4 ); -define( "PASSWD_MAX_LEN", 128 ); # Default language for displayed messages in the web interface. define("DEFAULT_LANG", "en"); diff --git a/web/template/login_form.php b/web/template/login_form.php index 21bdaa7..194083d 100644 --- a/web/template/login_form.php +++ b/web/template/login_form.php @@ -19,7 +19,7 @@ elseif (!$DISABLE_HTTP_LOGIN || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'])) print htmlspecialchars($_POST['user'], ENT_QUOTES); } ?>" /> <label for="passwd"><?php print __('Password') . ':'; ?></label> - <input type="password" name="passwd" id="passwd" size="30" maxlength="<?php print PASSWD_MAX_LEN; ?>" /> + <input type="password" name="passwd" id="passwd" size="30" /> <input type="checkbox" name="remember_me" id="remember_me" /> <label for="remember_me"><?php print __("Remember me"); ?></label> <input type="submit" class="button" value="<?php print __("Login"); ?>" /> -- 1.7.5.4
Heisann, Bra catch! :) Er du i Oslo om dagen? -- Cordially, Alexander Rødseth Arch Linux Trusted User (xyproto on IRC, trontonic on AUR)
Sorry, meant to send that message just to Stein Magnus Jodal. :) - Alexander
On Thu, Nov 24, 2011 at 11:19:29PM +0100, Stein Magnus Jodal wrote:
From c5798e0914075de8eba05ba57992ff23252bd491 Mon Sep 17 00:00:00 2001 From: Stein Magnus Jodal <stein.magnus@jodal.no> Date: Thu, 24 Nov 2011 22:51:05 +0100 Subject: [PATCH] Remove maxlength on password fields
The password field for login had a maxlength of 128 chars, while the password fields for account creation and modification, and for password resetting, had a maxlength of 32 chars. After this change, they will all behave in the same way.
Users with password managers will often use passwords longer than 32 characters, which was the previous maxlength when creating an account. Because of the password length, they will copy-paste the password into the browser. Most browsers does not notify the user that the text he pasted was cut of after a limit. If the login form also cuts of the password at the same length, the user will not notice anything until the site is changed to accept longer passwords, and his full password no longer matches the password the site has stored. In AUR's case, the login form accepted passwords of 128 characters length, so users trying to use longer passwords than 32 characters will notice the problem when first trying to log in.
Setting a maximum length on input fields makes sense where the input is stored directly to a database where the database field got a limited length. In the case of password fields, limiting the length make little sense as the password is hashed before being stored. The hash function's output has constant length, irrespective of the length of the input.
Out of curiosity - why would you need a password with more than 32 characters? If you use a password manager and create random passwords anyway, there's no need to create such long passwords. Assuming that your password contains lower-case and upper-case letters, as well as numbers, you won't gain any extra security when using passwords longer than ~22 characters (it'll be easier to brute-force a MD5 collision than finding the correct password in this case). Even if we used SHA-1, passwords with a length of 27 characters would already give you the maximum amount of security possible. After all, this patch still sounds reasonable to me. There's (almost) no loss of performance if we accept longer passwords. I'll rebase this on the style changes I'm currently working on. Thanks!
--- web/html/passreset.php | 4 ++-- web/lib/acctfuncs.inc.php | 4 ++-- web/lib/config.inc.php.proto | 1 - web/template/login_form.php | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/web/html/passreset.php b/web/html/passreset.php index 82be3ef..0792f72 100644 --- a/web/html/passreset.php +++ b/web/html/passreset.php @@ -109,11 +109,11 @@ html_header(__("Password Reset")); </tr> <tr> <td><?php echo __("Enter your new password:"); ?></td> - <td><input type="password" name="password" size="30" maxlength="32" /></td> + <td><input type="password" name="password" size="30" /></td> </tr> <tr> <td><?php echo __("Confirm your new password:"); ?></td> - <td><input type="password" name="confirm" size="30" maxlength="32" /></td> + <td><input type="password" name="confirm" size="30" /></td> </tr> </table> <br /> diff --git a/web/lib/acctfuncs.inc.php b/web/lib/acctfuncs.inc.php index 512e66c..b724580 100644 --- a/web/lib/acctfuncs.inc.php +++ b/web/lib/acctfuncs.inc.php @@ -84,7 +84,7 @@ function display_account_form($UTYPE,$A,$U="",$T="",$S="",
print "<tr>"; print "<td align='left'>".__("Password").":</td>"; - print "<td align='left'><input type='password' size='30' maxlength='32'"; + print "<td align='left'><input type='password' size='30'"; print " name='P' value='".$P."' />"; if ($A != "UpdateAccount") { print " (".__("required").")"; @@ -93,7 +93,7 @@ function display_account_form($UTYPE,$A,$U="",$T="",$S="",
print "<tr>"; print "<td align='left'>".__("Re-type password").":</td>"; - print "<td align='left'><input type='password' size='30' maxlength='32'"; + print "<td align='left'><input type='password' size='30'"; print " name='C' value='".$C."' />"; if ($A != "UpdateAccount") { print " (".__("required").")"; diff --git a/web/lib/config.inc.php.proto b/web/lib/config.inc.php.proto index 1f19651..aef2e34 100644 --- a/web/lib/config.inc.php.proto +++ b/web/lib/config.inc.php.proto @@ -15,7 +15,6 @@ define( "URL_DIR", "/packages/" ); define( "USERNAME_MIN_LEN", 3 ); define( "USERNAME_MAX_LEN", 16 ); define( "PASSWD_MIN_LEN", 4 ); -define( "PASSWD_MAX_LEN", 128 );
# Default language for displayed messages in the web interface. define("DEFAULT_LANG", "en"); diff --git a/web/template/login_form.php b/web/template/login_form.php index 21bdaa7..194083d 100644 --- a/web/template/login_form.php +++ b/web/template/login_form.php @@ -19,7 +19,7 @@ elseif (!$DISABLE_HTTP_LOGIN || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'])) print htmlspecialchars($_POST['user'], ENT_QUOTES); } ?>" /> <label for="passwd"><?php print __('Password') . ':'; ?></label> - <input type="password" name="passwd" id="passwd" size="30" maxlength="<?php print PASSWD_MAX_LEN; ?>" />
You should try to configure your MUA not to break long lines when sending patches... The best alternative is to use `git send-email` :)
+ <input type="password" name="passwd" id="passwd" size="30" /> <input type="checkbox" name="remember_me" id="remember_me" /> <label for="remember_me"><?php print __("Remember me"); ?></label> <input type="submit" class="button" value="<?php print __("Login"); ?>" /> -- 1.7.5.4
On Fri, Nov 25, 2011 at 12:16, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
Out of curiosity - why would you need a password with more than 32 characters? If you use a password manager and create random passwords anyway, there's no need to create such long passwords. Assuming that your password contains lower-case and upper-case letters, as well as numbers, you won't gain any extra security when using passwords longer than ~22 characters (it'll be easier to brute-force a MD5 collision than finding the correct password in this case). Even if we used SHA-1, passwords with a length of 27 characters would already give you the maximum amount of security possible.
I use the pwsafe password manager, which defaults to passwords with 160 bits of entropy, which usually means 32 chars with special chars or 39 chars with just numbers and letters in lower and upper case. I agree that there's no need for having passwords of this length, but passwords managers with longer default lengths than 32 chars do exist, and it's really no reason for stopping people from using long passwords. Having maxlength on a password field only suggests that the password is stored in plain text.
You should try to configure your MUA not to break long lines when sending patches... The best alternative is to use `git send-email` :)
Sorry, I'm usually using GitHub for all git cooperation, and Gmail is really suboptimal for sending patches :-) -- Stein Magnus Jodal
On Fri, Nov 25, 2011 at 01:30:05PM +0100, Stein Magnus Jodal wrote:
On Fri, Nov 25, 2011 at 12:16, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
Out of curiosity - why would you need a password with more than 32 characters? If you use a password manager and create random passwords anyway, there's no need to create such long passwords. Assuming that your password contains lower-case and upper-case letters, as well as numbers, you won't gain any extra security when using passwords longer than ~22 characters (it'll be easier to brute-force a MD5 collision than finding the correct password in this case). Even if we used SHA-1, passwords with a length of 27 characters would already give you the maximum amount of security possible.
I use the pwsafe password manager, which defaults to passwords with 160 bits of entropy, which usually means 32 chars with special chars or 39 chars with just numbers and letters in lower and upper case. I agree that there's no need for having passwords of this length, but passwords managers with longer default lengths than 32 chars do exist, and it's really no reason for stopping people from using long passwords. Having maxlength on a password field only suggests that the password is stored in plain text.
Identifying 160 bits of entropy with 32 chars is weird... Even if we only use alphabetic characters and digits, ceil(log(2 ^ 160) / log(2 * 26 + 10)) = 27 characters should be sufficient...
You should try to configure your MUA not to break long lines when sending patches... The best alternative is to use `git send-email` :)
Sorry, I'm usually using GitHub for all git cooperation, and Gmail is really suboptimal for sending patches :-)
You can add a link to your GitHub repository next time you send a patch so that I can pull and don't have to go through the pain of fixing the patch before feeding it to git-am(1).
-- Stein Magnus Jodal
On Fri, Nov 25, 2011 at 15:34, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
On Fri, Nov 25, 2011 at 01:30:05PM +0100, Stein Magnus Jodal wrote:
On Fri, Nov 25, 2011 at 12:16, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
You should try to configure your MUA not to break long lines when sending patches... The best alternative is to use `git send-email` :)
Sorry, I'm usually using GitHub for all git cooperation, and Gmail is really suboptimal for sending patches :-)
You can add a link to your GitHub repository next time you send a patch so that I can pull and don't have to go through the pain of fixing the patch before feeding it to git-am(1).
Is attaching the patch to the email okay? The HACKING file was rather specific on the patch being attached inline in the email. -- Stein Magnus Jodal
On Fri, Nov 25, 2011 at 03:43:57PM +0100, Stein Magnus Jodal wrote:
On Fri, Nov 25, 2011 at 15:34, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
On Fri, Nov 25, 2011 at 01:30:05PM +0100, Stein Magnus Jodal wrote:
On Fri, Nov 25, 2011 at 12:16, Lukas Fleischer <archlinux@cryptocrack.de> wrote:
You should try to configure your MUA not to break long lines when sending patches... The best alternative is to use `git send-email` :)
Sorry, I'm usually using GitHub for all git cooperation, and Gmail is really suboptimal for sending patches :-)
You can add a link to your GitHub repository next time you send a patch so that I can pull and don't have to go through the pain of fixing the patch before feeding it to git-am(1).
Is attaching the patch to the email okay? The HACKING file was rather specific on the patch being attached inline in the email.
Attachments are sometimes stripped. We really prefer inline patches. Once you get used to git-send-email(1), you'll never use anything else!
-- Stein Magnus Jodal
participants (3)
-
Alexander Rødseth
-
Lukas Fleischer
-
Stein Magnus Jodal