prop224: Add an introduction point onion key in the descriptor

So it turns out that this menagerie of keys in prop224 was missing yet one more key in it :D.

To extend to an introduction point, we need its onion key meaning we have to add it to the descriptor. Keep in mind that the client does NOT lookup the node in its consensus in order not to reveal which consensus it's using and avoid reachability issues with different consensus between service and client.

This goes in two fold. First add an onion key field in the spec and then implement it in an extra commit in legacy/trac#20657 (moved) code (which is being reviewed). I would like us to avoid making a branch depending on 8k lines of code from another unmerged ticket.

changed milestone to %Tor: 0.3.2.x-final in legacy/trac

added component::core tor/tor in Legacy / Trac milestone::Tor: 0.3.2.x-final in Legacy / Trac owner::dgoulet in Legacy / Trac parent::21888 in Legacy / Trac points::0.1 in Legacy / Trac priority::medium in Legacy / Trac prop224 in Legacy / Trac resolution::fixed in Legacy / Trac reviewer::asn in Legacy / Trac severity::normal in Legacy / Trac sponsor::R-must in Legacy / Trac status::closed in Legacy / Trac tor-hs in Legacy / Trac tor-spec in Legacy / Trac type::enhancement in Legacy / Trac labels

Trac:
Status: new to accepted
Parent: N/A to legacy/trac#20657 (moved)

Spec: ticket22979_01

Trac:
Status: accepted to needs_review

Replying to dgoulet:

Spec: ticket22979_01

LGTM.

Ok, time for the code to come! Considering this as a groundwork and should be based on git master.

Trac:
Parent: legacy/trac#20657 (moved) to legacy/trac#21888 (moved)
Status: needs_review to accepted

Branch: ticket22979_032_01

It's not that big so let me know if an Oniongit would be needed.

Trac:
Reviewer: N/A to asn
Points: 0.2 to 0.1
Status: accepted to needs_review

Replying to dgoulet:

Branch: ticket22979_032_01

It's not that big so let me know if an Oniongit would be needed.

LGTM, but two quick questions:

a) I assume that ip->onion_key will be initialized by the service code that is not yet included in the branch. So it's OK that it's not really filled with anything on the service-side, right?

b) Do we really want to goto err if we don't recognize the key type when decoding descriptor? Doesn't that influence our forward-compatibility that the key type field was supposed to be helping?

Replying to asn:

Replying to dgoulet:

Branch: ticket22979_032_01

It's not that big so let me know if an Oniongit would be needed.

LGTM, but two quick questions:

a) I assume that ip->onion_key will be initialized by the service code that is not yet included in the branch. So it's OK that it's not really filled with anything on the service-side, right?

Yes exactly. This is really just the "descriptor subsystem" support for decoding and encoding.

b) Do we really want to goto err if we don't recognize the key type when decoding descriptor? Doesn't that influence our forward-compatibility that the key type field was supposed to be helping?

Hmmm yes I think you are right. Then we have the same issue with enc-key... :S

Basically, if we don't have "ntor" following it, we should just ignore it and move on. See fixup commit 76533e10a.

From IRC: <+asn> LGTM

Actually, the fixup commit has been removed. Final branches:

Spec: ticket22979_01 Code: ticket22979_032_01

Trac:
Status: needs_review to merge_ready

Should this become an "At least once" element, so that we can someday have an ntor key and a something-else key?

Fixup commit on both spec and code. Basically, code one changes to T1N and the spec one mentions "Exactly once per ip".

I don't think that's what I meant: I meant that an intro point should be able to have an ntor onion key, and also (say) have some other kind of onion key for some day in the future, too, in case we add a successor to ntor.

So I think the code in decode_introduction_point needs to handle looking for the first onion-key ntor line. Otherwise it will ignore it if the ntor one isn't in the first position.

Also the spec should say something like "At least once per intro point; each key type must be unique per intro point"

Trac:
Status: merge_ready to needs_revision

That was fixed as part of the legacy/trac#20657 (moved) branch, and also merged upstream. See: https://gitweb.torproject.org/tor.git/commit/?id=471489ca035a50733205f97c8d589d80c58e36e8

Closing this for now.

Trac:
Status: needs_revision to closed
Resolution: N/A to fixed

closed

changed time estimate to 48m

mentioned in issue legacy/trac#23130 (moved)

moved from legacy/trac#22979 (moved)

added Feature + 1 deleted label and removed 1 deleted label

added Specifications + 1 deleted label and removed 1 deleted label