Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[message-tags] CLIENTTAGDENY isupport token #412

Merged
merged 4 commits into from May 20, 2020

Conversation

jwheare
Copy link
Member

@jwheare jwheare commented May 6, 2020

This implements the consensus (afaict) from #410

It also makes the allowance for moderating tags less specific to TAGMSG.

@progval
Copy link
Contributor

progval commented May 6, 2020

LGTM!

Should it be mentioned explicitly that clients can ignore CLIENTTAGDENY, and that their PRIVMSGs shouldn't be dropped entirely if they use a denied tag?

@jwheare
Copy link
Member Author

jwheare commented May 6, 2020

Yeah I can try to make that more explicit.

@jwheare
Copy link
Member Author

jwheare commented May 6, 2020

@progval better?

@progval
Copy link
Contributor

progval commented May 6, 2020

Great!

@prawnsalad
Copy link
Contributor

As someone that this will directly effect and will impact users using my client, I still don't fully follow the logic for this. I've asked before but was dismissed which means I can't even get to understand the logic. If someone could explain the following to me that would be great.

(I've read the previous threads, most of the points made are already solved problems as far as I can tell. Below in more detail.)

I understand that there is an optional implementation of tag whitelisting already, however the conversation seems to have jumped from some concerns to standardising the practise without any discussion of the impact it actually has or any better alternatives.

  1. There were security concerns with allowing arbitrary data in tags. The explanation for the security concerns was "I don't like that at all". What security concern does this spec fix exactly? If it's security related then there should be the specific attack and also the specific mitigation. Otherwise, literally nothing has been accomplished.

  2. One of the main points towards c2c tags is that clients can experiment and implement new features without having to wait for servers to notice, then acknowledge, then config or implement the features. If an IRC server blocks client tags then this innovation is entirely blocked.
    This has already been an issue with some IRC servers that didn't implement c2c tags fully despite advertising message-tags capabilities, and some that have a whitelist for tags, causing extra burden for client developers to know how to enable tags on each IRC server software for when users request support. That is assuming the users of the IRC server and/or general network users has gone out of their way to come to complain to the client developer of a broken client and actually stayed around to hear that it was a server blocking the client features. This actively happens already even before this is widespread. Unfortunately most of the users won't even do that, but only assume the client is broken.

  3. If a client has to determine which features are possible with their client tags, then mainstream (ie. not technical, not IRC literate) users will become confused with their client if "A (feature) may work but B wont, oh but C does work, but D wont work either", simply because the IRC network they connected to are arbitrarily limiting client tags. A mainstream user will however understand more clearly "this network will only support sending messages, nothing more". Why do we want to confuse IRC users? We should be simplifying the experience.

  4. As a user, if I have a client that supports (as an example, it could be anything a client wants to do) e2e encryption that uses some named c2c tags, why should a network admin refuse to allow me to use that feature of my client? It is using supported features of the IRC server and protocol, not using any abused tags, but I can't use it because some network admin decided it was not in my best interest.

  5. There was an issue raised regarding flooding or abuse of arbitrary tags towards a channel. How does this differ to normal abuse? If a user notices that they are getting abused by a c2c tag then this means that they are able to see the tag value or the side effect that the tag is having on their client. In this case, reporting the user to channel or network staff should be sufficient - exactly the same as it currently is for floods or other types of abuse towards a channel. If the abused user does not feel that this is sufficient then they also have the option to ignore the source of abuse or entirely disable c2c tags within their client without impacting everybody else on the network. So far there hasn't been any examples of abuse that this spec is trying to prevent specific to c2c tags, that I’ve seen. Any examples for this?

As it stands, if a network admin decides to limit a clients functionality that makes use of c2c tags then the obvious option for me as a client dev is to display a warning that client functionality has been restricted by the network and that only basic messaging is available. Listing specific disabled/enabled features is mental overhead and will be overlooked by users just making the client look broken in their eyes. Therefore a "basic messaging" or a "fully featured" client is what makes more sense here.

@progval
Copy link
Contributor

progval commented May 6, 2020

causing extra burden for client developers to know how to enable tags on each IRC server software for when users request support

This is what this PR aims to fix.

In this case, reporting the user to channel or network staff should be sufficient - exactly the same as it currently is for floods or other types of abuse towards a channel

Only if the staff has a client that can see these tags. They may not even be aware their client doesn't.

@jwheare
Copy link
Member Author

jwheare commented May 6, 2020

To avoid a long heated back and forth over this comment I’ll take this to IRC privately and we’ll see if we can resolve this.

@jwheare
Copy link
Member Author

jwheare commented May 11, 2020

I believe this has broad consensus. There are obviously still some unshared viewpoints around whether client tags should be blocked at all, but I think given that is something servers will be doing anyway, for various reasons, we should go ahead with this errata.

I've chatted to the people involved and there are disagreements concerning the reasons server authors/operators have given, but I feel they are still valid reasons. And I don't think anyone is outright vetoing this proposal.

With all that in mind, I will be merging this by the middle of the week.

@jwheare
Copy link
Member Author

jwheare commented May 13, 2020

I am actually going to delay this until I've tested an implementation. If anyone has implementations on e.g. testnets, please post here.

SadieCat added a commit to SadieCat/inspircd that referenced this pull request May 13, 2020
@jwheare
Copy link
Member Author

jwheare commented May 13, 2020

Thoughts on requiring that * appear before any negated tags? e.g. this is allowed:

CLIENTTAGDENY=*,-typing

This is not:

CLIENTTAGDENY=-typing,*

Means clients can iterate the list in order and set a "denied" flag on or off if there's a match.

Otherwise we need to specify that arguments are interpreted in a certain order.

This is only ambiguous for the deny-all-except-some case.

@k4bek4be
Copy link

k4bek4be commented May 13, 2020

I have an implementation for this available for testing on a public server (in fact for 8 days now). It is currently accessible via irc://aga.k4be.pl:6667/

@jwheare
Copy link
Member Author

jwheare commented May 14, 2020

I have implemented this in the IRCCloud web client to toggle the reply, react, edit and delete UI, and typing notifications when the respective tags are being blocked.

@k4bek4be
Copy link

It is now merged into unrealircd-5.0.5-dev.
unrealircd/unrealircd@0aa5fb6

@jwheare
Copy link
Member Author

jwheare commented May 19, 2020

This is probably OK to merge now. Will do so tomorrow if no further comments.

@jwheare jwheare merged commit 970640b into ircv3:master May 20, 2020
@jwheare jwheare deleted the client-tag-deny-isupport branch May 20, 2020 15:28
SadieCat added a commit to inspircd/inspircd that referenced this pull request May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants