#33545: assertion failure when "all zero" client auth key provided
-------------------------------------+-------------------------------------
Reporter: mcs | Owner: (none)
Type: defect | Status: reopened
Priority: High | Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor | Version: Tor: 0.4.4.0-alpha-
| dev
Severity: Normal | Resolution:
Keywords: 043-must security | Actual Points: 0.2
extra-review |
Parent ID: | Points:
Reviewer: dgoulet, nickm | Sponsor:
-------------------------------------+-------------------------------------
Comment (by cypherpunks):
Replying to [comment:11 asn]:
> With regards to the fix, I decided to take a similar approach to you but
do the all-zeroes check deeper into the
`parse_private_key_from_control_port()` which seemed like the right place
to do it. I also loosed the assert, and made the same check for the
filesystem client auth approach. It would be great if you could check out
my patch and please comment if you find any issues.
Thanks for finding the filesystem file-loading codepath. I'd naively
concluded that `hs_client_register_auth_credentials()` was the only place
that really called `digest256map_set(client_auths,` with new auths, so it
was the only place that needed a change, and hadn't realized that
`hs_config_client_authorization()` overwrites the entire map with a newly
constructed map of x25519 keys loaded from the filesystem.
I'd considered patching `parse_private_key_from_control_port()` at first,
but went with patching `hs_client_register_auth_credentials()` because it
seemed the lower level entrypoint, closer to where the invariant needed to
be maintained, and it could theoretically in the future have a different
caller, one that isn't the control port parsing code. If it isn't doing
the error checking now, that function could use a BUG() just like the
decryption code has, but then it still needs that new return value in the
enum added anyway.
(I'd put the unrelated
[https://gitgud.io/onionk/tor/-/commit/0c1d5fcbb7f6d513e41241b075f49001161eef44
comment documentation fix] in a separate commit instead of squashed with
the rest, but that's just aesthetics.)
gitweb.tpo and git.tpo seem to be down right now, incidentally. `fatal:
unable to access 'https://git.torproject.org/tor.git/': Failed to connect
to git.torproject.org port 443: No route to host`
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/33545#comment:12>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs
