Klipper: Do not insert secret data into history
ClosedPublic

Authored by hoffmannrobert on Apr 26 2018, 10:41 AM.

Details

Summary

Password manager tools like Keepassx offer an option to clear
the clipboard/selection after some time, e.g. 10 seconds,
after the password was copied to the clipboard. This works fine,
but unfortunately the password isn't removed from Klipper's
history. This is a great security risk, which may make the use
of password managers impossible.

This patch changes Klipper::applyClipChanges(const QMimeData* clipData)
where clipboard data is inserted into history. If the data has an
additional mime type 'x-kde-passwordManagerHint', it is not inserted
into history.

For this to work as designed, password managers should add the
additional mime type 'x-kde-passwordManagerHint' to the mimeData
like following when copying a password to the clipboard:

QMimeData* mimeDataClipboard = new QMimeData();
const QString secretStr = "secret";
QByteArray secretBa = secretStr.toUtf8();
mimeDataClipboard->setText(password);  // this is the password to copy
mimeDataClipboard->setData("x-kde-passwordManagerHint", secretBa);
clipboard->setMimeData(mimeDataClipboard, QClipboard::Clipboard);

if (clipboard->supportsSelection()) {
    // we cannot use the same QMimeData, it's already owned by clipboard
    QMimeData* mimeDataSelection = new QMimeData();
    mimeDataSelection->setText(password); // this is the password to copy
    mimeDataSelection->setData("x-kde-passwordManagerHint", secretBa);
    clipboard->setMimeData(mimeDataSelection, QClipboard::Selection);
}

Diff Detail

Repository
R120 Plasma Workspace
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Restricted Application added a project: Plasma. · View Herald TranscriptApr 26 2018, 10:41 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
hoffmannrobert requested review of this revision.Apr 26 2018, 10:41 AM
hoffmannrobert edited the summary of this revision. (Show Details)Apr 26 2018, 10:42 AM
hoffmannrobert edited the summary of this revision. (Show Details)Apr 26 2018, 10:48 AM
davidedmundson accepted this revision.Apr 26 2018, 10:48 AM
davidedmundson added a subscriber: davidedmundson.

Beautiful!
Do you have commit access?

This revision is now accepted and ready to land.Apr 26 2018, 10:48 AM

Can you link to the relevant Keepassx patch when you make it so I can help follow up there.

And it's probably something we also want to add to KWallet and maybe every password field we have.

And it's probably something we also want to add to KWallet and maybe every password field we have.

Since when can you Copy from password fields?

And it's probably something we also want to add to KWallet and maybe every password field we have.

Since when can you Copy from password fields?

I just locked my screen on X11 (I need the wacom), enabled echo mode, copied the content of the password field, unlocked and pasted and yes it worked.

dvratil added a subscriber: dvratil.May 1 2018, 8:06 PM
This revision was automatically updated to reflect the committed changes.

I was kinda hoping to see your keepassx patch merged before we did this in case of any changes, but given we have 5.13 beta soon, I've merged as-is