3 
    $salt = $this->get_salt($username);

    if (is_null($salt)) {
        return FALSE;
    }

    $password = sha1(SITE_KEY . $password . $salt);
    $sth = $this->db->prepare("SELECT id, username, active FROM user WHERE username = ? AND password = ?");
    $sth->setFetchMode(PDO::FETCH_OBJ);
    $sth->execute(array($username, $password));

    if (($result = $sth->fetch()) !== FALSE) {
        return $result;
    }

    return FALSE;

这就是让我担心的原因：

我没有误解登录方法。我只是不认为它应该返回那个对象。我可能错了，你所做的一切都很好，但我对此表示怀疑。您正在将数据库中的完整用户对象、密码和所有内容返回到可能不安全的脚本。有人可能会创建一个新文件，然后执行类似 var_dump($userObject); 之类的操作。并拥有所有这些信息

此外，我只是发现从某物返回“神奇”属性是不直观的。这只是在使用前未经验证的另一件事。如果您要将其移至 auth 类中的单独方法并让它返回“active”的值，您可以运行您需要的任何验证，而无需使 login.php 脚本更明智。

: 再看一遍，如果你以这种方式滥用该对象，你已经需要知道登录信息。在我看来，最好还是把它分开，以防出现某种泄漏。不是说我知道有人能做到这一点。只需将其视为一个潜在的漏洞。

我不会假装理解恶意用户会如何做某事。我只知道让他们更容易这样做似乎并不明智。也许它没有任何问题，我不是安全专家。我只是在戳一些东西，如果我要写它，我会改变，因为它们对我来说似乎很危险。如果您真的想知道它是否危险，我建议将该示例提交给常规的 stackoverflow 社区，看看他们怎么说。

他说的有道理吗？该方法将在我拥有的页面控制器中使用：

$user = $auth->login('username, password)

if ($user) {
//do some additional checks... set session variables
}
4

2 回答 2

8

不，您提供的代码不安全。

首先，您使用的是非迭代的简单哈希。有关原因的信息，请参阅此答案和此博客文章。

其次，为什么要分开存放盐？盐必须对每条记录都是唯一的，才能正常工作。所以把它放在密码旁边是合乎逻辑的。在这种情况下，您将进行两个查询而不是一个（假设盐不是从用户名派生的，并且确实是随机的）。如果它不是随机的，它应该是。有关更多信息，请参阅此帖子...

第三，由于数据库没有使用时间免疫比较，您将面临针对密码哈希的定时攻击。有关更多信息，请参阅本文和本文。

不要自己做这件事。确实很难安全地做到这一点。使用图书馆。你可以使用PHPASS或PHP-PasswordLib ...

编辑：致您朋友的评论：

我没有误解登录方法。我只是不认为它应该返回那个对象。我可能错了，你所做的一切都很好，但我对此表示怀疑。您正在将数据库中的完整用户对象、密码和所有内容返回到可能不安全的脚本。有人可能会创建一个新文件，然后执行类似 var_dump($userObject); 之类的操作。并拥有所有这些信息

这是一个有效的观点。但是到那时，如果用户可以注入var_dump($userObject)，他就可以运行$this->db->prepare("SELECT * FROM user");并获取您的所有用户数据，而不仅仅是他的。因此，尽管需要指出一些事情，但尝试确保安全是不值得的。除非您真的很肛门并将其全部放入数据库中的存储过程中并限制从表中的读取访问（因此您根本无法从用户表中进行选择）。但到那时，您需要从头开始重新设计整个应用程序，因为除了通过 SP 之外，您永远无法获得任何用户信息。这将很难做好（复杂性是安全的敌人）。

最后，我认为首先防御脚本注入会更好。尤其是如果您没有专业的管理员可以将您的服务器保护到防止这种攻击所需的可笑数量（PHP 进程上的 chroot 监狱等）。

此外，我只是发现从某物返回“神奇”属性是不直观的。这只是在使用前未经验证的另一件事。如果您要将其移至 auth 类中的单独方法并让它返回“active”的值，您可以运行您需要的任何验证，而无需使 login.php 脚本更明智。

有什么神奇的属性？您是否直接使用这些项目？或者您是否使用它们来填充用户对象（然后知道如何处理数据）......？仅凭该片段，我就看不到您朋友指出的问题...

: 再看一遍，如果你以这种方式滥用该对象，你已经需要知道登录信息。在我看来，最好还是把它分开，以防出现某种泄漏。不是说我知道有人能做到这一点。只需将其视为一个潜在的漏洞。

实际上，由于上​​面确定的定时攻击，您只需要知道一个有效的用户名......

我不会假装理解恶意用户会如何做某事。我只知道让他们更容易这样做似乎并不明智。也许它没有任何问题，我不是安全专家。我只是在戳一些东西，如果我要写它，我会改变，因为它们对我来说似乎很危险。如果您真的想知道它是否危险，我建议将该示例提交给常规的 stackoverflow 社区，看看他们怎么说。

总的来说，这是一个很好的建议。请记住，复杂性是安全的敌人。因此，如果增加难度会增加很多复杂性，那么添加新漏洞的机会就会显着增加（例如引入的定时攻击）。我犯了简单的错误。我有经过良好测试的库来处理复杂部分（如密码散列），但我编写的代码很简单。

考虑最强的加密算法：One Time Pad。该算法非常简单：

encrypted = plaintext XOR key

但是没有密钥是不可能解密的。所以算法的安全性实际上在于秘密，而不在于算法。这就是你想要的。算法应该是公开的，安全性不应该依赖于它的保密性。否则，它会因默默无闻而变得安全。在你崩溃之前，这会给你一种温暖的模糊感......

于 2012-05-11T19:57:10.403 回答
1

有关如何使用bcrypt散列密码的信息，请参阅此答案。它比您的更安全，因为它允许故意减慢哈希算法（因此，如果有人试图暴力破解，他们将被限制为每秒 1M 次左右的迭代，而不是 1B，这是几小时和几年之间的差异）。

此外，（与安全无关），您应该使用 PDO 的命名占位符，而不是未命名的占位符（:placeholder而不是 not ?）。

此外，您可能想要构建一个完整的User对象并将其返回，它会更容易使用（假设您有一个，您应该这样做！）。

于 2012-05-11T19:35:59.743 回答