Opened 5 years ago

Closed 5 years ago

#13267 closed defect (fixed)

escaped forward slashes in contact info

Reported by: Sebastian Owned by:
Priority: Medium Milestone:
Component: Metrics/Onionoo Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

both atlas and globe incorrectly show gabelmoo's contact line by "escaping" a '/' with an '\'. I suspect this might be an onionoo issue, as both apps get it wrong.

Child Tickets

Attachments (1)

0001-removes-the-escaping-backslashes-cf.-trac-13267.patch (1.8 KB) - added by iwakeh 5 years ago.

Download all attachments as: .zip

Change History (8)

comment:1 Changed 5 years ago by iwakeh

Additional info:
this is the beginning of gabelmoo's contact onionoo returns (timestamp 2014-09-27 09:00:00):

"contact":"4096R\\/261C5FBE77285F88FB0C343266C8C2D7C5AA446D Sebastian ... "

remains to see what data arrived originally in order to find the source of the "escape".

comment:2 Changed 5 years ago by iwakeh

The contact field from the voe document is correct, excerpt:

contact 4096R/261C5FBE77285F88FB0C343266C8C2D7C5AA446D Sebastian ...

(The unwanted escaping concerns many more contact infos.)

Inside Onionoo the escaping of the forward slash is a Gson feature.
A small patch is attached.

comment:3 Changed 5 years ago by karsten

Status: newneeds_revision

Good catch, Sebastian, and thanks for the patch, iwakeh. But we have to be careful when fixing this problem. Let me explain:

Here's a server descriptor published by another currently running relay, particularly the only relay that I found that has both characters / and ' in its contact line:

router DigitalBrains 83.161.152.50 21 0 0
or-address [2001:980:a370::8]:80
platform Tor 0.2.4.23 on Linux
protocols Link 1 2 Circuit 1
published 2014-09-28 06:57:28
fingerprint B41E 3847 4992 3487 4688 B2F3 54C6 764B 8589 EAF8
uptime 718153
bandwidth 1572864 10485760 2790125
extra-info-digest 7201355DF31896829B95CB970A63CD4D6BCFE93B
onion-key
-----BEGIN RSA PUBLIC KEY-----
MIGJAoGBAKcLROc5/W6X7bsL5XX3+miireX/kU8HW2mC+Yp3OVD2bqccP++kZCUj
XWWEjWv4nUJ8OnbbFAFey4+/Oe7cIoySqfOgQK04iIiU8JTs1/16vtjYLPSGg2JW
kZrvWIsHlX1+/4y1PRnjr0VdHgxQKfuDS/Rc4+HtUNcqbFo5QqpbAgMBAAE=
-----END RSA PUBLIC KEY-----
signing-key
-----BEGIN RSA PUBLIC KEY-----
MIGJAoGBAMT/Ev+LaTTtvmn+4zO3/w4XYcJTDdG7r+5G4Jsgkz3AwmMm6fUKZON5
QKDA3osPoC+R6sRR0PoWnXkWgab4nCOtVdzA9zyrcNHuMo4jFyhfO0QYK3jok+wv
oFhWygCleT0XIWtVrWCoA2kmM5iQpcfSry830cA2iwasRx9qNSVzAgMBAAE=
-----END RSA PUBLIC KEY-----
hidden-service-dir
contact 2048R/DE500B3E Peter Lebbing <tormail at (this router's nickname) dot com>
ntor-onion-key 39Iw4BEaOdFRfvLo2Ko1Eg3iLS54jKSr1SwcBpcBP3Y=
reject *:*
router-signature
-----BEGIN SIGNATURE-----
BJqOVb7/EACgyNf1LpuyDcWT5hHoFrsPOY0+GoyQwPyh5ejruCwju856aZ7yVoJk
Iv2adms2ONiXFtAlTawTybptcpgXGmNaTXAeWnyFMdrYpWVkej0Ha2zHVl7hXJoH
5cKnN3SNYvqdxXdtBM9bnyPLfyvlqTnmPlKRTrV3W/g=
-----END SIGNATURE-----

The hourly cronjob has a first phase during which it writes descriptor contents to a details status file. Here's what it writes for this particular relay:

"desc_published": "2014-09-29 06:51:59",
"last_restarted": "2014-09-19 23:28:14",
"bandwidth_rate": 1572864,
"bandwidth_burst": 10485760,
"observed_bandwidth": 3127441,
"advertised_bandwidth": 1572864,
"exit_policy": [
    "reject *:*"
],
"contact": "2048R\\/DE500B3E Peter Lebbing <tormail at (this router's nickname) dot com>",
"platform": "Tor 0.2.4.23 on Linux"

During the second phase the hourly cronjob reads details status files and writes them to details documents which will later be copied verbatim to servlet responses. Here's the relay's details document:

{
    "nickname": "DigitalBrains",
    "fingerprint": "B41E3847499234874688B2F354C6764B8589EAF8",
    "or_addresses": [
        "83.161.152.50:21",
        "[2001:980:a370::8]:80"
    ],
    "last_seen": "2014-09-29 08:00:00",
    "last_changed_address_or_port": "2014-09-26 09:00:00",
    "first_seen": "2014-09-26 09:00:00",
    "running": true,
    "flags": [
        "Fast",
        "Guard",
        "Running",
        "Stable",
        "Valid"
    ],
    "country": "nl",
    "country_name": "Netherlands",
    "latitude": 52.5,
    "longitude": 5.75,
    "as_number": "AS3265",
    "as_name": "XS4ALL Internet BV",
    "consensus_weight": 3400,
    "last_restarted": "2014-09-19 23:28:14",
    "bandwidth_rate": 1572864,
    "bandwidth_burst": 10485760,
    "observed_bandwidth": 3127441,
    "advertised_bandwidth": 1572864,
    "exit_policy": [
        "reject *:*"
    ],
    "exit_policy_summary": {
        "reject": [
            "1-65535"
        ]
    },
    "contact": "2048R\\/DE500B3E Peter Lebbing <tormail at (this router's nickname) dot com>",
    "platform": "Tor 0.2.4.23 on Linux",
    "advertised_bandwidth_fraction": 0.00012835582,
    "consensus_weight_fraction": 0.0001344956,
    "guard_probability": 0.0002506447,
    "middle_probability": 0.00015283491,
    "exit_probability": 0,
    "recommended_version": true
}

The easy fix would be to update the escapeJSON and unescapeJSON methods in DetailsDocument similar to iwakeh's patch. Newly written details documents would then not contain escaped forward slashes anymore. (There's no easy way to force rewriting details documents, but we could write a simple script or Java tool that goes over all files in out/details/, removes backslashes, and rewrites files.)

The harder fix would be to also apply iwakeh's patch. The reason is that the hourly cronjob would read old and new details status files, and it wouldn't handle old status files correctly. (I didn't go through all steps there, but I think this would break.) So, we'd have to first go through all old status files and update them and then apply the patch. This needs testing before we should apply it to the production system. Plus, we'll also have to do the easy fix to DetailsDocument, or our changes won't be effective at all.

iwakeh, would you want to prepare a revised patch and test it on a local Onionoo instance?

comment:4 Changed 5 years ago by iwakeh

Ah, right we have all those "old" files. Thanks for the detailed explanation!

I'll take another look at it.

comment:5 Changed 5 years ago by iwakeh

Status: needs_revisionnew

comment:6 Changed 5 years ago by karsten

Status: newneeds_review

While working on a fix, I discovered that we introduced a related bug a few months ago. Right now we're not only escaping / wrong, but also '...

Here's the contact line currently returned for the sample relay mentioned above:

"contact":"2048R\\/DE500B3E Peter Lebbing <tormail at (this router\\'s nickname) dot com>"

Please find my branch task-13267 with two commits, one to fix that recently introduced bug and one to stop escaping /.

Note that I'm ignoring my own advice by handling old files and new files the same. At least that didn't break horribly in local tests. I'm going to deploy this branch on the mirror now to see if it works.

I'd like to merge to master before the weekend, if possible. Review much appreciated.

comment:7 Changed 5 years ago by karsten

Resolution: fixed
Status: needs_reviewclosed

The mirror didn't break. Merging to master and deploying on the main instance. Closing. Thanks!

Note: See TracTickets for help on using tickets.