Feature #9415

use IPv4 netmasks in allow statements in config files

Added by Jim Pirzyk 9 months ago. Updated 7 months ago.

Status:Tests Insufficient Start date:09/10/2011
Priority:Normal Due date:
Assignee:Jim Pirzyk % Done:

0%
Category:plumbing
Target version:-
Affected Puppet version:2.7.3 Branch:
Keywords:
Votes: 0

Description

If I use the following template for the namespaceauth.conf file (extends to other config files too)

[fileserver]

allow *.<%= domain %>
allow <%= network_eth0 %>/<%= netmask_eth0 %>
allow 127.0.0.1

I get the following error:

Invalid pattern 192.168.1.0/255.255.255.0

In lieu of having a netmasklen_eth0 fact, I have supplied a patch to enable this syntax.

authstore.rb.patch - Patch to allow network/netmask format (819 Bytes) Jim Pirzyk, 09/10/2011 09:38 am

authstore_spec.rb.patch - authstore_spec.rb additional tests (1.1 kB) Jim Pirzyk, 09/11/2011 10:25 am

authstore.rb.patch (944 Bytes) Jim Pirzyk, 09/11/2011 03:26 pm

authstore_spec.rb.patch (1.9 kB) Jim Pirzyk, 09/11/2011 05:32 pm

Related issues

blocked by Puppet - Bug #9638: additional test cases for authstore allow entries In Topic Branch Pending Review 09/21/2011

History

#1
Updated by James Turnbull 9 months ago

  • Category set to plumbing
  • Status changed from Unreviewed to Tests Insufficient
  • Assignee set to Jim Pirzyk
  • Affected Puppet version set to 2.7.3

Jim – thanks for the patch! I think this needs some tests though for completeness.

#2
Updated by James Turnbull 9 months ago

The spec tests for authstore are here:

https://github.com/puppetlabs/puppet/blob/master/spec/unit/network/authstore_spec.rb

You should be able to replicate some of the existing ones for your change.

#3
Updated by Jim Pirzyk 8 months ago

I’ve attempted to write some additional tests for this new feature. Not exactly sure how the base system will handle IP addresses of 192.168.0.255 and 192.168.2.0 (broadcast and network addresses), but did put some extra tests for the first and last valid/invalid real IP addresses too.

#4
Updated by James Turnbull 8 months ago

I got four failed tests:

Failures:

  1) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.254
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.254", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  2) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.255
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.255", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  3) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.0
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.0", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  4) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.1
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.1", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

#5
Updated by Jim Pirzyk 8 months ago

Take 2 on the original patch

#6
Updated by James Turnbull 8 months ago

Still got test failures:

Failures:

  1) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.254
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.254", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  2) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.255
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.255", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  3) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.0
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.0", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

  4) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.1
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.1", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:85

#7
Updated by Jim Pirzyk 8 months ago

Update the authstore_spec file to test the CIDR format. Should come back with the same results as the newer /255.255.255.0 code.

#8
Updated by James Turnbull 8 months ago

Sadly not:

  1) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.254
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.254", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:97

  2) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.255
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.255", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:97

  3) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.0
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.0", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:97

  4) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.1
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.1", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:97

  5) Puppet::Network::AuthStore when checking a IP CIDR type of allow should not allow the IPv4 192.168.0.254
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.254", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:114

  6) Puppet::Network::AuthStore when checking a IP CIDR type of allow should not allow the IPv4 192.168.0.255
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.255", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:114

  7) Puppet::Network::AuthStore when checking a IP CIDR type of allow should not allow the IPv4 192.168.2.0
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.0", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:114

  8) Puppet::Network::AuthStore when checking a IP CIDR type of allow should not allow the IPv4 192.168.2.1
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.1", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:114

  9) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.254
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.254", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:131

  10) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.0.255
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.0.255", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:131

  11) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.0
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.0", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:131

  12) Puppet::Network::AuthStore when checking a IPv4 subnet type of allow should not allow the IPv4 192.168.2.1
     Failure/Error: @authstore.should_not be_allowed(ip, @serverip)
       expected allowed?("192.168.2.1", "192.168.1.1") to return false, got true
     # ./authstore_spec.rb:131

#9
Updated by James Turnbull 8 months ago

Maybe worth taking the test code and sending to the puppet-dev list?

#10
Updated by Jim Pirzyk 8 months ago

So the last update was a bit of a red herring. Seems that I might have found a bug outside of my change. My change is completely contained within Puppet::Network::AuthStore::parse()

Currently the parse() method returns the following:

parse(‘192.168.1.0/24’) => [ ‘ip’, ‘inexact’, 24, 192.168.1.0 ]

parse(‘192.168.1.0/255.255.255.0’) => [ ‘ip’, ‘inexact’, 24, 192.168.1.0 ]

So both cases are returning the same values (as they should be). If you take my changes out you get the following:

parse(‘192.168.1.0/24’) => [ ‘ip’, ‘inexact’, 24, 192.168.1.0 ]

parse(‘192.168.1.0/255.255.255.0’) => “Invalid pattern 192.168.1.0/255.255.255.0”

So we see my patch is trying to imitate existing behavior. Just for reference my initial patch actually returned the following:

[ ‘ip’, ‘inexact’, 192, 192.168.1.0 ]

Which is clearly wrong. Seems to me what ever code that is using the results of parse() is not quite correct in handling the resultant output.

My thought is we should open another bug that the ‘allow 192.168.1.0/24’ is not correct, pend this issue on that issue and have that worked on. Not sure I can find the problem but I can take a stab at it.

Thoughts?

#11
Updated by James Turnbull 8 months ago

  • Status changed from Tests Insufficient to Investigating

Sounds like a plan!

#12
Updated by Jim Pirzyk 8 months ago

Pending on Bug #9638

#13
Updated by James Turnbull 7 months ago

  • Status changed from Investigating to Tests Insufficient

Also available in: Atom PDF