Opened 6 weeks ago

Closed 5 weeks ago

#28245 closed defect (fixed)

Tor nodes with OpenSSL 1.1.1 can't communicate with each other

Reported by: asn Owned by: nickm
Priority: Very High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: openssl tor-tls tls-1.3 035-must 029-backport 033-backport 034-backport
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

We seem to have trouble in networks where both clients and relays are running openssl-1.1.1 . In particular, a chutney network on openssl-1.1.1 (11 sept 2018) will have its clients fail to bootstrap because they cant communicate any bytes after the SSL handshake is done.

The problem might be that 1.1.1 is the version that introduces TLS-1.3, so these nodes are trying to do TLS-1.3 with each other.

Thanks to teor, dgoulet, nickm for the debug help.

Child Tickets

Change History (13)

comment:1 Changed 6 weeks ago by asn

Exempts from the logs:

chan 2 changed from scheduler state IDLE to WAITING_FOR_CELLS
SSL 0x562355ca0410 is now in state SSL negotiation finished successfully [type=4098,val=1]
tor_tls_read(): read returned r=-1, err=-2
Oct 29 16:02:27.998 [info] control_event_bootstrap_problem(): Problem bootstrapping. Stuck at 10%: Finishing handshake with directory server. (IOERROR; IOERROR; count 1; recommendation ignore; host AA37ED4F6EB3F274991BE71915ECF5E7AF3DCFB2 at 127.0.0.1:5002)

comment:2 Changed 6 weeks ago by nickm

Priority: MediumVery High

comment:3 Changed 6 weeks ago by nickm

The "err=-2" part is normal: it means WANTREAD.

comment:4 Changed 6 weeks ago by nickm

Can we double-check that 0.3.4 also has this problem? 0.3.5 did a lot of TLS refactoring.

comment:5 in reply to:  4 Changed 6 weeks ago by asn

Replying to nickm:

Can we double-check that 0.3.4 also has this problem? 0.3.5 did a lot of TLS refactoring.

I'm fairly sure that maint-0.3.4 has this problem. In particular, the logs above are from:
Oct 29 15:58:36.490 [notice] Tor 0.3.4.8 (git-5da0e95e4871a0a1) opening new log file.

comment:6 Changed 5 weeks ago by nickm

Keywords: 035-must added

comment:7 Changed 5 weeks ago by nickm

I'm trying this on a chutney network. It seems that while clients are indeed failing as specified above, relays are successfully passing data back and forth with SSL_read and SSL_write.

comment:8 Changed 5 weeks ago by nickm

Keywords: 029-backport 033-backport 034-backport added
Owner: set to nickm
Status: newaccepted

After some experimentation, I think this is about how we configure our curves.

comment:9 Changed 5 weeks ago by nickm

So here's the process I used to try to track this down.

I started by adding logs all over to look at SSL_read() and SSL_write() per above. I noticed that not only were the clients getting the IO errors, but that relays were successfully using SSL_read() and SSL_write(). This made me think that there was something bogus on client connections.

I confirmed that something was going wrong in the handshake stage: I added a bogus version to the client service cell, and made the relays log when they saw it. They weren't getting the version at all, even though the client thinks that it was sending it. This helped me localize the problem to the handshake.h.

At this point, I edited the router_initialize_tls_context() function so that clients would create TLS contexts as if they were servers. This made chutney pass again! So I guessed that there was something in tor_tls_context_init_one() function, probably depending on the is_client flag, that was making clients not work.

I tried forcing the various if (! is_client) checks to if (1), and found that the one that was setting up EC curves was the problem.

comment:10 Changed 5 weeks ago by nickm

Status: acceptedneeds_review

See branch bug28245_029 ; it merges forward cleanly with git but not github. PR at https://github.com/torproject/tor/pull/490 .

See also branch bug28245_035; it's just the obvious merge. PR at https://github.com/torproject/tor/pull/491

comment:11 Changed 5 weeks ago by asn

Status: needs_reviewmerge_ready

This makes my chutney network bootstrap again as normal, and patch looks reasonable.

comment:12 Changed 5 weeks ago by nickm

okay, merged to 0.2.9 and forward.

comment:13 Changed 5 weeks ago by nickm

Resolution: fixed
Status: merge_readyclosed
Note: See TracTickets for help on using tickets.