Opened 5 years ago

Last modified 2 years ago

#13059 needs_revision defect

Create bad-relays file

Reported by: Sebastian Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-dirauth bad-relays interface
Cc: Actual Points:
Parent ID: #12898 Points: small
Reviewer: Sponsor:

Description

In the wake of #12899, it became apparent that redoing the approved-routers file is a good idea. It'll be replaced by a torrc-style file called bad-relays.

Child Tickets

TicketStatusOwnerSummaryComponent
#4477newRelays that are not directory authorities shouldn't load the approved-routers fileCore Tor/Tor
#15237newImprove tooling and usability for approved-routers file and its alliesCore Tor/Tor
#21148closedRestore documentation for the approved-routers fileCore Tor/Tor

Change History (28)

comment:1 Changed 5 years ago by nickm

fwiw, this also needs an automated migration tool. Also, have you talked with the other dirauth ops about how they feel about the migration?

comment:2 Changed 5 years ago by Sebastian

I haven't talked with other dirauth ops other than arma or weasel (who are the only ones voting on BadExit currently besides gabelmoo). Weasel is also the only one who had an auto-naming setup (other than gabelmoo), which dealt with the approved-routers file. At this point, I don't know what to gain from talking to them.

As for an automatic migration tool, it's super trivial and not trivial at the same time:

cp torrc bad-relays && cat approved-routers >> bad-relays

would totally work, but produce some warnings about unrecognized lines. Writing a tool to only extract the options which the new file supports is easy, but that would lose comments which explain why entries are there. Maybe I can provide a script to just comment out every non-badrelays-line for the migration or something, but really this should be super trivial for anyone to do.

comment:3 Changed 5 years ago by Sebastian

Hrm, I do need a conversion script because the relay named "a" in the approved-routers file gets considered as an abbreviation for AuthdirBadExit…

comment:4 Changed 5 years ago by Sebastian

Trivial script for conversion. It dumps all comments in the new file, preserves newlines, and removes all unknown lines (=the ones which belong to the naming system or the torrc).

#!/usr/bin/python                                                                                           

torrc = "./torrc"
approved_routers = "./approved-routers"
outfile = "./bad-relays"
outfile_reject = outfile + ".reject"

infiles = [torrc, approved_routers]

outf = open(outfile, 'w')
outf_rej = open(outfile_reject, 'w')

for f in infiles:
  for line in file(f, 'r'):
    l = line.strip().split(None, 1)
    if (len(l) == 0 or l[0].startswith('#')):
      outf.write(line)
    elif l[0].lower() in ['authdirbadexit',
    'authdirbadexitcc', 'authdirinvalid', 'authdirinvalidcc',
    'authdirreject', 'authdirrejectcc', '!badexit', '!invalid',
    '!reject', '' ]:
      if (len(l) == 1):
        outf.write(line)
      else:
        outf.write(l[0] + " " + l[1].replace(' ', '') + "\n")
    else:
      outf_rej.write(line)

comment:5 Changed 5 years ago by Sebastian

Status: newneeds_review

Patch in branch bug13059 in my repo (currently running on gabelmoo). Includes a new conversion script in contrib/

comment:6 Changed 5 years ago by Sebastian

There seems to be a bug here somewhere. Trying to track it down atm.

comment:7 Changed 5 years ago by Sebastian

Found the bug, testing in a privnet and on gabelmoo revealed no issues anymore. I'm keeping it running and will provide a new branch once #13078 has landed.

comment:8 Changed 5 years ago by phw

This seems related to #12723. It would be great if that file would be world-readable.

comment:9 Changed 5 years ago by Sebastian

I think you're confusing things a bit, this file is in a Tor dirauths data directory and these aren't world-readable.

But I could export the file via https and we could loop in karsten to see whether we want to archive it along with the consensus? If you'd want that, please file a new ticket for that, tho - it has nothing to do with the technical implementation I believe

comment:10 Changed 5 years ago by Sebastian

Status: needs_reviewneeds_revision

comment:11 Changed 5 years ago by nickm

On

{"AuthDirRejectCC", "AuthDirReject", 0, 0},

Are the syntaxes the same? Did AuthDirRejectCC take names wrapped in curly braces, the way that an authdirreject line will expect? if not, the transition tool needs to add braces.

Also:

  • I'd like the badrelays_t implementation stuff to be in its own file.
  • The function should be named badrelays_free(), I think: not free_badrelays_set().
  • The return values from routerset_contains_router() should have constant names, and should be documented. The code to convert them into strings should be its own function.
  • Why did you remove directory_remove_invalid() ? We'll need to run that whenever we reload the badrelays file, right?
  • There should probably be some unit tests here.

comment:12 Changed 4 years ago by Sebastian

Milestone: Tor: 0.2.6.x-finalTor: 0.2.7.x-final

comment:13 Changed 4 years ago by nickm

Keywords: 027-triaged-1-out added

Marking triaged-out items from first round of 0.2.7 triage.

comment:14 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:15 Changed 4 years ago by nickm

Points: small

comment:16 Changed 4 years ago by nickm

Keywords: pre028-patch added

comment:17 Changed 3 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.9.x-final

It is impossible that we will fix all 188 currently open 028 tickets before 028 releases. Time to move some out. This is my second pass through the "needs_revision" tickets, looking for things to move to 0.2.9.

Note that if the requested revisions happen in advance of the 0.2.8 freeze, they can get considered for 0.2.8.

comment:18 Changed 3 years ago by isabela

Milestone: Tor: 0.2.9.x-finalTor: 0.2.???

tickets market to be removed from milestone 029

comment:19 Changed 3 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:20 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:21 Changed 2 years ago by arma

Severity: Normal

I just made some tickets into children of this one, since they're all basically under this topic.

comment:22 Changed 2 years ago by arma

Might be that the recent landing of #1922 helps here!

comment:23 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:24 Changed 2 years ago by nickm

Keywords: 027-triaged-in added

comment:25 Changed 2 years ago by nickm

Keywords: 027-triaged-in removed

comment:26 Changed 2 years ago by nickm

Keywords: 027-triaged-1-out removed

comment:27 Changed 2 years ago by nickm

Keywords: pre028-patch removed

comment:28 Changed 2 years ago by nickm

Keywords: tor-dirauth bad-relays interface added

This is still a good idea, and I'd love us to pick it up again.

Note: See TracTickets for help on using tickets.