View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0004196unrealircdpublic2013-05-19 10:122013-05-23 12:16
ReporternenolodAssigned Tonenolod 
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Target Version3.4-alpha1Fixed in Version3.4-alpha1 
Summary0004196: TS6-style SID/UIDs
DescriptionImplement TS6-style SID/UIDs.

Things for discussion:

1. do we want to keep UIDs secret as required by TS6 (this will never be a conformant implementation of TS6)

2. how will the SID handshake go? PROTOCTL SID= token?
TagsNo tags attached.
3rd party modules

Relationships

has duplicate 0002483 resolvednenolod "Numerics" instead of nicknames over links 
child of 0004188 closed Unreal 3.4 alpha1 blockers 
child of 0004212 resolvedsyzop TS6-ish server protocol metabug 

Activities

nenolod

2013-05-19 10:12

reporter   ~0017593

this will probably be a 3.4-alpha2 thing, maybe 3.4-alpha1. not sure yet.

syzop

2013-05-19 16:45

administrator   ~0017598

1. Why would they need to be secret? What reason did they give? (On the other hand: why would we ever expose them?)

2. Sounds fine? :)

katsklaw

2013-05-19 23:46

reporter   ~0017605

Why not do a full TS6 conversion then? Services packages like Anope and Atheme can easily support the switch. Obviously this is a 3.4 thing so the current 3.2 modules will continue to be supported for sometime then.

nenolod

2013-05-20 00:03

reporter   ~0017606

katsklaw, as much as i would love to do that, we cannot do a full TS6 conversion and support linking to 3.2. :(

Atheme can already speak UID/SIDs to unreal with 3 lines of code added :)

nenolod

2013-05-20 02:46

reporter   ~0017611

As far as UID secrecy, the idea is that UIDs do not change for a session, so, they want to allow people to /nick to an alternate nickname to avoid being bothered. With the UID, this might be possible.

Also, do we want to introduce TS6 SAVE? It changes your nick to your UID in a collision instead of killing you off the network.

syzop

2013-05-20 11:22

administrator   ~0017623

Sorry, I didn't really understand the first part of your answer (on secrecy). I know UID's are unique for a session, but why would they have to be secret?

As for nickchanging on collision:
Does TS6 SAVE differ between equal creation TS and unequal TS?
What I'd want is:
* Different TS: oldest remains, newest gets forced nickchange (instead of kill)
* Equal TS: both get forced nickchange (instead of killing both)

Currently there's the lag collide attack by which you can highly annoy users if you're fast enough. That would be changed from a collision to a forced nick change. Still annoying, but less destructive.

Of course, be sure to get the implementation right, last thing we need are ghost users :p. Does it work well on a mixed SID/UID and noSID/noUID network?

grawity

2013-05-20 11:32

reporter   ~0017624

> Does TS6 SAVE differ between equal creation TS and unequal TS?

AFAIK, both TS6 and P10 use the following rules:

* equal TS: both users lose.

* different TS but same user@host: higher TS wins. (it's a connection from the same user – the old one might be a ghost or something.)

* different TS and user@host: lower TS wins.

syzop

2013-05-20 12:44

administrator   ~0017625

hmm, not sure about the 'same user@host' rules.
I've been in many situations where you want the reverse. So there's something to say about both (oldest vs newest).

Like, if I'm on a network and it splits, then I connect a second client to 'the other side' (split side) to see what's going on there. Then the networks merge, and my 1s connection gets killed. That's not what I wanted.

I'd say, keep the rules the same (lowest TS wins) and don't make exceptions for equal user@host

grawity

2013-05-20 14:03

reporter   ~0017626

1) Except you don't get killed, if the servers support SAVE you just get a forced nickchange to your UID... Not that it's a good idea to use the same nick for those connections if you're expecting the collision anyway.

2) From a user (non-oper)'s perspective, if the server I was connected to temporarily gets disconnected, I reconnect to another server (maybe automatically), and the old server gets relinked, I wouldn't want my new connection to be collided – I'd want the old ghost to die if it hadn't done so yet.

3) I'm really not an ircd developer, so this is opinion mostly, but if TS6 features are being adopted, it makes the most sense to me to also follow the same rules as other TS ircds use (not just Charybdis, but also Inspircd, Bahamut, &c.)

syzop

2013-05-20 18:13

administrator   ~0017627

We use (and will use) our own protocol, which always had (many) similarities with other protocols. It's not our intention to copy a protocol 1:1 -- in fact, we don't. So yes we are allowed to make our own decisions ;).

I'd like some feedback on this from other users: what do you think of the 'with same user@host the highest TS wins' logic ? Stealth, binki, Jobe, tabrisnet, others ?
See both my post and grawity's.

Note that this is just a detail. All the rest of SID/UID can already be implemented.

nenolod

2013-05-20 18:19

