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

NEW
Unassigned

Status

()

Bugzilla
Administration
5 years ago
2 years ago

People

(Reporter: Kyle Brantley, Unassigned)

Tracking

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
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).

Updated

5 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking4.4?
(Reporter)

Comment 2

5 years ago
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

5 years ago
Not a blocker, but something we will have to fix sooner or later as IPV6 addresses will become more frequent.
Assignee: general → administration
Component: Bugzilla-General → Administration
Flags: blocking4.4? → blocking4.4-

Updated

4 years ago
Assignee: administration → hugo.seabrook
Status: NEW → ASSIGNED

Comment 4

4 years ago
Created attachment 771950 [details] [diff] [review]
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
Attachment #771950 - Flags: review?(LpSolit)

Comment 5

4 years ago
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

4 years ago
(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

4 years ago
(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

Updated

4 years ago
Attachment #771950 - Attachment is obsolete: true
Attachment #771950 - Flags: review?(LpSolit)

Comment 8

4 years ago
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?
Attachment #771950 - Attachment is obsolete: false

Comment 9

4 years ago
Created attachment 772005 [details] [diff] [review]
v2 patch (now runtests compliant)
Attachment #771950 - Attachment is obsolete: true
Attachment #772005 - Flags: review?(LpSolit)

Comment 10

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

See my previous review. :)
Attachment #772005 - Flags: review?(LpSolit) → review-

Comment 11

4 years ago
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.

Updated

4 years ago
Assignee: hugo.seabrook → administration
Status: ASSIGNED → NEW
(Reporter)

Comment 12

4 years ago
Understandable...

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

Updated

2 years ago
Depends on: 1136137
Flags: blocking4.4-
You need to log in before you can comment on or make changes to this bug.