Created attachment 3062 [details] 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@example.com ssh-rsa AAAAB3NzaC1yc2EAAAAB.... blah@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
Created attachment 3128 [details] 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.
This is committed and will be in 7.7
closing resolved bugs as of 8.6p1 release