Can't type | ( pipe ) over vnc

Bug #1787267 reported by Phillip Susi
24
This bug affects 3 people
Affects Status Importance Assigned to Milestone
qemu (Ubuntu)
Bionic
Fix Released
Undecided
Phillip Susi
Cosmic
Fix Released
Undecided
Unassigned

Bug Description

[Impact]

After upgrading from 16.04 to 18.04, users are unable to type a | or > in their virtual machines when accessing them via vnc. This happened because upstream added a key to the keymap that can also produce these symbols and exists only on European keyboards, but it uses different modifier keys to produce these symbols so trying to type a | or a > results in a < instead. Simply removing this new non-existent scan code from the keymap resolves the issue.

[Test Case]

Start a virtual machine and connect to it via vnc. I am using "tightvnc" on Windows, and upstream indicated that some vnc clients send the raw scan code instead of keysyms, in which case this issue would not arise.
That doesn't have to start a lot and can be as trivial as :
$ qemu-system-x86_64 -vnc :1 -cdrom <path to ubuntu iso>
$ vncviewer 127.0.0.1:5901
# no need to fully boot that, just go to the menu
# now on a system with us keyboard (or just en_us input layout) try to use a | which will not work.

With the fix you will be able to type the chars again as you'd expect it.

[Regression Potential]

There should be no regression potential for actual en-us keyboards since this key does not exist. European keyboards may be unable to use this other key they have unless they set the correct keymap, but all of its symbols are duplicates from other keys on the keyboard, rendering it fully redundant.

[Notes]

Upstream has already added logic to pick the mapping the matches the current modifier keys to resolve this, so cosmic should be unaffected and so this patch only applies to bionic.

This was stalled a while due to other SRUs being in flight, in the meantime other Distros picked the same giving this some extra confidence:
- Arch: https://aur.archlinux.org/cgit/aur.git/tree/remove-problematic-evdev-86-key-from-en-us-keymap.patch?h=qemu-minimal
- Fedora: https://src.fedoraproject.org/rpms/qemu/c/f81be8f0261cce74799f946e99f23d57f8db7e17?branch=master

[Original Description]

I tried booting some Ubuntu images today on my Xen server where I use VNC to remotely access the VM, and am unable to type a | ( pipe ). I opened the keyboard layout utility and when I press the \ key, it recognizes it, but when I hold shift and press the key, it claims that I am pressing some key between left shift and Z that does not actually exist on US keyboards, and that types a > instead.

This did not used to happen, and probably started when I upgraded the server from 16.04 to 18.04 a while back and I just haven't been testing VMs since then.

CVE References

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

That key actually exists on my german keyboard :-)

It would be:
< - no modifier
> - with shift
| - with right alt (also called alt-gr)

I'd not at all see why your setup goes for a EU keyboard mapping in your setup, but you might try "right-alt + <that key>".

Qemu doesn't "do" anything there except passing through.
I'd more assume that might be an option of your VNC client, did you upgrade the system the VNC client is on as well at the same time?

Changed in qemu (Ubuntu):
status: New → Incomplete
Revision history for this message
Phillip Susi (psusi) wrote :

Yes, but why is it in the keyboard map for EN-US? Odd... no, I did not change the vnc client. I would think that vnc would be sending character codes, not scan codes, and so qemu must translate back to a scan code and it isn't doing it right.

Changed in qemu (Ubuntu):
status: Incomplete → New
Revision history for this message
Phillip Susi (psusi) wrote :

And holding right alt, shift, and pressing \ does not produce a | either.

Revision history for this message
Phillip Susi (psusi) wrote :

I just installed x11vnc in the vm and connected to that with the same vnc client and | works correctly there, so it must be qemu.

Revision history for this message
Phillip Susi (psusi) wrote :

The really weird thing is that pressing \ without shift sends the right scan code.... it's only when I try to shift it that the vm thinks it is getting that other weird key.

Revision history for this message
Phillip Susi (psusi) wrote :

I'm betting that it was this commit:

commit ab8f9d49d62c82a12409475547e4420a46da56ed
Author: Daniel P. Berrange <email address hidden>
Date: Wed Jan 17 16:41:15 2018 +0000

    hw: convert ps2 device to keycodemapdb

    Replace the qcode_to_keycode_set1, qcode_to_keycode_set2,
    and qcode_to_keycode_set3 tables with automatically
    generated tables.

I've emailed the author and qemu-devel mailing list about it.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Yeah I was following both and replied how to build.
Let me know if you run into further trouble with the package build.

Revision history for this message
Phillip Susi (psusi) wrote :

I'm still confused as to why the debian git repo is missing the pristine-tar branch and how to get the original tarball for the build to work.

Revision history for this message
Phillip Susi (psusi) wrote :

Just posted this to the mailing list:

Thanks. I checked out the 2.11 version that Ubuntu 18.04 shipped and found the problem:

commit a7815faffb2bd594b92aa3542d7b799cc89c5414
Author: Gerd Hoffmann <email address hidden>
Date: Thu Oct 5 17:33:30 2017 +0200

    pc-bios/keymaps: keymaps update

    Update the keymaps with the ones generated by qemu-keymap

    Signed-off-by: Gerd Hoffmann <email address hidden>
    Message-id: <email address hidden>

Prior to this commit, this file said it was generated from XKB, and was much smaller, not including the scan code for that other key. This commit added the 0x56 bar translation with altgr, and it wasn't until after 2.12 those smarts were inserted to pick the scancode that matches the current modifier keys.

So I think it should be as simple as removing this one line from the keymap file and we can SRU a fix!

C de-Avillez (hggdh2)
Changed in qemu (Ubuntu):
milestone: none → ubuntu-18.04.1
Revision history for this message
Phillip Susi (psusi) wrote :

Ok, what the hell? Whenever I build qemu from source ( got from apt-get source qemu ), my build adds some new dependencies including libcapstone3 that were not there before, and are are not installable.

Revision history for this message
Phillip Susi (psusi) wrote :

Ok, it seems to have optional dependencies that I had installed when trying to build the code from the debian git repo. Rebuilt in a clean chroot and it works great! Also noticed that the > key is affected, becoming < instead. This simple patch fixes it.

Revision history for this message
Phillip Susi (psusi) wrote :
Phillip Susi (psusi)
description: updated
Phillip Susi (psusi)
Changed in qemu (Ubuntu):
status: New → In Progress
tags: added: patch
C de-Avillez (hggdh2)
Changed in qemu (Ubuntu):
milestone: ubuntu-18.04.1 → none
Changed in qemu (Ubuntu Bionic):
milestone: none → bionic-updates
Changed in qemu (Ubuntu Cosmic):
status: In Progress → Fix Released
Changed in qemu (Ubuntu Bionic):
status: New → In Progress
Phillip Susi (psusi)
no longer affects: qemu (Ubuntu)
description: updated
Changed in qemu (Ubuntu Bionic):
assignee: nobody → Phillip Susi (psusi)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Hi,
I was tracking the upstream discussion closely and I'm glad you came here with a suggested fix.
With some minor polishing I added it to the packaging git for qemu.
I built a test version in a PPA at [1].

Never the less - probably since keycodes isn't my qemu-home-turf - I'm afraid that this change has a chance to fix an issue for some, but cause new issues for other users.
Therefore I'd want to go extra safe on this.

I'd suggest:
1. you test your case against the PPA I provided
2. I want to test on my own a matrix of qemu usages and check if/how they change due to that upgrade.
3. users in general are invited to test their case (the more languages/keyboards we have the better) I think I'm gonna send a call for testing to Ubuntu-server after we are done with our tests.

[1]: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/3380

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (3.4 KiB)

I tested a matrix of:
- ubuntu/xubuntu/lubuntu
- vga qxl / vga virtio
- keymap de / en-us / none
- input host keymapping de / en-us
- xtightvncviewer / tigervnc-viewer / spicy

Silly helper to spawn all of these:
#!/bin/bash
runguest () {
    local prefix=$1
    local kbd=$2
    local idx=$3
    local lidx=$(printf "%02d" $3)
    local vga=$4

    local cmd="qemu-system-x86_64 -spice port=50${lidx},password=pw -vga ${vga} -vnc :${idx}"

    if [ "${prefix}" == "u" ]; then
        realprefix=""
    else
        realprefix="${prefix}"
    fi
    cmd="${cmd} -cdrom ${realprefix}ubuntu-18.04-desktop-amd64.iso"

    if [ -n "${kbd}" ]; then
        cmd="${cmd} -k ${kbd}"
    else
        kbd="none"
    fi

    cmd="${cmd} -name '${lidx}-${prefix}-${kbd}-${vga}'"

    ${cmd} &
}

runguest "u" "de" 1 "qxl"
runguest "l" "de" 2 "qxl"
runguest "x" "de" 3 "qxl"
runguest "u" "en-us" 4 "qxl"
runguest "l" "en-us" 5 "qxl"
runguest "x" "en-us" 6 "qxl"
runguest "u" "" 7 "qxl"
runguest "l" "" 8 "qxl"
runguest "x" "" 9 "qxl"
runguest "u" "de" 11 "virtio"
runguest "l" "de" 12 "virtio"
runguest "x" "de" 13 "virtio"
runguest "u" "en-us" 14 "virtio"
runguest "l" "en-us" 15 "virtio"
runguest "x" "en-us" 16 "virtio"
runguest "u" "" 17 "virtio"
runguest "l" "" 18 "virtio"
runguest "x" "" 19 "virtio"

Results are a quadruple of DEl-DEk/DEl-USk/USl-DEk/USl-USk in this order
With the first element defining the clients current keyboard alyout (a.k.a. what is on my system)
DEl = German layout, USl = EN-US Layout
The second is the actual key pressedwith
DEk = AltGr+<, USk Shitf+# (on my German keyboard equaling the keycode for a | on en-us).
X means the key gave some odd control char or something like that.

After a while it became clear that the UI had no effect, so I skipped further U/X/Lubuntu differences

                    tight tiger spicy
1 U-DE-QXL <|<> <|<> <|<|
2 L-DE-QXL <|<> <|<> <|<|
3 X-DE-QXL <|<> <|<> <|<|
4 U-US-QXL <"<> <"<> <|<|
7 U-No-QXL <"<> <"<> <|<|
11 U-DE-VIRTIO <|<> <|<> <|<|
14 U-US-VIRTIO <"<> <"<> <|<|
17 U-No-VIRTIO <"<> <"<> <|<|

Insights:
- Spice ignores every config and behaves like a us querty keaboard.
- The VNC implementations has no influence
- The vga mode has no influence
- The Ubuntu Desktop type has no influence
- If US or no keycode is set then the none of the keys in vnc mode maps to a "|", instead it is a "
- With a en-us keyboard layout on the Client I can't reach a "|"

With the insights above the interesting tests after update from the PPA will be:

                    tight tiger spicy
1 U-DE-QXL <|<> <|<> <|<|
4 U-US-QXL \"\| \"\| <|<|
7 U-No-QXL \"\| \"\| <|<|

That would confirm that now qemu VNC with en-us keymaps (or not set) would have a keycode on US keyboard layouts to reach the | character.
Interestingly the very same change made it unreachable for non-US layout (client) to reach the | if no keymap or an en.us keymap is set.

But that is probably ok as -k is exactly meant to ...

Read more...

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Interested what your testing will show ...

Also I wondered since this is essentially a change to the data file of qemu-system-common:
qemu-system-common: /usr/share/qemu/keymaps/en-us

Further if this really is an issue and the right fix, I'm not sure about other languages - but should we at least fix en-gb as well as it is affected by the same?

Revision history for this message
Phillip Susi (psusi) wrote :

I'm not quite sure I understood everything you said. The patch I uploaded lets me type a | again. Of course, it will break that extra key on non US keyboards, but it didn't work in 16.04 anyhow, and all of the keys you can type with it can be typed using other keys.

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Ok, this will be made part of the next Qemu SRU

In the meantime it was also picked up in other places.
Upstream: bug 1738283
It seems to be the consensus to remove that definition just as I did in the PPA here.

Arch: https://aur.archlinux.org/cgit/aur.git/tree/remove-problematic-evdev-86-key-from-en-us-keymap.patch?h=qemu-minimal
Fedora: https://src.fedoraproject.org/rpms/qemu/c/f81be8f0261cce74799f946e99f23d57f8db7e17?branch=master

Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Well actually you did most of it Phillip, thanks!
I just tested and reviewed and did a bit patch polishing.

description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

I updated the SRU template a bit to make review easier for the SRU team later on.

description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Thanks you Phillip for your work on this!

Uploaded to Bionic-unapproved for SRU Team review

Revision history for this message
Łukasz Zemczak (sil2100) wrote : Proposed package upload rejected

An upload of qemu to bionic-proposed has been rejected from the upload queue for the following reason: "The SRU itself is good but currently the .changes file comes with 2 bugs that se".

Revision history for this message
Łukasz Zemczak (sil2100) wrote :

Rejection comment got cut: The SRU itself is good but currently the .changes file comes with 2 bugs that seem to be private (LP: #1780773, LP: #1790457). These need to be public for review and SRU tooling.

Revision history for this message
Łukasz Zemczak (sil2100) wrote : Please test proposed package

Hello Phillip, or anyone else affected,

Accepted qemu into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:2.11+dfsg-1ubuntu7.6 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Changed in qemu (Ubuntu Bionic):
status: In Progress → Fix Committed
tags: added: verification-needed verification-needed-bionic
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :

Tested with:
qemu-system-x86_64 -m 1G -vnc :1 -cdrom ubuntu-18.04.1-live-server-amd64.iso

Prior to the update the key expected for | returned only >
With EN-layout enabled and trying all keys I found no pipe symbol.

Update:
$ sudo apt install qemu-block-extra qemu-kvm qemu-system-common qemu-system-x86 qemu-utils
Reading package lists... Done
Building dependency tree
Reading state information... Done
The following package was automatically installed and is no longer required:
  grub-pc-bin
Use 'sudo apt autoremove' to remove it.
Suggested packages:
  samba vde2 ovmf
The following packages will be upgraded:
  qemu-block-extra qemu-kvm qemu-system-common qemu-system-x86 qemu-utils
5 upgraded, 0 newly installed, 0 to remove and 68 not upgraded.
Need to get 6782 kB of archives.
After this operation, 8192 B of additional disk space will be used.
Get:1 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 qemu-utils amd64 1:2.11+dfsg-1ubuntu7.6 [868 kB]
Get:2 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 qemu-system-common amd64 1:2.11+dfsg-1ubuntu7.6 [661 kB]
Get:3 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 qemu-block-extra amd64 1:2.11+dfsg-1ubuntu7.6 [39.0 kB]
Get:4 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 qemu-kvm amd64 1:2.11+dfsg-1ubuntu7.6 [13.2 kB]
Get:5 http://archive.ubuntu.com/ubuntu bionic-proposed/main amd64 qemu-system-x86 amd64 1:2.11+dfsg-1ubuntu7.6 [5201 kB]
Fetched 6782 kB in 1s (7023 kB/s)
(Reading database ... 86373 files and directories currently installed.)
Preparing to unpack .../qemu-utils_1%3a2.11+dfsg-1ubuntu7.6_amd64.deb ...
Unpacking qemu-utils (1:2.11+dfsg-1ubuntu7.6) over (1:2.11+dfsg-1ubuntu7.5) ...
Preparing to unpack .../qemu-system-common_1%3a2.11+dfsg-1ubuntu7.6_amd64.deb ...
Unpacking qemu-system-common (1:2.11+dfsg-1ubuntu7.6) over (1:2.11+dfsg-1ubuntu7.5) ...
Preparing to unpack .../qemu-block-extra_1%3a2.11+dfsg-1ubuntu7.6_amd64.deb ...
Unpacking qemu-block-extra:amd64 (1:2.11+dfsg-1ubuntu7.6) over (1:2.11+dfsg-1ubuntu7.5) ...
Preparing to unpack .../qemu-kvm_1%3a2.11+dfsg-1ubuntu7.6_amd64.deb ...
Unpacking qemu-kvm (1:2.11+dfsg-1ubuntu7.6) over (1:2.11+dfsg-1ubuntu7.5) ...
Preparing to unpack .../qemu-system-x86_1%3a2.11+dfsg-1ubuntu7.6_amd64.deb ...
Unpacking qemu-system-x86 (1:2.11+dfsg-1ubuntu7.6) over (1:2.11+dfsg-1ubuntu7.5) ...
Setting up qemu-block-extra:amd64 (1:2.11+dfsg-1ubuntu7.6) ...
Setting up qemu-utils (1:2.11+dfsg-1ubuntu7.6) ...
Processing triggers for man-db (2.8.3-2) ...
Setting up qemu-system-common (1:2.11+dfsg-1ubuntu7.6) ...
Setting up qemu-system-x86 (1:2.11+dfsg-1ubuntu7.6) ...
Setting up qemu-kvm (1:2.11+dfsg-1ubuntu7.6) ...

After the update:
Able to type the | with EN layout enabled on the place it is expected.
Shift+# on the German keyboard (EN layout applied).

Setting verified.

tags: added: verification-done verification-done-bionic
removed: verification-needed verification-needed-bionic
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qemu - 1:2.11+dfsg-1ubuntu7.6

---------------
qemu (1:2.11+dfsg-1ubuntu7.6) bionic; urgency=medium

  [ Christian Ehrhardt ]
  * Add cpu model for z14 ZR1 (LP: #1780773)
  * d/p/ubuntu/lp-1789551-seccomp-set-the-seccomp-filter-to-all-threads.patch:
    ensure that the seccomp blacklist is applied to all threads (LP: #1789551)
    - CVE-2018-15746
  * improve s390x spectre mitigation with etoken facility (LP: #1790457)
    - debian/patches/ubuntu/lp-1790457-s390x-kvm-add-etoken-facility.patch
    - debian/patches/ubuntu/lp-1790457-partial-s390x-linux-headers-update.patch

  [ Phillip Susi ]
  * d/p/ubuntu/lp-1787267-fix-en_us-vnc-pipe.patch: Fix pipe, greater than and
    less than keys over vnc when using en_us kemaps (LP: #1787267).

 -- Christian Ehrhardt <email address hidden> Wed, 29 Aug 2018 11:46:37 +0200

Changed in qemu (Ubuntu Bionic):
status: Fix Committed → Fix Released
Revision history for this message
Łukasz Zemczak (sil2100) wrote : Update Released

The verification of the Stable Release Update for qemu has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.