Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:  closed
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-

Comment (by nickm):

 (Also merged the spec patch)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:  closed
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by asn):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 Merged!!! Thanks a lot

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  merge_ready
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by asn):

 * status:  needs_review => merge_ready


Comment:

 Great. I squashed and made a PR here:
 https://github.com/torproject/tor/pull/1928

 Waiting to finish and will merge.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_review
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by Jaym):

 * status:  needs_revision => needs_review


Comment:

 Done! https://github.com/torproject/tor/pull/1926

 rebase on master fixed the appveyor issue, thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Things look good right now. Seems like the CI is broken tho. Any chance
 you could rebase to master (I think that should fix appveyor) and also fix
 the wide line in   `Wide:../src/feature/client/entrynodes.c:94` (I
 think that should fix travis).

 Thanks! We are close!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_review
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by Jaym):

 * status:  needs_revision => needs_review


Comment:

 - Added light doc about the smartlist_sort pattern
 - Change on guard-spec.txt: https://github.com/torproject/torspec/pull/119

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 As discussed in IRC we asked for some documentation on the smartlist_sort
 pattern,.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_review
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 Oh!  It sounds like this is ready for a fresh review then.  I'll put it
 back in needs_review.

 (Yeah, I agree about testing.  It's gotten to the point where when I write
 a test that passes the first time, I am suspicious that it isn't really
 testing the code.)

 As for the tests, it looks like you're getting this compilation error:
 {{{
 x86_64-w64-mingw32-gcc -DHAVE_CONFIG_H -I. -I..
 -DSHARE_DATADIR="\"/mingw64/share\"" -DLOCALSTATEDIR="\"/mingw64/var\""
 -DBINDIR="\"/mingw64/bin\"" -DTOR_UNIT_TESTS -I../src -I../src/ext
 -I../src/ext/trunnel -I../src/trunnel -I../src/ext -Isrc/ext
 -DSHARE_DATADIR="\"/mingw64/share\"" -DLOCALSTATEDIR="\"/mingw64/var\""
 -DBINDIR="\"/mingw64/bin\"" -DTOR_UNIT_TESTS  -DHAVE_MODULE_RELAY=1
 -DHAVE_MODULE_DIRAUTH=1 -DHAVE_MODULE_DIRCACHE=1 -I/mingw64/include
 -IC:/msys64/mingw64/include -L/mingw64/include  -g -O2 -fasynchronous-
 unwind-tables -Wall -fno-strict-aliasing -Werror  -Waddress -Warray-bounds
 -Wdate-time -Wdouble-promotion -Wduplicate-decl-specifier -Wduplicated-
 cond -Wenum-conversion -Wextra -Wfloat-conversion -Wignored-attributes
 -Wimplicit-fallthrough -Winit-self -Wlogical-op -Wmissing-field-
 initializers -Wmissing-format-attribute -Wmissing-noreturn
 -Wnormalized=nfkc -Wnull-dereference -Woverlength-strings -Woverride-init
 -Wshadow -Wshift-count-negative -Wshift-count-overflow -Wshift-negative-
 value -Wshift-overflow=2 -Wsizeof-array-argument -Wstrict-overflow=1
 -Wstring-compare -Wsuggest-attribute=format -Wsuggest-attribute=noreturn
 -Wswitch-bool -Wsync-nand -Wtrampolines -Wunused-but-set-parameter
 -Wunused-but-set-variable -Wunused-const-variable=2 -Wunused-local-
 typedefs -Wvariadic-macros  -W -Wfloat-equal -Wundef -Wpointer-arith
 -Wstrict-prototypes -Wmissing-prototypes -Wwrite-strings -Wredundant-decls
 -Wchar-subscripts -Wcomment -Wformat=2 -Wwrite-strings -Wnested-externs
 -Wbad-function-cast -Wswitch-enum -Waggregate-return -Wpacked -Wunused
 -Wunused-parameter  -Wold-style-definition -Wmissing-declarations -MT
 src/test/test-test_geoip.o -MD -MP -MF src/test/.deps/test-test_geoip.Tpo
 -c -o src/test/test-test_geoip.o `test -f 'src/test/test_geoip.c' || echo
 '../'`src/test/test_geoip.c
 bash.exe : In file included from ../src/lib/container/smartlist.h:18,
 At line:2 char:5
 + & $commandPath $args 2>&1
 + ~
 + CategoryInfo  : NotSpecified: (In file
 include...smartlist.h:18,:String) [], RemoteException
 + FullyQualifiedErrorId : NativeCommandError

  from ../src/core/or/or.h:29,
  from ../src/test/test_entrynodes.c:14:
 ../src/test/test_entrynodes.c: In function
 'test_entry_guard_correct_cascading_order':
 ../src/lib/smartlist_core/smartlist_core.h:40:55: error:
 'old_primary_guards' may be used uninitialized in this function [-Werror
 =maybe-uninitialized]
40 | #define smartlist_free(sl) FREE_AND_NULL(smartlist_t,
 smartlist_free_, (sl))
   |
 ^~~
 ../src/test/test_entrynodes.c:1825:16: note: 'old_primary_guards' was
 declared here
  1825 |   smartlist_t *old_primary_guards = smartlist_new();
   |^~
 mv -f src/test/.deps/test-test_geoip.Tpo src/test/.deps/test-test_geoip.Po
 }}}

 The issue here is that all of the `tt_...()` macros will run `goto done;`
 on failure.  The `done` block of
 `test_entry_guard_correct_cascading_order` is calling
 `smartlist_free(old_primary_guards)`... but that variable isn't actually
 declared until half-way through the function.  You can fix this error by
 declaring it earlier.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-

