$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;

这就是让我担心的:

I did not misunderstand the login method. I just don't think it should return that object. I may be wrong and what you are doing is perfectly fine, but I doubt it. You are returning the full user object from the database, password and all, to a potentially insecure script. Someone could potentially create a new file and then do something like var_dump($userObject); and have all that information

Also, I just find it unintuitive to return a "magic" property from something. It's just another one of those things that is not verified before being used. If you were to move that to a separate method in your auth class and have it return the value of "active" you could run any verification you needed to without your login.php script being any the wiser.

: Looked at it again, and while you would already need to know the login information if you were to abuse that object in that way. It is still better, in my opinion, to separate it in case there is some sort of leak. Not saying I know how someone would be able to do it. Just think of it as one less potential loop hole.

I don't pretend to understand how malicious users would do something. I just know that making it easier for them to do so doesn't seem wise. Maybe there isn't anything wrong with it, I'm not a security expert. I'm just poking at things that, if I were to write it, I would change because they seem hazardous to me. If you truly wish to know if it is dangerous or not, I'd suggest submitting that example to the regular stackoverflow community to see what they say.

他的话有道理吗？该方法将在我拥有的页面 Controller 中使用:

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

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

最佳答案

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

首先，您使用的是非迭代的简单哈希。参见 this answer和 this blog post有关原因的信息。

第二，为什么要单独存放盐？ salt 必须对于每条记录都是唯一的，这样才能正常工作。所以把它放在密码旁边的合乎逻辑的地方。在这种情况下，您将进行两个查询而不是一个查询(假设盐不是从用户名派生的，并且确实是随机的)。如果它不是随机的，它应该是。参见 this post更多信息...

第三，您会暴露于针对密码哈希的定时攻击，因为数据库未使用定时免疫比较。参见 this article和 this article获取更多信息。

不要自己做。安全地做到这一点真的很困难。使用图书馆。您可以使用 PHPASS或 PHP-PasswordLib ...

编辑:致您 friend 的评论:

I did not misunderstand the login method. I just don't think it should return that object. I may be wrong and what you are doing is perfectly fine, but I doubt it. You are returning the full user object from the database, password and all, to a potentially insecure script. Someone could potentially create a new file and then do something like var_dump($userObject); and have all that information

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

最后，我认为您首先可以远更好地防御脚本注入(inject)。特别是如果您没有专业的管理员来保护您的服务器以防止这种攻击所需的可笑数量(PHP 进程上的 chroot jail 等)。

Also, I just find it unintuitive to return a "magic" property from something. It's just another one of those things that is not verified before being used. If you were to move that to a separate method in your auth class and have it return the value of "active" you could run any verification you needed to without your login.php script being any the wiser.

有什么神奇的属性？您是否直接使用这些项目？还是您正在使用它们来填充用户对象(然后知道如何处理数据)...？就那个 snippit，我没有看到你的 friend 指出的问题......

: Looked at it again, and while you would already need to know the login information if you were to abuse that object in that way. It is still better, in my opinion, to separate it in case there is some sort of leak. Not saying I know how someone would be able to do it. Just think of it as one less potential loop hole.

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

I don't pretend to understand how malicious users would do something. I just know that making it easier for them to do so doesn't seem wise. Maybe there isn't anything wrong with it, I'm not a security expert. I'm just poking at things that, if I were to write it, I would change because they seem hazardous to me. If you truly wish to know if it is dangerous or not, I'd suggest submitting that example to the regular stackoverflow community to see what they say.

总的来说这是个好建议。请记住，复杂性是安全性的敌人。因此，如果让它变得更难会增加很多复杂性，那么添加新漏洞的机会就会大大增加(例如引入的定时攻击)。我在简单性方面犯了错误。我拥有经过良好测试的库，可以处理复杂的部分(如密码散列)，但我编写的代码很简单。

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

encrypted = plaintext XOR key

但是没有 key 是不可能解密的。所以算法的安全其实在于 secret ，而不在于算法。这就是你想要的。该算法应该是公开的，安全性不应依赖于它的保密性。否则它会通过默默无闻而变得安全。这会给你一种温暖的毛茸茸的感觉，直到你崩溃......

关于php - 这是一种安全的方法吗，哎呀，我们在Stack Overflow上找到一个类似的问题： https://stackoverflow.com/questions/10557440/