Opened 3 years ago

Last modified 3 months ago

#20021 needs_revision defect

Require ntor-onion-key in microdescriptors, descriptors

Reported by: nickm Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 034-triage-20180328, 034-removed-20180328
Cc: teor Actual Points:
Parent ID: Points: 1.1
Reviewer: Sponsor: Sponsor4

Description

Since we've merged #19163, authorities now reject any descriptor without an ntor key. Once authorities all are running that code, we can stop this at the parse level by requiring ntor-onion-key in descriptors and microdescriptors.

Child Tickets

Change History (18)

comment:1 Changed 3 years ago by teor

For the record, #19163 already rejects nodes that can't do ntor, but after parsing and descriptor downloads:

a76d528 Clients no longer download descriptors for relays without ntor

+  if (rs->version_known && !rs->version_supports_extend2_cells) {
+    /* We'd ignore it because it doesn't support ntor. */
+    return 0;
+  }

579a80d Clients avoid choosing nodes that can't do ntor

If we know a node's version, and it can't do ntor, consider it not running.
If we have a node's descriptor, and it doesn't have a valid ntor key,
consider it not running.

+    /* Don't choose nodes if we are certain they can't do ntor */
+    if (node->rs && !routerstatus_version_supports_ntor(node->rs, 1))
+      continue;
+    if ((node->ri || node->md) && !node_has_curve25519_onion_key(node))
+      continue;

comment:2 Changed 3 years ago by nickm

That's all true, and thanks for the note! What I was thinking of was making all of the code in routerparse which treats ntor-onion-key as an optional token instead treat it as a mandatory token.

comment:3 in reply to:  description ; Changed 3 years ago by arma

Replying to nickm:

Once authorities all are running that code

Specifically, that condition will happen when authorities are running at least 0.2.9.3-alpha.

comment:4 in reply to:  3 Changed 3 years ago by teor

Replying to arma:

Replying to nickm:

Once authorities all are running that code

Specifically, that condition will happen when authorities are running at least 0.2.9.3-alpha.

And we are sure they will never downgrade.

comment:5 Changed 3 years ago by nickm

Owner: set to nickm
Status: newaccepted

comment:6 Changed 3 years ago by nickm

My ticket20021 branch has the required change here, once we decide it's time to pull the trigger on this change. It will need further changes to make the unit tests pass again, and corresponding changes in dir-spec.txt

comment:7 Changed 3 years ago by teor

Milestone: Tor: 0.3.0.x-finalTor: 0.3.1.x-final

Looks good to me, leaving in needs-revision for the spec and unit tests.

Putting in 0.3.1, because it is likely that all authorities will be on 0.2.9 (and never downgrade) some time mid-2017. (Unless there is another bad bug where they all upgrade, or we coordinate an upgrade.)

comment:8 Changed 3 years ago by nickm

Keywords: TorCoreTeam201703 added

comment:9 Changed 3 years ago by nickm

Sponsor: Sponsor4

This can be done in sponsor4 since it affects MD design and relates to our long-term TAP removal.

comment:10 Changed 3 years ago by nickm

Status: acceptedneeds_revision

comment:11 Changed 3 years ago by nickm

Milestone: Tor: 0.3.1.x-finalTor: 0.3.2.x-final
Points: .11.1

I've fixed some of the test failures in ticket20021. But a real fix for the tests will be tricky:

1) I'd need to update the unit tests that use failing_routerdescs.inc so that they check the log messages and enforce the correct failure reasons.
2) I'd need to update the examples in failing_routerdescs.inc to include ntor keys.

Both of those are bigger than I'd expected, though the first would be quite worthwhile.

comment:12 Changed 2 years ago by nickm

Keywords: TorCoreTeam201703 removed

comment:13 Changed 2 years ago by nickm

Milestone: Tor: 0.3.2.x-finalTor: 0.3.4.x-final

comment:14 Changed 18 months ago by nickm

Keywords: 034-triage-20180328 added

comment:15 Changed 18 months ago by nickm

Keywords: 034-removed-20180328 added

Per our triage process, these tickets are pending removal from 0.3.4.

comment:16 Changed 18 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: unspecified

These needs_revision, tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if somebody does the necessary revision.

comment:17 Changed 3 months ago by nickm

Owner: nickm deleted
Status: needs_revisionassigned

comment:18 Changed 3 months ago by nickm

Status: assignedneeds_revision

None of these revisions are in my near-term plans.

Note: See TracTickets for help on using tickets.