Comment (by Jaym):

 Replying to [comment:28 nickm]:
 > Hi, how is this going?  I'd love to take a version of this in 0.4.4 if
 we can, to improve our security, but it might make more sense to take it
 early in 0.4.5 if it's not going to be done by the deadline (Jun 15).

 I'd love to see it in 0.4.4. I've made the requested unit test last Sunday
 and replied to asn's comment on the github pull request
 (https://github.com/torproject/tor/pull/1873). Btw asn, that was a good
 call, I caught a corner case issue.

 So currently, I am missing a patch in guard-spec.txt and I can workout the
 commit structure as soon as the code is flagged ok. I am sure all of this
 can be done this week :)

 Also, Travis and AppVeyor are complaining but I did not understand why (I
 found the logs hard to deciphers).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
-+-
 Reporter:  Jaym |  Owner:  (none)
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  High |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-spec prop271 prop310 044-should  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  nickm, asn   |Sponsor:
-+-
Changes (by nickm):

 * keywords:  tor-spec prop271 prop310 => tor-spec prop271 prop310
   044-should


Comment:

 Hi, how is this going?  I'd love to take a version of this in 0.4.4 if we
 can, to improve our security, but it might make more sense to take it
 early in 0.4.5 if it's not going to be done by the deadline (Jun 15).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm, asn|Sponsor:
--+

Comment (by asn):

 Hey thanks for the super fast revisions here! I left a comment on the new
 PR.

 Furthermore, I'd be more confidence if we had some more testing here. The
 test coverage of the branch in terms of LoC is great, but I would also
 like a test that tests the entire logic of this new feature. In
 particular, given that the changes are far from trivial, and change
 various behaviors in the code, I would like a test that tests most of
 that. In particular, I would be looking for something like: Load some
 guards to the sampled set (without using a state file), make sure that the
 sampled set has the sampled idxs correctly set, then create the confirmed
 set and make sure that's well ordered too, then make the primary guards
 list and make sure that's well ordered, finally ask for a guard and make
 sure that's what you would expect. Feel free to reuse code from other
 unittests of course...

 So, before this branch gets merged we would need to do the following two
 things:
 - Get that unittest in.
 - Answer the comment on the new PR.
 - Get the torspec patch for `guard-spec.txt` merged.
 - Squash the PR into more organized commits. Ideally this PR would be
 squashed down to max 3-4 logical commits. Something like the following
 commit structure would work, but feel free to improvise:
   - Commit 1: Use helper functions
 parse_from_state_set_vals/parse_from_state_handle_time
   - Commit 2: Implement prop310"
   - Commit 3: Fix existing unittests
   - Commit 4: Add unittests

 Thanks a lot for your work! :)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm, asn|Sponsor:
--+
Changes (by asn):

 * status:  needs_review => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm, asn|Sponsor:
--+
Changes (by Jaym):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm, asn|Sponsor:
--+

Comment (by Jaym):

 Thanks! rebased on https://github.com/torproject/tor/pull/1873

 + tried to improve based on asn's comments

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm, asn|Sponsor:
--+
Changes (by asn):

 * status:  needs_review => needs_revision
 * reviewer:  nickm => nickm, asn


