Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#3991 closed Bug (fixed)

Connection encryption stopped working

Reported by: ah Owned by: jordan
Priority: Normal Milestone: 2.21
Component: Transmission Version: 2.20
Severity: Critical Keywords: linux, encryption, required
Cc:

Description

Hello.

Since version 2.20, connection encryption stopped working, which makes the program unusable if set to "Require encryption", since no clients can connect. Setting transmission to "Allow encryption" still works, of course. There are no error logs in stdout, syslog, or the program's message log.

Running with OpenSSL 1.0c on linux x86_64. Let me know if you need more info.

Change History (27)

comment:1 Changed 11 years ago by jordan

Hi ah,

I'm not able to reproduce this behavior. Are you sure it's not just the swarm you're in? If you pick a well-seeded swarm does the behavior change?

comment:2 Changed 11 years ago by ah

I'm sure it's not the swarm, as I've verified the problem with well-seeded torrents from tuxdistro.com and ubuntu.com. I also tried re-compiling without any hardening options.

Reverting to transmission-2.13 (and libevent-1.4.14b) solves the problem.

comment:3 Changed 11 years ago by WillemB

I have a similar problem with connections in 2.20 too which wasn't in 2.20b4. I hardly get any peers, might be related to this ticket. I believe the last minute modifications in announcers.c might be the cause. Running Bittorrent on my PC gives instant download where transmission om my device gives 1 peer. In 2.20b4 I had more peers on the same torrents. I did not get to test the behaviour on non-encrypted peers. I'll be reverting to the old announcer.c to see if it works.

Realtek VENUS - Mipsel based media device

My 2 cents

Last edited 11 years ago by WillemB (previous) (diff)

comment:4 Changed 11 years ago by kroki

Same situation here: 2.20b1-4 works without any problems and the 2.20 has no peers any more.

Running on Fedora Core 14/64Bit

Kroki

Last edited 11 years ago by kroki (previous) (diff)

comment:5 Changed 11 years ago by jordan

Interesting. Well three people in one day is certainly a pattern worth looking into, but I'm still not seeing this behavior. Can you point me to a public domain .torrent to test this with?

comment:6 Changed 11 years ago by gunzip

make that 4 people. after 1 minute in a heavily-seeded torrent swarm of over 1000 i connected to only 3 peers using 2.20. switching back to 2.20b4 (11829) in same time i reached my peer limit 0f 90 connections. the difference is striking, and may not be limited to just encryption.

comment:7 Changed 11 years ago by jordan

...just to ask, you all wouldn't happen to be on private trackers that haven't whitelisted 2.20 yet, would you? If so, you'll see a dramatic drop in peers between 2.20b4 and 2.20 until your tracker whitelists 2.20...

comment:8 Changed 11 years ago by gunzip

@jordan: in my test in comment:6 it was a torrent from a public tracker.

comment:9 Changed 11 years ago by ah

Any trackers. As I've already mentioned, I verified the issue with torrents from tuxdistro.com and ubuntu.com (many others as well in the meantime).

comment:10 Changed 11 years ago by WillemB

I did some quick tests Encryption on (encryption=1) one non-encrypted peer (strange but true). Encryption off (encryption=0) downloading at >50kb/sec in no time which seems to be normal.

I do have crappy public trackers but the difference is big enough.

comment:11 Changed 11 years ago by jordan

I'm looking into this some more and will report back ASAP.

comment:12 Changed 11 years ago by jordan

Okay I think I've tracked the cause to a bug that's triggered by the way things get built for stable/official releases compared to betas and svn builds, and will have a commit for that shortly. We suspect this is only going to affect people who are building with configure/make from the tarball, rather than (say) the Mac users who download a dmg binary.

So, everyone on this ticket, does that sound right?

Did you build from a 2.20 tarball?

Is anyone here seeing this behavior in the mac client?

comment:13 Changed 11 years ago by jordan

  • Milestone changed from None Set to 2.21
  • Owner set to jordan
  • Severity changed from Normal to Critical
  • Status changed from new to assigned

comment:14 Changed 11 years ago by jordan

jordan * r11846 libtransmission/peer-io.c:

(libT) #3991 "Connection encryption stopped working" -- fixed.

This is a pretty straightfoward bug: the call to evbuffer_peek() should not have been wrapped in assert().

comment:15 Changed 11 years ago by gunzip

this issue is now solved on my end, using transmission-daemon 2.20+ (11852)

comment:16 Changed 11 years ago by ah

It was indeed built from tarball. I applied the r11846 diff and that fixed the problem.

Thank you.

comment:17 Changed 11 years ago by jordan

Thanks gunzip & ah :)

comment:18 Changed 11 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:19 Changed 11 years ago by reardon

Hmm, not the cleanest patch, as this now throws a (harmless) warning on release builds.

comment:20 Changed 11 years ago by jordan

I thought about that before commiting the patch, but figured that something like

#ifdef NDEBUG
evbuffer_peek( buf, byteCount, NULL, iovec, vecCount )
#else
const int n = evbuffer_peek( buf, byteCount, NULL, iovec, vecCount ); 
assert( n == vecCount ); 
#endif

would be worse, since it introduces duplicate code based on an #ifdef. If we change the arguments later on, they might not both be changed.

We could eliminate the warning and the redundancy by doing something like

#ifndef NDEBUG
const int n =
#endif
              evbuffer_peek( buf, byteCount, NULL, iovec, vecCount )
assert( n == vecCount ); 

...but even though this would make the compiler happy, it's by far the ugliest of the three approaches.

What would you suggest? :)

comment:21 Changed 11 years ago by reardon

I actually prefer the first, if I had my druthers (what are druthers, anyway?)

But the question really is: why are you assert()ing here?

comment:22 Changed 11 years ago by jordan

to confirm that the number of number of extents returned by evbuffer_peek() is the number that we thought would be returned. It's a new function in libevent2 and I want to assert that we're using it properly.

comment:23 Changed 11 years ago by zebulon501

I do build transmission 2.20 from the tarball (see http://synoblog.superzebulon.org/2011/02/transmission-2-20-spk/) and have the users complain a lot about the download and upload instability. I will apply r11846 and see what happens.

comment:25 Changed 11 years ago by zebulon501

jordan: thanks for the release, it is now available on http://synoblog.superzebulon.org/2011/02/transmission-2-21-spk/. It connects to peers much quicker and the upload speed limit is now respected.

By the way, release 2.21 is not advertised on the main page.

Last edited 11 years ago by zebulon501 (previous) (diff)

comment:26 Changed 11 years ago by ijuxda

Replying to jordan:

What would you suggest?

diff --git a/libtransmission/peer-io.c b/libtransmission/peer-io.c
index b815080..bebc028 100644
--- a/libtransmission/peer-io.c
+++ b/libtransmission/peer-io.c
@@ -808,7 +808,7 @@ evbuffer_peek_all( struct evbuffer * buf, size_t * setme_vecCount )
     struct evbuffer_iovec * iovec = tr_new0( struct evbuffer_iovec, vecCount );
     const int n = evbuffer_peek( buf, byteCount, NULL, iovec, vecCount );
     assert( n == vecCount );
-    *setme_vecCount = vecCount;
+    *setme_vecCount = n;
     return iovec;
 }

comment:27 Changed 11 years ago by jordan

Heh. It's ugly, but it's less ugly... :)

r11856

Note: See TracTickets for help on using tickets.