From 61c6da32244fcf12557676e5b32d99f52bdcbc61 Mon Sep 17 00:00:00 2001 From: Trevor Vaughan Date: Thu, 11 May 2017 13:32:33 -0400 Subject: [PATCH] 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 | 1 + CHANGELOG.md | 6 + lib/puppet/provider/sshd_config/augeas.rb | 15 +- metadata.json | 2 +- .../sshd_config/augeas/addressfamily_test | 144 ++++++++++++++++++ .../provider/sshd_config/augeas_spec.rb | 67 +++++++- 6 files changed, 223 insertions(+), 12 deletions(-) create mode 100644 spec/fixtures/unit/puppet/provider/sshd_config/augeas/addressfamily_test diff --git a/.gitignore b/.gitignore index 94145ab..1cce141 100644 --- a/.gitignore +++ b/.gitignore @@ -14,6 +14,7 @@ yardoc/ .rvmrc* .rbenv* .ruby-* +Gemfile.lock ## MAC OS .DS_Store diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b9c2a..a4135ab 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/puppet/provider/sshd_config/augeas.rb b/lib/puppet/provider/sshd_config/augeas.rb index fb418b0..dd4951c 100644 --- a/lib/puppet/provider/sshd_config/augeas.rb +++ b/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 diff --git a/metadata.json b/metadata.json index 9256356..4e2251e 100644 --- a/metadata.json +++ b/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", diff --git a/spec/fixtures/unit/puppet/provider/sshd_config/augeas/addressfamily_test b/spec/fixtures/unit/puppet/provider/sshd_config/augeas/addressfamily_test new file mode 100644 index 0000000..67b74da --- /dev/null +++ b/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 diff --git a/spec/unit/puppet/provider/sshd_config/augeas_spec.rb b/spec/unit/puppet/provider/sshd_config/augeas_spec.rb index eae54e3..db7f01a 100755 --- a/spec/unit/puppet/provider/sshd_config/augeas_spec.rb +++ b/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