Fix several problems in algorithm of deriving key filename from a prompt message supplied by ssh-add
Review Request #127569 - Created April 4, 2016 and submitted
|jriddell, lueck, mlaurent, whiting|
Fix an algorithm of deriving key filename from a prompt message
supplied by ssh-add: should fix problems with spaces in key file
ssh-addand fix extra space in KWallet
Obviously, it's a bad thing to do (especially given that nobody
guarantees that ssh-add won't use another set of messages someday or
it will be i18ned), but (1) there seem to be no workaround for it as
of now, (2) current version of ksshaskpass relies on it anyway, (3)
the algorithm is very broken.
Current algorithm relies on splitting a prompt string by spaces and
extracting certain nth fragment, counting from the end of
sequence. There are several major issues with it:
- When using
ssh-add -c, the message would be
Enter passphrase for /home/user/path/to/key (will confirm each use):, thus current
algorithm will extract
use)as a key file name, which is
obviously wrong. It's a hideous bug that goes unnoticed for the
majority of the people who either (a) do not use
-coption, or (b)
do not use more than 1 ssh key.
- Splitting by spaces obviously breaks on key filename that contain
spaces. Again, it mostly goes unnoticed by vast majority of the
- It generates extra space at the end of key filename, i.e. it
/home/user/.ssh/id_rsa. Not critical, but undermines efforts to
get key passphrase by filename in KWallet.
Proposed algorithm is clearly extracted in a distinct function and
clearly analyses 3 cases by rigid regexp patterns. In case if regexps
would fail, it issues a warning (seems like a good idea, so at least
it won't go completely unnoticed if it will break in future).
Tested it with all inputs that seem to be generated by ssh-add:
Enter passphrase for /home/user with space/.ssh/id_dsa (will confirm each use):
Bad passphrase, try again for /home/user with space/.ssh/id_dsa (will confirm each use):
Enter passphrase for /home/user with space/.ssh/id_dsa:
Bad passphrase, try again for /home/user with space/.ssh/id_dsa:
Provide workaround for a buggy KWallet key names used in previous
versions of ksshaskpass for smoother transition for regular users.
Previous versions of ksshaskpass had a bug: key names parsed from
ssh-add prompts were having extra trailing space (
). Given that
affected versions were deployed in the wild since almost the very
beginning of ksshaskpass, probably vast majority of users will be
affected by this bug. Fixing it as suggested in
https://git.reviewboard.kde.org/r/127569/ will break "compatibility"
and make users lose their passwords, which is inconvenient. This patch
ensures that if a key can't be found, we'll do extra check for key
with extra space, and, if found, rename it properly.
In "Added Files" there's a patch https://git.reviewboard.kde.org/media/uploaded/files/2016/04/04/33402a80-fc93-48d8-bd15-ada206d51776__your-patch2.patch that is different from the patch from "Download Diff" https://git.reviewboard.kde.org/r/127569/diff/raw/ Which is the good one? If the one from "added Files" is the good one, can you please remove it and use "Update" -> "Update Diff" to add the correct one?