Opened 2 years ago

Closed 2 years ago

#27549 closed defect (implemented)

hs-v3: Refactor the descriptor cookie computation code

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: CollecTor 1.7.0
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, hs-auth, refactoring
Cc: Actual Points:
Parent ID: #27544 Points:
Reviewer: asn Sponsor:

Description

These functions have very very similar code for computing hs_desc_build_authorized_client() and decrypt_descriptor_cookie() for computing the keys for the client authorization.

We should refactor this and consolidate since they do the same work on both sides (client and service).

Child Tickets

Change History (16)

comment:1 Changed 2 years ago by neel

Cc: neel added
Owner: set to neel
Status: newassigned

comment:2 Changed 2 years ago by neel

Cc: neel removed
Owner: neel deleted

comment:3 Changed 2 years ago by neel

Status: assignednew

comment:4 Changed 2 years ago by neel

Owner: set to neel
Status: newassigned

comment:5 Changed 2 years ago by neel

Owner: neel deleted

comment:6 Changed 2 years ago by neel

Status: assignednew

comment:7 Changed 2 years ago by dgoulet

Owner: set to dgoulet
Status: newaccepted

comment:8 Changed 2 years ago by dgoulet

Branch: ticket27549_035_01
PR: https://github.com/torproject/tor/pull/355

comment:9 Changed 2 years ago by dgoulet

Status: acceptedneeds_review

comment:10 Changed 2 years ago by dgoulet

Reviewer: asn

comment:11 Changed 2 years ago by asn

I pushed a fixup on my branch ticket27549_035_01 that kills the helper functions to simplify the patch.

The helper functions did not help with simplifying the code or with making it more abstract since the underlying code still needed to know the structure of keys to do:

-  memcpy(client_out->client_id,
-         get_client_id_from_keys(keystream), HS_DESC_CLIENT_ID_LEN);

Please review and mark as merge_ready if you like it.

comment:12 Changed 2 years ago by traumschule

Keywords: hs-auth added

comment:13 Changed 2 years ago by asn

Status: needs_reviewneeds_revision

Marking as needs_revision to make it obvious that an action is requested from dgoulet.

comment:14 Changed 2 years ago by dgoulet

Status: needs_revisionmerge_ready

lgtm. I've cherry pick the fixup:

Branch: ticket27549_035_01
PR: ​https://github.com/torproject/tor/pull/355

comment:15 Changed 2 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

comment:16 Changed 2 years ago by nickm

Milestone: Tor: 0.3.5.x-finalCollecTor 1.7.0
Resolution: implemented
Status: merge_readyclosed

Squashed and merged to master (0.3.6)

Note: See TracTickets for help on using tickets.