View Issue Details

IDProjectCategoryView StatusLast Update
0005603unrealircdpublic2020-05-14 09:16
ReporterLe_Coyote Assigned Tosyzop  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version5.0.3.1 
Fixed in Version5.0.5 
Summary0005603: TKL updates are gone
DescriptionApparently, 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
TagsNo tags attached.
3rd party modules

Relationships

has duplicate 0005672 closed Problem with gline and update (duration/reason update does not work) 

Activities

syzop

2020-04-12 15:17

administrator   ~0021440

Last edited: 2020-04-12 15:19

View 2 revisions

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

View 2 revisions

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 <syzop@vulnscan.org>
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

Issue History

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 View Revisions
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 View Revisions
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