wayland/clipboard: stop reading passwords?

[martinetd]

Background: I've started using wl-copy --paste-once recently, and it took me a while to figure out that it stopped working when mpv was running because mpv was reading my passwords.

So, for passwords specifically there's a thing with x-kde-passwordManagerHint -- if there's an offer with this mime type, mpv could ignore the update; it's probably not too hard to do, I might send a PR for it if it makes sense, and I'll be mostly happy...

But even for non-passwords I'm not sure I like the way this works, reading everything I could select (primary selection) or copy... One problem for example is that mpv will read data one chunk at a time and remember it all so I could just make a client streaming infinite data and crash mpv.
First, to make sure I understand why we're reading the clipboard on every update even if nothing uses it, I guess it's to avoid blocking whatever handled a key binding, as reading data from clipboard is easy to hang if the sender just doesn't send data when requested, so instead of e.g. hitting ctrl-v and then getting stuck only that background clipboard thread would get stuck and the previous data would still be available and all is well...?
I would have suggested only getting data if there's focus (or a lua script requesting clipboard if possible), but if there's no output we just can't tell if there's focus... I guess we could limit the buffer size we're reading in
get_selection_data() ? I'm frankly inclined to think I'd just prefer mpv to request data when hitting ctrl-v and sod it if the last offer came from a badly behaved client (perhaps with a timeout?)...
Or could the keybind trigger fetching data from background thread and have a callback from said thread handle adding to playlist or calling into lua?...

Anyway it's getting late for me and I'm losing focus; first thing first I'll open a PR with the x-kde-passwordthing in the next few days unless someone screams not to.

Thanks!

[na-na-hi]

So, for passwords specifically there's a thing with x-kde-passwordManagerHint -- if there's an offer with this mime type, mpv could ignore the update; it's probably not too hard to do, I might send a PR for it if it makes sense, and I'll be mostly happy...

I think the current mpv policy on wayland is to avoid compositor or application specific handlings like this.

so instead of e.g. hitting ctrl-v and then getting stuck only that background clipboard thread would get stuck and the previous data would still be available and all is well...?

This cannot be done because clipboard is accessed through properties and property access is always synchronous and happens in main player thread. Any delay in property access will cause the player to lock up. Async access to clipboard would require new commands.

I would have suggested only getting data if there's focus (or a lua script requesting clipboard if possible)

--clipboard-backends=vo does this and prevents mpv from reading clipboard when not in focus.

[martinetd]

Thanks for taking the time to answer!

I think the current mpv policy on wayland is to avoid compositor or application specific handlings like this.

This has x-kde-.. in the name but it's becoming a de-facto standard, e.g.

• wl-clipboard added wl-copy --sensitive ( bugaevc/wl-clipboard@e3c1b21 )
• fcitx5 "clipboard manager" module handles it ( https://github.com/fcitx/fcitx5/releases/tag/5.1.9 )
• from fcitx5's release note things like keepassxc also set it
• (I'd assume tools in KDE also handle it given the name...)

Given the merry mess that wayland is I assume that at some point someone will try to give this a more official name (... or even worse a new version of clipboard-related protocols), but I haven't seen anything about that, so I consider this to be a de facto standard for now -- at the very least this sounds more reasonable than a full rework of how the clipboard is handled e.g ↓

This cannot be done because clipboard is accessed through properties and property access is always synchronous and happens in main player thread. Any delay in property access will cause the player to lock up. Async access to clipboard would require new commands.

Sounds like it'd still be allowed to have the action in main thread kick in another thread to grab the content and modify the playlist after the action callback returned if someone wants to bother.

--clipboard-backends=vo does this and prevents mpv from reading clipboard when not in focus.

Thank you, I used that at first but I've made it so non-"privileged" programs cannot use ext-data-control-v1 anymore, so I guess I'm beating on a dead horse.
This question was mostly about raising awareness for me, if you (as in mpv contributors in general) are aware of the "problem" that sensitive data is being read and kept in memory (available in lua scripts!), then I'm happy with my workaround.

(If anyone cares, I asked for my compositor if it was possible to restrict such protocols natively in
niri-wm/niri#1773 (reply in thread) - it wasn't so I'm doing unholy things with https://github.com/martinetd/create_wayland_security_context_v1 to replace the main wayland socket with a "restricted" one, and only start programs that need such protocols with access to the original "privileged" socket)

[zefr0x]

So, for passwords specifically there's a thing with x-kde-passwordManagerHint -- if there's an offer with this mime type, mpv could ignore the update; it's probably not too hard to do, I might send a PR for it if it makes sense, and I'll be mostly happy...

I would like to note that x-kde-passwordManagerHint is used to prevent the KDE clipboard manager (the thing that store a history of your clipboard) from storing your password that was copied from a password manager, not to prevent usual applications from reading it.

[martinetd]

Thank you for pointing I was being rude, I've edited the "I didn't ask for" from the above, as it has nothing to do with the argument.

Now if you don't care that mpv, by default, slurps any secret going through the clipboard (and it's not just password managers which set the kde flag, but also API keys and stuff coming from browser which don't), then I guess I have nothing more to say -- as said in the other part of the thread I made it so mpv cannot use this privileged protocol and have no more stake in this. I'm sorry for everyone else.

Have a good day.

[CounterPillow]

I do agree that it's a bad feature and it should probably be off by default, but if it's not in there the usual wlroots ricers will shit and fart all over and cry when they can't have some really stupid setup that requires it.

[kasper93]

Should we disable bad features by default? /cc @na-na-hi ?

[na-na-hi]

Is it a "bad feature" when it can also be achieved with wl-clipboard which was required for clipboard before the native clipboard was added? wl-clipboard also uses the same "privileged protocol" yet no one complained. The compositor is responsible to define which clients are "privileged". mpv should not need to care about this.

If this is disabled then it won't achieve feature parity as wl-clipboard used previously by console.lua. For example, you cannot paste in console.lua in terminal anymore.

[kasper93]

The main problem is that Wayland/X11 backends are pooling clipboard all the time, in background, without any user interaction. Unlike Windows backend (or previous wl-clipboard solution) which reads data on demand. Wayland backed reads it as soon as offer is provided and stores in mpv heap which may never be accessed. As already mentioned this breaks single time clipboard. Regardless of clipboard-monitor option, which does nothing on Wayland/X11 in practice.

uses the same "privileged protocol" yet no one complained. The compositor is responsible to define which clients are "privileged". mpv should not need to care about this.

This I fully agree. I don't see anything "privileged" about this protocol when it can be used by any user application without any preconditions.

I don't have as strong opinion as others, but this sounds like a bug that with clipboard-monitor=no mpv still consumes any and all clipboard data, unprompted.

[rcombs]

See #17367 (comment) for my thoughts on this; tldr it sounds like the limitations of the Linux clipboard APIs mean that we should probably replace the property with an async command. PRs welcome.

As for the unexpected clipboard-monitor=no behavior, that definitely sounds like a bug in the Linux implementations and it'd be entirely reasonable to fix even if the async migration doesn't happen immediately.

[kasper93]

Fixed by: #17608