On Tue, Oct 4, 2011 at 1:05 PM, Steven Murdoch Steven.Murdoch@cl.cam.ac.uk wrote:
From a first look at 176 it looks good. Some comments and suggestions inline:
Thanks, Steven!
Terminological note: I use "client" below to mean the Tor instance (a client or a bridge or a relay) that initiates a TLS connection, and "server" to mean the Tor instance (a bridge or a relay) that accepts it.
There are still some initiator/responder below, which I've pointed out.
I've changed the above paragraph to make "client" and "initiator" are synonyms, as are "server" and "Responder".
On padding: agreed; I've added references to padding in the appropriate places, and referenced proposal 184.
To check the AUTHENTICATE cell, a server checks that all fields containing a hash contain the correct value, then verifies the signature. The server MUST ignore any extra bytes in the signed data after the SHA256 hash.
I'd suggest expanding on what "correct value" means for each of the fields. Some might be obvious but others are not (e.g. TIME, which I see later on is not currently checked).
I've changed that statement to say that all fields from TYPE through TLSSECRETS should be checked to make sure their "unique correct value as specified above."
If we allow padding cells then I think we should probably include them in SLOG and CLOG.
Agreed.
If the client was not able to include a non-zero TLSSECRET component, or the server can't check it, the answer is a little trickier. The server knows that it is not getting a replayed AUTHENTICATE cell, since the cell authenticates (among other stuff) the server's AUTH_CHALLENGE cell, which it has never used before. The server knows that it is not getting a MITM'd AUTHENTICATE cell, since the cell includes a hash of the server's link certificate, which nobody else should have been able to use in a successful TLS negotiation.
Is a zero TLSSECRET permitted anymore?
Nope; I've reworded this paragraph.
A signature of the TLSSECRET element on its own should be sufficient to prevent the attacks we care about, but because we don't necessarily have access to the TLS master secret when using a non-C TLS library, we can't depend on it. I added it anyway so that, if there is some problem with the rest of the protocol, clients and servers that _are_ written in C (that is, the official Tor implementation) can still be secure.
Again, it looks like TLSSECRET is now mandatory.
Also reworded this one.
Thanks,