Opened 4 years ago

Last modified 4 weeks ago

#16564 needs_revision enhancement

WIP: Reject bridge descriptors posted to non-bridge authorities

Reported by: arma Owned by: teor
Priority: High Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: work-in-progress, tor-bridge, easy, intro, ex-sponsor-19, practracker-refactor
Cc: catalyst Actual Points: 1
Parent ID: Points: 1
Reviewer: nickm Sponsor: Sponsor30-can

Description

Right now if my bridge descriptor gets uploaded to the directory authorities, poof I'm now a public relay, even if I didn't mean to be.

That's not the end of the world, since I am technically offering to be a relay already, and the only difference is that I didn't opt to publish my descriptor myself.

But still it seems like we should make the choice explicit inside the descriptor.

Child Tickets

TicketTypeStatusOwnerSummary
#30782defectclosedteorCache outdated fetched descriptors on directory authorities

Change History (29)

comment:1 Changed 4 years ago by arma

One implementation approach would be for BridgeRelay 1 to add a line to the descriptor, like "PublicRelay 0" or "BridgeRelay 1", and then authorities would look for this line and opt to not include such relays in their votes.

comment:2 Changed 4 years ago by arma

(This ticket might soon become tagged with SponsorR tor-hs too, since special and I are exploring the notion of exit bridges, and these futuristic exit bridges will have this same issue.)

comment:3 Changed 4 years ago by nickm

Keywords: SponsorU removed
Sponsor: SponsorU

Bulk-replace SponsorU keyword with SponsorU field.

comment:4 Changed 4 years ago by teor

Milestone: Tor: very long term

comment:5 Changed 3 years ago by isabela

Sponsor: SponsorUSponsorU-can

comment:6 Changed 3 years ago by nickm

Points: .2
Severity: Normal
Sponsor: SponsorU-can

comment:7 Changed 2 years ago by nickm

Keywords: easy intro added
Points: .2.1

comment:8 Changed 2 years ago by nickm

Milestone: Tor: very long termTor: unspecified

comment:9 Changed 2 years ago by nickm

Cc: isis added

comment:10 Changed 2 years ago by isis

Status: newneeds_information

What is the path that gets taken when the operator specifies "BridgeRelay 1" in their torrc that could lead to the descriptor being uploaded to a relay DirAuth? Is this possible?

comment:11 Changed 2 years ago by isis

Type: defectenhancement

comment:12 Changed 2 years ago by arma

A) You could do it by specifying "PublishServerDescriptor v3" if you wanted.

B) I could take your bridge descriptor and upload it to moria1. Then moria1 would include it in its v3 vote, and the other dir auths would learn about it and fetch the descriptor from moria1, and then it would be a relay.

Not the end of the world, but it seems cleaner for bridges to say what they wanted to be when they're writing it up and signing it.

comment:13 in reply to:  12 ; Changed 2 years ago by isis

Replying to arma:

A) You could do it by specifying "PublishServerDescriptor v3" if you wanted.


This one sounds like a bug?

B) I could take your bridge descriptor and upload it to moria1. Then moria1 would include it in its v3 vote, and the other dir auths would learn about it and fetch the descriptor from moria1, and then it would be a relay.


This is more convincing.

Not the end of the world, but it seems cleaner for bridges to say what they wanted to be when they're writing it up and signing it.


Yep, this makes sense and shouldn't be too hard, and I see it's already marked as a good intro ticket. :)

comment:14 Changed 2 years ago by arma

Status: needs_informationnew

comment:15 in reply to:  13 Changed 2 years ago by arma

Replying to isis:

Replying to arma:

A) You could do it by specifying "PublishServerDescriptor v3" if you wanted.


This one sounds like a bug?

That "feature" (in quotes) is around from the old days, when we were writing an overlay network and people wanted to put the pieces together as they saw fit -- you run a relay, and you publish to wherever you want to be collecting your relay descriptors.

