Re: [tor-bugs] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
-+-
 Reporter:  Hello71  |  Owner:  Hello71
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  034-deferred-20180602,   |  worksforme
  035-removed-20180711   |  Actual Points:
Parent ID:   | Points:
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by Hello71):

 * status:  needs_revision => closed
 * resolution:   => worksforme


--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
-+-
 Reporter:  Hello71  |  Owner:  Hello71
 Type:  enhancement  | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  unspecified
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  034-deferred-20180602,   |  Actual Points:
  035-removed-20180711   |
Parent ID:   | Points:
 Reviewer:  catalyst |Sponsor:
-+-

Comment (by Hello71):

 I don't think there's enough left here to bother revising. Please 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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+

Comment (by cypherpunks):

 Replying to [comment:5 Hello71]:
 > Hm... I see, you are correct. I really really hope that nowadays
 everybody has random seed persistence, but it is of course better to be
 conservative here.

 Unfortunately, the random seed takes quite some time (on the order of
 minutes) to actually take effect. The seed is written to the non-blocking
 character device which triggers the `random_write` file operation which
 uses `write_pool` to send the data to the input pool. Unfortunately it can
 take a while for the secondary pools to receive the seed, since they have
 to wait for the `push_to_pool` workqueue function to be triggered. On
 newer Linux kernels (using ChaCha20 rather than SHA-1 for the non-blocking
 character device), the input pool is queried every 5 minutes, and it only
 reseeds the stream cipher if more than 128 bits of entropy have been
 collected in the input pool since the last reseed.

 If you do not check for `EINTR` (and avoid the blocking behavior
 altogether), then even if you are using a persistent random seed, you will
 end up obtaining potentially predictable random data.

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+

Comment (by catalyst):

 When thinking about how to describe the user-visible parts of this change,
 I realized that the previous code would loop on `EINTR`, while the patch
 causes a failure and disables `getrandom()` thereafter.  This is unlikely
 to be a problem in practice, because libevent seems to always set
 `SA_RESTART`, which should prevent us from getting `EINTR`.

 Maybe we should mention this in the changes file.  On the other hand,
 maybe the conservative and likely harmless thing to do is to leave the
 existing loop as it is, even if it doesn't ever end up looping.  If we
 restore the loop, I think the remaining parts of the patch are some
 comment improvements and handling of a (also unlikely) short-read
 condition.

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+
Changes (by nickm):

 * status:  merge_ready => needs_revision


Comment:

 Code seems plausible.  Could somebody please write a changes file for this
 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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+
Changes (by catalyst):

 * status:  needs_review => merge_ready


Comment:

 Looks good to me! I made a squashed and rebased patch in
 https://github.com/torproject/tor/pull/107 to double check coveralls
 results.

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+
Changes (by catalyst):

 * 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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+

Comment (by catalyst):

 Replying to [comment:6 Hello71]:
 > Ah, I remember. I read the EINTR part of the ERRORS section, and I
 interpreted it to mean that we will never receive EINTR, contrary to your
 quote. I filed https://bugzilla.kernel.org/show_bug.cgi?id=199711 to ask
 which one it is.
 Thanks for filing the kernel bug report! It looks like libevent might
 always set `SA_RESTART` when installing signal handlers on systems with
 `sigaction()`? (at least based on my quick skim of the source)

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+

Comment (by Hello71):

 Ah, I remember. I read the EINTR part of the ERRORS section, and I
 interpreted it to mean that we will never receive EINTR, contrary to your
 quote. I filed https://bugzilla.kernel.org/show_bug.cgi?id=199711 to ask
 which one it is.

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+

Comment (by Hello71):

 Hm... I see, you are correct. I really really hope that nowadays everybody
 has random seed persistence, but it is of course better to be conservative
 here.

 You can see the Travis output at https://travis-
 ci.org/Hello71/tor/builds/376061939 (via https://travis-
 ci.org/Hello71/tor/branches).

--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+
Changes (by catalyst):

 * status:  needs_review => needs_revision


Comment:

 Thanks for the patch!  The `getrandom(2)` manpage on one of my Ubuntu VMs
 says
 {{{
The behavior when a call to getrandom() that is blocked  while
 reading
from  /dev/urandom  is  interrupted  by a signal handler depends on
 the
initialization state of the entropy buffer and  on  the  request
 size,
buflen.  If the entropy is not yet initialized, then the call will
 fail
with the EINTR error.
 }}}
 so in the case of tor starting up soon after boot, I think it might be
 possible to get `EINTR` if tor receives a signal while blocked on
 insufficient entropy.  Arguably we want to retry in this case.  (Also I
 would be interested in hearing if there are good reasons to treat this as
 a bug when it occurs anyway.)

 Please make a GitHub pull request for your revised patches so CI can run
 on them.  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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:  catalyst  |Sponsor:
--+
Changes (by dgoulet):

 * reviewer:   => catalyst


--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * milestone:   => Tor: 0.3.4.x-final


--
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] #26040 [Core Tor/Tor]: Improve getrandom handling

[Tor Bug Tracker & Wiki]

#26040: Improve getrandom handling
--+--
 Reporter:  Hello71   |  Owner:  Hello71
 Type:  enhancement   | Status:  needs_review
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+--
Changes (by Hello71):

 * status:  assigned => 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