Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#4591 closed defect (fixed)

Don't set SSL_MODE_NO_AUTO_CHAIN during renegotiation.

Reported by: asn Owned by: nickm
Priority: High Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tls correctness tor-relay
Cc: arma Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

If during the renegotiation's ClientHello a client passes:

 if (tor_tls_client_is_using_v2_ciphers(ssl, ADDR(tls))) {

in tor_tls_server_info_callback() (or tor_tls_got_client_hello() in master), we don't send a full certificate chain to the client.

The above if statement should only be examined during the initial SSL handshake.

There was already a

/*XXXX_TLS keep this from happening more than once! */ 

comment that never got implemented.

Child Tickets

Change History (9)

comment:1 Changed 8 years ago by nickm

Milestone: Tor: 0.2.3.x-final
Owner: set to nickm
Priority: normalmajor
Status: newaccepted

comment:2 Changed 8 years ago by nickm

Keywords: tls correctness added

comment:3 Changed 8 years ago by nickm

Status: acceptedneeds_review

Untested patch against maint-0.2.2 in branch "bug4591" in my public repository.

comment:4 Changed 8 years ago by asn

I tested this by connecting to a relay with OpenSSL s_client, requesting renegotiation and checking the length of the Certificate record. The certificate record on the first handshake is 600 bytes (or so) smaller than the certificate on the renegotiation handshake (since during renegotiation the bridge sends two certificates in the Certificate record). It seems to fix the bug.

The patch looks OK too.

The OpenSSL command I used is:

openssl s_client -cert ./example.pem -tls1 -msg -showcerts -connect localhost:6666

comment:5 Changed 8 years ago by nickm

Cc: arma added

Okay, so my remaining question is whether to merge this into 0.2.2 or 0.2.3. I am on the fence, and would like a second opinion. Roger, what do you think here?

comment:6 Changed 8 years ago by arma

So the bug is that if the client does something different than the client we wrote does, the ssl handshake won't work? And we found it by learning more about renegotiation, rather than by having somebody report it because their alternate client is broken?

If so, sounds like an 0.2.3 thing.

comment:7 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Okay, merging to 0.2.3. If we find out it should have gone into 0.2.2, we can merge it there too pretty easily.

comment:8 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:9 Changed 7 years ago by nickm

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