0005603: TKL updates are gone - UnrealIRCd Bug Tracker

ID	Project	Category	View Status	Date Submitted	Last Update

0005603	unreal	ircd	public	2020-03-26 22:26	2020-05-14 09:16

Reporter	Le_Coyote	Assigned To	syzop
Priority	normal	Severity	minor	Reproducibility	always
Status	resolved	Resolution	fixed
Product Version	5.0.3.1
Fixed in Version	5.0.5

Summary	0005603: TKL updates are gone
Description	Apparently, this was done on purpose: https://bugs.unrealircd.org/view.php?id=4976 But this is a regression. Case in point, an anti-spam script that we have places soft shuns on client connections from "weird" countries. But sometimes it appears to misfire, and there is no way to tell a faulty script from an existing TKL at runtime (apart from manually searching through them all).
Steps To Reproduce	- apply TKL to target with specified duration - apply TKL to same target with different duration => No message from IRCd
Tags	No tags attached.

3rd party modules

syzop 2020-04-12 15:17 administrator ~0021440 Last edited: 2020-04-12 15:19	Some background: the 'tkl update' notice was never meant for this in the first place, it has always been meant for server syncing tkl updates (so when two servers link). Anyway, on the subject of "altering existing LINES" set by people... I think it would be better to just reject the LINE / SPAMFILTER with a generic error message. Opinions? :)

Lord255 2020-04-12 17:00 reporter ~0021451	uhm. so you say it would be better to not let ppl update eachothers xlines, so if they want to "update" it, they have to remove and then re-add it. make sense. (better for tracking i presume, unless the whole thing could be point to the person who changed the things)

Le_Coyote 2020-04-14 13:46 reporter ~0021468	In terms of userfriendliness (even if those users are opers), I think the ability to extend an existing TKL is a useful tool to have. If the final decision is to reject such changes, then IMHO it MUST generate an error message towards the client. However, if internally a "tkl update" is not convenient when it comes from an oper, can Unreal decide do remove the corresponding line and place the new one instead, for convenience's sake? In which case the tkl update used for server syncing does not need to be messed with, and from the oper's point of view, usability remains good :)

Lord255 2020-04-16 13:47 reporter ~0021496	giving and error could be a solution, or the update leaves a log entry or something and updates the person who added the tkl gets updated, then its fine. so for investigation, it's important to know who put the tkl first and who updated it and why. since its not possible to give a reason for the update (or it should be a feature), i would say that give an error and the oper can decide to remove and re-add it with the updated info.

syzop 2020-05-14 08:54 administrator ~0021570 Last edited: 2020-05-14 08:55	For new and other people joining in. As mentioned in my first post, TKL updating was never meant to be done by end-users. It was only meant for when servers link then they would choose which one wins and then a notice was sent. The problem is that if a TKL entry already exists, and we get a new one, because the updating was meant for server linking, we follow certain 'merge' rules, such as 'highest duration wins' and the lowest alphabetical setter wins, that sort of stuff. So using TKL updates by end-users will not work OK. As an example: you can enlarge a TKL from 10m to 100m but once it is at 100m you cannot lower it to 50m. So I do not recommend using it now. Just to be clear, THIS NOT NEW. It has been like that in 4.x and even in 3.2.x. Again, it was never meant to be used as a tool for end-users to 'update' glines. So the current behavior is a bug, not a feature, and it behaves buggy as well. Since, people obviously are trying to update *LINES, usually by mistake, it is clear that we need to handle it better. I will just outright reject it from end-users now as a fix. Doing automatic removal+readd is more convenient to the oper but too troublesome at the moment in the code and.. as also said here.. we don't know the intentions of the setter... eg did that person really want to shorten the duration of the ban, etc. It would only be guessing. I don't think it's a major issue anyway to see the gline rejected, because when do you deliberately gline an IP/host that is already glined? It is usually by mistake. Sure, I can think of theoretical cases when it can be useful, that is easy, but those happen rarely.

syzop 2020-05-14 09:16 administrator ~0021571	Fixed now, thanks for the report. No more hidden changes to LINEs by opers :) https://github.com/unrealircd/unrealircd/commit/941b745be278de422995b6f32c7ec0ce3e92b1b3 commit 941b745be278de422995b6f32c7ec0ce3e92b1b3 Author: Bram Matthys <[email protected]> Date: Thu May 14 09:08:57 2020 +0200 Give an error when trying to place an LINE that already exists. Then the oper may decide if the original entry should indeed be removed and re-added, or if (s)he should not touch it. These are usually done by mistake anyway. Updating existing entries by end-users was never intended and did not work properly anyway (see bug comments). Issue reported by Le_Coyote and armyn in https://bugs.unrealircd.org/view.php?id=5603

Date Modified	Username	Field	Change
2020-03-26 22:26	Le_Coyote	New Issue
2020-04-12 15:17	syzop	Note Added: 0021440
2020-04-12 15:18	syzop	Assigned To	=> syzop
2020-04-12 15:18	syzop	Status	new => feedback
2020-04-12 15:19	syzop	Note Edited: 0021440
2020-04-12 17:00	Lord255	Note Added: 0021451
2020-04-14 13:46	Le_Coyote	Note Added: 0021468
2020-04-16 13:47	Lord255	Note Added: 0021496
2020-05-14 08:30	syzop	Relationship added	has duplicate 0005672
2020-05-14 08:54	syzop	Note Added: 0021570
2020-05-14 08:55	syzop	Note Edited: 0021570
2020-05-14 09:16	syzop	Status	feedback => resolved
2020-05-14 09:16	syzop	Resolution	open => fixed
2020-05-14 09:16	syzop	Fixed in Version	=> 5.0.5
2020-05-14 09:16	syzop	Note Added: 0021571