Opened 19 months ago
Closed 10 months ago
#23107 closed defect (fixed)
prop224: Optimize hs_circ_service_get_intro_circ() digest calculation
Reported by: | asn | Owned by: | neel |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: 0.3.4.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | prop224, prop224-extra, tor-hs, optimization, 032-unreached |
Cc: | neel@… | Actual Points: | |
Parent ID: | Points: | ||
Reviewer: | asn | Sponsor: |
Description
Our prop224 function for getting an intro circ given its intro object re-calculates the relay identity fpr all the time:
if (ip->base.is_only_legacy) { uint8_t digest[DIGEST_LEN]; if (BUG(crypto_pk_get_digest(ip->legacy_key, (char *) digest) < 0)) { goto end; } circ = hs_circuitmap_get_intro_circ_v2_service_side(digest);
We could shove that in the hs_service_intro_point_t
object as well to cut some digest calculations.
Child Tickets
Attachments (2)
Change History (14)
comment:1 Changed 17 months ago by
Keywords: | 032-unreached added |
---|---|
Milestone: | Tor: 0.3.2.x-final → Tor: unspecified |
comment:2 Changed 10 months ago by
Cc: | neel@… added |
---|
comment:3 Changed 10 months ago by
Owner: | set to neel |
---|---|
Status: | new → assigned |
comment:4 Changed 10 months ago by
I have created a patch for this patch under the filename b23107-r001.patch
. I have also used the cached digest in register_intro_circ()
.
comment:5 Changed 10 months ago by
Status: | assigned → needs_review |
---|
comment:6 Changed 10 months ago by
Points: | 0.2 |
---|---|
Reviewer: | → dgoulet |
Sponsor: | SponsorR-can |
Status: | needs_review → needs_revision |
Thanks neel!
Couple of things.
- We should set the digest right away when creating the legacy key instead of this:
memset(ip->digest, 0, sizeof(ip->digest));
- We don't need to check for mem is zero when using the legacy IP because if
is_only_legacy
is set, then digest + legacy_key MUST be set. Actually, considering that we set the digest when an IP is allocated, we don't need this whole if().
- I would rename the
digest
to maybe something likelegacy_key_digest
because there are so many keys in that object that we should just be precise about what digest that is.
Huge thanks!
comment:8 Changed 10 months ago by
Milestone: | Tor: unspecified → Tor: 0.3.4.x-final |
---|---|
Status: | needs_revision → needs_review |
comment:9 Changed 10 months ago by
Reviewer: | dgoulet → asn |
---|
comment:10 Changed 10 months ago by
Patch looks good to me.
Please see my branch bug23107
which is neel's patch, with a smaller commit msg, and a tiny amendment to the changes file so that make check-changes
does not complain. Here is a PR too: https://github.com/torproject/tor/pull/76
Neel thanks for all the code! BTW, since you are a regular contributor now, you should consider forking the torproject tor repo on github and sending us branch names (or PRs) on the tickets instead of attaching .patch files. It's a smoother development procedure :) Thanks again! :)
comment:11 Changed 10 months ago by
Status: | needs_review → merge_ready |
---|
comment:12 Changed 10 months ago by
Resolution: | → fixed |
---|---|
Status: | merge_ready → closed |
lgtm; I pushed an extra commit to fix an extra white-space left in the header. No biggy!
Merged. Thanks Neel/asn!
Mark a large number of tickets that I do not think we will do for 0.3.2.