ioerror / tlsdate

secure parasitic rdate replacement
Other
362 stars 74 forks source link

minor building issues on debian testing #180

Open ghost opened 8 years ago

ghost commented 8 years ago

Hi,

I ran into a few issues compiling on debian testing.

1: configure.ac - _AS_IF([test "x${OPTPOLARSSL}" != "xno"

in configure.ac, the AS_IF call checking for PolarSSL produces errors on autoreconf -i with autoconf 2.69-9.

I fixed that quick'n'dirty using

m4_pattern_allow([AS_IF])
m4_pattern_allow([AC_DEFINE])
m4_pattern_allow([AC_MSG_ERROR])

but I'm sure there are more solid solutions.

2: configure.ac - _PKG_CHECKMODULES([LIBEVENT])

when pkg-config is not installed, configure throws a syntax error. I fixed that adding

dnl fix error `PKG_CHECK_MODULES(LIBEVENT, libevent >= 2.0)` when pkg-config is not present
dnl Testing presence of pkg-config
AC_MSG_CHECKING([pkg-config m4 macros])
if test m4_ifdef([PKG_CHECK_MODULES], [yes], [no]) == yes; then
   AC_MSG_RESULT([yes]);
else
   AC_MSG_RESULT([no]);
   AC_MSG_ERROR([pkg-config is required. See pkg-config.freedesktop.org])
fi

Is there a better solution for this?

3: SSLv3_client_method() was dropped in Debian's OpenSSL

I fixed that using the following diff:

diff --git a/src/tlsdate-helper-plan9.c b/src/tlsdate-helper-plan9.c
index 3c532aa..12ecb1f 100644
--- a/src/tlsdate-helper-plan9.c
+++ b/src/tlsdate-helper-plan9.c
@@ -978,10 +978,15 @@ run_ssl (uint32_t *time_map, int time_is_an_illusion)
   {
     verb ("V: using SSLv23_client_method()\n");
     ctx = SSL_CTX_new(SSLv23_client_method());
+/*
+ * SSLv3 is obsolete
+*/
+/*
   } else if (0 == strcmp("sslv3", protocol))
   {
     verb ("V: using SSLv3_client_method()\n");
     ctx = SSL_CTX_new(SSLv3_client_method());
+*/
   } else if (0 == strcmp("tlsv1", protocol))
   {
     verb ("V: using TLSv1_client_method()\n");
diff --git a/src/tlsdate-helper.c b/src/tlsdate-helper.c
index 877c67e..e625a10 100644
--- a/src/tlsdate-helper.c
+++ b/src/tlsdate-helper.c
@@ -1133,10 +1133,15 @@ run_ssl (uint32_t *time_map, int time_is_an_illusion, int http)
   {
     verb ("V: using SSLv23_client_method()");
     ctx = SSL_CTX_new(SSLv23_client_method());
+/*
+ * SSLv3_client_method() is obsolete
+*/
+/*
   } else if (0 == strcmp("sslv3", protocol))
   {
     verb ("V: using SSLv3_client_method()");
     ctx = SSL_CTX_new(SSLv3_client_method());
+*/
   } else if (0 == strcmp("tlsv1", protocol))
   {
     verb ("V: using TLSv1_client_method()");
diff --git a/src/tlsdate.c b/src/tlsdate.c
index dd7f993..ecba968 100644
--- a/src/tlsdate.c
+++ b/src/tlsdate.c
@@ -88,7 +88,10 @@ usage (void)
            " [-n|--dont-set-clock]\n"
            " [-H|--host] [hostname|ip]\n"
            " [-p|--port] [port number]\n"
+/*
            " [-P|--protocol] [sslv23|sslv3|tlsv1]\n"
+*/
+           " [-P]--protocol] [sslv23|tlsv1]\n"
            " [-C|--certcontainer] [dirname|filename]\n"
            " [-v|--verbose]\n"
            " [-V|--showtime] [human|raw]\n"

4: tlsdated is not completely free()ing all allocated memory when exiting

With ASAN (gcc) enabled, LeakSanitizer reports memory leaks after tlsdated exists:

/opt/tlsdate/sbin > ./tlsdated -p -w
Can't read seconds from /var/cache/tlsdated/timestamp: Permission denied
initial time sync type: system-clock
[event:action_sync_and_save] time setter is gone!
[event:handle_time_setter] time setter could not open save file
tlsdate-setter exitting: code:EXITED status:2 pid:31394 uid:1000
tlsdated clean up finished; exiting!

=================================================================
==31393==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 272 byte(s) in 2 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x7f46bc687c05 in event_new (/usr/lib/x86_64-linux-gnu/libevent-2.0.so.5+0x12c05)

Direct leak of 40 byte(s) in 1 object(s) allocated from:
    #0 0x7f46bd01d4e1 in __interceptor_calloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x944e1)
    #1 0x559125a2b852 in add_source_to_conf src/tlsdated.c:227
    #2 0x559125a3f413 in parse_source src/tlsdated.c:287
    #3 0x559125a3f413 in load_conf src/tlsdated.c:378
    #4 0x559125a18631 in main src/tlsdated.c:466
    #5 0x7f46bb5deb44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

Direct leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x559125a1c2d8 in conf_parse src/conf.c:78
    #2 0x559125a35373 in load_conf src/tlsdated.c:312
    #3 0x559125a18631 in main src/tlsdated.c:466
    #4 0x7f46bb5deb44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

Direct leak of 20 byte(s) in 1 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x7f46bb63e9e9 in strdup (/lib/x86_64-linux-gnu/libc.so.6+0x819e9)

Indirect leak of 360 byte(s) in 15 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x559125a1c2d8 in conf_parse src/conf.c:78
    #2 0x559125a35373 in load_conf src/tlsdated.c:312
    #3 0x559125a18631 in main src/tlsdated.c:466
    #4 0x7f46bb5deb44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

Indirect leak of 330 byte(s) in 37 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x7f46bb63e9e9 in strdup (/lib/x86_64-linux-gnu/libc.so.6+0x819e9)

Indirect leak of 136 byte(s) in 1 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x7f46bc687c05 in event_new (/usr/lib/x86_64-linux-gnu/libevent-2.0.so.5+0x12c05)

Indirect leak of 48 byte(s) in 2 object(s) allocated from:
    #0 0x7f46bd01d37a in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x9437a)
    #1 0x559125a1c7b1 in conf_parse src/conf.c:78
    #2 0x559125a35373 in load_conf src/tlsdated.c:312
    #3 0x559125a18631 in main src/tlsdated.c:466
    #4 0x7f46bb5deb44 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21b44)

SUMMARY: AddressSanitizer: 1230 byte(s) leaked in 60 allocation(s).

There seems to be the need for some garbage collection.

ghost commented 8 years ago

Oh, just FYI, on another debian testing box, where I "accidently" had installed libpolarssl-dev, I ran into another issue (with clang 3.5.2, not relevant for me though):

src/tlsdate-helper.c:840:9: error: unknown type name 'x509_cert'; did you mean 'x509_crt'?
  const x509_cert *certificate;
        ^~~~~~~~~
        x509_crt
/usr/include/polarssl/x509_crt.h:100:1: note: 'x509_crt' declared here
x509_crt;
^
src/tlsdate-helper.c:850:3: warning: implicit declaration of function 'x509parse_dn_gets' is invalid in C99 [-Wimplicit-function-declaration]
  x509parse_dn_gets(buf, 1024, &certificate->subject);
  ^
src/tlsdate-helper.c:853:30: error: no member named 'rsa' in 'struct _x509_crt'
  public_key = &certificate->rsa;
                ~~~~~~~~~~~  ^
src/tlsdate-helper.c:984:3: error: unknown type name 'x509_cert'; did you mean 'x509_crt'?
  x509_cert cacert;
  ^~~~~~~~~
  x509_crt
/usr/include/polarssl/x509_crt.h:100:1: note: 'x509_crt' declared here
x509_crt;
^
src/tlsdate-helper.c:990:30: error: use of undeclared identifier 'x509_cert'; did you mean 'x509_crt'?
  memset (&cacert, 0, sizeof(x509_cert));
                             ^
/usr/include/polarssl/x509_crt.h:100:1: note: 'x509_crt' declared here
x509_crt;
^
src/tlsdate-helper.c:1004:17: warning: implicit declaration of function 'x509parse_crtfile' is invalid in C99 [-Wimplicit-function-declaration]
        if (0 > x509parse_crtfile(&cacert, ca_cert_container))
                ^
src/tlsdate-helper.c:1008:17: warning: implicit declaration of function 'x509parse_crtpath' is invalid in C99 [-Wimplicit-function-declaration]
        if (0 > x509parse_crtpath(&cacert, ca_cert_container))
                ^
src/tlsdate-helper.c:1107:3: warning: implicit declaration of function 'x509_free' is invalid in C99 [-Wimplicit-function-declaration]
  x509_free (&cacert);
  ^
4 warnings and 4 errors generated.

x509_crt is the easy part. I'm not familiar with PolarSSL, but from 1.2 to 1.3 they have restructured the RSA module, from rsa_context ctx to pk_context ctx. So my quick fix changing "public_key = &certificate->rsa" to "public_key = &certificate->pk" compiled, but CCLD fails.

src/src_tlsdate_helper-tlsdate-helper.o: In function `check_key_length':
/home/daniel/code/tlsdate/src/tlsdate-helper.c:850: undefined reference to `x509parse_dn_gets'
src/src_tlsdate_helper-tlsdate-helper.o: In function `run_ssl':
/home/daniel/code/tlsdate/src/tlsdate-helper.c:1008: undefined reference to `x509parse_crtpath'
/home/daniel/code/tlsdate/src/tlsdate-helper.c:1004: undefined reference to `x509parse_crtfile'
/home/daniel/code/tlsdate/src/tlsdate-helper.c:1107: undefined reference to `x509_free'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Makefile:1704: recipe for target 'src/tlsdate-helper' failed

this needs more investigation if mbedTLS 1.3 has to work.

vapier commented 8 years ago

(1) AS_IF is provided by autoconf itself. autoconf-2.69-6 on my system defines it in /usr/share/autoconf/m4sugar/m4sh.m4. same goes for all the other AC/AS macros you describe. tlsdate must not mess around with m4_pattern_allow here.

(2) if you want to build tlsdate from git and/or run autotools, you must install the relevant dev packages. that includes pkg-config which provides pkg.m4. not a bug in tlsdate.

(3) tlsdate probably could use some symbol checks. OpenSSL is not friendly when it comes to disabling crypto funcs as it basically breaks the ABI.

(4) this isn't necessarily a bug. having a program call free() on all allocated buffers before calling exit() is both entirely pointless and a waste of CPU/resources. the kernel will automatically free all the memory when programs exits.

(5) if you want to request polarssl support, please file a new issue. creating one bug report with a ton of unrelated issues only creates a mess.