View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0004189unrealircdpublic2013-05-13 07:242013-05-23 05:52
ReporternenolodAssigned Tonenolod 
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Target Version3.4-alpha1Fixed in Version3.4-alpha1 
Summary0004189: remove ziplinks support (nobody should be using it by now anyway)
Descriptionwe should either move the ziplinks code to a lowerlevel component of the stack instead of dopacket() or remove it.

frankly, i think we should just remove it as ssl makes it redundant, and we shouldn't really encourage linking servers without ssl in 2013.

(services doesn't count here, none of those support ziplinks)

tl;dr: what do we want to do with this? if nobody objects, i'm just going to nuke it when i rewrite the buffer code.
TagsNo tags attached.
Attached Files
4189_remove_ziplink.diff (64,047 bytes)
4189_remove_ziplink_3.4.diff (65,375 bytes)
3rd party modules

Relationships

child of 0004188 closed Unreal 3.4 alpha1 blockers 

Activities

syzop

2013-05-14 19:24

administrator   ~0017565

I'll reply here:

I'm tending towards removal, but I wonder about the figures.
* SSL can compress, is this always enabled by default? (if both sides support it)
* Is there much overhead due to SSL? I think it's only 1-2 bytes per packet or something? and rounding on 16 bytes blocks? That's it? or?

Let me know so we can make a good decision.

nenolod

2013-05-14 22:44

reporter   ~0017566

SSL compression is always enabled since SSLv3 onwards, and is a requirement of TLS.

The overhead of SSL on the compression is indeed minimal.

syzop

2013-05-14 23:43

administrator   ~0017567

Last edited: 2013-05-14 23:43

 Ok then, I'm fine with removal.

Can you add something that if link::options::zip is specified in the config file, we would print out some statement (fatal error) saying they should use SSL/TLS because (among other things) it does compression? <- but then a proper sentence, of course.

nenolod

2013-05-15 05:03

reporter   ~0017568

Okay, I will do this after I finish knocking down the remainder of the tokens code.

falconkirtaran

2013-05-15 08:36

reporter   ~0017569

Last edited: 2013-05-15 08:38

 Expunged everything having to do with ziplinks at all ever.

Tested two servers (same build) could link and clients could connect and communicate without apparent issues.

It may be necessary to hg remove include/zip.h and src/zip.c.

nenolod

2013-05-15 16:53

reporter   ~0017570

This patch does not apply to Unreal 3.4 tree, did you do it against the 3.2 tree?

I will work to integrate it in regardless.

In the future, make sure that you've run: hg checkout unreal34 to get the 3.4 tree.

syzop

2013-05-15 21:55

administrator   ~0017571

who's falconkirtaran? ;P
(hi)

nenolod

2013-05-16 04:03

reporter   ~0017572

Falcon is a friend of mine who is interested in helping out on Unreal 3.4 project.

He's particularly good at auditing code and figuring out what it is doing, which is useful as it means I don't have to tell him what every piece of hairy 2.8+ircd-dal code is doing ;)

falconkirtaran

2013-05-16 04:12

reporter   ~0017573

Indeed. I'm a security consultant / IDS developer at Leviathan, and I've been using unrealircd for almost a decade. Figured I might as well do something for it.

falconkirtaran

2013-05-16 05:58

reporter   ~0017575

There. New patch actually made from the 3.4 tree, which should actually apply.

Bonus: fixed a potentially unsafe sprintf in send_proto().

nenolod

2013-05-16 08:36

reporter   ~0017576

Removal patch committed: http://hg.unrealircd.org/hg/unreal/rev/529b168ff8c1

Before we can close this out, need to have a notice saying that ziplinks are deprecated and recommending TLS as an alternative.

syzop

2013-05-16 21:13

administrator   ~0017578

Falcon: Ok great :). Good to have you onboard.

falconkirtaran

2013-05-19 09:20

reporter   ~0017588

Thanks, syzop!

Where should the notice go?

nenolod

2013-05-19 10:06

reporter   ~0017591

Put it in the config handling stuff, if you see "zip" in the options, flag a warning. (might need to keep the bitfield entry for this)

nenolod

2013-05-19 12:44

reporter   ~0017597

Warning is added in this commit: http://hg.unrealircd.org/hg/unreal/rev/256547419ad6

Closing it out.

Issue History

Date Modified Username Field Change
2013-05-13 07:24 nenolod New Issue
2013-05-13 07:24 nenolod Status new => assigned
2013-05-13 07:24 nenolod Assigned To => nenolod
2013-05-13 07:25 nenolod Relationship added child of 0004188
2013-05-14 19:24 syzop Note Added: 0017565
2013-05-14 22:44 nenolod Note Added: 0017566
2013-05-14 23:43 syzop Note Added: 0017567
2013-05-14 23:43 syzop Note Edited: 0017567
2013-05-15 05:03 nenolod Note Added: 0017568
2013-05-15 08:36 falconkirtaran Note Added: 0017569
2013-05-15 08:37 falconkirtaran File Added: 4189_remove_ziplink.diff
2013-05-15 08:38 falconkirtaran Note Edited: 0017569
2013-05-15 16:53 nenolod Note Added: 0017570
2013-05-15 21:55 syzop Note Added: 0017571
2013-05-16 04:03 nenolod Note Added: 0017572
2013-05-16 04:12 falconkirtaran Note Added: 0017573
2013-05-16 05:57 falconkirtaran File Added: 4189_remove_ziplink_3.4.diff
2013-05-16 05:58 falconkirtaran Note Added: 0017575
2013-05-16 08:36 nenolod Note Added: 0017576
2013-05-16 21:13 syzop Note Added: 0017578
2013-05-19 09:20 falconkirtaran Note Added: 0017588
2013-05-19 10:06 nenolod Note Added: 0017591
2013-05-19 12:44 nenolod Note Added: 0017597
2013-05-19 12:44 nenolod Status assigned => resolved
2013-05-19 12:44 nenolod Fixed in Version => 3.4-alpha1
2013-05-19 12:44 nenolod Resolution open => fixed