Comment:

 Hello, thanks for all the work here! I actually missed the proposal and
 the ticket so this caught me by surprise today. This seems really well
 thought all-in-all and a solid patch, and the test adjustments look good
 to.

 I added a few comments to the PR. It's mainly documentation requests to
 polish some parts that I didn't comprehend.

 Also, can the next revision come with a new PR, which contains the changes
 in the top commits? This can be done with a rebase. Isolating the changes
 of this PR was not easy for me because the relevant commits were scattered
 around the log.

 Thanks a lot!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by nickm):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+

Comment (by Jaym):

 Hello Nick,

 Thanks for the review!

 Things have been fixed and github looks happy now.

 Summary: The problem was apparently due to a qsort() behavior difference
 between my setup and appveyor running on windows (I wasn't experiencing
 the failure). Yet, the test was doing something weird; i.e., setting
 confirmed_idx of different guards to the same value, which is not
 something that seems ok to test. It means unstable behaviors with sorted
 lists, and we don't like unstable behaviors :)

 I've slightly modified the original test since now the confirmed_guards
 are sorted by sampled_idx, and it makes sure that the confirmed guards are
 sorted as expected, with valid confirmed_idx (i.e., not the same).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Sorry for the delay: the last week or so has been hugely busy.

 The changes look good, but CI still isn't passing.  It looks like these
 tests are failing on appveyor:
 {{{
 entrynodes/confirming_guards: [forking] Skipping 1 testcases.
   FAIL ../src/test/test_entrynodes.c:1513: assert(g1->confirmed_idx OP_EQ
 0): 2 vs 0
   [confirming_guards FAILED]
 entrynodes/confirming_guards_reasonably_future: [forking] Skipping 1
 testcases.
   FAIL ../src/test/test_entrynodes.c:1513: assert(g1->confirmed_idx OP_EQ
 0): 2 vs 0
   [confirming_guards_reasonably_future FAILED]
 entrynodes/confirming_guards_reasonably_past: [forking] Skipping 1
 testcases.
   FAIL ../src/test/test_entrynodes.c:1513: assert(g1->confirmed_idx OP_EQ
 0): 2 vs 0
   [confirming_guards_reasonably_past FAILED]
 }}}

 I'm also seeing more practracker issues on Travis:
 {{{
 problem function-size
 /src/feature/client/entrynodes.c:entry_guard_parse_from_state() 272
 }}}

 For this one we should probably look for some code we can extract into a
 new function, to simplify this function.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by Jaym):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_revision
 Priority:  High  |  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by nickm):

 * priority:  Medium => High
 * status:  needs_review => needs_revision


Comment:

 Hi!  Initial notes here.  Sorry for the delay in the review.

 First thing is, continuous integration isn't passing. It looks like there
 might be a memory leak in the unit tests?  You can test that yourself by
 building with --enable-fragile-hardening and running the tests.

 Also there seems to be a practracker failure:
 {{{
 problem file-size /src/feature/client/entrynodes.h 652
 }}}
 You can suppress this warning by editing the
 scripts/maint/practracker/exceptions.txt file and increasing the number on
 that line.

 The code itself looks well-written and straightforward.  I have some
 suggestions for perhaps making it more robust; I have left them on the
 github request.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+

Comment (by teor):

 Replying to [comment:14 Jaym]:
 > The pull request has been updated.
 >
 > On loading, Tor sets the sampled_idx to the confirmed_idx. That should
 keep older clients to behave the same (and not reordering primary guards).
 On the next state save, the sampled_idx should be made dense.
 >
 > Also, the patch applies now ordering when it seems necessary (a couple
 of redundant orderings have been removed).

 Thanks!

 > Also, I was concerned by the fact that Tor assumes integrity of the
 state when loading it. If some application has write access to this file,
 making the client rotate guards until a chosen one is found shouldn't be
 too much of a hard task. Is that kind of threat relevant?

 An attacker who can modify files on the local system could do many worse
 things. So those attacks are not really part of tor's threat model. To
 defend against those kinds of attacks, people should use an amnesiac
 system like TAILS.

 File corruption is a risk, though. And tor could detect file corruption
 earlier with checksums. But that's a different ticket :-)

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop310  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by gk):

 * keywords:  tor-spec prop271 prop308 => tor-spec prop271 prop310


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #32088 [Core Tor/Tor]: Proposal 310 - choose guards in sampled order (was: Proposal 308 - choose guards in sampled order)

[Tor Bug Tracker & Wiki]

#32088: Proposal 310 - choose guards in sampled order
--+
 Reporter:  Jaym  |  Owner:  (none)
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.4.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-spec prop271 prop308  |  Actual Points:
Parent ID:| Points:
 Reviewer:  nickm |Sponsor:
--+
Changes (by gk):

 * status:  needs_revision => needs_review


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs