Re: [tor-bugs] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  closed
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:  fixed
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by nickm):

 * status:  merge_ready => closed
 * resolution:   => fixed
 * milestone:  Tor: 0.2.9.x-final => Tor: 0.3.0.x-final


Comment:

 Not backporting.

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  merge_ready
 Priority:  Medium|  Milestone:  Tor: 0.2.9.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:  dgoulet   |Sponsor:
--+
Changes (by nickm):

 * keywords:  029-backport, review-group-16 => 029-backport
 * milestone:  Tor: 0.3.0.x-final => Tor: 0.2.9.x-final


Comment:

 Squashed as `bug21420_029_squashed` ; merged to 0.3.0; marking for
 possible 0.2.9 backport.  (I suggest no backport here.)

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
---+---
 Reporter:  mmcloughlin|  Owner:  nickm
 Type:  defect | Status:  merge_ready
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  029-backport, review-group-16  |  Actual Points:
Parent ID: | Points:
 Reviewer:  dgoulet|Sponsor:
---+---
Changes (by dgoulet):

 * status:  needs_review => merge_ready


Comment:

 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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
---+---
 Reporter:  mmcloughlin|  Owner:  nickm
 Type:  defect | Status:  needs_review
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  029-backport, review-group-16  |  Actual Points:
Parent ID: | Points:
 Reviewer:  dgoulet|Sponsor:
---+---
Changes (by nickm):

 * reviewer:   => dgoulet


--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
---+---
 Reporter:  mmcloughlin|  Owner:  nickm
 Type:  defect | Status:  needs_review
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  029-backport, review-group-16  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 I've tried to add more explanatory comments in bug21420_029, with an
 explanation of why we're trying to do it this way (basically: we thought
 it would help with fingerprinting).

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
---+---
 Reporter:  mmcloughlin|  Owner:  nickm
 Type:  defect | Status:
   |  needs_revision
 Priority:  Medium |  Milestone:  Tor:
   |  0.3.0.x-final
Component:  Core Tor/Tor   |Version:
 Severity:  Normal | Resolution:
 Keywords:  029-backport, review-group-16  |  Actual Points:
Parent ID: | Points:
 Reviewer: |Sponsor:
---+---

Comment (by mmcloughlin):

 Looks like the certificate verification allows wiggle room at either end:
 2 days on the expiry and 30 days on the start date.

 
https://github.com/torproject/tor/blob/67eb6470d711b36d1b855e6423ce7bbb302af834/src/common/tortls.c#L897-L900

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by mmcloughlin):

 Should we be concerned that none of these certificates were rejected as
 invalid?

 
https://github.com/torproject/torspec/blob/b5c4adb0c2b04d90e14d75dbe8fcae4bfc738b0f
 /tor-spec.txt#L570-L571

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by dgoulet):

 * status:  needs_review => needs_revision


Comment:

 This comment, I can't understand the why nor the what (starting at the
 "instead"):

 {{{
   /* Our certificate lifetime will be cert_lifetime no matter what, but if
 we
* start cert_lifetime in the past, we'll have 0 real lifetime.  instead
 we
* start up to (cert_lifetime - min_real_lifetime - start_granularity)
 in
* the past. */
 }}}

 I do understand that we absolutely want "cert_lifetime" but then the
 explanation for how we do that is confusing to me. We "start up to" what
 exactly? and what is this "in the past"? Trying to understand: we use the
 lifetime value we want minus some values which are the minimum real
 lifetime (basically the minimum allowed for lifetime of a cert?) and then
 a "granularity" that I don't know why we use that. I see this comment
 `Lastly, be sure to start on a day boundary.` but no why.

 And then the code is kind of the same thing but intuitively is reverse :).

 {{{
   time_t earliest_start_time = now - cert_lifetime + min_real_lifetime +
 start_granularity;
 }}}

 The math aren't that difficult but are easily confusing especially with a
 lifetime concept so I would really love to see a unit test testing the
 boundaries. And this whole snippet of code could even be extracted in a
 separate function for clarity, documentation and easier testing.

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 I've commented stuff, renamed stuff, and fixed the comparison in a fixup
 commit on the same branch `bug21420_029` .

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_revision
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 Replying to [comment:5 arma]:
 >
 > Maybe some more comments to explain what we're computing, and *why*,
 would help?

 Okay, will do.

 > Looking at the origin commit, it seems maybe I wanted to say "- 2 days",
 not "+ 2 days". Would that resolve everything here?

 I thought about it, but if you had done that, it would be possible to have
 the start time be "now - lifetime - 2 days".  And the end time would be
 "start + lifetime", which would result in an already-expired certificate.

 > Maybe I am deeply confused, but won't
 > {{{
 > +  if (earliest_start_time < now)
 > +earliest_start_time = now;
 > +  start_time = crypto_rand_time_range(earliest_start_time, now);
 > }}}
 > trigger the assert in crypto_rand_time_range() that min < max, since
 we'll be passing it "now, now"?

 ohhh, yeah. Better fix that too.

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+

Comment (by arma):

 I think you're right that commit 0196647 is the problem here.

 I suspect you're right that fixing the underlying math is the right
 answer.

 However, I'm unable to follow the math here:
 {{{
 -  start_time = crypto_rand_time_range(now - cert_lifetime, now) +
 2*24*3600;
 +  const time_t min_real_lifetime = 2*24*3600;
 +  time_t earliest_start_time = now - cert_lifetime + min_real_lifetime;
 +  if (earliest_start_time < now)
 +earliest_start_time = now;
 +  start_time = crypto_rand_time_range(earliest_start_time, now);
 }}}

 Maybe some more comments to explain what we're computing, and *why*, would
 help?

 Looking at the origin commit, it seems maybe I wanted to say "- 2 days",
 not "+ 2 days". Would that resolve everything here?

 Maybe I am deeply confused, but won't
 {{{
 +  if (earliest_start_time < now)
 +earliest_start_time = now;
 +  start_time = crypto_rand_time_range(earliest_start_time, now);
 }}}
 trigger the assert in crypto_rand_time_range() that min < max, since we'll
 be passing it "now, 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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:  nickm
 Type:  defect| Status:  needs_review
 Priority:  Medium|  Milestone:  Tor: 0.3.0.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:  029-backport  |  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+
Changes (by nickm):

 * status:  assigned => needs_review
 * keywords:   => 029-backport


Comment:

 Hm.  It looks like we started using that approach in 0196647970a91d, but
 I'm not at all sure that's right.  I think we wanted to do something like
 choosing a start time at the start of a day, between this most recent
 midnight, and up to cert_lifetime in the past, but making sure that we
 don't wind up with an expiration time in the past.

 My branch `bug21420_029` in my public git repository [1] tries to fix
 this.  I've marked it as a possible backport to 0.2.9, but I believe it's
 safe to leave this as-is in existing tors, since
 check_cert_lifetime_internal() is called with a 30-day future tolerance.

 [1]
 
https://gitweb.torproject.org/nickm/tor.git/commit/?h=bug21420_029=d839f798a5812fc81fcc5b4b06604ed08dc2e558
 for the HTML version;
 https://git.torproject.org/nickm/tor.git for the repository itself.

--
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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+
 Reporter:  mmcloughlin   |  Owner:
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:  Tor: 0.3.0.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.0.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] #21420 [Core Tor/Tor]: Link certificate start date in the future

[Tor Bug Tracker & Wiki]

#21420: Link certificate start date in the future
--+-
 Reporter:  mmcloughlin   |  Owner:
 Type:  defect| Status:  new
 Priority:  Medium|  Milestone:
Component:  Core Tor/Tor  |Version:
 Severity:  Normal| Resolution:
 Keywords:|  Actual Points:
Parent ID:| Points:
 Reviewer:|Sponsor:
--+-
Changes (by mmcloughlin):

 * component:  - Select a component => Core Tor/Tor


--
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