Adds support for building Lighttpd with wolfSSL #92

dgarske · 2018-09-21T17:31:43Z

Adds support for building Lighttpd with wolfSSL as a TLS and Crypto provider. Adds a new ./configure --with-wolfssl option.

gstrauss

Thank you for putting this together. I have done a quick once-over and there are a number of items that can be addressed. I would like to spend a bit more time looking through the wolfSSL callbacks code, so I may have more comments.

Shortly, I will push some changes from this patch which are not wolfSSL-related, so please look for those and remove them from this pull request.

gstrauss · 2018-09-26T00:57:25Z

configure.ac

+
+
+dnl Check for wolfSSL
+ENABLED_WOLFSSL=no


The convention using in lighttpd configure.ac is WITH_xxxxx. Please use WITH_WOLFSSL

doc/wolfssl/README

gstrauss · 2018-09-26T01:04:59Z

doc/wolfssl/README

+
+Prior to the log being opened, log messages will be directed to stderr.
+
+To use lighttpd logging the following function is called with a lighttpd


Why is this mentioned here?
"arguement" is spelled "argument"
While I do not particularly like the historical lighttpd log_error_write() interface, your example is not minimal. The format string "s" indicates as single string following, so ... "s", "desired message"), not ... "ss", "", "desired message")

gstrauss · 2018-09-26T01:06:55Z

src/server.c

@@ -2048,6 +2048,10 @@ int main (int argc, char **argv) {
    }
  #endif

+  #ifdef DEBUG_WOLFSSL


This belongs in mod_openssl.c in the mod_openssl_init() function, which is called when the module is loaded. The call does not belong in server.c

gstrauss · 2018-09-26T01:22:15Z

src/first.h

@@ -55,4 +55,8 @@

 #define UNUSED(x) ( (void)(x) )

+#ifdef HAVE_WOLFSSL_SSL_H


This does not belong here. There is only compiler and C library setup here. What was your reasoning for adding something so wolfSSL specific here, to be included in every file?

If the other modules which check for "#if defined HAVE_LIBSSL && defined HAVE_OPENSSL_SSL_H" should be isolated into a new lighttpd header, such as sys-crypto.h, then let's create such an abstraction.

gstrauss · 2018-09-26T01:26:18Z

src/mod_authn_file.c

