sshd_config: fix adding multiple entries when no Match block is prese…

…nt, as the

set_value method takes a different path.

Fix deleting of excess entries when reducing the number of values.

Positioning has been improved so that new entries for the same setting name
will be added immediately after existing ones.

lib/puppet/provider/sshd_config/augeas.rb

@@ -29,7 +29,7 @@ def self.path_label(path)
 
   def self.get_value(aug, pathx)
     aug.match(pathx).map do |vp|
-      # Bug in Augeas lens, counter isn't reset to 1 so check for any int
+      # Augeas lens does transparent multi-node (no counte reset) so check for any int
       if aug.match("#{vp}/*[label()=~regexp('[0-9]*')]").empty?
         aug.get(vp)
       else
@@ -40,7 +40,7 @@ def self.get_value(aug, pathx)
     end.flatten
   end
 
-  def self.set_value(aug, path, value)
+  def self.set_value(aug, base, path, value)
     if path =~ /.*\/(((Allow|Deny)(Groups|Users))|AcceptEnv|MACs)(\[\d\*\])?/
       # Make sure only our values are used
       aug.rm("#{path}/*")
@@ -56,16 +56,33 @@ def self.set_value(aug, path, value)
       value = value.clone
 
       # Change any existing settings with this name
+      lastsp = nil
       aug.match(path).each do |sp|
-        aug.set(sp, value.shift)
+        val = value.shift
+        if val.nil?
+          aug.rm(sp)
+        else
+          aug.set(sp, val)
+          lastsp = sp
+        end
       end
 
       # Insert new values for the rest
       value.each do |v|
-        unless aug.match("#{path}/../Match").empty?
-          aug.insert("#{path}/../Match[1]", path_label(path), true)
+        if lastsp
+          # After the most recent same setting (lastsp)
+          aug.insert(lastsp, path_label(path), false)
+          aug.set("#{path}[last()]", v)
+        else
+          if aug.match("#{base}/Match").empty?
+            aug.set("#{path}[last()+1]", v)
+          else
+            # before the match block so it's in the main section
+            aug.insert("#{base}/Match[1]", path_label(path), true)
+            aug.set("#{path}[last()]", v)
+          end
         end
-        aug.set("#{path}[last()]", v)
+        lastsp = aug.match("#{path}[last()]")[0]
       end
     end
   end
@@ -131,12 +148,12 @@ def self.match_conditions(resource=nil)
   def self.entry_path(resource)
     path = "/files#{self.file(resource)}"
     key = resource[:key] ? resource[:key] : resource[:name]
-    if resource[:condition]
-      cond_str = self.match_conditions(resource)
-      "#{path}/Match#{cond_str}/Settings/#{key}"
+    base = if resource[:condition]
+      "#{path}/Match#{self.match_conditions(resource)}/Settings"
     else
-      "#{path}/#{key}"
+      path
     end
+    { :base => base, :path => "#{base}/#{key}" }
   end
 
   def self.match_exists?(resource)
@@ -157,7 +174,7 @@ def self.match_exists?(resource)
 
   def exists? 
     aug = nil
-    entry_path = self.class.entry_path(resource)
+    entry_path = self.class.entry_path(resource)[:path]
     begin
       aug = self.class.augopen(resource)
       not aug.match(entry_path).empty?
@@ -189,12 +206,8 @@ def create
         unless self.class.match_exists?(resource)
           aug = self.class.create_match(resource, aug)
         end
-      else
-        unless aug.match("#{path}/Match").empty?
-          aug.insert("#{path}/Match[1]", key, true)
-        end
       end
-      self.class.set_value(aug, entry_path, resource[:value])
+      self.class.set_value(aug, entry_path[:base], entry_path[:path], resource[:value])
       augsave!(aug)
     ensure
       aug.close if aug
@@ -206,7 +219,7 @@ def destroy
     path = "/files#{self.class.file(resource)}"
     begin
       aug = self.class.augopen(resource)
-      entry_path = self.class.entry_path(resource)
+      entry_path = self.class.entry_path(resource)[:path]
       aug.rm(entry_path)
       aug.rm("#{path}/Match[count(Settings/*)=0]")
       augsave!(aug)
@@ -224,7 +237,7 @@ def value
     path = "/files#{self.class.file(resource)}"
     begin
       aug = self.class.augopen(resource)
-      entry_path = self.class.entry_path(resource)
+      entry_path = self.class.entry_path(resource)[:path]
       self.class.get_value(aug, entry_path)
     ensure
       aug.close if aug
@@ -237,7 +250,7 @@ def value=(value)
     begin
       aug = self.class.augopen(resource)
       entry_path = self.class.entry_path(resource)
-      self.class.set_value(aug, entry_path, value)
+      self.class.set_value(aug, entry_path[:base], entry_path[:path], value)
       augsave!(aug)
     ensure
       aug.close if aug

spec/fixtures/unit/puppet/sshd_config/nomatch

@@ -0,0 +1,132 @@
+#	$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.
+
+#Port 22
+#AddressFamily any
+ListenAddress 0.0.0.0
+ListenAddress ::
+
+# 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