View Issue Details
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005434||unreal||ircd||public||2019-10-08 19:33||2020-01-20 15:34|
|Status||closed||Resolution||no change required|
|Summary||0005434: sasl code improvements|
|Description||Hi, I've been working on some improvements for sasl. I thought I'd share the patch here since I can't find a way to publish my local repository on github.|
Basically this is the summary of the changes I'd like to make:
1) use sendto_one instead of sendto_server
2) properly reset sasl_out counter which makes sasl_complete counter redundant.
3) with the help of sasl_out, we now inform sasl agent if a known auth session is aborted
4) abort sasl auth if we receive '*' as authenticate data and use ERR_SASLABORTED numeric instead
5) don't copy string to sasl_agent string if origin matches sasl_agent
6) remove make_user in cmd_sasl because it is already called in cmd_svslogin and it is pointless to be called in cmd_sasl
7) reject sasl messages if auth session has been aborted
8) reject svslogin message if it has been sent more than once per auth session
Please look into the patch and please take these changes into consideration.
|3rd party modules|
sasl-diff.txt (21,744 bytes)
I forgot to mention that sasl_complete has been re-purposed to reject repeated svslogin message.
What do you think?
Thanks for the patch. I didn't know you intended to make such a large(r) cleanup. Earlier in UnrealIRCd 5 development that would be perfectly fine but I'm currently heading towards an alpha4 release and - in general - I am at the end of all the cleanups that I want to do in U5. So.. well.. let's see after alpha4. Let's talk then.
Did you test the patch in a multiserver setup like [A]-[B]-[C]-[services]?
For both a 100% U5 network and also a mixed network where you have both U4 and U5 servers.
It's easy to make a mistake in this code. The current code is not written by me and looks rather weird at places.
||See previous comment. I can understand that this will take some time for you to test.|
No I've not tested this code yet. I will try to find time to set up a test network in the next few days.
Since sasl.c only handles local clients and any s2s messages that are not directed to us are relayed across without altering anything, theoretically the patch should not cause issues in multiserver setup unless I've used the wrong variable or accidentally altered the content of sendto_* function
||Ok understood. Yeah it's really vital to test these things.|
I could keep this open but the truth is that you probably don't have time for it and neither will I do any of these changes until UnrealIRCd 6, so.. better to just close it rather than keep it lingering.
For the future: to avoid disappointment, please contact me prior to major code rewrites, I hate to see this happen!
|2019-10-08 19:33||dboyz||New Issue|
|2019-10-08 19:33||dboyz||Tag Attached: sasl|
|2019-10-08 19:33||dboyz||File Added: sasl-diff.txt|
|2019-10-08 19:35||dboyz||Note Added: 0020945|
|2019-10-09 15:58||syzop||Note Added: 0020946|
|2019-10-09 15:59||syzop||Note Edited: 0020946||View Revisions|
|2019-10-14 09:11||syzop||Note Added: 0020954|
|2019-10-14 09:11||syzop||Assigned To||=> syzop|
|2019-10-14 09:11||syzop||Status||new => feedback|
|2019-10-14 09:11||syzop||Note Added: 0020955|
|2019-10-14 17:29||dboyz||Note Added: 0021035|
|2019-10-14 18:17||syzop||Note Added: 0021036|
|2020-01-20 15:33||syzop||Status||feedback => closed|
|2020-01-20 15:33||syzop||Resolution||open => no change required|
|2020-01-20 15:33||syzop||Note Added: 0021233|
|2020-01-20 15:34||syzop||Summary||sasl improvements => sasl code improvements|
|2020-01-20 15:34||syzop||Description Updated||View Revisions|