[Bug 447847] New: Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Summary: Review Request: unbound - Validating, recursive, and
                    caching DNS(SEC) resolver
           Product: Fedora
           Version: rawhide
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: medium
          Priority: medium
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]
                CC: [e-mail],[e-mail]

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: tp://ftp.xelerance.com/unbound/unbound-1.0.0-1.src.rpm
Description: Unbound is a validating, recursive, and caching DNS(SEC) resolver.

The C implementation of Unbound is developed and maintained by NLnet
Labs. It is based on ideas and algorithms taken from a java prototype
developed by Verisign labs, Nominet, Kirei and ep.net.

Unbound is designed as a set of modular components, so that also
DNSSEC (secure DNS) validation and stub-resolvers (that do not run
as a server, but are linked into an application) are easily possible.

output from rpmlint:

unbound.x86_64: E: non-standard-uid /var/unbound unbound
unbound.x86_64: E: non-standard-gid /var/unbound unbound
unbound.x86_64: E: non-standard-dir-perm /var/unbound 0700
unbound.x86_64: E: non-standard-uid /var/unbound/unbound.conf unbound
unbound.x86_64: E: non-standard-gid /var/unbound/unbound.conf unbound
unbound.x86_64: W: non-standard-dir-in-var unbound
unbound.x86_64: W: dangerous-command-in-%preun rm
unbound.x86_64: W: incoherent-subsys /etc/rc.d/init.d/unbound $prog

Currently talking to upstream about the problem of including a static ldns shipped with unbound, versus using the real standaline ldns.

The dangerous rm in %preun is the chroot /var/unbound getting cleaned up. The chroot() is currently somewhat variable, eg needing the real /etc/resolv.conf, which is manipulated in the init script, since hard links might not work across /etc/ and /var

I am not sure about the incoherent-subsys warning. It seems similar to other packages used. Using prog=$(basename $exec) did not help.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

https://bugzilla.redhat.com/show_bug.cgi?id=447847

------- Additional Comments From [e-mail]  2008-05-22 12:36 EST -------
ldns 1.3.x is required, but not yet released by upstream. So once that is
released and updated in fedora (I am the ldns maintainer), the static lib issue
will go away.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

https://bugzilla.redhat.com/show_bug.cgi?id=447847

------- Additional Comments From [e-mail]  2008-05-28 16:47 EST -------
ldns 1.3.0 (prerelease) addressed these problems, and the build now uses the
installed ldns properly. I've uploaded new rpm and spec file:

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.0-2.fc9.src.rpm

ldns-1.3.0 should make it in the repo in the next day or so when upstream
releases it.

currently rpmlint says:
unbound.x86_64: E: non-standard-uid /var/unbound unbound
unbound.x86_64: E: non-standard-gid /var/unbound unbound
unbound.x86_64: E: non-standard-uid /var/unbound/unbound.conf unbound
unbound.x86_64: E: non-standard-gid /var/unbound/unbound.conf unbound
unbound.x86_64: W: non-standard-dir-in-var unbound
unbound.x86_64: W: dangerous-command-in-%preun rm
unbound.x86_64: W: incoherent-subsys /etc/rc.d/init.d/unbound $prog

I am not sure about the incoherent-subsys error.....

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

https://bugzilla.redhat.com/show_bug.cgi?id=447847

------- Additional Comments From [e-mail]  2008-07-16 14:36 EST -------
updated to 1.0.1

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.1-1.fc9.src.rpm

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Adam Tkac  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|                            |[e-mail]
    Customer Facing|                            |---

--- Comment #4 from Adam Tkac   2008-08-11 09:41:30 EDT ---
It would be nice to have this software in Fedora. I'm going to take care about
review.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #5 from Adam Tkac   2008-08-11 10:21:26 EDT ---
rpmlint output:
unbound.x86_64: W: non-standard-uid /var/unbound unbound
unbound.x86_64: W: non-standard-gid /var/unbound unbound
unbound.x86_64: W: non-standard-uid /var/unbound/unbound.conf unbound
unbound.x86_64: W: non-standard-gid /var/unbound/unbound.conf unbound
unbound.x86_64: W: non-standard-dir-in-var unbound
unbound.x86_64: W: incoherent-subsys /etc/rc.d/init.d/unbound $prog
^^ OK

