On Dec 14, 2009, at 9:58 AM, Markus Roberts wrote:

>>> This removes some of the IPv4 centricism from authstore's handling
>>> of IP addresses.  It isn't full IPv6 support (and doesn't even fully
>>> handle all the cases within its limited scope, as ruby's IPAddr
>>> library does not work with hybrid addresses), but it should simplify
>>> adding IPv6 support when the time comes.
>>
>> Are you sure about that?
>
> Yes.
>
>> embedded = IPAddr.new("::192.168.0.1")
>> => #<IPAddr: IPv6:0000:0000:0000:0000:0000:0000:c0a8:0001/ 
>> ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff>
>>
>> norm = IPAddr.new("192.168.0.1")
>> => #<IPAddr: IPv4:192.168.0.1/255.255.255.255>
>>
>> norm.ipv4_mapped()
>> => #<IPAddr:  
>> IPv6 
>> : 
>> 0000 
>> : 
>> 0000 
>> : 
>> 0000 
>> :0000:0000:ffff:c0a8:0001/0000:0000:0000:0000:0000:0000:ffff:ffff>
>>
>> Now there are mask issues there which mean comparison isn't true in
>> this case
>
> The comparison being wrong is enough to say that it doesn't work,  
> IMHO.
>
>> What explicit cases does ipaddr not work for?
>
> For example:
>
> IPAddr.new("0:0:0:0:0:FFFF:129.144.52.38")
> ArgumentError: invalid address
>       from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ 
> ruby/1.8/ipaddr.rb:464:in
> `in6_addr'
>       from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ 
> ruby/1.8/ipaddr.rb:432:in
> `initialize'
>       from (irb):54:in `new'
>       from (irb):54
>       from :0
>
>> but adding internal regex based ipv6 parsing feels wrong.
>
> The internal validity checking has nothing to do with the IPv6 stuff,
> it's the core of the bugfix, which was necessitated because passing
> things that aren't valid IP addresses to IPAddr.new() can cause a
> variety of undesired (and possibly time consuming) behaviors.  I
> believe it may even be possible (though I have not tried this, and
> don't think it's a serious concern in practice) to craft a DoS string
> which would take down a puppetmaster.  Just like with SQL, etc.
> passing external data through to library routines without sanity
> checking it is a bad idea.

The short answer here is that we're often seeing 10x worse performance  
on tests because IPAddr is doing a DNS reverse lookup when it  
shouldn't, and this fixes that.

Any attempt to use regexes to match all cases is somewhat bound for  
failure, but this is better than nothing, IMO.

-- 
It is absurd to divide people into good and bad. People are either
charming or tedious. -- Oscar Wilde
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

--

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.


Reply via email to