Use constant-time comparison for rpcapd password hash verification#1660
Use constant-time comparison for rpcapd password hash verification#1660kodareef5 wants to merge 1 commit intothe-tcpdump-group:masterfrom
Conversation
rpcapd/daemon.c:1554 compares crypt() password hashes using strcmp(), which returns early on the first character mismatch. Replace with a constant-time XOR accumulation loop. While the crypt() call dominates the overall timing, using constant-time comparison for authentication credentials is defense-in-depth best practice.
|
Speaking of existing solutions to this problem, memcmp(3) refers to consttime_memequal(3), which is even shorter. More generally, it would be nice to find a way not to have to solve such problems in the first place, similarly to how git can work over SSH, but implements neither SSH nor the involved authentication. |
|
Good point about On the broader question — I agree that delegating auth to SSH is the better long-term model. But as long as rpcapd's password authentication path exists and is usable, the hash comparison in that path should be constant-time. Even if most users are on SSH, the code is still compiled and callable. |
| snprintf(errbuf, PCAP_ERRBUF_SIZE, "Authentication failed"); | ||
| return -1; | ||
| /* | ||
| * Use constant-time comparison to prevent timing |
There was a problem hiding this comment.
To be precise, the comparison is constant-time in the "the time is padded so that shorter strings take the same time as longer strings", but the strlen() calls are O(N).
There was a problem hiding this comment.
"...and the comparison doesn't stop as soon as it knows that the strings aren't equal".
We could just take the NetBSD code and add it as rpcap/missing/consttime_memequal.c, and use that if the OS doesn't have it.
Yes, this is probably the right thing for now. |
Presumably on the server side a "git daemon", or whatever, is run as the ssh command run by the client, so the server doesn't need to implement SSH or authentication, the SSH daemon does; what does it do on the client side, e.g. in the |
|
On the client side git does similarly, i.e. instead of implementing an SSH client git executes it. See |
rpcapd/daemon.cline 1554 comparescrypt()password hashes usingstrcmp(), which returns early on the first character mismatch. Replace with a constant-time XOR accumulation loop that examines all characters regardless of match position.While
crypt()dominates the overall timing, using constant-time comparison for authentication credentials is defense-in-depth best practice.