Bug 2520 - ssh-keygen: sanitize ANSI escape sequences in key comment
Summary: ssh-keygen: sanitize ANSI escape sequences in key comment
Status: CLOSED FIXED
Alias: None
Product: Portable OpenSSH
Classification: Unclassified
Component: ssh-keygen (show other bugs)
Version: 7.1p1
Hardware: amd64 Linux
: P5 minor
Assignee: Damien Miller
URL:
Keywords:
Depends on:
Blocks: V_7_5
  Show dependency treegraph
 
Reported: 2016-01-06 06:15 AEDT by Roland Hieber
Modified: 2018-04-06 12:26 AEST (History)
2 users (show)

See Also:


Attachments
public key with ANSI escape sequences (410 bytes, application/vnd.ms-publisher)
2016-01-06 06:15 AEDT, Roland Hieber
no flags Details
screenshot showing the output from ssh-keygen on the public key (7.55 KB, image/png)
2016-01-06 06:16 AEDT, Roland Hieber
no flags Details
proposed patch (1.42 KB, patch)
2016-01-06 06:19 AEDT, Roland Hieber
no flags Details | Diff
sanitise escape sequences but not valid UTF-8 when the locale supports it (1.89 KB, patch)
2017-02-10 14:00 AEDT, Damien Miller
dtucker: ok+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Hieber 2016-01-06 06:15:44 AEDT
Created attachment 2775 [details]
public key with ANSI escape sequences

I noticed that ssh-keygen prints most non-printable characters in the comment as-is when showing the fingerprint of a key. This can lead to confusing output on the terminal when the comment contains ANSI escape characters which are interpreted by the terminal. The attached public key file serves as an example, which, when fingerprinted on my Linux terminal, looks like this:

$ ssh-keygen -E sha256 -lf test.pub
1024 MD5:de:ad:be:ef:00:7h:15:15:af:0r:6e:d0:ha:5h:00:00 nobody@example.org (RSA)

... in nice rainbow colors (see attached screenshot). Also note that a SHA256 hash was requested whereas the output is an MD5 hash (which also contains invalid characters, so it cannot really be an MD5 hash...), but you get the point that, in general, this technique can be used to suppress the real fingerprint of a key and let the user see a different one.

For this reason, I suggest applying the attached patch (based on commit 271df81 from the OpenSSH Portable GitHub repository), which emplys strvis() to escape possibly dangerous characters in the comment prior to printing it to the terminal.  This should serve as a sufficient workaround for the obfuscating escape behaviour of the underlying terminal emulator.
Comment 1 Roland Hieber 2016-01-06 06:16:57 AEDT
Created attachment 2776 [details]
screenshot showing the output from ssh-keygen on the public key
Comment 2 Roland Hieber 2016-01-06 06:19:32 AEDT
Created attachment 2777 [details]
proposed patch
Comment 3 Damien Miller 2016-01-06 18:11:48 AEDT
This is not going to be popular with users who have UTF-8 or other non-ASCII characters in their comments. We need something like that proposed for bug #2058
Comment 4 Damien Miller 2017-02-10 14:00:24 AEDT
Created attachment 2940 [details]
sanitise escape sequences but not valid UTF-8 when the locale supports it

This patch uses Ingo's recent mprintf API to safely render strings while preserving UTF-8 characters when the locale supports them.
Comment 5 Damien Miller 2017-02-10 14:38:34 AEDT
Fix applied. This will be in OpenSSH 7.5

commit a287c5ad1e0bf9811c7b9221979b969255076019
Author: djm@openbsd.org <djm@openbsd.org>
Date:   Fri Feb 10 03:36:40 2017 +0000

    upstream commit
    
    Sanitise escape sequences in key comments sent to printf
    but preserve valid UTF-8 when the locale supports it; bz#2520 ok dtucker@
    
    Upstream-ID: e8eed28712ba7b22d49be534237eed019875bd1e
Comment 6 Damien Miller 2018-04-06 12:26:41 AEST
Close all resolved bugs after release of OpenSSH 7.7.