unbound.x86_64: W: dangerous-command-in-%preun rm
Why do you have to call rm? I think rpm could handle remove of such directories
automatically.

build process:
you could see something like

Depend daemon/daemon.c
Depend daemon/acl_list.c
Build services/listen_dnsport.c
Build services/localzone.c

Is it intentional that gcc parameters are not written to output? It doesn't
look fine for me.

spec file:
is it really needed ship static libunbound.a library?

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #6 from Paul Wouters   2008-09-16 17:10:59 EDT ---
Hi, I missed the emails telling me someone was reviewing this. Sorry.

I've updated unbound to version 1.0.2

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.2-1.fc9.src.rpm

As for your comments:

- hiding the gcc command is what upstream does. It is not something we do.
- the rm command is used because unbound creates these files on startup.
  I will look into pre-creating these as part of the package, so they are
  removed when the package is uninstalled without using the rm command
manually.
- the static library does not need shipping in the -devel package, i remove it
in the next release

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #7 from Paul Wouters   2008-09-16 17:11:58 EDT ---
I'll also update the user addition scheme to comply with the new packaging
guidelines.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Ralf Corsepius  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[e-mail]

--- Comment #8 from Ralf Corsepius   2008-09-18 02:38:03 EDT ---
(In reply to comment #6)
> the static library does not need shipping in the -devel package, i remove it
>
> in the next release

MUSTFIX: There is shared libunbound, so you must not ship it as part of the
-devel package. If you really feel you must ship static libs (feel strongly
encouraged not to do so), move the lib*.a to a -static package.

MUSTFIX:
/var/unbound violates the FHS. Check the FHS for details, but this likely
should be /var/lib/unbound.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #9 from Paul Wouters   2008-10-10 00:07:51 EDT ---
I've fixed up all issues and created a new release.

rmplint now only shows:

unbound.x86_64: W: non-standard-uid /var/run/unbound unbound
unbound.x86_64: W: non-standard-gid /var/run/unbound unbound

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.2-2.fc9.src.rpm
initscript:  ftp://ftp.xelerance.com/unbound/unbound.init

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Adam Tkac  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|[e-mail]    |[e-mail]
               Flag|                            |fedora-review?

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #10 from Adam Tkac   2008-10-17 07:22:31 EDT ---
source files match upstream: YES
package meets naming and versioning guidelines: YES
specfile is properly named, is cleanly written and uses macros consistently:
YES
dist tag is present: YES
build root is correct: YES
license field matches the actual license: YES
license is open source-compatible: YES (BSD)
License text included in package: YES
latest version is being packaged: YES
BuildRequires are proper: YES

compiler flags are appropriate: NO
- look on http://people.redhat.com/atkac/unbound-build.patch. I think that
patch should be included in main source. It shows build parameters

%clean is present: YES
package builds in mock (Rawhide/x86_64): YES
debuginfo package looks complete: YES
rpmlint is silent: OK (messages written below are fine)
unbound.x86_64: W: non-standard-uid /var/run/unbound unbound
unbound.x86_64: W: non-standard-gid /var/run/unbound unbound

final provides and requires look sane: OK
%check is present and all tests pass: OK (check is not present)
no shared libraries are added to the regular linker search paths: NO
- it seems that it is possible write "clients" for unbound. Due this reason I
think the best will be create unbound-libs subpackage which will contain 
libunbound.so.0 and libunbound.so.0.14.0 libraries

owns the directories it creates: YES
doesn't own any directories it shouldn't: YES
no duplicates in %files: YES
file permissions are appropriate: VERIFY
- isn't better to make configuration files and chroot non-readable by "others"?

scriptlets: NO
- I think you should add unbound group as well. Look on 
https://fedoraproject.org/wiki/Packaging/UsersAndGroups

Please correct/explain problematic points written above.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #11 from Paul Wouters   2008-10-19 15:52:41 EDT ---
- I've changed the make command to run with QUIET=no, so it does not require a
patch.
- the chroot contains no secret information, and is just there for security in
case the unbound process would get compromised with a buffer overflow. Since
there is nothing to hide, using world-readable seems appropriate.
- I've created a separate libs package as you recommended
- I've added the unbound group

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.2-3.fc9.src.rpm
initscript:  ftp://ftp.xelerance.com/unbound/unbound.init

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #12 from Adam Tkac   2008-10-21 11:03:17 EDT ---
All problems were fixed except one - compiler flags:

http://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags

"Standard" approach

%build
export CFLAGS=$RPM_OPT_FLAGS
./configure

doesn't pass CFLAGS to gcc, I don't know why. But something like

sed -i 's/CFLAGS=@CFLAGS@/CFLAGS=@CFLAGS@ '"$RPM_OPT_FLAGS"'/' Makefile.in

works.

I recommend put %configure command to build section (but it isn't needed).
Otherwise all looks fine for me.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #13 from Paul Wouters   2008-10-21 11:40:40 EDT ---
Ahh, i see. I used:

%{__make} CFLAGS="$RPM_OPT_FLAGS" QUIET=no %{?_smp_mflags}

Which seems to work, but causes a previous warning to be an error now. So I'll
have to make a patch :)

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #14 from Mamoru Tasaka   2008-10-21 11:42:15 EDT ---
Just for CFLAGS issue

(In reply to comment #12)
> %build
> export CFLAGS=$RPM_OPT_FLAGS
> ./configure
>
> doesn't pass CFLAGS to gcc, I don't know why.

Because configure.ac contains:
---------------------------------------------------------------
    35  
    36  CFLAGS=
    37  AC_AIX
---------------------------------------------------------------
The line 36 is the culprit.
---------------------------------------------------------------
sed -i.cflags -e '/^CFLAGS=$/d' configure
---------------------------------------------------------------
before calling %configure will save CFLAGS.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #15 from Paul Wouters   2008-10-21 12:08:55 EDT ---
Ahh, we lost -D_GNU_SOURCE. I added this to the make line, since it seems it is
the only part of CFLAGS that we lose.

I also disabled interface-automatic, as this caused unbound to listen on
0.0.0.0 and ::0 instead of just localhost.

Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.2-4.fc9.src.rpm
initscript:  ftp://ftp.xelerance.com/unbound/unbound.init

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #16 from Paul Wouters   2008-10-22 12:46:01 EDT ---
Spec URL: ftp://ftp.xelerance.com/unbound/unbound.spec
SRPM URL: ftp://ftp.xelerance.com/unbound/unbound-1.0.2-5.fc9.src.rpm
initscript:  ftp://ftp.xelerance.com/unbound/unbound.init

* Wed Oct 22 2008 Paul Wouters  - 1.0.2-5
- Only call ldconfig in -libs package
- Move configure into build section
- devel subpackage should only depend on libs subpackage

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Adam Tkac  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-review?              |fedora-review+

--- Comment #17 from Adam Tkac   2008-10-29 07:02:40 EDT ---
All problems written in previous comments are fixed (especially from comment
#10 which contains detail review steps). Reviewed

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Paul Wouters  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |fedora-cvs+

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Paul Wouters  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs+                 |fedora-cvs?

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

--- Comment #18 from Paul Wouters   2008-10-31 09:44:25 EDT ---
New Package CVS Request
=======================
Package Name: unbound
Short Description: Validating, recursive, and caching DNS(SEC) resolver
Owners: pwouters
Branches: F-10 F-9 EL-5
InitialCC:

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Kevin Fenzi  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|fedora-cvs?                 |fedora-cvs+

--- Comment #19 from Kevin Fenzi   2008-10-31 12:58:59 EDT ---
cvs done.

[Bug 447847] Review Request: unbound - Validating, recursive, and caching DNS(SEC) resolver

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=447847

Paul Wouters  changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE