Bug#798773: postinst script handles comments in config file incorrectly

[Christian Hofstaedtler]

Control: severity -1 serious

* Stephen Frost  [151006 15:06]:
> > I was going to write which users are affected by this bug, but now
> > that I think more about it, I'm not sure which users are affected.
> > As you have some interest, I assume you have run into the bug? How
> > did your configuration look like?
> 
> Yes, I did run into this bug while upgrading the hidden PowerDNS master
> for postgresql.org.  The configuration we have is relatively simple.  My
> recollection of how it happened is:
> 
> Installed an older version (3.4.1?), modified /etc/powerdns/pdns.conf
> with a few simple changes- allow-axfr-ips, master=yes, slave=yes, etc.
> 
> Then at some point down the road, attempted to upgrade to 3.4.6 (in
> jessie backports), and it blew up on that error.  To fix it, I recall
> modifying pdns.conf *and* pdns.conf.ucf-dist.  It's possible I didn't
> have to change both, but I think I tried changing pdns.conf first and it
> didn't help, so I changed pdns.conf.ucf-dist also and that got me past
> the issue.  Apologies for not having more detailed notes, I was a bit
> anxious to get it fixed. :)

I think the multiple upgrades are what causes this really.

pdns.conf as installed by the wheezy version will only have a single
line matching "include=...", where 3.4.x will have multiple lines,
like this:

# include-dir   Include *.conf files from this directory
# include-dir=/etc/powerdns/pdns.conf.d

$ grep include foo.conf | awk -F '=' '{print $2}'

/etc/powerdns/pdns.conf.d
$

Therefore I think most users doing the wheezy -> jessie upgrade are
off fine.

(Security) Updates in jessie and updates jessie -> stretch will fail.

> I'll see about building a test jessie VM, installing the version in
> jessie, modifying the config, and then doing an upgrade, and see if I
> can reproduce the error.

Appreciated.

-- 
 ,''`.  Christian Hofstaedtler 
: :' :  Debian Developer
`. `'   7D1A CFFA D9E0 806C 9C4C  D392 5C13 D6DB 9305 2E03
  `-



signature.asc
Description: PGP signature

Bug#798773: postinst script handles comments in config file incorrectly

[Christian Hofstaedtler]

Stephen,

* Stephen Frost  [151006 04:12]:
> That said, I do think it's worthwhile to see about fixing these
> particular install failures, and the proposed change looks like it would
> at least do that.

I agree that it would make the situation better for some users.

I was going to write which users are affected by this bug, but now
that I think more about it, I'm not sure which users are affected.
As you have some interest, I assume you have run into the bug? How
did your configuration look like?

> > > Is there anything I can do to help?
> > 
> > I'm thinking of deleting most of the code in the postinst for
> > stretch.
> 
> Are you thinking about simply assuming that /etc/powerdns/pdns.d is the
> PDNSDIR ...

Yes, because the files the postinst touches are meant to be the
files that the previous version of the pdns-server package has
shipped.

> and anything else is up to the user to address?

If the user has moved or renamed the pdns.d dir, or changed the
include= dir to point to something else entirely, then we have no
business of touching (and moving!) the users config files at all.

As the postinst doesn't have a version check right now, it
1) is wrong for versions after jessie,
2) we have to look at all previously released binary packages to see
the original intent and which conffiles have previously been installed.
I haven't done that check yet.

> > Not sure what to do about jessie. Given that this bug has existed
> > since 2006, maybe it's not terribly important to fix in jessie.
> 
> I disagree.  Perhaps I'm being naive, but having the relatively simple
> case, where /etc/powerdns/pdns.d is the directory and the configuration
> has been only mildly tweaked, failure during upgrades is not a good
> position for us to be in.
> 
> I have to admit that I'm not up to speed on current policy, but I'm
> happy to try and implement whatever the correct solution is.  I'm sure
> there are other packages which have include directories, is there a
> clear "right way" to handle this?

I don't think there's much policy here except for the normal "don't
touch stuff that isn't yours" - i.e. preserve user changes,
especially if they are to/in files that aren't installed by the
package.

There's some other complication - include= is the old name of the
include directory setting; jessie's pdns does include-dir= instead. [1]

> Thanks!

Thank you for caring about this,
Christian

[1] https://doc.powerdns.com/md/authoritative/settings/#include-dir
-- 
 ,''`.  Christian Hofstaedtler 
: :' :  Debian Developer
`. `'   7D1A CFFA D9E0 806C 9C4C  D392 5C13 D6DB 9305 2E03
  `-

Bug#798773: postinst script handles comments in config file incorrectly

[Stephen Frost]

Christian,

* Christian Hofstaedtler (z...@debian.org) wrote:
> * Stephen Frost  [151006 04:12]:
> > That said, I do think it's worthwhile to see about fixing these
> > particular install failures, and the proposed change looks like it would
> > at least do that.
> 
> I agree that it would make the situation better for some users.
> 
> I was going to write which users are affected by this bug, but now
> that I think more about it, I'm not sure which users are affected.
> As you have some interest, I assume you have run into the bug? How
> did your configuration look like?

Yes, I did run into this bug while upgrading the hidden PowerDNS master
for postgresql.org.  The configuration we have is relatively simple.  My
recollection of how it happened is:

Installed an older version (3.4.1?), modified /etc/powerdns/pdns.conf
with a few simple changes- allow-axfr-ips, master=yes, slave=yes, etc.

Then at some point down the road, attempted to upgrade to 3.4.6 (in
jessie backports), and it blew up on that error.  To fix it, I recall
modifying pdns.conf *and* pdns.conf.ucf-dist.  It's possible I didn't
have to change both, but I think I tried changing pdns.conf first and it
didn't help, so I changed pdns.conf.ucf-dist also and that got me past
the issue.  Apologies for not having more detailed notes, I was a bit
anxious to get it fixed. :)

I'll see about building a test jessie VM, installing the version in
jessie, modifying the config, and then doing an upgrade, and see if I
can reproduce the error.

> > > > Is there anything I can do to help?
> > > 
> > > I'm thinking of deleting most of the code in the postinst for
> > > stretch.
> > 
> > Are you thinking about simply assuming that /etc/powerdns/pdns.d is the
> > PDNSDIR ...
> 
> Yes, because the files the postinst touches are meant to be the
> files that the previous version of the pdns-server package has
> shipped.

Right.

> > and anything else is up to the user to address?
> 
> If the user has moved or renamed the pdns.d dir, or changed the
> include= dir to point to something else entirely, then we have no
> business of touching (and moving!) the users config files at all.

Agreed.  We just need to be able to sanely detect that and act
accordingly.

> As the postinst doesn't have a version check right now, it
> 1) is wrong for versions after jessie,
> 2) we have to look at all previously released binary packages to see
> the original intent and which conffiles have previously been installed.
> I haven't done that check yet.

Yeah, that doesn't sound like much fun. :)

> > > Not sure what to do about jessie. Given that this bug has existed
> > > since 2006, maybe it's not terribly important to fix in jessie.
> > 
> > I disagree.  Perhaps I'm being naive, but having the relatively simple
> > case, where /etc/powerdns/pdns.d is the directory and the configuration
> > has been only mildly tweaked, failure during upgrades is not a good
> > position for us to be in.
> > 
> > I have to admit that I'm not up to speed on current policy, but I'm
> > happy to try and implement whatever the correct solution is.  I'm sure
> > there are other packages which have include directories, is there a
> > clear "right way" to handle this?
> 
> I don't think there's much policy here except for the normal "don't
> touch stuff that isn't yours" - i.e. preserve user changes,
> especially if they are to/in files that aren't installed by the
> package.

Right.

> There's some other complication - include= is the old name of the
> include directory setting; jessie's pdns does include-dir= instead. [1]
> 
> > Thanks!
> 
> Thank you for caring about this,

Absolutely, I like PDNS in general and given that we're using it to run
postgresql.org, we really want it to work well. :)

I'm also a DD, btw, though I haven't done much Debian-related work
recently.  I'd be happy to help out with maintaining PowerDNS though, as
we're using it.

Thanks!

Stephen


signature.asc
Description: Digital signature

Bug#798773: postinst script handles comments in config file incorrectly

[Stephen Frost]

Greetings,

Any chance we could get this bug fixed?  Looks like a pretty
straight-forward change.

Is there anything I can do to help?

Thanks!

Stephen


signature.asc
Description: Digital signature

Bug#798773: postinst script handles comments in config file incorrectly

[Christian Hofstaedtler]

* Stephen Frost  [151005 22:57]:
> Any chance we could get this bug fixed?  Looks like a pretty
> straight-forward change.

Only on the surface it's a straight-forward change. When you look
closer at the postinst code, I'm quite sure the code only works when
the config var hasn't been changed from the default.
(Or it does work, but you end up with ucf hijacking files the
package doesn't ship.)

> Is there anything I can do to help?

I'm thinking of deleting most of the code in the postinst for
stretch.

Not sure what to do about jessie. Given that this bug has existed
since 2006, maybe it's not terribly important to fix in jessie.

-- 
 ,''`.  Christian Hofstaedtler 
: :' :  Debian Developer
`. `'   7D1A CFFA D9E0 806C 9C4C  D392 5C13 D6DB 9305 2E03
  `-



signature.asc
Description: PGP signature

Bug#798773: postinst script handles comments in config file incorrectly

[Stephen Frost]

Christian,

Thanks for the quick reply!

* Christian Hofstaedtler (z...@debian.org) wrote:
> * Stephen Frost  [151005 22:57]:
> > Any chance we could get this bug fixed?  Looks like a pretty
> > straight-forward change.
> 
> Only on the surface it's a straight-forward change. When you look
> closer at the postinst code, I'm quite sure the code only works when
> the config var hasn't been changed from the default.
> (Or it does work, but you end up with ucf hijacking files the
> package doesn't ship.)

Looking at the postinst code, I think I see what you mean.

That said, I do think it's worthwhile to see about fixing these
particular install failures, and the proposed change looks like it would
at least do that.

> > Is there anything I can do to help?
> 
> I'm thinking of deleting most of the code in the postinst for
> stretch.

Are you thinking about simply assuming that /etc/powerdns/pdns.d is the
PDNSDIR and anything else is up to the user to address?

> Not sure what to do about jessie. Given that this bug has existed
> since 2006, maybe it's not terribly important to fix in jessie.

I disagree.  Perhaps I'm being naive, but having the relatively simple
case, where /etc/powerdns/pdns.d is the directory and the configuration
has been only mildly tweaked, failure during upgrades is not a good
position for us to be in.

I have to admit that I'm not up to speed on current policy, but I'm
happy to try and implement whatever the correct solution is.  I'm sure
there are other packages which have include directories, is there a
clear "right way" to handle this?

Thanks!

Stephen


signature.asc
Description: Digital signature

Bug#798773: postinst script handles comments in config file incorrectly

[Leon Weber]

Package: pdns-server
Version: 3.4.1-4+deb8u3

Hi,

The postinst script tries to find any “include=…” lines in pdns.conf in
order to find the pdns config directory.  However, the script fails to
ignore comments in the configuration file, so “#include=…” lines (or
basically any lines containing “include”) will match as well.  This can
cause error messages from ucfr such as

Setting up pdns-server (3.4.1-4+deb8u3) ...
ucfr: *** ERROR: Need exactly two arguments, got 3

[ … ucfr usage output follows … ]

dpkg: error processing package pdns-server (--configure):
 subprocess installed post-installation script returned error exit
 status 3
Errors were encountered while processing:
 pdns-server
E: Sub-process /usr/bin/dpkg returned an error code (1)

The culprit lies in pdns-server.postinst line 20:

PDNSDIR=`cat $PDNSCONF | grep include | awk -F '=' '{print $2}'`

I think an easy, but effective fix could be as simple as:

PDNSDIR=`cat $PDNSCONF | grep '^include' | awk -F '=' '{print $2}'`

Regards,

-- Leon.


pgpQLg6fCl_Fm.pgp
Description: PGP signature