Opened 8 years ago

Closed 8 years ago

Last modified 9 months ago

#4371 closed defect (fixed)

We close a connection to a relay whose time is more than an hour in the future

Reported by: arma Owned by:
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-client
Cc: bastik.public@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

#4370 is about warnings about scary times. Those are worrying to users but not actually harmful. The real problem is that we declare the cert invalid and close the connection if the relay we're talking to is 61 minutes in the future. That's new (and much too strict) behavior.

The previous behavior was to not care about the clock the other guy claims to have, so long as he talks the Tor protocol. Things will likely go poorly if he's more than a week or two out of date (e.g. since somebody's onion key will probably be wrong now), but that's no reason to give up without trying.

In fact, I think log_cert_lifetime() and tor_tls_check_lifetime() are in 0.2.2.x but never called?

So what do we give up by not checking the time on certs in 0.2.3.x either? The cert is valid for a whole year, so I'm not sure what attacks we resolve by being this precise. (If we still want to check if it's *past* the one year mark, I wouldn't object.)

Pointed out by "bastik_tor".

Child Tickets

Change History (13)

comment:1 Changed 8 years ago by bastik

Cc: bastik.public@… added

comment:2 Changed 8 years ago by arma

My directory authority doesn't consider this sort of failed handshake to be a successful reachability attempt, so it has stopped voting Running for this relay (nickname 0xABCD).

I wonder why the other directory authorities are still giving it the Running flag.

comment:3 Changed 8 years ago by nickm

I am fine with raising the acceptable range, but having *no* liveness checks on the cert at all seems like a less good idea. Shall we say 48 hours?

comment:4 in reply to:  3 ; Changed 8 years ago by rransom

Replying to nickm:

I am fine with raising the acceptable range, but having *no* liveness checks on the cert at all seems like a less good idea. Shall we say 48 hours?

We nuked the liveness checks on our TLS certificates in the fix for #4014, by increasing cert lifetimes to one year. Any certificate which is ‘not yet valid’ should be accepted.

comment:5 Changed 8 years ago by stars

Hi,

I get the same problem with the last Tor git:. Tor v0.2.3.7-alpha (git-4a7225d4c97b956f) (with bufferevents). (Running on Linux x86_64)

The problem was happens after the winter time change in my country ( CH ). I suspect that the problem come from that..

Best regards

SwissTorExit

comment:6 Changed 8 years ago by stars

i have missed my log :

nov. 07 19:08:19.511 [Warning] Certificate not yet valid: is your system clock set incorrectly?
nov. 07 19:08:19.512 [Warning] (certificate lifetime runs from Nov 8 00:27:20 2011 GMT through Nov 7 00:27:20 2012 GMT. Your time is Nov 07 18:08:19 2011 GMT.)

comment:7 in reply to:  4 Changed 8 years ago by nickm

Status: newneeds_review

Replying to rransom:

Replying to nickm:

I am fine with raising the acceptable range, but having *no* liveness checks on the cert at all seems like a less good idea. Shall we say 48 hours?

We nuked the liveness checks on our TLS certificates in the fix for #4014, by increasing cert lifetimes to one year. Any certificate which is ‘not yet valid’ should be accepted.

I still get squirmy with the idea of "no checking." How about we require liveness within 48 hours in the past, and within a month in the future.

How about branch "bug4371" in my public repository?

comment:8 Changed 8 years ago by Sebastian

In your comment here you say 48 hours in the past, month in the future, your code does the opposite, I think?

Also, should this be a protocol warning? At least if we haven't heard this from a few places before, maybe? Otherwise people might still get confused by the warning.

Otherwise the branch looks good.

comment:9 Changed 8 years ago by nickm

Okay, tried to fix those. How's the branch look now?

comment:10 Changed 8 years ago by Sebastian

Looks good now. I wonder if anyone would argue we should actually print out this message when we start getting it from more than a few places, but I won't be the one to argue that. I'd be happy with a merge now.

comment:11 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

think I might agree with you, but I want to call that more of a general "can we bootstrap a more trusted time estimate from multiple untrusted sources" question, and it's not this bug in particular.

Squashed, merged, closing. Thanks again!

comment:12 Changed 7 years ago by nickm

Keywords: tor-client added

comment:13 Changed 7 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.