Please review pull request #701: Bug/2.7.x/14127 ssh auth keys fails on completely empty line opened by (daniel-pittman)
Description:
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 [email protected]
- Opened: Mon Apr 23 21:55:04 UTC 2012
- Based on: puppetlabs:2.7.x (0ee259f3c26608ba58afe5fbd5bb6f6489c0cefb)
- Requested merge: daniel-pittman:bug/2.7.x/14127-ssh-auth-keys-fails-on-completely-empty-line (b4d1c654320ed9f1e5368879d0edb7aec89e4ff0)
Diff follows:
diff --git a/lib/puppet/provider/ssh_authorized_key/parsed.rb b/lib/puppet/provider/ssh_authorized_key/parsed.rb
index 622c250..bef811d 100644
--- a/lib/puppet/provider/ssh_authorized_key/parsed.rb
+++ b/lib/puppet/provider/ssh_authorized_key/parsed.rb
@@ -1,17 +1,14 @@
require 'puppet/provider/parsedfile'
-
-
- Puppet::Type.type(:ssh_authorized_key).provide(
- :parsed,
- :parent => Puppet::Provider::ParsedFile,
- :filetype => :flat,
-
+Puppet::Type.type(:ssh_authorized_key).provide(
+ :parsed,
+ :parent => Puppet::Provider::ParsedFile,
+ :filetype => :flat,
:default_target => ''
) do
desc "Parse and generate authorized_keys files for SSH."
- text_line :comment, :match => /^#/
- text_line :blank, :match => /^\s+/
+ text_line :comment, :match => /^\s*#/
+ text_line :blank, :match => /^\s*$/
record_line :parsed,
:fields => %w{options type key name},
diff --git a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
index 9d747c0..1b45764 100755
--- a/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
+++ b/spec/unit/provider/ssh_authorized_key/parsed_spec.rb
@@ -63,6 +63,30 @@ def genkey(key)
genkey(key).should == "from=\"192.168.1.1\",no-pty,no-X11-forwarding ssh-rsa AAAAfsfddsjldjgksdflgkjsfdlgkj root@localhost\n"
end
+ it "should parse comments" do
+ result = [{ :record_type => :comment, :line => "# hello" }]
+ @provider_class.parse("# hello\n").should == result
+ end
+
+ it "should parse comments with leading whitespace" do
+ result = [{ :record_type => :comment, :line => " # hello" }]
+ @provider_class.parse(" # hello\n").should == result
+ end
+
+ it "should skip over lines with only whitespace" do
+ result = [{ :record_type => :comment, :line => "#before" },
+ { :record_type => :blank, :line => " " },
+ { :record_type => :comment, :line => "#after" }]
+ @provider_class.parse("#before\n \n#after\n").should == result
+ end
+
+ it "should skip over completely empty lines" do
+ result = [{ :record_type => :comment, :line => "#before"},
+ { :record_type => :blank, :line => ""},
+ { :record_type => :comment, :line => "#after"}]
+ @provider_class.parse("#before\n\n#after\n").should == result
+ end
+
it "should be able to parse name if it includes whitespace" do
@provider_class.parse_line('ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC7pHZ1XRj3tXbFpPFhMGU1bVwz7jr13zt/wuE+pVIJA8GlmHYuYtIxHPfDHlkixdwLachCpSQUL9NbYkkRFRn9m6PZ7125ohE4E4m96QS6SGSQowTiRn4Lzd9LV38g93EMHjPmEkdSq7MY4uJEd6DUYsLvaDYdIgBiLBIWPA3OrQ== fancy user')[:name].should == 'fancy user'
@provider_class.parse_line('from="host1.reductlivelabs.com,host.reductivelabs.com",command="/usr/local/bin/run",ssh-pty ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAAAgQC7pHZ1XRj3tXbFpPFhMGU1bVwz7jr13zt/wuE+pVIJA8GlmHYuYtIxHPfDHlkixdwLachCpSQUL9NbYkkRFRn9m6PZ7125ohE4E4m96QS6SGSQowTiRn4Lzd9LV38g93EMHjPmEkdSq7MY4uJEd6DUYsLvaDYdIgBiLBIWPA3OrQ== fancy user')[:name].should == 'fancy user'
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
