Opened 6 months ago

Closed 4 months 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 6 months ago by neel

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

comment:2 Changed 6 months ago by neel

Cc: neel removed
Owner: neel deleted

comment:3 Changed 6 months ago by neel

Status: assignednew

comment:4 Changed 6 months ago by neel

Owner: set to neel
Status: newassigned

comment:5 Changed 6 months ago by neel

Owner: neel deleted

comment:6 Changed 6 months ago by neel

Status: assignednew

comment:7 Changed 5 months ago by dgoulet

Owner: set to dgoulet
Status: newaccepted

comment:8 Changed 5 months ago by dgoulet

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

comment:9 Changed 5 months ago by dgoulet

Status: acceptedneeds_review

comment:10 Changed 4 months ago by dgoulet

Reviewer: asn

comment:11 Changed 4 months 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 4 months ago by traumschule

Keywords: hs-auth added

comment:13 Changed 4 months 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 4 months 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 4 months ago by nickm

Milestone: Tor: unspecifiedTor: 0.3.5.x-final

comment:16 Changed 4 months 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.