Opened 8 years ago

Closed 8 years ago

#6963 closed defect (fixed)

Contradiction between specs and code to detect a V2 handshake

Reported by: cced Owned by: cypherpunks
Priority: Low Milestone: Tor: 0.2.4.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: easy spec tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:


This is a contradiction on how to differentiate V1 and V2 handshake. It's confusing if someone wants to implement his own version of Tor (JTor, silvertunnel,...).

tor-spec (and 130-v2-conn-protocol):

In "certificates up-front" (a.k.a "the v1 handshake"),
[...]  The initiator's ClientHello MUST NOT include any
    ciphersuites other than:

The actual implementation:

   /* Now we need to see if there are any ciphers whose presence means we're
    * dealing with an updated Tor. */
   for (i = 0; i < sk_SSL_CIPHER_num(session->ciphers); ++i) {
     SSL_CIPHER *cipher = sk_SSL_CIPHER_value(session->ciphers, i);
     const char *ciphername = SSL_CIPHER_get_name(cipher);
     if (strcmp(ciphername, TLS1_TXT_DHE_RSA_WITH_AES_128_SHA) &&
         strcmp(ciphername, TLS1_TXT_DHE_RSA_WITH_AES_256_SHA) &&
         strcmp(ciphername, SSL3_TXT_EDH_RSA_DES_192_CBC3_SHA) &&
         strcmp(ciphername, "(NONE)")) {
       log_debug(LD_NET, "Got a non-version-1 cipher called '%s'", ciphername);
       // return 1;
       goto dump_list;

So, in practice, the use of the SSL_DHE_DSS_WITH_3DES_EDE_CBC_SHA (SSL3_TXT_EDH_DSS_DES_192_CBC3_SHA) cipher suite is considered as a sign of the use of the V2 handshake. This contradicts all specifications.

(According to 124-tls-certificates and tor.git history, two of these cipher suites {AES_256 and DSS} have never been used in Tor v0/1, they seem to be include for better censorship resistance).

Child Tickets

Change History (5)

comment:1 Changed 8 years ago by rransom

Component: Quality Assurance and TestingTor Relay
Keywords: easy added; torspec removed
Milestone: Tor: 0.2.4.x-final
Priority: normalminor

Tor relays cannot currently use a DSA-based ciphersuite, and will never use one (ECDSA is less unlikely). The code should probably check for that ciphersuite, though, just to make sure its behaviour can never differ from the spec in the future.

comment:2 Changed 8 years ago by nickm

Keywords: spec added

comment:3 Changed 8 years ago by nickm

Keywords: tor-relay added

comment:4 Changed 8 years ago by nickm

Component: Tor RelayTor

comment:5 Changed 8 years ago by nickm

Resolution: fixed
Status: newclosed

should be fixed in 490c777859b96223421e247675367c792a031c75 ; thanks!

Note: See TracTickets for help on using tickets.