Re: [tor-bugs] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  closed
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:  implemented
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by nickm):

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


Comment:

 Merging to master (which is now 0.3.4).

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by nickm):

 I think this is something we should plan for "no backport" on:  We've done
 this allocation in every version of Tor that's had the current relay
 crypto algorithm, ever.  Unless we found that it had a '''massive'''
 performance impact, I think it's not a backport candidate.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by teor):

 Do you think we should backport this change?
 Or are CPU usage / RAM fragmentation bugs not severe enough to backport?

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => merge_ready


Comment:

 Okay.  Squashed it, and holding it in merge_ready till the 0.3.4 merge
 window opens.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by dgoulet):

 lgtm;

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 Yeah: I don't see an attack from not zeroing it, but I think it's
 generally good to zero crypto stuff reflexively.

 I've added a couple of fixup commits; better now?

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by dgoulet):

 Replying to [comment:6 nickm]:
 > The rationale for not using crypto_digest_assign() here is that
 crypto_digest_t is opaque, so it can't be stack-allocated.  Does that seem
 plausbile?
 >
 > I agree that the memset should be memwipe.

 Ah indeed. Ok makes sense for both. I'm not sure we need to zero it here
 also but maybe it is security related so no strong opinion.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by nickm):

 The rationale for not using crypto_digest_assign() here is that
 crypto_digest_t is opaque, so it can't be stack-allocated.  Does that seem
 plausbile?

 I agree that the memset should be memwipe.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by Hello71):

 * cc: alex_y_xu@… (added)


--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+

Comment (by Hello71):

 also, this is the only non-test caller of crypto_digest_assign, so it
 seems that we could obsolete that.

 also, I'm suspicious of the memset(0). I don't totally understand, but
 either this is security-critical, and we should use memwipe, or it isn't,
 and don't bother zeroing it at all. I suspect gcc will optimize this
 memset out anyways.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by dgoulet):

 * status:  needs_review => needs_revision
 * reviewer:   => dgoulet


Comment:

 Why not use `crypto_digest_assign()` and `crypto_digest_t` in the new
 checkpoint data structure (inline)?

 Asking because `crypto_digest_checkpoint()` and `crypto_digest_restore()`
 really looks like the assign function.

 Also extra whitespaces:

 {{{
 +crypto_digest_checkpoint(crypto_digest_checkpoint_t *checkpoint,
 +  const crypto_digest_t *digest)
 }}}

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  accepted => needs_review


Comment:

 Branch `bug24914` has a simple patch for this.

--
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] #24914 [Core Tor/Tor]: Maybe make relay_digest_matches() not use tor_malloc()

[Tor Bug Tracker & Wiki]

#24914: Maybe make relay_digest_matches() not use tor_malloc()
--+
 Reporter:  dgoulet   |  Owner:  nickm
 Type:  defect| Status:  accepted
 Priority:  Medium|  Milestone:  Tor: 0.3.4.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  tor-relay |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  new => accepted
 * owner:  (none) => nickm
 * milestone:  Tor: 0.3.3.x-final => 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