[lighttpd] -mod-wolfssl inhibited by missing library functionality

[ghost]

lighttpd:1.4.56-3
lighttpd-mod-wolfssl: 1.4.56-3

Maintainer: @flyn-org
Tag: @gstrauss

libwolfssl24: 4.5.0-stable-2
Maintainer: @cotequeiroz

Environment: mvebu/cortexa9

---

# lighttpd -tt -f /etc/lighttpd/lighttpd.conf

(../src/mod_wolfssl.c.2052) SSL: WARNING: SNI callbacks *crippled* in wolfSSL library build

#14139 (comment)

wolfssl in openwrt was built without support for SNI, something that severely limits wolfssl use in a web server.

[gstrauss]

This is an issue with wolfssl in openwrt. It isn't something that can be fixed in lighttpd.

There are some additional choice words in my comment in https://git.lighttpd.net/lighttpd/lighttpd1.4/src/branch/master/src/mod_wolfssl.c#L2036

At the time -- and I believe this is still the case -- wolfssl built without --enable-opensslall was missing functionality that is required in modern web servers. I had reached out to wolfssl developers in July. I'll try again.

[gstrauss]

openwrt/package/libs/wolfssl/Makefile contains

CONFIGURE_ARGS += \
        --enable-opensslextra \
        --enable-sni \
        --enable-stunnel \

While that might work for stunnel, that does not necessarily help lighttpd using libwolfssl. Unfortunately, wolfssl is very, very, very, very, very messy with preprocessor defines that are over-used, inconsistently used, poorly named, and poorly designed (poorly levelized; poor API interface). Adding --enable-lighty would not make things work in the current wolfssl release v4.5.0. Due to inconsistent application of the overly-complex preprocessor defines, there are code paths which may silently fail in wolfssl unless wolfssl is built with --enable-opensslall

An example of the wolfssl design failures: HAVE_OPENSSLALL should be a feature set, which enables HAVE_SNI and other things. HAVE_OPENSSLALL should not be all over the code. Rather, the HAVE_SNI and other features it enabled should be in the code. Same goes for HAVE_STUNNEL, which should be a feature set which enables a set of HAVE_<feature>. Instead, HAVE_OPENSSLALL and HAVE_STUNNEL (and many others) are littered inconsistently throughout the code.

Here's some preprocessor defines in wolfssl v4.5.0 wrapping struct members, function definitions, and calls to those functions.

#if defined(OPENSSL_ALL) || (defined(OPENSSL_EXTRA) && (defined(HAVE_STUNNEL) || \
                             defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY) || \
                             defined(WOLFSSL_HAPROXY) || defined(WOLFSSL_OPENSSH) ))
    CallbackSniRecv sniRecvCb;
    void*           sniRecvCbArg;
#endif

