View Issue Details

IDProjectCategoryView StatusLast Update
0005434unrealircdpublic2019-10-14 18:17
ReporterdboyzAssigned Tosyzop 
PrioritynormalSeveritytweakReproducibilityN/A
Status feedbackResolutionopen 
Product Version5.0.0-alpha3 
Target VersionFixed in Version 
Summary0005434: sasl improvements
DescriptionHi, 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.

Thanks!
Tagssasl
3rd party modules

Activities

dboyz

2019-10-08 19:33

reporter  

sasl-diff.txt (21,744 bytes)

dboyz

2019-10-08 19:35

reporter   ~0020945

I forgot to mention that sasl_complete has been re-purposed to reject repeated svslogin message.

What do you think?

syzop

2019-10-09 15:58

administrator   ~0020946

Last edited: 2019-10-09 15:59

View 2 revisions

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.

syzop

2019-10-14 09:11

administrator   ~0020954

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.

syzop

2019-10-14 09:11

administrator   ~0020955

See previous comment. I can understand that this will take some time for you to test.

dboyz

2019-10-14 17:29

reporter   ~0021035

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

syzop

2019-10-14 18:17

administrator   ~0021036

Ok understood. Yeah it's really vital to test these things.

Issue History

Date Modified Username Field Change
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