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

Prevent Sending Passwords from the Clipboard #1893

Open
cesar19004 opened this issue Nov 27, 2024 · 8 comments · May be fixed by #1894
Open

Prevent Sending Passwords from the Clipboard #1893

cesar19004 opened this issue Nov 27, 2024 · 8 comments · May be fixed by #1894
Labels
enhancement A request for a feature or additional functionality triage An issue that needs confirmation and labeling

Comments

@cesar19004
Copy link

cesar19004 commented Nov 27, 2024

Describe your request

Auto-sharing the clipboard to a mobile device is really useful; however, when using password managers, you might not want to share passwords, as they could be stored insecurely in the mobile device's clipboard manager.

Proposed solution

Use clipboard MIME data to determine if content should be shared. Some password managers set the appropriate MIME data to mark content as private, as demonstrated in this example from KeePassXC.

KDE Connect already uses the x-kde-passwordManagerHint MIME data to check for passwords. It also includes a config option to control whether sharing passwords is permitted.

Alternatives

No response

GSConnect version

58

Installed from

GNOME Extensions website

GNOME Shell version

47

Linux distribution/release

Ubuntu 24.10

Additional context

No response

@cesar19004 cesar19004 added the enhancement A request for a feature or additional functionality label Nov 27, 2024
@github-actions github-actions bot added the triage An issue that needs confirmation and labeling label Nov 27, 2024
@ferdnyc
Copy link
Member

ferdnyc commented Nov 27, 2024

Hmm. I'll take a look at this, I thought we only accepted text/plain data for clipboard sharing (and maybe text/htmlas well), and would reject anything else... But I could be misremembering.

To be honest I'm not sure how many Linux applications correctly set the clipboard data MIME type, but I agree we should do what we can to prevent passwords being shared unintentionally.

@cesar19004
Copy link
Author

Thanks for taking the time.
If I'm correct, these are the MIME types accepted by GSConnect:

const TEXT_MIMETYPES = [
    'text/plain;charset=utf-8',
    'UTF8_STRING',
    'text/plain',
    'STRING',
];

And these are the MIME types of passwords returned by the GNOME evaluator when running St.Clipboard.get_default().get_mimetypes(Meta.SelectionType.SELECTION_CLIPBOARD):

0: text/plain;charset=utf-8
1: x-kde-passwordManagerHint
2: text/plain

For now, maybe a simple validation to check that x-kde-passwordManagerHint is not present in the MIME types should be enough, while an option in the config to control password sharing is being considered.

@ferdnyc
Copy link
Member

ferdnyc commented Nov 28, 2024

@cesar19004

Mmm, it looks like currently we special-case ignore any clipboard with text/uri-list in its mimetypes...

async _gtkUpdateText() {
const mimetypes = await new Promise((resolve, reject) => {
this._clipboard.request_targets((clipboard, atoms) => resolve(atoms));
});
// Special case for a cleared clipboard
if (mimetypes.length === 0)
return this._applyUpdate('');
// Special case to ignore copied files
if (mimetypes.includes('text/uri-list'))
return;

I don't see why we couldn't extend that to also exclude x-kde-passwordManagerHint... although it maybe seems a bit odd to see x-kde-passwordManagerHint clipboard contents in a GNOME Shell session? (Still no harm in doing it, tho.)

@ferdnyc
Copy link
Member

ferdnyc commented Nov 28, 2024

My one slight worry with this is whether there might be users who rely on copy-pasting passwords to their mobile device, who we might be breaking things for if we disable this across-the-board.

ferdnyc added a commit to ferdnyc/gnome-shell-extension-gsconnect that referenced this issue Nov 28, 2024
Since some password managers set a clipboard mimetype to signal
when the clipboard contains password data, ignore the clipboard
contents in those cases, to avoid exposing passwords over the
link to the mobile device.

Fixes: GSConnect#1893
@ferdnyc ferdnyc linked a pull request Nov 28, 2024 that will close this issue
@cesar19004
Copy link
Author

Agreed, using x-kde-passwordManagerHint in GNOME Shell seems odd. Also, it doesn't seem to be widely adopted at the moment and passwords copied from some password managers could still be shared. However, in the meantime, it shouldn’t cause harm to validate it while we wait for a more standard or better alternative.

@cesar19004
Copy link
Author

Perhaps, to avoid breaking the copy-pasting of passwords for users who use it, it could be added as an experimental option disabled by default

@daniellandau
Copy link
Member

I would think having a trusted password manager in one device and copy pasting passwords over gsconnect to another is a valid, perhaps even quite normal use case. So I wouldn't make it the default to exclude those. Saves on having to figure out syncing a keepass database for example and dealing with sync conflicts.

I think it would be weird for us to set an x-kde mimetype, but aren't we just talking about handling that when set by others? KeepassXC is definitely used in gnome desktops.

@cesar19004
Copy link
Author

Hi @daniellandau,
Yes, by setting that option to be disabled by default, I meant that it would only prevent sending passwords when the option is enabled.

I think it would be weird for us to set an x-kde mimetype, but aren't we just talking about handling that when set by others? KeepassXC is definitely used in gnome desktops.

Yeah, I see no reason to set that MIME type, only to read it. KeepassXC is quite popular, so I agree that it's definitely used by GNOME users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A request for a feature or additional functionality triage An issue that needs confirmation and labeling
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants