Opened 3 years ago

Closed 3 years ago

#20573 closed enhancement (duplicate)

The build_mac() function in hs_descriptor.c should be made generic for the entire code base

Reported by: dgoulet Owned by: dgoulet
Priority: Medium Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, prop224, crypto, TorCoreTeam201611
Cc: Actual Points:
Parent ID: Points: 0.2
Reviewer: Sponsor: SponsorR-can

Description

Move build_mac() out of its file, put it in a common place, rename it with a better name and improve by adding the length of the salt and mac key to the construction so we don't assume fixed size length.

Child Tickets

Change History (5)

comment:1 Changed 3 years ago by dgoulet

Keywords: TorCoreTeam201611 added

comment:2 Changed 3 years ago by dgoulet

Status: newneeds_review

Ok took a stab at it. See branch: ticket20573_030_01.

One thing that could be an improvement is maybe renaming the function to contain "256" or else offer a "bits" variable for the size of the SHA3 or we think SHA3-256 is what should be used all the time :).

comment:3 Changed 3 years ago by dgoulet

Owner: set to dgoulet
Status: needs_reviewaccepted

Hrm #19043 has a patch for this already in commit ba3e8694 so let's back off on this one and see how it goes with #19043.

comment:4 Changed 3 years ago by dgoulet

More information. Turns out that the one in #19043 is different as it doesn't take the key/iv length in the construction and is called "HMAC" which is not entirely accurate. So what I propose here is that in the process of reviewing #19043, we can take this branch as a fixup commit for #19043 or once merged upstream, this ticket will become "fix upstream with better version of the function".

comment:5 Changed 3 years ago by dgoulet

Resolution: duplicate
Status: acceptedclosed

After some discussion on IRC, the right implementation will go in #19043 so closing this as duplicate. Thanks to chelseakomlo and asn for being part of the decision!

Note: See TracTickets for help on using tickets.