Skip to content
Permalink
Browse files

Remove support for all less-safe values of safePasswords.

  • Loading branch information
kohler committed Feb 7, 2020
1 parent 092f087 commit c7242d3f3ee3d238714b7a970acef2b49024d5e4
Showing with 33 additions and 111 deletions.
  1. +1 −4 profile.php
  2. +0 −8 scripts/script.js
  3. +0 −12 src/conference.php
  4. +11 −28 src/contact.php
  5. +2 −12 src/distoptions.php
  6. +14 −35 src/userstatus.php
  7. +0 −1 test/cdb-options.php
  8. +0 −1 test/options.php
  9. +5 −10 test/test04.php
@@ -553,7 +553,7 @@ function echo_modes($hlbulk) {

// obtain user json
$UserStatus->set_user($Acct);
$userj = $UserStatus->user_json(["include_password" => true]);
$userj = $UserStatus->user_json();
if (!$useRequest
&& $Me->privChair
&& $Acct->is_empty()
@@ -623,9 +623,6 @@ function echo_modes($hlbulk) {
if (isset($Qreq->redirect)) {
echo Ht::hidden("redirect", $Qreq->redirect);
}
if ($Me->privChair) {
echo Ht::hidden("whichpassword", "");
}

if ($UserStatus->has_messages()) {
$status = 0;
@@ -8050,14 +8050,6 @@ handle_ui.on("js-delete-user", function (event) {
$d.on("submit", "form", function () { addClass(f, "submitting"); });
});

handle_ui.on("js-plaintext-password", function (event) {
foldup.call(this);
var open = $(this).closest(".foldo, .foldc").hasClass("foldo");
var form = $(this).closest("form")[0];
if (form && form.whichpassword)
form.whichpassword.value = open ? "t" : "";
});

var profile_ui = (function ($) {
return function (event) {
if (hasClass(this, "js-role")) {
@@ -585,14 +585,6 @@ function crosscheck_options() {
}
$this->_date_format_initialized = false;

// set safePasswords
if (!get($this->opt, "safePasswords")
|| (is_int($this->opt["safePasswords"]) && $this->opt["safePasswords"] < 1)) {
$this->opt["safePasswords"] = 0;
} else if ($this->opt["safePasswords"] === true) {
$this->opt["safePasswords"] = 1;
}

// set defaultFormat
$this->default_format = (int) get($this->opt, "defaultFormat");
$this->_format_info = null;
@@ -1653,10 +1645,6 @@ function saved_searches() {

// users

function password_storage_cleartext() {
return $this->opt["safePasswords"] < 1;
}

function external_login() {
return isset($this->opt["ldapLogin"]) || isset($this->opt["httpAuthLogin"]);
}
@@ -1483,26 +1483,14 @@ private function password_hash_method() {
}

private function check_password_encryption($hash, $iscdb) {
if ($iscdb)
$safe = $this->conf->opt("contactdb_safePasswords", 2);
else
$safe = $this->conf->opt("safePasswords");
if ($safe < 1
|| ($method = $this->password_hash_method()) === false
|| ($hash !== "" && $hash[0] !== " " && $safe == 1))
return false;
else if ($hash === "" || $hash[0] !== " ")
return true;
else
return $hash[1] !== "\$"
|| password_needs_rehash(substr($hash, 2), $method);
return $hash === ""
|| $hash[0] !== " "
|| $hash[1] !== "\$"
|| password_needs_rehash(substr($hash, 2), $this->password_hash_method());
}

private function hash_password($input) {
if (($method = $this->password_hash_method()) !== false)
return " \$" . password_hash($input, $method);
else
return $input;
return " \$" . password_hash($input, $this->password_hash_method());
}

function check_password($input, $info = null) {
@@ -1637,18 +1625,13 @@ function sendAccountInfo($sendtype, $sensitive) {
$template = "@createaccount";
}
} else if ($sendtype === "forgot") {
if ($this->conf->opt("safePasswords") <= 1
&& $this->plaintext_password()) {
$template = "@accountinfo";
} else {
if (!$cdbu && $this->conf->contactdb()) {
error_log("{$this->conf->dbname}: {$this->email} local capability");
}
$capmgr = $this->conf->capability_manager($cdbu ? "U" : null);
$rest["capability"] = $capmgr->create(CAPTYPE_RESETPASSWORD, array("user" => $this, "timeExpires" => time() + 259200));
$this->conf->log_for($this, null, "Password link sent " . substr($rest["capability"], 0, 8) . "...");
$template = "@resetpassword";
if (!$cdbu && $this->conf->contactdb()) {
error_log("{$this->conf->dbname}: {$this->email} local capability");
}
$capmgr = $this->conf->capability_manager($cdbu ? "U" : null);
$rest["capability"] = $capmgr->create(CAPTYPE_RESETPASSWORD, array("user" => $this, "timeExpires" => time() + 259200));
$this->conf->log_for($this, null, "Password link sent " . substr($rest["capability"], 0, 8) . "...");
$template = "@resetpassword";
} else {
$template = "@accountinfo";
}
@@ -107,18 +107,8 @@

// PASSWORD SECURITY
//
// safePasswords Controls how passwords are stored in the database. If
// false, passwords are stored in plaintext; if true, user-
// chosen passwords are stored as cryptographic hashes with
// random salts, which are less vulnerable to cracking.
// Randomly-generated passwords, such as those generated
// for new accounts, are stored in plaintext for usability
// reasons; set safePasswords to 2 to opportunistically
// upgrade these passwords to cryptographic hashes.
// chairHidePasswords If true, then chairs cannot view or modify other
// users' passwords. Defaults to false.

$Opt["safePasswords"] = 2;
// chairHidePasswords If true, then chairs cannot modify other users'
// passwords. Defaults to false.


// PAPER STORAGE
@@ -108,10 +108,6 @@ static function unparse_json_main(UserStatus $us, $cj, $args) {
$cj->$k = $x;
}

if (get($args, "include_password")
&& ($pw = $user->plaintext_password()))
$cj->__passwords = ["", "", $pw];

if ($user->roles)
$cj->roles = self::unparse_roles_json($user->roles);

@@ -752,14 +748,10 @@ static function parse_request_main(UserStatus $us, $cj, Qrequest $qreq, $uf) {
if (!$us->conf->external_login()
&& !$us->user->is_empty()
&& $us->viewer->can_change_password($us->user)
&& (isset($qreq->upassword) || isset($qreq->upasswordt))) {
if ($qreq->whichpassword === "t" && $qreq->upasswordt) {
$pw = $pw2 = trim($qreq->upasswordt);
} else {
$pw = trim((string) $qreq->upassword);
$pw2 = trim((string) $qreq->upassword2);
}
$cj->__passwords = [(string) $qreq->upassword, (string) $qreq->upassword2, (string) $qreq->upasswordt];
&& isset($qreq->upassword)) {
$pw = trim((string) $qreq->upassword);
$pw2 = trim((string) $qreq->upassword2);
$cj->__passwords = [(string) $qreq->upassword, (string) $qreq->upassword2];
if ($pw === "" && $pw2 === "") {
/* do nothing */;
} else if ($pw !== $pw2) {
@@ -769,24 +761,26 @@ static function parse_request_main(UserStatus $us, $cj, Qrequest $qreq, $uf) {
} else if ($us->viewer->can_change_password(null)
&& strcasecmp($us->viewer->email, $us->user->email)) {
$cj->new_password = $pw;
} else if ($us->user->check_password(trim((string) $qreq->oldpassword))) {
$cj->new_password = $pw;
} else {
if ($us->user->check_password(trim((string) $qreq->oldpassword)))
$cj->new_password = $pw;
else
$us->error_at("password", "Incorrect current password. New password ignored.");
$us->error_at("password", "Incorrect current password. New password ignored.");
}
}

// PC components
if (isset($qreq->pctype) && $us->viewer->privChair) {
$cj->roles = (object) array();
$pctype = $qreq->pctype;
if ($pctype === "chair")
if ($pctype === "chair") {
$cj->roles->chair = $cj->roles->pc = true;
if ($pctype === "pc")
}
if ($pctype === "pc") {
$cj->roles->pc = true;
if ($qreq->ass)
}
if ($qreq->ass) {
$cj->roles->sysadmin = true;
}
}

$follow = [];
@@ -973,7 +967,7 @@ static function render_password(UserStatus $us, $cj, $reqj, $uf) {
echo '<div id="foldpassword" class="profile-g foldc ',
($us->has_problem_at("password") ? "fold3o" : "fold3c"),
'">';
$pws = get($reqj, "__passwords", ["", "", ""]);
$pws = get($reqj, "__passwords", ["", ""]);
// Hit a button to change your password
echo Ht::button("Change password", ["class" => "ui js-foldup fn3", "data-fold-target" => "3o"]);
// Display the following after the button is clicked
@@ -990,25 +984,10 @@ static function render_password(UserStatus $us, $cj, $reqj, $uf) {
echo '<div class="', $us->control_class("password", "f-i"), '">
<div class="f-c">New password</div>',
Ht::password("upassword", $pws[0], ["size" => 52, "class" => "fn", "autocomplete" => $us->autocomplete("new-password")]);
if ($us->user->plaintext_password() && $us->viewer->privChair) {
echo Ht::entry("upasswordt", $pws[2], ["size" => 52, "class" => "fx", "autocomplete" => $us->autocomplete("new-password")]);
}
echo '</div>
<div class="', $us->control_class("password", "f-i"), ' fn">
<div class="f-c">Repeat new password</div>',
Ht::password("upassword2", $pws[1], ["size" => 52, "autocomplete" => $us->autocomplete("new-password")]), "</div>\n";
if ($us->user->plaintext_password()
&& ($us->viewer->privChair || $us->conf->password_storage_cleartext())) {
echo " <div class=\"f-h\">";
if ($us->conf->password_storage_cleartext())
echo "The password is stored in our database in cleartext and will be mailed to you if you have forgotten it, so don’t use a login password or any other high-security password.";
if ($us->viewer->privChair) {
if ($us->conf->password_storage_cleartext())
echo " <span class=\"sep\"></span>";
echo '<span class="n"><a class="ui js-plaintext-password" href=""><span class="fn">Show password</span><span class="fx">Hide password</span></a></span>';
}
echo "</div>\n";
}
echo "</div></div>"; // .fx3 #foldpassword
}

@@ -7,7 +7,6 @@
$Opt["dbPassword"] = "m5LuaN23j26g";
$Opt["shortName"] = "Testconf I";
$Opt["longName"] = "Test Conference I";
$Opt["safePasswords"] = true;
$Opt["passwordHmacKey"] = "W9dY7CVZPA2fFdVjNSVD66gMTbazagzHkm8ygL5uyxZnWA55";
$Opt["contactName"] = "Eddie Kohler";
$Opt["contactEmail"] = "ekohler@hotcrp.lcdf.org";
@@ -7,7 +7,6 @@
$Opt["shortName"] = "Testconf I";
$Opt["longName"] = "Test Conference I";
$Opt["paperSite"] = "http://hotcrp.lcdf.org/test/";
$Opt["safePasswords"] = true;
$Opt["passwordHmacKey"] = "6MFZ8fnvAudRVRn4CXsMNrqVjSvTZqrVBFLEBfxRvxvsEjWn";
$Opt["contactName"] = "Eddie Kohler";
$Opt["contactEmail"] = "ekohler@hotcrp.lcdf.org";
@@ -37,15 +37,13 @@ function save_password($email, $encoded_password, $iscdb = false) {
$user_chair = $Conf->user_by_email("chair@_.com");
$marina = "marina@poema.ru";

$Conf->set_opt("safePasswords", 0);
$Conf->set_opt("contactdb_safePasswords", 0);
user($marina)->change_password("rosdevitch", 0);
xassert_eqq(password($marina), "");
xassert_eqq(password($marina, true), "rosdevitch");
xassert_neqq(password($marina, true), "");
xassert(user($marina)->check_password("rosdevitch"));
$Conf->qe("update ContactInfo set password=? where contactId=?", "rosdevitch", user($marina)->contactId);
xassert_eqq(password($marina), "rosdevitch");
xassert_eqq(password($marina, true), "rosdevitch");
xassert_neqq(password($marina), "");
xassert_neqq(password($marina, true), "");
xassert(user($marina)->check_password("rosdevitch"));

// different password in localdb => both passwords work
@@ -69,7 +67,7 @@ function save_password($email, $encoded_password, $iscdb = false) {
// update local password only
save_password($marina, "ncurses", false);
xassert_eqq(password($marina), "ncurses");
xassert_eqq(password($marina, true), "isdevitch");
xassert_neqq(password($marina, true), "ncurses");
xassert(user($marina)->check_password("ncurses"));

// logging in with global password makes local password obsolete
@@ -87,11 +85,8 @@ function save_password($email, $encoded_password, $iscdb = false) {
// restore to "this is a cdb password"
user($marina)->change_password("isdevitch", 0);
xassert_eqq(password($marina), "");
save_password($marina, "isdevitch", true);
xassert_eqq(password($marina, true), "isdevitch");

// start upgrading passwords
$Conf->set_opt("safePasswords", 2);
$Conf->set_opt("contactdb_safePasswords", 2);
// current status: local password is empty, global password "isdevitch"

// checking an unencrypted password encrypts it

0 comments on commit c7242d3

Please sign in to comment.
You can’t perform that action at this time.