Bug/2.7.x/14127 ssh auth keys fails on completely empty line #701

Merged
merged 4 commits into from Apr 30, 2012

Conversation

Projects
None yet
3 participants
Contributor

slippycheeze commented Apr 23, 2012

The ssh_authorized_keys grammer, defined for ParsedFile, is wrong about both
comments and blank lines. Specifically, it failed in two key ways:

It failed if a comment didn't start on character one; any whitespace caused
the comment to be ignored, which is absolutely incorrect by the official
OpenSSH implementation.

It also failed because a "blank" line was defined as a line that contained one
or more whitespace characters at the start - and nothing more!

This failed in a bunch of ways, starting with assuming that a comment on a
line with leading whitespace was actually a blank line.

This change fixes both of those bugs, as well as adding appropriate tests.

(See rsa_key_allowed_in_file in auth-rsa.c for the parser, at least in the
current as of this commit version of OpenSSH. That is official enough for me
to assume that everyone will behave that way ;)

Signed-off-by: Daniel Pittman daniel@puppetlabs.com

slippycheeze and others added some commits Apr 23, 2012

@slippycheeze slippycheeze (#14127) ssh_authorized_keys grammer fails on blank lines.
The `ssh_authorized_keys` grammer, defined for ParsedFile, is wrong about both
comments and blank lines.  Specifically, it failed in two key ways:

It failed if a comment didn't start on character one; any whitespace caused
the comment to be ignored, which is absolutely incorrect by the official
OpenSSH implementation.

It also failed because a "blank" line was defined as a line that contained one
or more whitespace characters at the start - and nothing more!

This failed in a bunch of ways, starting with assuming that a comment on a
line with leading whitespace was actually a blank line.

This change fixes both of those bugs, as well as adding appropriate tests.

(See `rsa_key_allowed_in_file` in `auth-rsa.c` for the parser, at least in the
 current as of this commit version of OpenSSH.  That is official enough for me
 to assume that everyone will behave that way ;)

Signed-off-by: Daniel Pittman <daniel@puppetlabs.com>
b4d1c65
@stschulte stschulte maint: refactor integration specs for ssh_authorized_key
Replace all before blocks and instance variables with rspec let methods
and move the stubbing of File.chmod that happened in all tests to the
top to reduce code duplication.
85ee441
@stschulte stschulte (#14127) Add integration tests for ssh_authorized_key
Test the behaviour of the ssh_authorized key type when modifying a file
with different kind of text lines (inline comment and blank lines)
402a425
Member

stschulte commented Apr 23, 2012

@daniel-pittman I extended the integration tests for ssh_authorized_key to test the behaviour. I've opened a pull request to merge it into your branch if you want to include it.

pcarlisle merged commit ac7bd6b into puppetlabs:2.7.x Apr 30, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment