[pkg] CurveDNS - review

[Stéphane Neveu]

> Hi Stéphane

Hi Lucas !
,
>
> On Tue, 20 Jun 2017 13:58:20 +0200
> Stéphane Neveu <stefneveu at gmail.com> wrote:
>> May I ask you to review my first package : CurveDNS ?
>> It's on alioth...
>
> Thanks for your work!  I had a look at your package. Disclaimer: I'm a
> fairly new contributor myself, doing a review mostly to learn something
> myself and hopefully take some load off the DDs.
>

Thank you for reviewing my package :)

>
> I noticed the source includes (and compiles+uses) the nacl library.
> While that library is already packaged in Debian, it's unfortunately
> not available as shared object.
>
> I don't really know how to proceed here, none of our options look
> particularly promising:
> * Keep the copy of the nacl library: this means that in case of security
>   fixes in the nacl library, these need to be applied to your package
>   separately.
> * Build-Depend on libnacl-dev and use the provided `libnacl.a` for
>   linking:  This means that after a security update your package "only"
>   needs to be recompiled.  But an incompatible ABI change in libnacl
>   might break your package without warning…
> * Convince the maintainer of nacl to provide it as shared library
>   (upstream does not support this, so this may involve a lot of work and
>   may not be doable): This would be what we want, security patches can
>   affect you directly and ABI compatibility can be handled somewhat
>   gracefully.
>
> I hope someone here can provide us with guidance what to do.
>

Yes, I don't know how to deal with that really. Should I ask on
mentors mailing list ?
I'm not sure to be able to convince the maintainer.

>
> Some possible problems I've noticed when going through the files:
> * To what extend does the software need daemontools?  Can't systemd or
>   sysvinit run it also?  This part creates a lot of complexity (also in
>   the maintainer scripts) and I would like to know if we really need
>   this.

In fact, the whole documentation on their website is based on
daemontools but ok I'll try to use systemd / sysVinit instead :)

>   I also don't understand why we need to start multilog in ExecStartPost
>   in the systemd unit file (not present in the init script).  Can't we
>   let curvedns log to journal?  Then we can also drop the whole
>   envuidgid stuff and select the correct user in the unit file directly.
> * I think with your hardening_support.patch, only the CPPFLAGS from
>   `dpkg-buildflags` are applied and not the CFLAGS or LDFLAGS…

Meaculpa, I'll change that, thanks.

> * debian/curvedns.config: don't you need to db_get in order to be able
>   to use $RET (it's commented out)?

Suprisingly, it does not work with db_get whereas it does when
commented out... did I missed something ? yes probably.

> * debian/curvedns.{postrm,prerm}: you should not do something like
>
>       if not_my_phase; then
>           exit 0
>       fi
>       #commands here
>
>   because that will prevent #DEBHELPER# later to execute what it may
>   need to in other phases.  Instead do something like:
>
>       if my_phase; then
>           #commands here
>       fi
>
> * Suggestions for "beautification":
>   - debian/control: make whitespace consistent, don't mix tabs and
>     spaces

Thank you, it's done.

>   - debian/copyright: link to format could be https :)

Well the tls certificate doesn't seem to be valid :/
https://curvedns.on2it.net/

>   - debian/{dirs,install,manpages}: rename to debian/curvedns.{...} for
>     consistency
>   - permissions in the repository for the maintainer scripts are
>     inconsistent (only some of them have execute permission)
>

Thank you, done :)


Stephane