pi-hole / FTL

The Pi-hole FTL engine
https://pi-hole.net
Other
1.34k stars 187 forks source link

dev: webserver doesn't properly detect mbedtls dependency #1966

Open oliv3r opened 1 month ago

oliv3r commented 1 month ago

When building the latest development-v6 branch, mbedtls isn't properly guarded out.

CMake will detect the missing mbed tls and say that it won't compile with TLS; however the webserver will still be compiled regardless, resulting in an obvious missing header error.

In file included from /src/src/args.c:56:
/src/src/webserver/x509.h:13:10: fatal error: mbedtls/entropy.h: No such file or directory
   13 | #include <mbedtls/entropy.h>
      |          ^~~~~~~~~~~~~~~~~~~
compilation terminated.
gmake[2]: *** [src/CMakeFiles/FTL.dir/build.make:76: src/CMakeFiles/FTL.dir/args.c.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:413: src/CMakeFiles/FTL.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2

Not sure if the lack of TLS should mean: no webserver; or if the webserver could be build without tls.

btw, even adding mbed-tls still bombs, no idea why/how this is caused.

[ 76%] Building C object src/webserver/civetweb/CMakeFiles/civetweb.dir/civetweb.c.o
/src/src/webserver/civetweb/mod_mbedtls.inl: In function 'mbed_ssl_accept':
/src/src/webserver/civetweb/mod_mbedtls.inl:216:73: error: 'mbedtls_ssl_context' has no member named 'MBEDTLS_PRIVATE'
  216 |         DEBUG_TRACE("TLS connection %p accepted, state: %d", ssl, (*ssl)->MBEDTLS_PRIVATE(state));
      |                                                                         ^~
/src/src/webserver/civetweb/civetweb.c:245:53: note: in definition of macro 'DEBUG_TRACE'
  245 |                 log_web("DEBUG: " fmt " (%s:%d)", ##__VA_ARGS__, short_path(__FILE__), __LINE__); }
      |                                                     ^~~~~~~~~~~
/src/src/webserver/civetweb/mod_mbedtls.inl:216:91: error: 'state' undeclared (first use in this function); did you mean 'stat'?
  216 |         DEBUG_TRACE("TLS connection %p accepted, state: %d", ssl, (*ssl)->MBEDTLS_PRIVATE(state));
      |                                                                                           ^~~~~
/src/src/webserver/civetweb/civetweb.c:245:53: note: in definition of macro 'DEBUG_TRACE'
  245 |                 log_web("DEBUG: " fmt " (%s:%d)", ##__VA_ARGS__, short_path(__FILE__), __LINE__); }
      |                                                     ^~~~~~~~~~~
/src/src/webserver/civetweb/mod_mbedtls.inl:216:91: note: each undeclared identifier is reported only once for each function it appears in
  216 |         DEBUG_TRACE("TLS connection %p accepted, state: %d", ssl, (*ssl)->MBEDTLS_PRIVATE(state));
      |                                                                                           ^~~~~
/src/src/webserver/civetweb/civetweb.c:245:53: note: in definition of macro 'DEBUG_TRACE'
  245 |                 log_web("DEBUG: " fmt " (%s:%d)", ##__VA_ARGS__, short_path(__FILE__), __LINE__); }
      |                                                     ^~~~~~~~~~~
/src/src/webserver/civetweb/mod_mbedtls.inl: In function 'mbed_ssl_handshake':
/src/src/webserver/civetweb/mod_mbedtls.inl:242:63: error: 'mbedtls_ssl_context' has no member named 'MBEDTLS_PRIVATE'
  242 |         DEBUG_TRACE("TLS handshake rc: %d, state: %d", rc, ssl->MBEDTLS_PRIVATE(state));
      |                                                               ^~
/src/src/webserver/civetweb/civetweb.c:245:53: note: in definition of macro 'DEBUG_TRACE'
  245 |                 log_web("DEBUG: " fmt " (%s:%d)", ##__VA_ARGS__, short_path(__FILE__), __LINE__); }
      |                                                     ^~~~~~~~~~~
/src/src/webserver/civetweb/mod_mbedtls.inl:242:81: error: 'state' undeclared (first use in this function); did you mean 'stat'?
  242 |         DEBUG_TRACE("TLS handshake rc: %d, state: %d", rc, ssl->MBEDTLS_PRIVATE(state));
      |                                                                                 ^~~~~
/src/src/webserver/civetweb/civetweb.c:245:53: note: in definition of macro 'DEBUG_TRACE'
  245 |                 log_web("DEBUG: " fmt " (%s:%d)", ##__VA_ARGS__, short_path(__FILE__), __LINE__); }
      |                                                     ^~~~~~~~~~~
gmake[2]: *** [src/webserver/civetweb/CMakeFiles/civetweb.dir/build.make:76: src/webserver/civetweb/CMakeFiles/civetweb.dir/civetweb.c.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:559: src/webserver/civetweb/CMakeFiles/civetweb.dir/all] Error 2
gmake: *** [Makefile:136: all] Error 2
DL6ER commented 1 month ago

Building without mbedtls

This should work, I will check out what is going wrong and fix this. It's probably just what the error you have seen suggests: We have to guard all the Pi-hole X.509 features. The webserver is meant to be functional even without TLS.

mbedtls compile errors

Please check the version you have installed, FTL is meant to be built against the latest stable release, 3.5.0. I will add an internal check against the minimum supported version to print a clear error message.

Check out the v6.0 documentation draft for a detailed step-by-step to compile mbedtls from source as system-provided versions tend to be ancient.

oliv3r commented 1 month ago

Not sure if I want to go down that rabbit hole :) I just did apk add mbedtls-dev :p (on alpine:latest or alpine:edge)

DL6ER commented 1 month ago

I did just try installing mbedTLS v3.6.0 and recompiling FTL against it and it worked flawlessly. You could also use Pi-hole's ftl-build multi-arch containers instead of trying to create your own environment if you prefer.

Anyway, I don't think this is a rabbit hole, it's just the few lines I put up there that needs to be executed. I will soon try to debug this further but alpine:edge is currently broken concerning gmp preventing any FTL builds at the moment. I have reported this upstream but resolution is pending.

oliv3r commented 1 month ago

I'm actually not really trying to set up my own build env, but rather trying to package it for Alpine.

For now, its packaged fine, but for 2.25.2 the patches are needed and for Dev, as you said things are broken.

DL6ER commented 1 month ago

@oliv3r Please check again with the latest development-v6. Meanwhile, we changed the C standard from 11 to 17 and made the code compatible with clang. This involved a huge amount of small fixes and changes. The code does not compile flawlessly using both gcc and clang on alpine:latest.