Ensure that ListenAddress follows AddressFamily (#34)

This appears to be undocumented, but ListenAddress must follow
AddressFamily when specified in the sshd_config file.

Closes #33

.gitignore

@@ -14,6 +14,7 @@ yardoc/
 .rvmrc*
 .rbenv*
 .ruby-*
+Gemfile.lock
 
 ## MAC OS
 .DS_Store

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## 2.5.1
+
+- Bugfix Release:
+  - Allow multiple values for GlobalKnownHostsFile (#GH 32)
+  - Ensure that AddressFamily comes before ListenAddress (#GH 34)
+
 ## 2.5.0
 
 - Implement instances for sshkey (only for non-hashed entries)

lib/puppet/provider/sshd_config/augeas.rb

@@ -165,10 +165,10 @@ def self.match_conditions(resource=nil)
 
   def self.match_exists?(aug, resource)
     cond_str = resource[:condition] ? self.match_conditions(resource) : ''
-    not aug.match("$target/Match#{cond_str}").empty?
+    !aug.match("$target/Match#{cond_str}").empty?
   end
 
-  def create 
+  def create
     base_path = self.class.base_path(resource)
     augopen! do |aug|
       key = resource[:key] ? resource[:key] : resource[:name]
@@ -178,9 +178,18 @@ def create
           aug.set("$target/Match[last()]/Condition/#{k}", v)
         end
       end
-      if key.downcase == 'port' and not aug.match("#{base_path}/ListenAddress").empty?
+      if key.downcase == 'port' && !aug.match("#{base_path}/ListenAddress").empty?
         aug.insert("#{base_path}/ListenAddress[1]", key, true)
       end
+
+     if key.downcase == 'listenaddress' && !aug.match("#{base_path}/AddressFamily").empty?
+       aug.insert("#{base_path}/AddressFamily", key, false)
+     end
+
+     if key.downcase == 'addressfamily' && !aug.match("#{base_path}/ListenAddress").empty?
+       aug.insert("#{base_path}/ListenAddress", key, true)
+     end
+
       self.class.set_value(aug, base_path, "#{base_path}/#{key}", key, resource[:value])
     end
   end

metadata.json

@@ -1,6 +1,6 @@
 {
   "name": "herculesteam-augeasproviders_ssh",
-  "version": "2.5.0",
+  "version": "2.5.1",
   "author": "Dominic Cleal, Raphael Pinson",
   "summary": "Augeas-based ssh types and providers for Puppet",
   "license": "Apache-2.0",

spec/fixtures/unit/puppet/provider/sshd_config/augeas/addressfamily_test

@@ -0,0 +1,144 @@
+#	$OpenBSD: sshd_config,v 1.81 2009/10/08 14:03:41 markus Exp $
+
+# This is the sshd server system-wide configuration file.  See
+# sshd_config(5) for more information.
+
+# This sshd was compiled with PATH=/usr/local/bin:/bin:/usr/bin
+
+# The strategy used for options in the default sshd_config shipped with
+# OpenSSH is to specify options with their default value where
+# possible, but leave them commented.  Uncommented options change a
+# default value.
+
+#ListenAddress 0.0.0.0
+#ListenAddress ::
+
+#AddressFamily any
+
+# The default requires explicit activation of protocol 1
+#Protocol 2
+
+# HostKey for protocol version 1
+#HostKey /etc/ssh/ssh_host_key
+# HostKeys for protocol version 2
+#HostKey /etc/ssh/ssh_host_rsa_key
+#HostKey /etc/ssh/ssh_host_dsa_key
+
+# Lifetime and size of ephemeral version 1 server key
+#KeyRegenerationInterval 1h
+#ServerKeyBits 1024
+
+# Logging
+# obsoletes QuietMode and FascistLogging
+#SyslogFacility AUTH
+SyslogFacility AUTHPRIV
+#LogLevel INFO
+
+# Authentication:
+
+AllowGroups sshusers admins
+
+#LoginGraceTime 2m
+PermitRootLogin without-password
+#StrictModes yes
+#MaxAuthTries 6
+#MaxSessions 10
+
+#RSAAuthentication yes
+#PubkeyAuthentication yes
+#AuthorizedKeysFile	.ssh/authorized_keys
+#AuthorizedKeysCommand none
+#AuthorizedKeysCommandRunAs nobody
+
+# For this to work you will also need host keys in /etc/ssh/ssh_known_hosts
+#RhostsRSAAuthentication no
+# similar for protocol version 2
+#HostbasedAuthentication no
+# Change to yes if you don't trust ~/.ssh/known_hosts for
+# RhostsRSAAuthentication and HostbasedAuthentication
+#IgnoreUserKnownHosts no
+# Don't read the user's ~/.rhosts and ~/.shosts files
+#IgnoreRhosts yes
+
+# To disable tunneled clear text passwords, change to no here!
+#PasswordAuthentication yes
+#PermitEmptyPasswords no
+PasswordAuthentication yes
+
+# Change to no to disable s/key passwords
+#ChallengeResponseAuthentication yes
+ChallengeResponseAuthentication no
+
+# Kerberos options
+#KerberosAuthentication no
+#KerberosOrLocalPasswd yes
+#KerberosTicketCleanup yes
+#KerberosGetAFSToken no
+#KerberosUseKuserok yes
+
+# GSSAPI options
+#GSSAPIAuthentication no
+GSSAPIAuthentication yes
+#GSSAPICleanupCredentials yes
+GSSAPICleanupCredentials yes
+#GSSAPIStrictAcceptorCheck yes
+#GSSAPIKeyExchange no
+
+# Set this to 'yes' to enable PAM authentication, account processing, 
+# and session processing. If this is enabled, PAM authentication will 
+# be allowed through the ChallengeResponseAuthentication and
+# PasswordAuthentication.  Depending on your PAM configuration,
+# PAM authentication via ChallengeResponseAuthentication may bypass
+# the setting of "PermitRootLogin without-password".
+# If you just want the PAM account and session checks to run without
+# PAM authentication, then enable this but set PasswordAuthentication
+# and ChallengeResponseAuthentication to 'no'.
+#UsePAM no
+UsePAM yes
+
+# Accept locale-related environment variables
+AcceptEnv LANG LC_CTYPE LC_NUMERIC LC_TIME LC_COLLATE LC_MONETARY LC_MESSAGES
+AcceptEnv LC_PAPER LC_NAME LC_ADDRESS LC_TELEPHONE LC_MEASUREMENT
+AcceptEnv LC_IDENTIFICATION LC_ALL LANGUAGE
+AcceptEnv XMODIFIERS
+
+#AllowAgentForwarding yes
+#AllowTcpForwarding yes
+#GatewayPorts no
+#X11Forwarding no
+X11Forwarding yes
+#X11DisplayOffset 10
+#X11UseLocalhost yes
+#PrintMotd yes
+#PrintLastLog yes
+#TCPKeepAlive yes
+#UseLogin no
+#UsePrivilegeSeparation yes
+#PermitUserEnvironment no
+#Compression delayed
+#ClientAliveInterval 0
+#ClientAliveCountMax 3
+#ShowPatchLevel no
+#UseDNS yes
+#PidFile /var/run/sshd.pid
+#MaxStartups 10
+#PermitTunnel no
+#ChrootDirectory none
+
+# no default banner path
+#Banner none
+
+# override default of no subsystems
+Subsystem	sftp	/usr/libexec/openssh/sftp-server
+
+AddressFamily any
+
+# Example of overriding settings on a per-user basis
+Match User anoncvs
+	X11Forwarding no
+	AllowTcpForwarding no
+	ForceCommand cvs server
+
+Match Host *.example.net User *
+    AllowAgentForwarding no
+    ListenAddress 10.1.1.2

spec/unit/puppet/provider/sshd_config/augeas_spec.rb

@@ -59,7 +59,7 @@
 
     context "when declaring two resources with same key" do
       it "should fail with same name" do
-        expect do 
+        expect do
           apply!(
             Puppet::Type.type(:sshd_config).new(
               :name      => "X11Forwarding",
@@ -79,7 +79,7 @@
       end
 
       it "should fail with different names, same key and no conditions" do
-        expect do 
+        expect do
           apply!(
             Puppet::Type.type(:sshd_config).new(
               :name      => "X11Forwarding",
@@ -99,7 +99,7 @@
       end
 
       it "should not fail with different names, same key and different conditions" do
-        expect do 
+        expect do
           apply!(
             Puppet::Type.type(:sshd_config).new(
               :name      => "X11Forwarding",
@@ -170,7 +170,7 @@
           :target    => target,
           :provider  => "augeas"
         ))
-  
+
         aug_open(target, "Sshd.lns") do |aug|
           expect(aug.match("Match[2]/Settings/ListenAddress[preceding-sibling::Port]").size).to eq(1)
         end
@@ -226,7 +226,7 @@
           :target   => target,
           :provider => "augeas"
         ))
-  
+
         aug_open(target, "Sshd.lns") do |aug|
           expect(aug.get("AllowUsers/1")).to eq("ssh")
           expect(aug.get("AllowUsers/2")).to eq("foo")
@@ -386,13 +386,13 @@
           :target   => target,
           :provider => "augeas"
         ))
-  
+
         aug_open(target, "Sshd.lns") do |aug|
           expect(aug.match("*[label()=~regexp('PasswordAuthentication', 'i')]").size).to eq(1)
           expect(aug.get("PasswordAuthentication")).to eq("no")
         end
       end
-  
+
       it "should not replace settings case insensitively when on Augeas < 1.0.0" do
         provider_class.stubs(:supported?).with(:post_resource_eval)
         provider_class.stubs(:supported?).with(:regexpi).returns(false)
@@ -402,7 +402,7 @@
           :target   => target,
           :provider => "augeas"
         ))
-  
+
         aug_open(target, "Sshd.lns") do |aug|
           expect(aug.match("GSSAPIAuthentication").size).to eq(1)
           expect(aug.match("GSSAPIauthentIcAtion").size).to eq(1)
@@ -544,4 +544,55 @@
       expect(@logs.first.message.include?(target)).to eq(true)
     end
   end
+
+  ['AddressFamily', 'ListenAddress'].each do |key|
+    context "when adding AddressFamily" do
+      values = {
+        'AddressFamily' => 'any',
+        'ListenAddress' => '1.2.3.4'
+      }
+
+      let(:tmptarget) { aug_fixture("addressfamily_test") }
+      let(:target) { tmptarget.path }
+      let(:key) { key }
+      let(:value) { values[key] }
+
+      it "should ensure that AddressFamily comes before ListenAddress" do
+        provider_class.stubs(:target).returns(target)
+
+        def create_file_hash(provider_stub)
+
+          fh = Hash.new
+
+          provider_stub.instances.each { |p|
+            condition = p.get(:condition)
+            condition = 'Global' if condition == :absent
+
+            fh[condition] ||= []
+            fh[condition] << p.get(:name)
+          }
+
+          return fh
+        end
+
+        apply!(Puppet::Type.type(:sshd_config).new(
+          :name     => key,
+          :value    => value,
+          :target   => target,
+          :provider => "augeas"
+        ))
+
+        file_hash = create_file_hash(provider_class)
+
+        file_hash.keys.each do |section|
+          # Make sure our test input is correctly broken
+          af_index = file_hash[section].index('AddressFamily')
+          la_index = file_hash[section].index('ListenAddress')
+          if af_index && la_index
+            expect(af_index).to be < la_index
+          end
+        end
+      end
+    end
+  end
 end