Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

修改网站 密码 和 cookie 的散列计算方式 #252

Merged
merged 5 commits into from
Nov 10, 2023
Merged

修改网站 密码 和 cookie 的散列计算方式 #252

merged 5 commits into from
Nov 10, 2023

Conversation

BANKA2017
Copy link
Collaborator

@BANKA2017 BANKA2017 commented Nov 10, 2023 •
edited
Loading

修改

  • 默认（PHP >= 5.5.0）使用 password_hash() + PASSWORD_BCRYPT 用以替代原有的 md5(md5(md5($pwd))) / 用户自定义计算方法，如果运行主机的 PHP 版本不支持的这个方法，将会回退到原有的方法
  • 使用 password_verify()（PHP >= 5.5.0）校验用户登录时提交的密码，若不支持该方法同样会回退到原有方法计算并比较
  • 使用 hash_hmac 计算密码哈希的 hmac-sha256 作为 cookie，密钥为 id 加上密码哈希本身，由于我对加密了解不深，这样做是否妥当，有没有更好的方法还请在下方评论指出
  • 数据表 tc_users 中记录帐号密码的列 pw 的类型将会修改成 TEXT 以应对更长的密码哈希
  • 除了会被踢下线以外，用户不会有任何影响

会有什么影响？

  • 已登录的用户会被强制下线，因为 Cookie 会改变
  • 在支持 password_hash 等方法的主机上
    • 新注册和修改密码的用户将会保存被 password_hash() 处理过的密码哈希
    • 没有进行密码修改的用户将会保持原样
  • 不支持的设备上不会有任何变化
  • 调用了 EncodePwd() 方法的插件需要调用 VerifyPwd($pwd, $hash) 校验哈希是否合法，而不再是重新生成一遍再比较是否相同，由于 Bcrypt 的耗时还是挺高的，可能需要考虑继续使用 EncodePwd() 还是换别的方法

其他

附带的那个 update5.00to5.01.php 请以最终版本号为准

@BANKA2017 BANKA2017 requested a review from n0099 November 10, 2023 09:34
@n0099
Copy link
Collaborator

n0099 commented Nov 10, 2023 •
edited
Loading

回顾经典之 SomeBottle/OdIndex#21
cc @yangbowen
建议大脑升级argon2id

@BANKA2017
Copy link
Collaborator Author

建议大脑升级argon2id

这个我看文档有这种那种的前提条件不太好搞，通用点比较好

另外我比较关心 cookie 那里那样计算 hmac sha256 有没有什么问题
@n0099

@n0099
Copy link
Collaborator

n0099 commented Nov 10, 2023 •
edited
Loading

建议立即前往早已于2022年11月31日四叶内耗正式结束后彻底灭亡的泛银河系格雷科技邪恶组织四叶重工之流亡四叶残部zulip本部 https://z.n0099.net 致电上海贵族信安底层壬上壬m$ft精神mvp反unix哲学头子倒FSF急先锋EFF精神会员杨博文阁下

kenvix
kenvix approved these changes Nov 10, 2023
View reviewed changes
Copy link
Member

@kenvix kenvix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

建议大脑升级argon2id

不赞同。考虑到本软件经常在各种电子垃圾级硬件环境部署，Argon2的开销可能过大。个人认为，攻破本软件的用户几乎不会有任何收益，没有任何使用复杂散列算法的必要，能防止撞彩虹表就已经可以了。

关心 cookie 那里那样计算 hmac sha256 有没有什么问题

我觉得有问题，hash_hmac('sha256', $p['pw'], $p['id'] . $p['pw']) 这里几乎没有体现HMAC的作用，和直接SHA256是一样的

但是也无所谓，理由同上。

@BANKA2017
Copy link
Collaborator Author

但是也无所谓，理由同上。

行，我补齐另外几个版本文件就合并

@n0099
Copy link
Collaborator

n0099 commented Nov 10, 2023

protip:bduss+stoken可以直接改绑无需短信验证

@BANKA2017
Copy link
Collaborator Author

这个 pr 真正在修的小漏洞一直以来都没被人利用（至少明面上没见过报告）确实说明收益实在太低了，没人愿意去干……

@BANKA2017 BANKA2017 merged commit 4822472 into MoeNetwork:master Nov 10, 2023
@BANKA2017 BANKA2017 deleted the fix-crypto branch November 10, 2023 15:47
@yangbowen
Copy link

考虑到本软件经常在各种电子垃圾级硬件环境部署，Argon2的开销可能过大。

这类算法的开销都是可以通过一个参数来 tune 的，对于 Argon2 来说就是 内存消耗 枚举次数 并行度 ……

@yangbowen
Copy link

yangbowen commented Nov 11, 2023 •
edited
Loading

  • 使用 hash_hmac 计算密码哈希的 hmac-sha256 作为 cookie，密钥为 id 加上密码哈希本身，由于我对加密了解不深，这样做是否妥当，有没有更好的方法还请在下方评论指出

由于 HMAC 的内部构造杜绝了 SHA-256 的 Length extension attack 问题，所以在 SHA-256 不被破解的前提下， HMAC-SHA256 可以看作一个伪随机函数的。
事实上甚至对于已经被攻击的哈希算法如 MD5 ，使用 HMAC 都可能会比直接使用它本身来得安全——一些攻击方式对 HMAC 并不适用。
更通用的 MAC 不能这么用，如用于 AEAD 的 GMAC 、 Poly1305 等会在 nonce 复用的情况下丧失全部的安全性。但 HMAC 是没问题的。
不过（不影响安全性），为什么不直接以密码哈希为密钥计算 id 的 HMAC 作为 cookie 呢？

@yangbowen
Copy link

yangbowen commented Nov 11, 2023 •
edited
Loading

建议大脑升级argon2id

不赞同。考虑到本软件经常在各种电子垃圾级硬件环境部署，Argon2的开销可能过大。个人认为，攻破本软件的用户几乎不会有任何收益，没有任何使用复杂散列算法的必要，能防止撞彩虹表就已经可以了。

本软件的 cookie 可能没有很大价值，但用户的明文密码值得最大的努力去保护，因为用户有很大可能在其它更重要的地方重复使用同一个密码。虽然获知本平台的密码等价物可能价值不大，但如果一个弱的密码哈希算法导致过于容易字典穷举用户的明文密码，那么这是一个值得重视的安全问题。
从这个角度上说，这个密码哈希算法比 cookie 的加密算法更重要。因此我也仍旧推荐 Argon2 。

建议大脑升级argon2id

这个我看文档有这种那种的前提条件不太好搞，通用点比较好

在这里尝试不同的参数组合并选取合理的性能-安全性取舍。
并遵循这里的建议。
这些参数就是为了让它有更大的通用性的。

@BANKA2017
Copy link
Collaborator Author

BANKA2017 commented Nov 11, 2023 •
edited
Loading

为什么不直接以密码哈希为密钥计算 id 的 HMAC 作为 cookie 呢

确实是这样的，我当时想得复杂了

我也仍旧推荐 Argon2

目前用的是 BCrypt 处理密码，应该也行？

由于只会在 登录/改密码/注册 时会调用，效率这方面应该还行

@yangbowen

@kenvix
Copy link
Member

kenvix commented Nov 11, 2023 •
edited
Loading

考虑到本软件经常在各种电子垃圾级硬件环境部署，Argon2的开销可能过大。

这类算法的开销都是可以通过一个参数来 tune 的，对于 Argon2 来说就是 内存消耗 枚举次数 并行度 ……

你tune来tune去就算tune成最低开销也比不过现在的简单方法🤣

@BANKA2017
Copy link
Collaborator Author

我不太关心落库时存的是什么，改 BCrypt 只是顺手行为，算是一种双保险吧，真被脱裤时，数据库里面明文bduss和stoken都比这个密码哈希有用

@yangbowen
Copy link

yangbowen commented Nov 11, 2023 •
edited
Loading

为什么不直接以密码哈希为密钥计算 id 的 HMAC 作为 cookie 呢

确实是这样的，我当时想得复杂了

如果直接使用 SHA-256 的话就需要这种复杂的考虑。 HMAC 不受 length extension 影响所以无所谓。

我也仍旧推荐 Argon2

目前用的是 BCrypt 处理密码，应该也行？

由于只会在 登录/改密码/注册 时会调用，效率这方面应该还行

你tune来tune去就算tune成最低开销也比不过现在的简单方法🤣

bcrypt 行，或者 PBKDF2 也行。无非就是在多大程度上提高暴力穷举用户密码的开销的问题。
说到底以这种“反正这东西破解了也价值不大”的逻辑您就算直接传明文密码也可以这么说。反正指出穷举用户明文密码后去更重要的平台尝试是一个很实际的危险。至于在 对用户密码的保护/消耗的计算性能/消耗的开发人员精力 上最终要如何取舍自然是您等的事。真这么在意性能的话，不用PHP立省百分之八十

@n0099
Copy link
Collaborator

n0099 commented Nov 11, 2023

真这么在意性能的话，不用PHP立省百分之八十

kndil神在贴吧辅助工具头子鸡血神和starry神双重指导下发威了！ https://github.com/BANKA2017/tbsign_go

@BANKA2017 BANKA2017 linked an issue Nov 11, 2023 that may be closed by this pull request
5.00 及以前的版本存在相同密码用户可互相访问的漏洞 #254
Open
2 tasks
@Huozhangqi Huozhangqi mentioned this pull request Nov 24, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@n0099 n0099 n0099 approved these changes

@kenvix kenvix kenvix approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

5.00 及以前的版本存在相同密码用户可互相访问的漏洞
4 participants
@BANKA2017 @n0099 @yangbowen @kenvix