openssh bugs - Sep 2017 - [Bug 2786] New: New OpenSSH fails to parse public keys with bogus whitespace

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

            Bug ID: 2786
           Summary: New OpenSSH fails to parse public keys with bogus
                    whitespace
           Product: Portable OpenSSH
           Version: 7.5p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 3062
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3062&action=edit
do not be strict about whitespace in public keys

OpenSSH prior 6.7 were more benevolent about whitespace around public
key blob. The first of the following keys is parsed properly, but the
second is not:

ssh-rsa AAAAB3NzaC1yc2EAAAAB.... blah at example.com
ssh-rsa  AAAAB3NzaC1yc2EAAAAB.... blah at example.com

This was changed in the commit [1], which moved away from uudecode()
function (which was skipping bogus whitespace) to sshbuf_b64tod()
function (which is already getting zero-length buffer from
sshkey_read(), because strchr(cp, ' ') returns the immediately
following space in the above example).

The documentation is clear that there should be only single whitespace,
but being benevolent about the whitespace is good, especially in case
of handling public keys, which can come from various sources, for
example users pasting them through web interface (github, gitlab, ...).
The attached one-line patch is skipping the whitespace and returns to
the old behavior.

This bug is based on the Red Hat bugzilla [2].

[1] https://github.com/openssh/openssh-portable/commit/8668706d
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1493406

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3062|0                           |1
        is obsolete|                            |
                 CC|                            |djm at mindrot.org
   Attachment #3128|                            |ok?
              Flags|                            |

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Created attachment 3128
  --> https://bugzilla.mindrot.org/attachment.cgi?id=3128&action=edit
refactor sshkey_read, allow more whitespace

IMO sshkey_read() needs a more comprehensive refactor since it's quite
crufty and internally inconsistent, especially in how it allowed tab
characters in some places and not others.

This moves the big switch statement to the start of the function; it
was previously used to select v1/v2 key parsing but now only functions
as a test of key->type. Moving it allows the the whole function to be
outdented.

It switches from strchr to strcspn to find spaces, and now accepts tabs
anywhere where whitespace is allowed.

Finally, it no longer modifies its argument during parsing but I'll
commit the flipping of its argument to const until later to avoid
colliding other in-progress diffs.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net
   Attachment #3128|ok?                         |ok?(dtucker at dtucker.net)
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2782


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2782
[Bug 2782] Tracking bug for OpenSSH 7.7 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
This is committed and will be in 7.7

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
closing resolved bugs as of 8.6p1 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2786

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #3128|ok?(dtucker at dtucker.net)    |
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.