reporter   ~0017628

higher TS means the 1s connection wins.

in some IRCds, it is possible to refer to the user by their UID even when they are not using it as their nickname. that is basically, what we need to decide whether we care about or not.

if we do care, we need to add a find_named_person() and use it in places that accept user input.

syzop

2013-05-20 18:26

administrator   ~0017630

Ah okay, so you mean a privacy issue? If a user knows you by UID he can track/find you even though you nickchange and are not in the same channels?
Off-topic: We actually have a similar issue with DCCALLOW, a user can track you even if you nickchange. (not so nice)

It would be better if we avoid the privacy issue, I think. Does it make the code more complex / ugly ? Like will we now get .... = IsServer(x) ? find_person(..) : find_named_person(..) all over the place?
Because if it's much of a hassle then.... well.. ;p

nenolod

2013-05-20 18:35

reporter   ~0017631

We could hide it with a macro, I think.

#define find_id(cptr, id) IsServer(cptr) ? find_person(id) : find_named_person(id)

Or we could split the commands into separate STAT_CLIENT and STAT_SERVER handlers, which would be cleaner imo, and have a nice side effect of cleaning up many potential problems in the S2S part (running client commands etc).

syzop

2013-05-20 18:36

administrator   ~0017632

Or we could change the meaning of find_person: find_person("personyouarelookingfor", whoislooking)

So you can: find_person(nick, sptr)

And depending on 'sptr' you allow UID lookup or not. And if it's NULL then you allow the lookup.

Right now the 2nd argument is always NULL everywhere in the code anyway, so then it would actually do something useful.

Same for find_client, except two places that probably can change.

Jobe

2013-05-20 18:42

reporter   ~0017634

With regards to "same user@host, highest TS wins" logic, I think the best direction is to keep it the same as is currently implemented in Unreal, that is as already stated, lowest wins as if user@host were different. As far as a configuration option to allow choosing the alternate highest TS wins logic is concerned, it would somehow require a means to ensure that the setting is the same network wide and not just warn that it's different due to the technical issues that could arise from it differing across links.

As for find_person(), I agree it would be far simpler to handle a check on who is looking within the function instead of outside.

nenolod

2013-05-20 18:46

reporter   ~0017635

find_client() should always search for UIDs.

find_person() should wrap find_client() if sptr is Server, else search only the nickname hashtable.

syzop

2013-05-20 18:52

administrator   ~0017636

> find_client() should always search for UIDs.

I mention it because in some cases users can use find_client(), such as in WHOIS. Well.. just look around.

> find_person() should wrap find_client() if sptr is Server, else search only the nickname hashtable.

I presume you still mean that find_person will always ensure that the returned target is a user (otherwise you'll get crashes). If so, agree.

We posted at about the same time regarding find_id / modifying find_person. What do you think of my idea?
As for splitting commands out in client / server, I wouldn't do that (or at least not now), as it's far too much work.

syzop

2013-05-20 18:55

administrator   ~0017637

Last edited: 2013-05-20 18:56

 Added as child for alpha1, since we agreed - when you removed numerics - that this will take priority and would be done within a few weeks (since then).

EDIT: that's for SID, not necessarily for UID, but makes sense to implement them at the same time.

Jobe

2013-05-20 18:58

reporter   ~0017638

Regarding separate message handlers, personally I have always been a fan of and preferred handlers to be separate depending on the type of client issuing the command. Typical handler types used by most IRCds are unregistered, user, oper and server. Which as stated adds an extra level of sanity and safety to message handling. (Would also eliminate a certain S2S crash bug caused by a message handler assuming that the message only ever comes from users) But yes I agree it is a lot of work to do, and in a lot of cases isn't as simple as cut/copy/pasting code. Of course it could be possible to implement a means of phasing such a system in over time.

syzop

2013-05-20 19:02

administrator   ~0017639

Jobe:

(regarding user@host) yeah regards to configurability I thought of the same issue, so that's not an option, too error prone. I tend to agree with no special rules, as you said.

(regarding last comment) yeah the main issue is time. If we wish to do this, then better to make it a separate feature suggestion instead of link it to this bug. And indeed, we can do it gradually, instead of all at once. Just like we did with CommandAdd / add_Command 'back in the days' :p.

nenolod

2013-05-20 22:39

reporter   ~0017644

Syzop, your idea is fine. I will go with that.

nenolod

2013-05-21 04:02

reporter   ~0017645

current status:

PROTOCTL SID= handshake done

next up: SID command to burst a server with SID.
then we will do a UID command.

after that, it is just modifying all the sendto_server() calls to send a version with UID/SID. we have PROTO_SID for that, use it in the caps/nocaps fields as needed.

i'll do a few, and then i am sure falcon or somebody can do the rest. it's boring :P

nenolod

2013-05-21 09:44

reporter   ~0017647

SID core is done, validated with 3.2 and 3.4 mixed network (in 3.2 network it downgrades gracefully to SERVER message).

We should use a macro to choose ID or full-name on sending messages.

nenolod

2013-05-21 12:01

reporter   ~0017648

So, next, we need UID command.

I propose we take the NICKv2 and modify it so that the SID is the prefix and the servername parameter is the UID.

Anything else we should do here?

syzop

2013-05-21 21:06

administrator   ~0017650

Great.

> I propose we take the NICKv2 and modify it so that the SID is the prefix and the servername parameter is the UID.

I see. Yeah, clever, saves adding a new parameter.
And you won't have direction problems either, because NICK messages always come from that direction. (Sorry I don't know how to say this in normal English ;p)

