Please use system-wide polarssl library if installed #457

Closed
Habbie opened this Issue Apr 26, 2013 · 14 comments

Projects

None yet

1 participant

@Habbie
Member
Habbie commented Apr 26, 2013

Hi,

PowerDNS comes with its own PolarSSL sources. This makes it harder to track security issues, and Debian policy (4.13) says that packages in Debian should not do this.

See Debian Bug #671856 (http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=671856)

Please consider adapting the configure script that it looks for the polarssl lib first in a system-wide directory before resorting to the local PolarSSL. Additionally, a configure option --use-internal-libpolarssl could be used to influence that decision.

I have been told that coreutils, for example, does it right and offers some code that can be adapted for PowerDNS. I didn't verify this.

Greetings
Marc

@Habbie Habbie was assigned Apr 26, 2013
@Habbie Habbie closed this Apr 26, 2013
@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
Please pull Zugschlus/debian-pdns@0dfb745
It's a patch by Evgeni Golov which will make configure look for a system-wide installation of PolarSSL and use this installation if it's found. The local convenience copy of PolarSSL will only be used if there is no system-wide PolarSSL.

This patch has been tested by me on Debian sid.

Greetings
Marc

@Habbie
Member
Habbie commented Apr 26, 2013

Author: peter
Thank you for this patch. One more request before we pull it: If there is a system wide polarssl, we would like to give the user the option to still use our included polarssl. Would this be doable?

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
I'd have to ask Evgeni to implement this. I didn't write the patch myself.

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
EHLO, Evgeni here.

I pushed an updated version to !GitHub: https://github.com/evgeni/pdns/compare/system-polarssl
The branch contains two patches:

  1. dc0ea145 - thats what Marc submitted ealier, rebased against your SVN trunk
  2. 3130bc17 - the update to make --without-system-polarssl override the findings and use the internal copy of PolarSSL

Enjoy
Evgeni

@Habbie
Member
Habbie commented Apr 26, 2013

Author: peter
With the two patches, both ./configure and ./configure --without-system-polarssl are generating
HAVE_LIBPOLARSSL_FALSE=''
HAVE_LIBPOLARSSL_TRUE='#'

Thus causing PowerDNS to build with the included polarssl.

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
I don't know too much about things, but it actually seems like even ./configure generates a pdns/Makefile referencing the included polarssl while it should use the system polarssl:

s$ grep 1.1.2 pdns/Makefilepdns_server_DEPENDENCIES = ext/polarssl-1.1.2/library/libpolarssl.a
pdnssec_DEPENDENCIES = ext/polarssl-1.1.2/library/libpolarssl.a
DIST_SUBDIRS = ext/polarssl-1.1.2 backends
AM_CXXFLAGS = -DSYSCONFDIR="${prefix}/etc" -DLIBDIR="${exec_prefix}/lib" -DLOCALSTATEDIR="/var/run" -Ibackends/bind -pthread $(LUA_CFLAGS) $(SQLITE3_CFLAGS) -Iext/polarssl-1.1.2/include -DPDNS_ENABLE_LUA
SUBDIRS = ext/polarssl-1.1.2 backends
pdns_server_LDADD = ext/polarssl-1.1.2/library/libpolarssl.a
pdnssec_LDADD = ext/polarssl-1.1.2/library/libpolarssl.a
tsig_tests_LDFLAGS = -Lext/polarssl-1.1.2/library

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
(Evgeni):
Aye, there is a bug I somehow overlooked when porting to the two-step detection.
Will try to debug it later, head isn't clear enough atm.

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
Fixed version up at https://github.com/evgeni/pdns/compare/system-polarssl
improvement is in d43e4574

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
As requested, the version at https://github.com/evgeni/pdns/compare/system-polarssl now checks for PolarSSL >= 1.0
Also, rebased against current svn trunk.
Enjoy

~Evgeni

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
I will run some tests over the weekend and report their results here.

Greetings
Marc

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
I have run the following checks:

  • check out svn head
  • autoreconf -i
  • automake --add-missing
  • verify that configure --help says --without-system-polarssl
  • install ccache

On Debian sid (with system PolarSSL 1.1.4 installed)

  • ./configure --with-modules="" --with-dynmodules=""
  • time make -j 10
  • ldd pdns/pdns_server | grep polar => "libpolarssl.so.0 => /usr/lib/libpolarssl.so.0 (0x00007f855020f000)"
  • ./configure --with-modules="" --with-dynmodules="" --without-system-polarssl
  • time make -j 10
  • ldd pdns/pdns_server | grep polar => no output

on Debian squeeze (with system PolarSSL 0.12.1)

  • ./configure --with-modules="" --with-dynmodules=""
    • checking whether we will try to link in system PolarSSL... yes
    • checking PolarSSL version >= 1.0... no
  • time make -j 10
  • ldd pdns/pdns_server | grep polar => no output
  • ./configure --with-modules="" --with-dynmodules="" --without-system-polarssl
  • time make -j 10
  • ldd pdns/pdns_server | grep polar => no output

If you want me to do additional tests before this is committed, please let me know.

@Habbie
Member
Habbie commented Apr 26, 2013

Author: peter
I have changed the test to require 1.1 instead of 1.0, and committed as r2697. Thanks!

@Habbie
Member
Habbie commented Apr 26, 2013

Author: anon
thanks for merging!

FIY, you also could have checked the whole version directly by using POLARSSL_VERSION_NUMBER, like this:

#if POLARSSL_VERSION_NUMBER < 0x01010000
#error invalid version
#endif
@Habbie
Member
Habbie commented Apr 26, 2013

Author: peter
Good call! Applied, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment