Opened 6 months ago

Closed 4 months ago

#24526 closed enhancement (fixed)

Make it clear that multi-relay operators are expected to set a working ContactInfo and proper MyFamily

Reported by: cypherpunks Owned by: nickm
Priority: Medium Milestone: Tor: 0.3.3.x-final
Component: Core Tor/Tor Version:
Severity: Critical Keywords: tor-doc, man, review-group-28, security-low, 032-backport
Cc: Actual Points:
Parent ID: #24497 Points:
Reviewer: catalyst Sponsor:

Description

As per discussion with David on bad-relays@ I'm opening this ticket as he requested.

We want to make it clear to tor relay operators that setting a proper ContactInfo (working email address) and MyFamily (fully mutual configuration) is strongly encouraged (required?) for relay operators that run more than 3 (?) tor instances, relays showing up without such configuration likely raise a red flag and might get rejected from the network.

places to update:

  • manual page:
    • ContactInfo
    • MyFamily

Child Tickets

Change History (27)

comment:1 Changed 6 months ago by irl

Parent ID: #24497

comment:2 Changed 6 months ago by dgoulet

Keywords: tor-doc man added
Milestone: Tor: 0.3.3.x-final

comment:3 Changed 6 months ago by ahf

Would it make sense to deprecate the ContactInfo field in torrc and replace it with a more structured format?

For example:

  • ContactName the name that the relay operator would like to be identified as.
  • ContactEmail the email address the relay operator would like to be contacted via.
  • ContactComment (optional) an additional comment that will be rendered in the contact field in the descriptor.

We could then let Tor continue to use the contact field in the relay descriptor and render it using the three pre-defined fields:

"{{ContactName}} <{{ContactEmail}}>" if ContactComment is unset or empty.
"{{ContactName}} <{{ContactEmail}}> ({{ContactComment}})" if ContactComment is set.

We could warn the operator if the relay has ORPort set, but ContactName and ContactEmail is missing.

I don't know if this has been discussed before elsewhere, but I have the feeling that people are using this field in some slightly unstructured manners.

comment:5 Changed 5 months ago by cypherpunks

Part 1: Make ContactInfo mandatory for operators running multiple relays / bridges:

based on the email from David (4 Dec 2017 18:07:08 -0500) on bad-relays I'm submitting a patch to make it clear to relay/bridge operators that a contact information is mandatory if you run more than one relay.

This should make bad-relays@ cases less problematic where we have a presumed relay group like [ 1 ] and have no way to contact the operator before removing these relays from the tor network (because historically such groups turned out to be malicious).

I didn't add to the man page what happens if you do not set it (your relays might get removed if we detect a very likely group of relays without contactInfo [ 1 ]), because that would make it longer.

This adds a single and clear statement to the ContactInfo entry.

tor.1.txt:

@@ -1716,7 +1716,8 @@
 [[ContactInfo]] **ContactInfo** __email_address__::
     Administrative contact information for this relay or bridge. This line
     can be used to contact you if your relay or bridge is misconfigured or
-    something else goes wrong. Note that we archive and publish all
+    something else goes wrong. ContactInfo must be set if you run more than
+    one relay or bridge. Note that we archive and publish all
     descriptors containing these lines and that Google indexes them, so
     spammers might also collect them. You may want to obscure the fact
     that it's an email address and/or generate a new address for this

[ 1 ] https://nusenu.github.io/OrNetRadar/2017/11/30/a6

As David suggested after the man page is updated (this should get backported), he plans to announce this change on the blog.

Last edited 5 months ago by cypherpunks (previous) (diff)

comment:6 Changed 5 months ago by cypherpunks

Status: newneeds_review

comment:7 Changed 5 months ago by nickm

I've incorporated that sentence, and noted the requirement in a few more places, in a new branch bug24526 in my public repository. I've put the warnings in separate paragraphs so that they stand out more. Still needs_review.

comment:8 Changed 5 months ago by cypherpunks

Thanks for making it even more clear and adding it to MyFamily as well!

MyFamily MUST be set if you run more than one relay or bridge.

Maybe we should make it clear that it is not enough to just set it but to also be a proper (mutual) MyFamily?
I removed " or bridge." since the man page also says: "Do not list any bridge relay ..."

A proper (mutual) MyFamily MUST be set if you run more than one relay across multiple /16 networks.

comment:9 Changed 5 months ago by cypherpunks

the changes file says:

Document in more places...

That was not documented / was not a requirement before, right? So the changes file could say something like the following?

Documentation:

  • Operators running multiple relays or bridges are required to set ContactInfo.
  • Operators running multiple relays are required to set a proper (mutual) MyFamily.

I'm also no longer convinced we should make any distinction between single relay and group relay operator.

comment:10 Changed 5 months ago by nickm

Keywords: review-group-28 added

comment:11 Changed 5 months ago by tom

There is a typo in the patch in src/config/torrc.sample.in - a stray 'n' at the beginning of a line.

Aside from that, seems fine, aside from the obvious issue that we are using a capital MUST to declare something that is a policy preference with no technical enforcement mechanism.

comment:13 Changed 4 months ago by nickm

Status: needs_reviewneeds_revision

comment:14 Changed 4 months ago by nickm

Owner: set to nickm
Status: needs_revisionaccepted

comment:15 Changed 4 months ago by nickm

Status: acceptedneeds_review

I've added a series of fixup commits to my bug24526 branch to try to fix the issues matched above. Shall I squash and commit?

comment:16 Changed 4 months ago by catalyst

Reviewer: catalyst
Status: needs_reviewmerge_ready

Looks good to me! Commit message typo: s/requied/required/

comment:17 Changed 4 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

fixed and squashed and merging; thanks!

comment:18 Changed 4 months ago by cypherpunks

thanks for merging!

Will you backport this change?

comment:19 Changed 4 months ago by nickm

Not planning to: we mostly don't backport documentation. Maybe to 0.3.2?

comment:20 Changed 4 months ago by cypherpunks

yes please at least to 0.3.2 :)

comment:21 Changed 4 months ago by Sebastian

Resolution: implemented
Severity: NormalCritical
Status: closedreopened

Something went really wrong here. This is what the docs now say:

Do not list any bridge relay as it would compromise its concealment.
[...]
MyFamily **must** be set correctly if you run more than one relay or bridge. (That is, every relay should list all the others as described above.)

bridges should definitely not be mentioned below.

comment:22 Changed 4 months ago by cypherpunks

Yes, I mentioned that in comment 8 as well but it got ignored.

comment:23 Changed 4 months ago by Sebastian

Trivial patch in myfamily in my repo. I also fixed the issue of saying that MyFamily must be set and then in parens say that it should be set.

comment:24 Changed 4 months ago by Sebastian

Status: reopenedneeds_review

comment:25 in reply to:  23 Changed 4 months ago by catalyst

Status: needs_reviewmerge_ready

Replying to Sebastian:

Trivial patch in myfamily in my repo. I also fixed the issue of saying that MyFamily must be set and then in parens say that it should be set.

Thanks! This patch looks good to me. Sorry for missing the bridge thing the first time around.

comment:26 Changed 4 months ago by teor

Keywords: security-low 032-backport added

Did the original patch get backported to 0.3.2?
If it did, then we need to backport this fix to 0.3.2 as well, because bridge disclosure is a security issue.

comment:27 Changed 4 months ago by nickm

Resolution: fixed
Status: merge_readyclosed

Merged sebastian's fix to master; cherry-picked both to maint-0.3.2

Note: See TracTickets for help on using tickets.