@@ -600,7 +600,15 @@ static void apr_sha_encode(const char *pw, char *result, size_t nbytes) {
    unsigned char digest[20];
    size_t base64_written;

+#ifdef HAVE_WOLFSSL_SSL_H


see above suggestion for sys-crypto.h

If wolfSSL does not provide SHA1(), then we can create a macro such as

#define SHA1(str,len,digest) do { SHA_CTX sha1; SHA1_Init(&sha1); SHA1_Update(&sha1, (const unsigned char *)(str), (len)); SHA1_Final((digest), &sha1); }

To avoid potential conflict with user-variable named 'sha1', I would use something a bit more unique, or make this a static inline function. lighttpd is built to support (most of) C99.

Actually, if wolfSSL does not provide SHA1() then it does not need to be an inline function. It could simply be provided in algo_sha1.[ch].

gstrauss · 2018-09-26T01:27:53Z

src/mod_authn_file.c

@@ -646,6 +654,7 @@ static handler_t mod_authn_file_htpasswd_basic(server *srv, connection *con, voi
            #endif
           #endif
           #ifdef USE_OPENSSL_CRYPTO /* (for MD4_*() (e.g. MD4_Update())) */


Please add a comment that says that wolfSSL defines this if MD4 is not provided, e.g.

#ifndef NO_MD4 /*(defined if support for MD4 is not built into wolfSSL)*/

MD4 is disabled in wolfSSL by default. However, MD4 is an available wolfSSL configure option, should the user desire it.

gstrauss · 2018-09-26T01:29:13Z

src/mod_dirlisting.c

@@ -87,6 +87,7 @@ static excludes_buffer *excludes_buffer_init(void) {
 	return exb;
 }



Incorrect (or at least misleading). This looks to be part of warnings generated if lighttpd is built ./configure --without-pcre and should be addressed separately.

gstrauss · 2018-09-26T01:34:50Z

src/mod_secdownload.c

@@ -18,7 +18,15 @@
 #ifdef USE_OPENSSL_CRYPTO
 #include <openssl/evp.h>
 #include <openssl/hmac.h>
-#endif


I have run into the CONST_STR_LEN() and CONST_BUF_LEN() issues with other inline macros, such as strncasecmp() and would rather can the affected two lines of code to replace CONST_BUF_LEN(), but without reaching into the buffer innards directly. In other words, use buffer_string_length(), which is an inline function and the extra few instructions will be dwarfed by the HMAC calculations.

gstrauss · 2018-09-26T01:51:15Z

autogen.sh

@@ -10,5 +10,7 @@ fi

 # old autoreconf/aclocal versions fail hard if m4 doesn't exist
 mkdir -p m4
+# some versions fail if it doesn't exist
+touch ./config.rpath


Is this the right thing to do? Would you provide more details? (autoconf can be quite opaque and obtuse. lighttpd does include some m4 scripts to address some shortcomings or limitations in some distributions. Is there a better way to get past the error you are seeing? When are you seeing an error? What versions/platforms?)

Related, lighttpd supports multiple build systems, so if we add --with-wolfssl to configure.ac, we should also update the CMake, SCons, and meson build scripts, too.

gstrauss · 2018-09-26T01:54:22Z

#if defined HAVE_LIBSSL && defined HAVE_OPENSSL_SSL_H
#define USE_OPENSSL_CRYPTO
#endif

Related to my comment about creating a sys-crypto.h, to enable use of wolfSSL, we may need to move the above logic from a few files into sys-crypto.h, too.

gstrauss · 2018-09-26T06:44:30Z

src/mod_openssl.c

+    server* srv = (server*)ctx;
+    WC_RNG  rng;
+
+    (void)ssl;


Please use the UNUSED() macro for unused variables instead of explicitly casting to (void). In the future, the UNUSED() macro might include attribute((unused)) for compilers which support that attribute.

TimParrish · 2018-09-28T15:43:41Z

Thank you for the feedback, we are working to address these comments and will push to this branch when they are ready.

gstrauss · 2018-10-01T05:31:15Z

doc/wolfssl/README

-server *srv arguement:
-    log_error_write(srv, __FILE__, __LINE__, "ss", "", "desired message");
+server *srv argument:
+    log_error_write(srv, __FILE__, __LINE__, "s", "", "desired message");


The new code is incorrect and the last argument is unused. Please remove this code example from the documentation. log_error_write() is part of the lighttpd basic interfaces and is not specific to mod_openssl or wolfSSL.

gstrauss · 2018-10-01T05:37:26Z

Please rebase dgarske:lighttpd_wolfssl against lighttpd:master and force push dgarske:lighttpd_wolfssl so that changes that I have made to lighttpd:master are not adding noise to the patches here. Also, if you like, you may squash into the original commit the changes made to address my feedback.

gstrauss · 2018-10-04T19:58:29Z

Repeating what I posted 4 days ago:
Please rebase dgarske:lighttpd_wolfssl against lighttpd:master and force push dgarske:lighttpd_wolfssl so that changes that I have made to lighttpd:master are not adding noise to the patches here. Also, if you like, you may squash into the original commit the changes made to address my feedback.

Do not include patches which are unrelated to wolfSSL in this pull request. Do not include patches related to compiling ./configure --without-pcre. Those are completely unnecessary and a waste of both my time and yours since I have already addressed these in lighttpd git master and have asked you to rebase your patch without them.

dgarske · 2018-10-04T20:23:56Z

@gstrauss : Thanks for the feedback. I will finalize this cleanup right now based on your feedback and force push a rebase/squash merge.

…rovider. Adds a new ./configure `--with-wolfssl` option.

gstrauss · 2018-10-05T05:27:34Z

The PK callbacks code looks to be removed in the latest push. Was that intentional?

Please re-review the feedback I have submitted. I hope I am misunderstanding you comment about "finalized this cleanup", since there are a number of remaining items from my feedback that need to be addressed, including the re-appearance of the private key in your most recent push.

requires wolfSSL library version 3.15.3 or later https://www.wolfssl.com/ https://github.com/wolfSSL/wolfssl (thx dgarske) x-ref: "Adds support for building Lighttpd with wolfSSL" #92

gstrauss · 2018-10-08T04:06:44Z

I updated lighttpd git master with support for wolfSSL, including build support for autoconf, meson, CMake, and SCons.

Remaining items for this pull request include only the doc/wolfssl contributions and the potential re-addition of the PK callbacks. Please remove the private keys, remove the reference to wolfSSL_Debugging_ON() in the doc, remove trailing spaces in doc and other files.

RFE for wolfSSL: the wolfSSL openssl compatibility headers should all #include <wolfssl/options.h> instead of lighttpd having to do it.

gstrauss · 2018-10-08T04:37:01Z

After further review of the remaining patch, I am going to close this pull request. The gist is that wolfssl needs to be built with ./configure --enable-lighty, and then lighttpd needs to be built with ./configure --with-wolfssl

If you would like to add back code for the PK callbacks, please submit a more specific pull request.

If you post the documentation that you have written on the wolfssl.com website somewhere under support, then I will link to it from lighttpd's online documentation https://redmine.lighttpd.net/projects/lighttpd/wiki/Docs_SSL

Thank you for your contribution. Given the work put into the wolfSSL openssl-compatibility layer, the changes needed for lighttpd to use wolfSSL were quite small: e9f223d e1f21b2 0074b6d

dgarske · 2018-10-08T16:21:47Z

Hi @gstrauss,

Thanks so much for your work on integrating the lighttpd changes to support wolfSSL! I pulled down the latest master and confirmed the changes work correctly with automake (./configure --with-wolfssl=yes) and cmake (cmake -DWITH_WOLFSSL=ON ..).

We will work on getting documentation posted on our website so you can add a link to it here (https://redmine.lighttpd.net/projects/lighttpd/wiki/Docs_SSL). For now other folks can find the "old" documentation here: https://github.com/dgarske/lighttpd1.4/tree/lighttpd_wolfssl/doc/wolfssl

The PK callback code was example code for supporting external crypto hardware for ECC operations. That code is 100% optional shouldn't have been in the upstream submission.

Thanks,
David Garske, wolfSSL

gstrauss · 2018-10-09T00:23:33Z

Thank you @dgarske

Where may I submit an RFE for wolfSSL? The wolfSSL openssl compatibility headers should all #include <wolfssl/options.h> instead of lighttpd having to do it.

dgarske · 2018-10-09T16:45:51Z

Hi @gstrauss : I've captured this enhancement request internally, but feel free to submit a GitHub issue for it here: https://github.com/wolfSSL/wolfssl/issues

I agree it seems like inclusion of the wolfssl/options.h could happen in our library.

Thanks,
David Garske, wolfSSL

roytam1 · 2018-12-04T08:06:36Z

I wonder if I can use static wolfssl/wolfcrypt for lighttpd use? (my system in centos5 and I don't want to change system's openssl-0.9.8)

dgarske · 2018-12-10T22:25:17Z

@roytam1 : The wolfSSL library can be built as a shared library and installed without affecting your existing openssl version. They are two different libraries and can both be installed.

To use lighttpd with wolfSSL the steps are:

WolfSS: ./configure --enable-lighty && make && sudo make install
Lighttpd: ./configure --with-wolfssl=yes && make && sudo make install

roytam1 · 2018-12-18T04:26:04Z

@dgarske it seems not working?

mod_openssl.c: In function ‘network_ssl_servername_callback’:
mod_openssl.c:401:9: warning: implicit declaration of function ‘SSL_set_client_CA_list’ [-Wimplicit-function-declaration]
         SSL_set_client_CA_list(
         ^
mod_openssl.c: In function ‘network_init_ssl’:
mod_openssl.c:983:16: warning: passing argument 2 of ‘wolfSSL_CTX_set_tlsext_servername_callback’ from incompatible pointer type [enabled by default]
                s->ssl_ctx, network_ssl_servername_callback) ||
                ^
In file included from /usr/local/include/wolfssl/openssl/ssl.h:35:0,
                 from mod_openssl.c:15:
/usr/local/include/wolfssl/ssl.h:2816:17: note: expected ‘CallbackSniRecv’ but argument is of type ‘int (*)(struct SSL *, int *, struct server *)’
 WOLFSSL_API int wolfSSL_CTX_set_tlsext_servername_callback(WOLFSSL_CTX *,
                 ^
mod_openssl.c:984:13: error: invalid use of void expression
             !SSL_CTX_set_tlsext_servername_arg(s->ssl_ctx, srv)) {
             ^
make[3]: *** [mod_openssl.lo] Error 1

gstrauss · 2018-12-18T05:22:09Z

Did you build and install WolfSSL with the following?
./configure --enable-lighty && make && sudo make install

roytam1 · 2018-12-18T05:44:49Z

./configure --enable-lighty && make && sudo make install

alright, so having other --enable- {other-applications} causing compile failure.

gstrauss · 2018-12-18T14:20:37Z

@roytam1 that is incomplete information and likely belongs as an issue on WolfSSL issue page rather than here.

dgarske · 2018-12-18T18:57:35Z

Hi @roytam1 : It looks like there are some missing openssl compatiblity API's in wolfSSl with the latest Lighttpd. The last version we ported to and support is Lighttpd v1.4.51 on Aug 15, 2018 (a9e131f)

Attached are the sources and patch for Lighttpd
lighttpd-1.4.51.tar.gz
Lighttpd_1_4_51.txt

See README in wolfStartup folder for details.

Thanks,
David Garske, wolfSSL

dgarske · 2018-12-18T19:06:35Z

@roytam1 : Also it looks like OPENSSL_NO_TLSEXT should get defined to prevent the above errors with mod_openssl.c. On my builds that gets defined as the top of mod_openssl.c:27. If its not getting defined try adding CFLAGS="-DOPENSSL_NO_TLSEXT" to your Lighttpd `./configure.

Please make sure you are using the following steps:

wolfSSL:

./autogen.sh (if cloned from GitHub directly)
./configure --enable-lighty && make
sudo make install
sudo ldconfig

Lighttpd:

./autogen.sh (if cloned from GitHub directly)
./configure --with-wolfssl=yes
make

Thanks,
David Garske, wolfSSL

roytam1 · 2018-12-20T13:06:02Z

Hi @roytam1 : It looks like there are some missing openssl compatiblity API's in wolfSSl with the latest Lighttpd. The last version we ported to and support is Lighttpd v1.4.51 on Aug 15, 2018 (a9e131f)

@dgarske would you please create new pull request for this?

dgarske · 2018-12-20T19:38:38Z

@roytam1 : I've updated the wolfSSL to the latest Lighttpd commit in master a1077d1.

Many of the original fixes from this PR were already done in Lighttpd master. The changes are minimal and you can find the latest branch here:
https://github.com/dgarske/lighttpd1.4/tree/wolfssl_lighttpd

See the README in wolfStartup for details.

The steps above for building are still valid.

Thanks,
David Garske, wolfSSL

gstrauss requested changes Sep 26, 2018

View reviewed changes

gstrauss reviewed Sep 26, 2018

View reviewed changes

gstrauss reviewed Oct 1, 2018

View reviewed changes

dgarske force-pushed the lighttpd_wolfssl branch from 18eaf24 to 4c255d1 Compare October 1, 2018 19:14

Adds support for building Lighttpd with wolfSSL as a TLS and Crypto p…

be48999

…rovider. Adds a new ./configure `--with-wolfssl` option.

dgarske force-pushed the lighttpd_wolfssl branch from 717b6d4 to be48999 Compare October 4, 2018 23:06

gstrauss closed this Oct 8, 2018

gstrauss mentioned this pull request Oct 28, 2018

include wolfssl/options.h in settings.h wolfSSL/wolfssl#1899

Closed

dgarske deleted the lighttpd_wolfssl branch December 20, 2018 19:36


		Prior to the log being opened, log messages will be directed to stderr.

		To use lighttpd logging the following function is called with a lighttpd

		@@ -55,4 +55,8 @@

		#define UNUSED(x) ( (void)(x) )

		#ifdef HAVE_WOLFSSL_SSL_H

		@@ -87,6 +87,7 @@ static excludes_buffer *excludes_buffer_init(void) {
		return exb;
		}

Adds support for building Lighttpd with wolfSSL #92

Adds support for building Lighttpd with wolfSSL #92

Conversation

dgarske commented Sep 21, 2018

gstrauss left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gstrauss commented Sep 26, 2018

Choose a reason for hiding this comment

TimParrish commented Sep 28, 2018 • edited Loading

Choose a reason for hiding this comment

gstrauss commented Oct 1, 2018

gstrauss commented Oct 4, 2018 • edited Loading

dgarske commented Oct 4, 2018

gstrauss commented Oct 5, 2018

gstrauss commented Oct 8, 2018 • edited Loading

gstrauss commented Oct 8, 2018

dgarske commented Oct 8, 2018

gstrauss commented Oct 9, 2018

dgarske commented Oct 9, 2018

roytam1 commented Dec 4, 2018

dgarske commented Dec 10, 2018

roytam1 commented Dec 18, 2018

gstrauss commented Dec 18, 2018

roytam1 commented Dec 18, 2018

gstrauss commented Dec 18, 2018

dgarske commented Dec 18, 2018

dgarske commented Dec 18, 2018

roytam1 commented Dec 20, 2018

dgarske commented Dec 20, 2018

TimParrish commented Sep 28, 2018 •

edited

Loading

gstrauss commented Oct 4, 2018 •

edited

Loading

gstrauss commented Oct 8, 2018 •

edited

Loading