[tor-bugs] #22781 [Core Tor/Tor]: hs: Unify link specifier API/ABI

[Tor Bug Tracker & Wiki]

#22781: hs: Unify link specifier API/ABI
---------------------------------------+-----------------------------------
 Reporter:  dgoulet                    |          Owner:  dgoulet
     Type:  enhancement                |         Status:  needs_information
 Priority:  Medium                     |      Milestone:  Tor:
                                       |  0.3.3.x-final
Component:  Core Tor/Tor               |        Version:
 Severity:  Normal                     |     Resolution:
 Keywords:  tor-cell, tor-relay, ipv6  |  Actual Points:
Parent ID:  #24403                     |         Points:  1
 Reviewer:                             |        Sponsor:  SponsorV-can
---------------------------------------+-----------------------------------
Changes (by dgoulet):

 * status:  needs_revision => needs_information


Comment:

 I want to make sure we nail down an interface that we like/want before
 going further.

 1. EXTEND cell: The `extend_cell_t` object contains basically all the
 possible ways to extend. Through `extend_cell_format()`, we take what's in
 it and create `link_specifier_t` (trunnel).

  The relationship is:
  - `extend_cell_format(): extend_cell_t -> link_specifier_t`
  - `extend_cell_parse(): link_specifier_t -> extend_cell_t` (Note that
 this is for EXTEND2).

 2. HSv3: We have `hs_desc_link_specifier_t` which represent only one
 possible way to extend (IPv4, IPv6, Legacy ID or ed25519).

  Relationship is:
  - `encode_link_specifiers(): hs_desc_link_specifier_t ->
 link_specifier_t`
  - `decode_link_specifiers(): link_specifier_t ->
 hs_desc_link_specifier_t`

 That is what we have right now using link specifiers interface (trunnel)
 and some intermediate object. I think a good move forward would be so
 abstract the high level link specifier object to something generic that
 every subsystem could use. In other words, replace
 `hs_desc_link_specifier_t` and `extend_cell_t` to be the same object.

 They differ right now because the HS one contains *one* specific link
 specifier and the extend cell object contains *all* possible link
 specifiers. In the spirit of reducing memory allocation, I think it is
 reasonable to have one generic object containing all possible link
 specifiers. Something like:

 {{{
 tor_addr_port_t addrport_v4;
 tor_addr_port_t addrport_v6;
 uint8_t legacy_id[DIGEST_LEN];
 ed25519_public_key_t ed_id;
 }}}

 We mandate that v4 and legacy_id be present which means we could either
 check for "mem zero" on IPv6 and ed_id to decide to include them or we add
 a flag saying "v6 defined" to maybe be more efficient. Parsing cells is
 certainly part of our fast path in my opinion so any optimization is a
 win. Downside here is if we ever want to include more than one type of
 specifiers in a cell/descriptor that is for example 2 different IPv4?

 My original branch (`ticket22781_032_01`) has already some work started
 there with `lspec_t` being a generic higher level object that encodes to a
 `link_specifier_t` and vice versa. However, it would need to contain all
 the possible links.

 Am I on a path that we like?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/22781#comment:6>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online