Code review comment for lp:~yolanda.robla/ubuntu/saucy/bind9/server_banner

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Thanks for your patch! :)

This merge is attempting to change something like this:
$ host -c chaos -t txt version.bind <server>
...
version.bind descriptive text "9.9.2-P1"

to:
$ host -c chaos -t txt version.bind <server>
...
version.bind Ubuntu "9.9.2-P1"

but the patch is wrong, because it changes the 'descriptive text' in the dig command itself, not what is returned by the server. Eg:

On the machine with the updated bind9 packages:
$ host -c chaos -t txt version.bind 127.0.0.1
...
version.bind Ubuntu "9.9.2-P1"
$ host -c chaos -t txt version.bind ns1.canonical.com
...
version.bind Ubuntu "9.8.1-P1"

On some other machine:
$ host -c chaos -t txt version.bind <server with updated bind>
...
version.bind descriptive text "9.9.2-P1"
$ host -c chaos -t txt version.bind ns1.canonical.com
...
version.bind descriptive text "9.8.1-P1"

I think you'd be better off updating the version if you were going to pursue this so that the server returns something like this instead:
$ host -c chaos -t txt version.bind <server with updated bind>
...
version.bind descriptive text "Ubuntu 9.9.2-P1"

However, https://blueprints.launchpad.net/ubuntu/+spec/servercloud-s-server-app-banner-updates states that the rationale is 'When looking at Ubuntu Server, some services such as ssh say "Debian" rather than "Ubuntu"'. This is not the case with bind9. In fact, it doesn't even show the specific Ubuntu version. For example on saucy, I currently have 1:9.9.2.dfsg.P1-2ubuntu2 installed, but the server reports itself as 9.9.2-P1. Understanding that hiding the OS vendor from the banner can be considered security through obscurity, adding it doesn't seem hugely beneficial and I'd prefer that we not make this banner change. If you feel strongly about the change, this the sort of change that should be upstreamed to Debian, and I suggest you take it up with lamont (he is the Debian maintainer and cares for our Ubuntu packages as well) and report back the outcome.

Thanks again

review: Disapprove

« Back to merge proposal