#if defined(OPENSSL_ALL) || defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || \
    defined(WOLFSSL_HAPROXY)
    int SNI_Callback(WOLFSSL* ssl)
    {

    #if defined(OPENSSL_ALL) || defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY)
                if((ret=SNI_Callback(ssl)))

The next version of wolfssl (not yet released) does add HAVE_LIGHTY to more places, and I'll update lighttpd mod_wolfssl.c to check for that wolfssl version once it is released.

In the meantime, would openwrt developers suggest that I modify lighttpd code to check for HAVE_STUNNEL? I could do that in a patch for openwrt lighttpd build, but I do not want to add a "wolfssl-stunnel-specific" define into lighttpd code, as it really doesn't belong in lighttpd code.

@cotequeiroz would you consider adding --enable-lighty to the openwrt wolfssl Makefile? Would you consider adding --enable-opensslall? --enable-opensslall is the safest addition to avoid potential silent failures due to wolfssl code "not running" since code is not there to run. If you're willing to entertain these, then I'll put together some builds to see what the size difference is between the current openwrt wolfssl build --enable-opensslextra --enable-sni --enable-stunnel, versus a build with --enable-opensslall Thanks.

[cotequeiroz]

I was just looking for the SNI functions at the current wolfssl binaries with ldtest, and found both of the ones around https://git.lighttpd.net/lighttpd/lighttpd1.4/src/branch/master/src/mod_wolfssl.c#L2053
to be present:

# dltest /usr/lib/libwolfssl.so.24 wolfSSL_CTX_set_servername_callback
SUCCESS: Loaded /usr/lib/libwolfssl.so.24
SUCCESS: Found wolfSSL_CTX_set_servername_callback
# dltest /usr/lib/libwolfssl.so.24 wolfSSL_CTX_set_servername_arg
SUCCESS: Loaded /usr/lib/libwolfssl.so.24
SUCCESS: Found wolfSSL_CTX_set_servername_arg

Reading you latest explanations, it may not be an indication that it will actually work.

I'll try to see how much larger the library is going to be with the suggested options.

[gstrauss]

From a quick look, I think that SNI will work in the current build, due to --enable-stunnel. When I coded mod_wolfssl in lighttpd, it is likely that I did not want to add to the fragile complexity of wolfssl preprocessor defines by propagating non-lighttpd-specific defines into lighttpd.

As for silent failures, I don't have an example this second, but I seem to recall frustration with other TLS features not working with lighttpd mod_wolfssl until I enabled --enable-opensslall. If I remember correctly, the missing functionality had to do with client certificate validation, which is an optional feature not required for general use, but may be a part of more advanced web server configurations.

[gstrauss]

FYI: one example of silent failure with client certificate verification in lighttpd mod_wolfssl: in wolfssl wolfSSL_dup_CA_list() is a stub function. It failed silently. I submitted a patch to wolfSSL team on 2 Jul and it took them over 4 months to accept it. wolfSSL/wolfssl#3098 The patch will presumably be part of the next wolfssl release.

[gstrauss]

Bug in wolfssl:
wolfssl/wolfcrypt/settings.h contains:

#if defined(WOLFSSL_NGINX) || defined(WOLFSSL_QT) || defined(OPENSSL_ALL)
    #define SSL_CTRL_SET_TLSEXT_HOSTNAME 55
#endif

and if SSL_CTRL_SET_TLSEXT_HOSTNAME is not defined, then code in wolfSSL_ctrl in src/ssl.c does not execute. How many inconsistencies in wolfssl preprocessor directives can you spot in these next few lines from src/ssl.c? Note that HAVE_STUNNEL is not mentioned here, so I hope that it does not rely on this code path. (I have not checked but it probably does not rely on this since SSL_CTRL_SET_TLSEXT_HOSTNAME is probably not defined) lighttpd does not rely on this code path.

#if defined(OPENSSL_ALL) || defined(WOLFSSL_ASIO) || defined(WOLFSSL_HAPROXY) \
    || defined(WOLFSSL_NGINX) || defined(WOLFSSL_QT)

long wolfSSL_ctrl(WOLFSSL* ssl, int cmd, long opt, void* pt)
{
    WOLFSSL_ENTER("wolfSSL_ctrl");
    if (ssl == NULL)
        return BAD_FUNC_ARG;

    switch (cmd) {
        #if defined(WOLFSSL_NGINX) || defined(WOLFSSL_QT) || defined(OPENSSL_ALL)
        case SSL_CTRL_SET_TLSEXT_HOSTNAME:
            WOLFSSL_MSG("Entering Case: SSL_CTRL_SET_TLSEXT_HOSTNAME.");
        #ifdef HAVE_SNI
            if (pt == NULL) {
                WOLFSSL_MSG("Passed in NULL Host Name.");
                break;
            }
            return wolfSSL_set_tlsext_host_name(ssl, (const char*) pt);
        #else
            WOLFSSL_MSG("SNI not enabled.");
            break;
        #endif /* HAVE_SNI */
        #endif /* WOLFSSL_NGINX || WOLFSSL_QT || OPENSSL_ALL */
        default:
            WOLFSSL_MSG("Case not implemented.");
    }
    (void)opt;
    (void)pt;
    return WOLFSSL_FAILURE;
}

[gstrauss]

I submitted a pull request to wolfssl (wolfSSL/wolfssl#3538) to put all SNI-related code behind HAVE_SNI instead of the more complex (and inconsistent) set of preprocessor directives currently used.

For openwrt, what do you recommend for lighttpd mod_wolfssl and SNI? For openwrt, should I submit a patch which checks for HAVE_STUNNEL?

Separately, client certificate validation in lighttpd mod_wolfssl will not be an available feature with current openwrt build of wolfssl unless the openwrt build of wolfssl chooses to build with --enable-opensslall. I defer to @cotequeiroz for whether or not this is a worthwhile tradeoff in openwrt. Either way, I do hope that @cotequeiroz will add --enable-lighty to the openwrt build of wolfssl. Thanks.

[cotequeiroz]

I've just tried the suggested options. The size increase is not dramatic:

$ ls -orS
total 852
-rw-r--r-- 1 equeiroz 282089 Dec  5 16:18 original.ipk
-rw-r--r-- 1 equeiroz 282401 Dec  5 16:17 lighty.ipk
-rw-r--r-- 1 equeiroz 304612 Dec  5 16:28 opensslall.ipk

@flyn-org
I was doing a clean build of openwrt with lighttpd selected, but I forgot to add the wolfssl backend. Compilation failed because openssl was not found. I've looked at the Makefile, and found this:

ifneq ($(strip $(CONFIG_LIGHTTPD_SSL)),)
  ifeq ($(CONFIG_PACKAGE_lighttpd-mod-openssl),)
  ifeq ($(CONFIG_PACKAGE_lighttpd-mod-mbedtls),)
  ifeq ($(CONFIG_PACKAGE_lighttpd-mod-wolfssl),)
  ifeq ($(CONFIG_PACKAGE_lighttpd-mod-gnutls),)
  ifeq ($(CONFIG_PACKAGE_lighttpd-mod-nss),)
    CONFIG_PACKAGE_lighttpd-mod-openssl=m
  endif
  endif
  endif
  endif
  endif
endif

What's the intent here?

[gstrauss]

@cotequeiroz the intent of that Makefile snippet is to ensure that at least one TLS module is selected if CONFIG_LIGHTTPD_SSL is enabled. Since mod_openssl was previously the only option, mod_openssl is the one selected for compatibility.

[cotequeiroz]

A failproof way to do this is by adding some symbols to Package/lighttpd/config:

config LIGHTTPD_SSL_DEPENDS
        bool
        depends on LIGHTTPD_SSL
        default PACKAGE_lighttpd-mod-mbedtls || PACKAGE_lighttpd-mod-wolfssl || PACKAGE_lighttpd-mod-gnutls || PACKAGE_lighttpd-mod-nss

config LIGHTTPD_SSL_SELECT
        tristate
        depends on LIGHTTPD_SSL
        default m if !LIGHTTPD_SSL_DEPENDS
        select PACKAGE_lighttpd-mod-openssl

The same logic could be applied to mod-authn_file selection.

[gstrauss]

Thanks for those pointers @cotequeiroz I'll do some experiments for a future update.

I traced my development and found the reason why lighttpd mod_wolfssl is coded to require OPENSSL_ALL for SNI. If wolfssl is built --enable-sni, then one reasonably expects that SNI will work. However, that is not the case with the overly complex and inconsistent wolfssl preprocessor logic. Only --enable-opensslall works 100% of the time without relying on a mishmash of other preprocessor directives like HAVE_STUNNEL.

To be practical, I'll consider having lighttpd mod_wolfssl check for HAVE_STUNNEL, even though it is messy.

I am sorry that so many people (including me) have wasted time on this. Hopefully, the wolfssl team will get the message in wolfSSL/wolfssl#3538

[cotequeiroz]

I've just sent a patch series to the openwrt mailing list, enabling both --enable-lighty, and --enable-opensslall. Since wolfssl is used as the default TLS/crypto backend, size matters, so I'm sending it as an RFC. Sorry, I should have CC'ed you, but I realized I hadn't after sending it 🤦‍♂️.

[gstrauss]

Thanks @cotequeiroz

For my part, I filed #14153 with a patch for lighttpd mod_wolfssl SNI detection, and I'll include the patch in the next official lighttpd release.

FYI: this is the problematic code (similar in three places in wolfssl) in the current v4.5.0 release of wolfssl. Notice that neither HAVE_SNI or HAVE_LIGHTY are present. Without the preprocessor logic below, the SNI callbacks are not executed in wolfssl.

    #if defined(OPENSSL_ALL) || defined(HAVE_STUNNEL) || defined(WOLFSSL_NGINX) || defined(WOLFSSL_HAPROXY)
                if((ret=SNI_Callback(ssl)))
                    return ret;
                ssl->options.side = WOLFSSL_SERVER_END;
    #endif

[cotequeiroz]

The wolfssl changes I proposed were accepted: openwrt/openwrt@f31c9cd and openwrt/openwrt@4e8a001

@n8v8R, or anyone, can you test to see if it works now?

[gstrauss]

Thanks, @cotequeiroz
I needed to fix some newly discovered bugs in lighttpd, and lighttpd 1.4.57 has just been released.
I'll try to test with wolfssl on openwrt this weekend.

[gstrauss]

I can confirm that SNI is working now for lighttpd with mod_wolfssl. Tested with lighttpd 1.4.59 and wolfssl 4.6.0 on

DISTRIB_RELEASE='19.07.6'
DISTRIB_TARGET='ar71xx/generic'
DISTRIB_ARCH='mips_24kc'

(Sorry for the delay in getting back to this)