syzop

2013-05-21 21:12

administrator   ~0017651

I know it's all in the works right now, so when you have time for it (not now): could you update the serverprotocol.html file with the new protocol.

nenolod

2013-05-23 10:00

reporter   ~0017658

This bug will track simply implementing and documenting the UID/SID core.

Validating against UID/SID leaks and UID/SID-specific sendto_server() message variants will happen in alpha2 on another bug.

nenolod

2013-05-23 12:11

reporter   ~0017659

Trunk as of right now has a very rough cut at a full SID/UID solution. Cleanups are needed, but it is good enough for alpha1.

[23/05/2013 10:09:11] -> :00A UID kaniini_ 0 1369303362 kaniini turtle.dereferenced.org 00AAAAAAB 0 +iwx testnet-5A087B2.dereferenced.org testnet-5A087B2.dereferenced.org xhtOfA== :William Pitcock
[23/05/2013 10:09:11] m_uid(): new user on `irc-lax0.dereferenced.org': irc-lax0.dereferenced.org
[23/05/2013 10:09:11] user_add(): kaniini_ ([email protected]) -> irc-lax0.dereferenced.org
[23/05/2013 10:09:11] <- :999AAAAAC NOTICE 00AAAAAAB :Services are presently running in debug mode, attached to a console. You should take extra caution when utilizing your services passwords.

As a bonus, since VHP support is implied by SID/UID support, Atheme now understands the VHP capab too.

nenolod

2013-05-23 12:16

reporter   ~0017660

Marking this done, as we have implemented the scope of this work.

Cleanups and other tasks are on the metabug, which will be resolved before beta phase.

Issue History

Date Modified Username Field Change
2013-05-19 10:12 nenolod New Issue
2013-05-19 10:12 nenolod Note Added: 0017593
2013-05-19 16:45 syzop Note Added: 0017598
2013-05-19 23:46 katsklaw Note Added: 0017605
2013-05-20 00:03 nenolod Note Added: 0017606
2013-05-20 02:46 nenolod Note Added: 0017611
2013-05-20 09:51 nenolod Relationship added has duplicate 0002483
2013-05-20 11:22 syzop Note Added: 0017623
2013-05-20 11:32 grawity Note Added: 0017624
2013-05-20 12:44 syzop Note Added: 0017625
2013-05-20 14:03 grawity Note Added: 0017626
2013-05-20 18:13 syzop Note Added: 0017627
2013-05-20 18:19 nenolod Note Added: 0017628
2013-05-20 18:26 syzop Note Added: 0017630
2013-05-20 18:35 nenolod Note Added: 0017631
2013-05-20 18:36 syzop Note Added: 0017632
2013-05-20 18:42 Jobe Note Added: 0017634
2013-05-20 18:46 nenolod Note Added: 0017635
2013-05-20 18:52 syzop Note Added: 0017636
2013-05-20 18:53 syzop Relationship added child of 0004188
2013-05-20 18:55 syzop Note Added: 0017637
2013-05-20 18:56 syzop Note Edited: 0017637
2013-05-20 18:58 Jobe Note Added: 0017638
2013-05-20 19:02 syzop Note Added: 0017639
2013-05-20 22:39 nenolod Note Added: 0017644
2013-05-21 04:02 nenolod Note Added: 0017645
2013-05-21 09:44 nenolod Note Added: 0017647
2013-05-21 12:01 nenolod Note Added: 0017648
2013-05-21 21:06 syzop Note Added: 0017650
2013-05-21 21:12 syzop Note Added: 0017651
2013-05-23 10:00 nenolod Note Added: 0017658
2013-05-23 10:08 nenolod Relationship added child of 0004212
2013-05-23 12:11 nenolod Note Added: 0017659
2013-05-23 12:16 nenolod Note Added: 0017660
2013-05-23 12:16 nenolod Status new => resolved
2013-05-23 12:16 nenolod Fixed in Version => 3.4-alpha1
2013-05-23 12:16 nenolod Resolution open => fixed
2013-05-23 12:16 nenolod Assigned To => nenolod