View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0004079unrealircdpublic2012-01-22 10:162012-03-25 19:56
ReporternenolodAssigned Tosyzop  
PrioritynormalSeverityfeatureReproducibilityhave not tried
Status resolvedResolutionfixed 
Fixed in Version3.2.10-rc1 
Summary0004079: [feature] add SASL support using PUIDs
Descriptionhi,

using pseudo-UIDs (instead of UUIDs), we can implement a lightweight version of SASL which allows authentication with services.

i have a patch which is mostly completed for this, but it needs to be rebased and refactored. i will attach it later this week.

a services-side implementation of this is already present in atheme-7 GIT master, if you want an idea of how the protocol works, but it is still work-in-progress.
TagsNo tags attached.
Attached Files
sasl-rebase.patch (13,974 bytes)
sasl-rebased-v2.patch (14,427 bytes)
3rd party modules

Activities

nenolod

2012-03-06 04:57

reporter   ~0016939

For some reason it won't let me attach the patch to the tracker.

So, you can download it here: http://tortois.es/~nenolod/sasl-rebase.patch

nenolod

2012-03-07 02:03

reporter   ~0016943

Right, so, sasl-rebase.patch has the patch. It seems to be working, but more testing is needed.

katsklaw

2012-03-07 03:12

reporter   ~0016944

+1

warg

2012-03-07 03:21

reporter   ~0016945

+1

SnoFox

2012-03-07 17:43

reporter   ~0016950

Woo!
+1

Justasic

2012-03-14 07:24

reporter   ~0016951

+1

KindOne

2012-03-14 07:29

reporter   ~0016952

+1

syzop

2012-03-24 10:08

administrator   ~0016953

Last edited: 2012-03-24 10:11

 Ok, just shooting a few questions. Sorry for not getting back sooner... even now I've only quickly read through the code. As you may understand I didn't setup a (new for me) services package just for this. If you have some written documentation somewhere online, let me know, I did find the charybdis doc/sasl.txt which helped a little.
Here it goes:
* Shouldn't sasl_agent[] be NICKLEN+1?
* The SASL message is broadcasted to all servers, is that correct? It looks that way, both in the case that 1) you have no idea who will 'answer', and 2) even when a destination is known. #2 sounds odd. But even for #1, wouldn't it be better if a destination server can be configured? Maybe even default to the services-server. It sounds illogical to share authentication requests (etc.) with all servers in a network. What if multiple servers would answer?
* Some minimal bounds checking on the 'slot' would be nice
* Is there a race condition between the stage where a user is authenticating (has just sent his last AUTHENTICATE), the client dropping out, and a new client taking this slot? Or is this properly handled? If services lag a few seconds then this may be a realistic scenario. (Not that the time frame matters, since any race condition should be addressed). If this is already addressed, then good.

nenolod

2012-03-25 00:17

reporter   ~0016954

hi,

yes, sasl_agent[] should probably be NICKLEN + 1.

since we have services-server setting, we can indeed change the broadcast to send a message directly to the server containing it.

some bounds checking should indeed be applied to decode_puid() -- no disagreement there.

however, i think we should perhaps put a cookie onto the PUID, which would allow for limited session correlation (which would certainly stop confusion between AUTHENTICATE messages - as bogus ones could be discarded due to not having matching cookie).

thoughts?

nenolod

2012-03-25 00:38

reporter   ~0016955

sasl-rebased-v2.patch implements the requested changes, including usage of a per-session cookie (generated by getrandom16() which should be plenty sufficient)

syzop

2012-03-25 10:25

administrator   ~0016956

Great! Committed :)

http://hg.unrealircd.com/hg/unreal/rev/db9bd1f1d7f2
- Added support for SASL, patch from nenolod (0004079).

syzop

2012-03-25 12:41

administrator   ~0016957

Just fixed a crash if you sent 'AUTHENTICATE' without parameters.

Another question, shouldn't 'AUTHENTICATE' be ignored/denied when the user is fully registered? That's what I read in doc/sasl.txt.

nenolod

2012-03-25 19:05

reporter   ~0016958

hi,

i guess removing M_REGISTERED from the AUTHENTICATE message would do the trick for that.

syzop

2012-03-25 19:55

administrator   ~0016959

Right.

Also had to fix two other crash bugs, and I added some code so not any random user can run the SVSLOGIN / SASL commands.

http://hg.unrealircd.com/hg/unreal/rev/f07846413bb2

Issue History

Date Modified Username Field Change
2012-01-22 10:16 nenolod New Issue
2012-03-06 04:54 nenolod Status new => has patch
2012-03-06 04:57 nenolod Note Added: 0016939
2012-03-06 17:31 ohnobinki File Added: sasl-rebase.patch
2012-03-07 02:03 nenolod Note Added: 0016943
2012-03-07 03:12 katsklaw Note Added: 0016944
2012-03-07 03:21 warg Note Added: 0016945
2012-03-07 17:43 SnoFox Note Added: 0016950
2012-03-14 07:24 Justasic Note Added: 0016951
2012-03-14 07:29 KindOne Note Added: 0016952
2012-03-24 10:08 syzop Note Added: 0016953
2012-03-24 10:10 syzop Note Edited: 0016953
2012-03-24 10:11 syzop Note Edited: 0016953
2012-03-25 00:17 nenolod Note Added: 0016954
2012-03-25 00:38 nenolod File Added: sasl-rebased-v2.patch
2012-03-25 00:38 nenolod Note Added: 0016955
2012-03-25 10:25 syzop Note Added: 0016956
2012-03-25 10:25 syzop Status has patch => resolved
2012-03-25 10:25 syzop Fixed in Version => 3.2.10-rc1
2012-03-25 10:25 syzop Resolution open => fixed
2012-03-25 10:25 syzop Assigned To => syzop
2012-03-25 12:41 syzop Note Added: 0016957
2012-03-25 12:42 syzop Status resolved => acknowledged
2012-03-25 12:42 syzop Resolution fixed => open
2012-03-25 19:05 nenolod Note Added: 0016958
2012-03-25 19:55 syzop Note Added: 0016959
2012-03-25 19:55 syzop Status acknowledged => resolved
2012-03-25 19:55 syzop Resolution open => fixed