I could see taking it out, in that "closing a door of what Tor can be" sort of way, but also I don't see any harm in leaving it in.

comment:16 Changed 2 years ago by catalyst

Cc: catalyst added

comment:17 Changed 21 months ago by isis

Owner: set to isis
Points: .11
Priority: MediumHigh
Sponsor: SponsorM
Status: newaccepted

A nicer way to do this, once #18329 is merged, would be to have the dirauths check the uploaded server descriptors for "bridge-distribution-request" lines, since all bridges will have them. I can make a patch that rejects bridges from the consensus, which we should probably be doing.

comment:18 Changed 8 months ago by gaba

Cc: isis removed
Owner: isis deleted
Status: acceptedassigned

comment:20 Changed 4 months ago by catalyst

Sponsor: SponsorMSponsor19-can

comment:21 Changed 8 weeks ago by teor

Actual Points: 0.5
Milestone: Tor: unspecifiedTor: 0.4.2.x-final
Owner: set to teor
Summary: Add a line to bridge descriptors specifying they're bridges?Reject bridge descriptors posted to non-bridge authorities

I have a draft tor and spec patch for this change.

But I still need to write tests:

  • bridge-distribution-request descriptors are accepted by:
    • bridge authorities
    • bridge clients
  • other roles reject bridge-distribution-request descriptors:
    • non-bridge authorities
    • non-bridge clients
    • bridge and non-bridge relays

I think I'll find some missing cases by writing these tests.

Last edited 8 weeks ago by teor (previous) (diff)

comment:22 Changed 7 weeks ago by gaba

Actual Points: 0.5

Add 'actual points' when the ticket get closed.

Last edited 7 weeks ago by gaba (previous) (diff)

comment:23 in reply to:  22 Changed 7 weeks ago by teor

Actual Points: 0.5

Replying to gaba:

Add 'actual points' when the ticket get closed.

I talked with Gaba, and it is ok to use actual points to track the time I spend on a ticket.

comment:24 Changed 6 weeks ago by gaba

Keywords: ex-sponsor-19 added

Adding the keyword to mark everything that didn't fit into the time for sponsor 19.

comment:25 Changed 6 weeks ago by phw

Sponsor: Sponsor19-canSponsor30-can

Moving from Sponsor 19 to Sponsor 30.

comment:26 Changed 6 weeks ago by teor

Actual Points: 0.50.6
Keywords: work-in-progress added
Status: assignedneeds_review
Summary: Reject bridge descriptors posted to non-bridge authoritiesWIP: Reject bridge descriptors posted to non-bridge authorities

I would like an initial review of the design of this change:
https://github.com/torproject/tor/pull/1084

This pull request:

  • changes the purpose of general-purpose descriptors with bridge-distribution-request line to the bridge purpose
  • rejects:
    • bridge descriptors uploaded to a non-bridge authority from relays
    • downloaded bridge descriptors, when expecting relay descriptors
    • controller requests that try to add a bridge descriptor to the list of relay descriptors

I wonder if I am missing some rejection cases, and if those cases are all covered by changing the descriptor purpose.

I still need to write some unit tests, see:
https://trac.torproject.org/projects/tor/ticket/16564#comment:21

This branch is based on #30780, because of a conflict in was_router_added_t. If #30780 merges to master, I can rebase on master to remove those commits from the pull request.

Last edited 6 weeks ago by teor (previous) (diff)

comment:27 Changed 6 weeks ago by teor

Actual Points: 0.61
Keywords: practracker-refactor added

I did a refactor to reduce the size of router_add_to_routerlist(), and found and fixed #30782.

comment:28 Changed 5 weeks ago by dgoulet

Reviewer: nickm

comment:29 Changed 4 weeks ago by nickm

Sorry for the delay here -- I've left a set of comments on the branch.

comment:30 Changed 4 weeks ago by nickm

Status: needs_reviewneeds_revision
Note: See TracTickets for help on using tickets.