-
Notifications
You must be signed in to change notification settings - Fork 419
fix: Onion v3 parse/format order and render lowercase #4090
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.
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
fix: Onion v3 parse/format order and render lowercase #4090
Conversation
👋 Thanks for assigning @tnull as a reviewer! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4090 +/- ##
==========================================
+ Coverage 88.60% 88.61% +0.01%
==========================================
Files 176 176
Lines 132070 132130 +60
Branches 132070 132130 +60
==========================================
+ Hits 117015 117082 +67
+ Misses 12386 12378 -8
- Partials 2669 2670 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM.
Could be worth extending the test to do a round-trip on the string to parsed address back to string -- this would've caught the uppercase issue. Could also add an assert on the version to be the default (3).
👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer. |
Tor v3 addresses are base32 of 35 bytes laid out as: [32-byte ed25519 pubkey][2-byte checksum big-endian][1-byte version] see: https://spec.torproject.org/rend-spec/encoding-onion-addresses.html * Parsing now slices 0..32 = pubkey, 32..34 = checksum (BE), 34 = version. * Display now appends pubkey + checksum + version and lowercases base32. * Adds/updates test vector accordingly.
6d19ce7
to
f9fc879
Compare
Updated! |
🔔 1st Reminder Hey @wpaulino! This PR has been waiting for your review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
After running differential fuzzing on the parsing of
init
Lightning messages between c-lightning and rust-lightning, I found that rust-lightning parses and displays OnionV3 addresses incorrectly.Tor v3 addresses are base32 of 35 bytes laid out as:
[32-byte ed25519 pubkey][2-byte checksum big-endian][1-byte version]
see: https://spec.torproject.org/rend-spec/encoding-onion-addresses.html