Leaving my draft here both so you can read it and so I don't lose it:

> Indeed, the reason for this is that in authd we are presenting a
qrcode to perform weblogin and that doesn't work.

This seems a reasonable justification for an SRU in principle then, now
that it's documented. Thank you for the update.

The remaining questions are whether this way of fixing it is the best
one for the SRU. And even though I agree the SRU is justified in
principle, we should remain open to the possibility that the risk
associated with this particular change outweighs the benefits. Given the
security sensitive area being touched I'd like a firm +1 from the
security team please, that considers both the code changes and the risk.

Seth> Are ascii-encoded qr codes known to be insufficient?

I think this is a good question but leave it to the desktop team to
decide.

> The test case for it, in a such complex setup isn't as easy to test as
the minimal reproducer that it's enough to cover the fix and to ensure
there are no regressions in the normal behavior.

I think that's fine for the usual requirement that the test case be
reproducible by anybody and it looks like a good initial proxy test for
the issue being resolved. However, you're requesting this SRU for a
particular purpose, and it's an invasive change. I'd like you to verify
that your actual purpose is also fulfilled end-to-end during SRU
verification please, to ensure that before we publish to -updates that
further tweaks won't be required and there will be no reason to abandon
the change. Otherwise we would have burdened users with a download,
update and regression risk for no reason. Please add a verification that
your final purpose actually works to the Test Plan. If changes to other
packages are also needed for that, then they should be tested in
-proposed all together.

> PAM info messsages are exposed as generic SSH messages, they're the
only ones of this kind AFAIK, but I wanted to be generic enough so that
normal ssh usage should be checked too.

> PAM it is, but keyboard interactive authentication (which is the only
case that triggers these messages) is not, so the change doesn't affect
PAM behavior of ssh per se, only if the server has enabled keyboard
interactive authentication, which PAM modules can trigger.

OK, but there are cases where we recommend doing that, such as for
HOTP/TOTP configuration: https://ubuntu.com/server/docs/openssh-server.
Maybe it's worth adding to the Test Plan to ensure that those steps
still work?

> So banners and other messages should be checked for regressions.

Please could you add steps on how you specifically intend to do this to
the Test Plan?

I asked around the SRU team for a second opinion, and I got back some
additional concerns:

0) This should probably have received a review from the security team
before uploading to the development release.

1) If it filtered out control characters and restricted itself to UTF-8
printable that would be less scary. Could you consider that, please?

2) What happens if the client and server encodings do not agree? This
isn't entirely theoretical. I'm told that Japan has a national encoding
standard that is not Unicode which is still in widespread use and it may
be the case that this is still what we use on Ubuntu when installing in
Japanese.

-- 
You received this bug notification because you are a member of Ubuntu
Touch seeded packages, which is subscribed to openssh in Ubuntu.
https://bugs.launchpad.net/bugs/2077576

Title:
  SSH client doesn't handle properly non-ASCII chars

Status in openssh package in Ubuntu:
  Fix Released
Status in openssh source package in Focal:
  Incomplete
Status in openssh source package in Jammy:
  Incomplete
Status in openssh source package in Noble:
  Fix Released

Bug description:
  [ Impact ]

  Non-ascii visible chars (including back-slashes, new lines and so) are
  not properly rendered by clients, showing their octal visualization.

  Such as:

    Hello SSHD \\ We love \360\237\215\225!

  Instead of:

    Hello SSHD \ We love 🍕!

  This is particularly an issue when a server has configured keyboard
  interactive authentication and a PAM module wants to show non-ASCII
  characters such as a QR code for web authentication:

  When using an ubuntu server running authd for web authentication we
  may end up having the login qrcode rendered such as

  
\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210\342\226\210
                          https://ubuntu.com
                                 1337

  Which is clearly unreadable.

  [ Test case ]

  ## Server preparation

  Enable PAM and keyboard interactive authentication in a ssh server:

  Add a configuration file such as:
   /etc/ssh/sshd_config.d/test-ssh-pam.conf

  Containing:

  UsePAM yes
  KbdInteractiveAuthentication yes
  # This was working already; here to check potential regressions
  ForceCommand bash -c "echo Hello from SSHD \ We also love 🍕!; $SHELL"

  It's also suggested to check for regressions using a `Banner` option
  in sshd, pointing to a file with utf-8 contents.

  Restart the server:

    sudo systemctl restart ssh.service

  Edit the sshd PAM configuration file, adding as first line:

    auth    requisite pam_echo.so Hello SSHD \ We love 🍕!

  Can be done with the command:
    sudo sed '1 iauth    requisite pam_echo.so Hello SSHD! \\ We love 🍕!' \
     -i /etc/pam.d/sshd

  ## Client test

  In the same host:

   ssh -o PubkeyAuthentication=no \
       -o PasswordAuthentication=no \
       -o PreferredAuthentications=keyboard-interactive \
       $USER@localhost

  The client should show:

  Hello SSHD \ We love 🍕!
  ($USER@localhost) Password:
  ...
  Hello from SSHD \ We also love 🍕!

  Retry the same with another host and without keyboard authentication
  enabled in the server side.

  To verify the fix in more complex scenario it's possible to follow the 
instructions of configuring authd:
   - https://github.com/ubuntu/authd/wiki/05--How%E2%80%90to-log-in-over-SSH

  Once authd is configured, the user should be able to scan a QrCode
  from a ssh session.

  ## Cleanup

  Revert the changes done in the cleanup phase, after test is done

  sudo sed '/pam_echo\.so/d' -i /etc/pam.d/sshd
  sudo rm /etc/ssh/sshd_config.d/test-ssh-pam.conf

  [ Regression potential ]

  SSH info messages are not shown by the client. Even though those
  aren't covered by this change, it's important to check for regressions
  in any output that SSH exposes to the user. So banners and other
  messages should be checked for regressions.

  These kind of messages are normally shown only when PAM *and* keyboard
  interaction are enabled in the server side, so it should not affect
  the default ubuntu servers behavior.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/openssh/+bug/2077576/+subscriptions


-- 
Mailing list: https://launchpad.net/~touch-packages
Post to     : touch-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~touch-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to