wrong exit-list annotation

Reported and analyzed by Wojtek Porczyk, see metrics-team ml.

Next steps:

• (if possible) create test to prevent similar in future
• apply patch
• prepare data correction (script and documentation)
• test script on mirror
• hotfix release 1.1.2

changed milestone to %CollecTor 1.1.2

added component::metrics/collector milestone::CollecTor 1.1.2 owner::iwakeh priority::high resolution::fixed severity::normal status::closed type::defect labels

Should a patch be applied immediately on mirror or main to have one source of recent correct data?

Trac:
Milestone: N/A to CollecTor 1.1.2

We're not losing any data right now, just adding the wrong @type descriptor, right? If so, no need to apply the patch immediately, IMO.

You're right. The mail by Wojtek mentions that stem fails to parse the exit-lists. But, knowing the bug the descriptors could easily be pre-processed by the downloading party. Thus, no immediate changes to any CollecTor mirror necessary.

For what it's worth the trouble here for stem is simply that it uses the @type to infer the descriptor type. If they specify a 'descriptor_type' argument when they parse the descriptors stem will ignore the @type field and should be happy with these...

https://stem.torproject.org/api/descriptor/descriptor.html#stem.descriptor.init.parse_file

Cheers! -Damian

Trac:
Owner: metrics-team to iwakeh
Status: new to assigned

My branch is ready for review. There are three commits: one for the fix, one for a possible unclosed reader (which could be omitted), and one for the shell script.

The fix is running on mirror collector.sky-ip.org and the files there were fixed with the provided script. Documentation is added as comment in the script.

Please test carefully.

Trac:
Status: assigned to needs_review

Regarding the test: I added a note to tickets #20543 (moved). The restructuring is a prerequisite for creating meaningful tests here.

dcccaaa looks good.

7752b6d looks good.

5bc248e looks good and works, though I have two tweaks:

@@ -2,7 +2,7 @@
 #
 #  Only for upgrading from 1.1.0 or 1.1.1!!!
 #
-#  Script for correcting ext-list annotations.
+#  Script for correcting exit-list annotations.
 #  See task-21195 for details.
 #  Replaces 'torperf' with 'tordnsel' in files
 #  and archives.
@@ -36,7 +36,8 @@
     tar xf $ARC;
     fix $TEMP exit-list-$ym
     mv $ARC $ARC-old;
-    tar --remove-files -cJf exit-list-$ym.tar.xz exit-list-$ym
+    tar --remove-files -cf exit-list-$ym.tar exit-list-$ym
+    xz -9e exit-list-$ym.tar
     mv $TEMP/exit-list-$ym.tar.xz $ARC
     echo "--> $ARC is done."
 done;

The idea is to use xz -9e just like we used that option when creating original tarballs. Not that it matters much, but why not be consistent.

Okay, how do we proceed? Should I merge your commits (with the tweak) and put out 1.1.2? And would you want to announce it when it's out?

Trac:
Status: needs_review to merge_ready

Replying to karsten:

...

The idea is to use xz -9e just like we used that option when creating original tarballs. Not that it matters much, but why not be consistent.

All fine.

Okay, how do we proceed? Should I merge your commits (with the tweak) and put out 1.1.2? And would you want to announce it when it's out?

Sounds like a good plan!

Here's a pre-release. Please take a look and let me know if that's good to go. Thanks!

Looks fine:

• offline building from the supplied sources resulted in identical bytecode
• ant checks and test tasks passed
• signed jars passed verification

I want to make the announcement after the exit-lists on collector.tpo are fixed. Could you add a comment here when that is done?

Thanks!

There, you'll find the tarball and signature in the usual place, and all exit lists provided by https://collector.torproject.org/ contains the correct @type annotation. Please announce when you're ready, and please close this ticket afterwards. Thanks!

All finished and announced.

Closing.

Thanks!

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed