Opened 10 months ago

Closed 6 weeks ago

#21031 closed defect (implemented)

Please don't remove ClientDNSRejectInternalAddresses

Reported by: Sebastian Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: review-group-23
Cc: Actual Points: .1
Parent ID: Points: .5
Reviewer: dgoulet Sponsor:

Description

I have a private tor network that lives in 10.0.0.0/8, and removing this option would make that network unusable.

Child Tickets

TicketStatusOwnerSummaryComponent
#21522closednickmClientDNSRejectInternalAddresses should not be deprecated in test networks.Core Tor/Tor

Change History (10)

comment:1 Changed 10 months ago by nickm

Hm. We have a category of testing-only options. I wonder if we should consider this to be in a new category of options that you can only change if you have changed your default authorities (like we do right now for setting a relay's IP to a private address).

comment:2 Changed 10 months ago by nickm

Points: .5

comment:3 Changed 10 months ago by nickm

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

Let's not remove any deprecated options in 0.3.0.

comment:4 Changed 6 months ago by nickm

Keywords: 031-reach added
Owner: set to nickm
Status: newaccepted

Putting these tickets in 031-reach and assigning them to me. The minimum to do is "nothing"; the maximum is "come up with a better deprecation path that doesn't break testing".

comment:5 Changed 5 months ago by nickm

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

A better refactoring here will need to wait for 0.3.2

comment:6 Changed 5 months ago by nickm

Keywords: 031-reach removed

comment:7 Changed 6 weeks ago by nickm

Actual Points: .1
Status: acceptedneeds_review

How about the approach in ticket21031 ? It renames the option to TestingClientDNSRejectInternalAddresses; it adds the old option name as an alias, and it makes it illegal to change the option except in a testing network.

Or should this look at something other than "testing network", like the set of default authorities?

comment:8 Changed 6 weeks ago by nickm

Keywords: review-group-23 added

Put 0.3.2 needs_review and merge_ready tickets into review-group-23.

comment:9 Changed 6 weeks ago by dgoulet

Reviewer: dgoulet
Status: needs_reviewmerge_ready

Sounds like a good approach to me.

Code lgtm;

comment:10 Changed 6 weeks ago by nickm

Resolution: implemented
Status: merge_readyclosed

merged!

Note: See TracTickets for help on using tickets.