860151 - When configuring sslbase with an IPv6 only DNS label, bugzilla fails to resolve the hostname

Status: NEW

(Bugzilla :: Administration, task)

Description

[Kyle Brantley]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

Installed bugzilla on Fedora 18 (bugzilla-4.2.5-1.fc18.noarch). Went to configure the urlbase and sslbase parameters with a IPv6 only DNS hostname (AAAA, no A records).


Actual results:

Bugzilla validated urlbase without issue. However, it is refusing to validate sslbase, instead insisting that the host could not be resolved. 


Expected results:

Bugzilla should have been able to identify that the host was valid and proceed.

Comment 1

[Kyle Brantley]

Bugzilla/Config/Util.pm:122:

    my $iaddr = inet_aton($host) || return "The host $host cannot be resolved";

inet_aton will only resolve an IPv4 address by design (http://perldoc.perl.org/Socket.html#%24ip_address-%3D-inet_aton-%24string).

Comment 2

[Kyle Brantley]

After tweaking data/params (thanks, Simon!) to manually define sslbase, I'm able to see the value in editparams.cgi?section=core#sslbase_desc and configure require_ssl with no ill effects. Everything appears to be working.

Comment 3

[Frédéric Buclin]

Not a blocker, but something we will have to fix sooner or later as IPV6 addresses will become more frequent.

Comment 4

[Simon Green]

Attachment: v1 patch

This is not my preferred solution, but is the best one I can think of given that upstream only mandate a minimum of 5.10.1. Not sure why, since that version is well can truly EOF from upstream Perl.

Anyway, this version does away with actually checking that the connection port is actually available.

Regards,
Hugo

Comment 5

[Simon Green]

Should probably also mention that I haven't tested this on a Windows installation. Someone should in case Net::DNS has issues in Windows. Net::DNS should be available in any half decent Linux operating system (including Ubuntu and CentOS at least)

Regards,
Hugo

Comment 6

[Frédéric Buclin]

(In reply to Hugo Seabrook from comment #4)
> that upstream only mandate a minimum of 5.10.1. Not sure why, since that
> version is well can truly EOF from upstream Perl.

Perl 5.10.1 is still used in several Linux distros, including RHEL6, and so we cannot require a newer version yet. Why does it matter for your patch?

Comment 7

[Simon Green]

(In reply to Frédéric Buclin from comment #6)
> Perl 5.10.1 is still used in several Linux distros, including RHEL6, and so
> we cannot require a newer version yet.

Fair enough.

> Why does it matter for your patch?

If we were using 5.14 or later, I would have used inet_pton (which is built in to perl's Socket module) rather than Net::DNS.

Regards,
Hugo

Comment 8

[Frédéric Buclin]

Comment on attachment 771950 [details] [diff] [review]
v1 patch

>=== modified file 'Bugzilla/Config/Common.pm'

>-        if ($url !~ m#^https://([^/]+).*/$#) {
>+        if ($url !~ m#^https://([^:/]+).*/$#) {

With this change? $host will no longer contain the port appended to the URL, if present.



>=== modified file 'Bugzilla/Install/Requirements.pm'

>+        package => 'Net-DNS',

Do we really want to make this module mandatory? It's only used when checking the sslbase parameter, and if there is an IPv4 address available, this module is not required. Also, Net::DNS has several other problems:

- It has a memory leak which is still unfixed, see:
  https://rt.cpan.org/Public/Bug/Display.html?id=86414
  https://rt.cpan.org/Public/Bug/Display.html?id=84601
  This means we should require a newer (currently unreleased) version where this memory leak is fixed.

- In combination with an old version of Win32::API, it will make the application crash on Windows, see:
  https://rt.cpan.org/Public/Bug/Display.html?id=82090.
  Per this bug report, Win32::API 0.64 breaks Net::DNS, but Bugzilla requires Win32::API 0.55 only. So this minimum version should be bumped, but I have no idea which version is required at minimum to not crash Net::DNS.



>=== modified file 'Bugzilla/Util.pm'

>+use Net::DNS;

Why moving this code here? It would be loaded all the time despite it's only used when entering a new value for sslbase.


>+    my $query = $res->search($domain, 'AAAA');

Why not doing $res->search($domain) only? Is the 2nd argument important?

Comment 9

[Simon Green]

Attachment: v2 patch (now runtests compliant)

Comment 10

[Frédéric Buclin]

Comment on attachment 772005 [details] [diff] [review]
v2 patch (now runtests compliant)

See my previous review. :)

Comment 11

[Simon Green]

Based on LpSolit's issues with Net::DNS (which are perfectly valid), I propose we leave this bug as unfixed until we require 5.14 and can use inet_ptoa to resolve IPv6 addresses. At this stage, there are very few sites that are IPv6 only, and for those that are, they can manually hack the code to not do the check.

Regards,
Hugo.

Comment 12

[Kyle Brantley]

Understandable...

Given that, would it be possible to update the documentation to include a note regarding manually configuring sslbase in this situation?