Re: [Tails-dev] tordate issue in Tails 0.14~rc1

Supprimer ce message

Répondre à ce message
Auteur: anonym
Date:  
À: The Tails public development discussion list
Sujet: Re: [Tails-dev] tordate issue in Tails 0.14~rc1
21/10/12 10:41, intrigeri wrote:
> hi,
>
> anonym wrote (18 Oct 2012 11:18:43 GMT) :
>> Implemented in bugfix/tordate_vs_tor_0.2.3.x
>
> Looks good.
>
> I'm worried about the reliance on exact log messages wording,
> but well...


Absolutely.

> Here follows some nitpicking on implementation details.
>
> I'm in doubt with the "grep -m 1" on Tor log file: don't we want the
> *last* occurrence of e.g. "certificate lifetime", rather than the
> first one, in case there would be several? This may very well be
> a rare edge case, or even impossible, but still, if we indeed want the
> last occurrence, I'd rather see this expressed explicitly in code.


At first I thought no, but there is indeed a rare edge case:

1. Start Tails with an incorrect clock.
2. For some reason tordate quits without finishing (so no
$TORDATE_DONE_FILE is written).
3. Leave the Tails session for at least a month.
4. Try to connect again, so tordate is run once more. The time will be
set to some time ~1 month back (when we ran step 1) so Tor's TLS
connection will fail.

Beyond that, what you say makes more sense any way. Fixed.

> Same for:
>>      grep -q "\[warn\] Certificate \(not yet valid\|already expired\)." \
>>          /var/log/tor/log


I'm usure what you mean here. Care to clarify?

I can see a small issue here since we only look for the existence of
such a line, so it may be an old line from a previous (failed) Tor
bootstrap attempt that's not relevant any more. That could make
is_clock_way_off() return true even if our clock is perfect, and hence
have it set to some old cert's valid-after date. AFAICT it will only
cause a problem in scenarios like the one I described in steps 1-4 above
(i.e. they rely on the same Tails session running for > a month).

I think we can just leave this one. Otherwise I suppose we could look
into clearing the Tor log each time it starts.

> About "grep -m 1 SIMPLIFIED_PATTERN | sed ...": I think this can be
> done in a more robust way with sed only, which would also be a way to
> get the last occurrence if that's what we want.


I can go as far as `sed -n s/.../.../p` to grop the grep, but I won't go
further (please do yourself if you feel it's important). More advanced
sed than that *usually* ends up making the code a lot harder to read.
OTOH, if there is a super simple *and* readable way to also only write
the last match in the above sed stanza, I'd be happy to learn about it.

> Please use long options (e.g. --max-count, --only-matching, etc.):
> more readable, easier to review and maintain.
>
> About "grep | tail -n1": I think "tac | grep --max-count 1",
> or sed alone, may be a bit cheaper. Not that we care much.


Again I think I'll go with what's easier to read, i.e. the `tail` approach.

>> Grep Tor's log instead of GETINFO the control port.
>> Through extensive testing it seems that opening a controller and/or
>> issuing "GETINFO status/bootstrap-phase" during early bootstrap gets
>> Tor stuck at "Bootstrapped 5%".
>
> Argh :( Upstream bug report?


Once I have something reproducible I will report it.

> Weird fact: Vidalia seems to be able to do the same without triggering
> this bug.


Vidalia does it rather differently; it gets the bootstrap progress from
STATUS_CLIENT events, not by GETINFO-bombing with a new control port
connection (AUTHENTICATE...QUIT) for each query. :)

> Nitpicking (about the commented-out tor_control_getinfo()): according
> to the Tor control protocol spec, even if Tor MAY accept LF alone,
> "Controllers SHOULD always send CRLF".


Fixed :).

>> (merged it into experimental).
>
> It seems you forgot to push.


Pushed now.

Cheers!