New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache module duplicates extra response headers; moddecompress error #85

Closed
LQ2-apostrophe opened this Issue Nov 10, 2017 · 8 comments

Comments

Projects
None yet
2 participants
@LQ2-apostrophe
Copy link

LQ2-apostrophe commented Nov 10, 2017

Edit: A new update of this issue is written in detail here.

I am using Ubuntu 16.04 and I have successfully compiled and installed OpenLiteSpeed 1.4.28 with BoringSSL based on the solution given by @darkspadez here.

Since then, there has been two minor errors, but I don't know which part in the source code causes them.

Problem 1: OLS cache module sometimes mimics the same HSTS header that I have set in virtual host configurations.

In all my virtual host configurations, I set up a custom response header in Context tab like this:

Type: Static
URI: /
Location: $DOC_ROOT
Accessible: Yes
Extra Headers: Strict-Transport-Security: max-age=31536000; includeSubDomains
Rewrite Base: /
Rewrite Rules: rewriteFile /path/to/.htaccess

There is only one custom header like that, for each virtual host.

I have set up cache module's parameters like this:

enableCache 0
qsCache 1
reqCookieCache 1
respCookieCache 1
ignoreReqCacheCtrl 1
ignoreRespCacheCtrl 0
enablePrivateCache 1
privateExpireInSeconds 1000
expireInSeconds 1000
storagePath cachedata
checkPrivateCache 1
checkPublicCache 1

I have installed LSPHP 7.1 (with OPcache) from your repository. My websites are using WordPress. I am using LSCWP for caching and minifying HTML, JS and CSS.

Before, when I compiled OLS with OpenSSL 1.0.2, I did not see double HSTS headers. Sometimes links to minified JS and CSS didn't have HSTS header returned along, but that's not a big problem.

But now I see double HSTS headers, even on websites where I don't use LSCWP.

Strict-Transport-Security: max-age=31536000; includeSubDomains (Original header)
Strict-Transport-Security: max-age=31536000; includeSubDomains (Imitation)

Sometimes the accidental imitation is incomplete:

Strict-Transport-Security: max-age=31536000; includeSubDomains (Original header)
Strict-Transport-Security: 31536000; includeSubDomains (Imitation)

This imitation problem only happens on links of cached PHP resources (including pages, posts and minified assets). Not on links of static assets (JS/CSS/images) under $DOC_ROOT.

Problem 2: moddecompress error

This problem also happens after I compiled OLS with BoringSSL.

I haven't activated moddecompress. But sometimes I get this error, can be seen on OLS dashboard and the error logs of my virtual hosts.

[<some external IP>:<port>] [moddecompress+++] compressbuf in 0, inflate/deflate return -5

Please provide a fix to the problems I have mentioned above.

Additional info: For BoringSSL, I install Go library from existing Ubuntu repository:

sudo apt-get install golang-go
@darkspadez

This comment has been minimized.

Copy link

darkspadez commented Nov 10, 2017

For problem #1, Do you have any other plugins or settings for HSTS? I just tried on a clean install with the BSSL patch and 1.4.28 and I only receive one header. I tested this with cache and without cache. Is it possible to get a copy of your httpd_config.xml? You can remove sensitive information.

For problem #2, This does seem like a bug but should not break anything. This has to do with GZIP compression and does not require to load a separate module. For this one, do you have anyway to reproduce this or is it completely random?

@LQ2-apostrophe

This comment has been minimized.

Copy link
Author

LQ2-apostrophe commented Nov 11, 2017

For problem 1, here is the file. httpd_config.zip

I have two other plugins with the ability of writing HSTS headers, but I turned off their HSTS settings, before and after I reinstalled OLS with BoringSSL. I have checked their behavior in another WordPress installation at localhost, they don't output HSTS headers when I turn off their HSTS settings.

And problem 2 maybe random. But exactly the same error message with some external IPs. I have checked these IPs on iplocation.net and found that they are hosted by either Microsoft Azure or Google.

@LQ2-apostrophe

This comment has been minimized.

Copy link
Author

LQ2-apostrophe commented Nov 11, 2017

@darkspadez
I have just updated the first post of this issue a little bit to clarify problem 1.

Also, I have some new updates of problem 1. It appears that this problem only happens on a WordPress installation. But there is a worse problem: This weird imitation happens on every extra header that I write in Extra Headers field.

Set up the test

I set up two different listeners A and B. Both are using valid certificates. For each listener, I link with a seperate virtual host (A, B). Then I set up each virtual host with a seperate document root. Virtual host A contains simple PHP files that do not change results. Virtual host B contains a completely new WordPress installation.

Here are the detailed settings I used for each virtual host.

Generic settings:

Index Files: index.php

Some important SSL settings:

Chained Certificate: Yes

Protocol Version: TLS 1.2, TLS 1.3
Ciphers: ECDHE-RSA-CHACHA20-POLY1305:ECDHE-RSA-AES256-GCM-SHA384:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-SHA384:ECDHE-RSA-AES128-SHA256
Enable ECDH Key Exchange: Yes
Enable DH Key Exchange: No

Enable SPDY/HTTP2: SPDY/2, SPDY/3, HTTP/2

Some important context settings in virtual hosts:

Type: Static
URI: /
Location: $DOC_ROOT
Accessible: Yes

Extra Headers:
Strict-Transport-Security: max-age=31536000; includeSubDomains
Test-1: This-is-dummy
Test-2: This-dummy-header-is-used-for-checking-weird-behavior

Cache module configuration, shows that the private cache is turned on:

enableCache 0
qsCache 1
reqCookieCache 1
respCookieCache 1
ignoreReqCacheCtrl 1
ignoreRespCacheCtrl 0
enablePrivateCache 1
privateExpireInSeconds 1000
expireInSeconds 1000
storagePath cachedata
checkPrivateCache 1
checkPublicCache 1

Rewrite configurations does not matter, but I list them here for completeness.

Enable Rewrite: Yes
Log Level: 0

Rewrite Rules:
RewriteRule ^index\.php$ - [L]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule . /index.php [L]

The tree under $DOC_ROOT, in virtual host A:

$DOC_ROOT/
├─ index.php
└─ step2/
   └─ index.php

The content of index.php:

<?php
echo "Hello World!";

And in virtual host B, I set up a new WordPress installation. I use MariaDB 10.0.31 for the database.

Test steps

Please note that I use relative URLs here. I use browser's developer tools to watch the response headers.

Initial load (not cached)

A: Navigate /, then /step2/.
B: Enter home page, then Hello World post.

Second load (private cache hit)

A: Navigate back to /. Optionally navigate to /step2/ again.
B: Return back to home page. Optionally enter Hello World again.

After that, clear the cache by removing all items in $SERVER_ROOT/cachedata. Then redo this process above.

Test results

Initial load

This duplication problem does not happen on both A and B.

Second load

A: This problem does not happen.
B: Duplication happens with all extra headers:

Strict-Transport-Security: max-age=31536000; includeSubDomains
Strict-Transport-Security: max-age=31536000; includeSubDomains
Test-1: This-is-dummy
Test-1: This-is-dummy
Test-2: This-dummy-header-is-used-for-checking-weird-behavior
Test-2: This-dummy-header-is-used-for-checking-weird-behavior

How interesting!

@LQ2-apostrophe LQ2-apostrophe changed the title Cache module accidentally output the same HSTS header; moddecompress error Cache module duplicates extra response headers; moddecompress error Nov 11, 2017

@darkspadez

This comment has been minimized.

Copy link

darkspadez commented Nov 13, 2017

Taking a look I see that you are using Private Cache. Is there a specific reason for this?

I was able to reproduce the error and seems it is related to private cache. Will have the developer take a look.

@LQ2-apostrophe

This comment has been minimized.

Copy link
Author

LQ2-apostrophe commented Nov 14, 2017

I am using private cache because my websites use HTTPS.

@darkspadez

This comment has been minimized.

Copy link

darkspadez commented Nov 14, 2017

Private Cache is more needed when your sites have pages they need to cache to specific users/sessions. You can use Public Cache and still use HTTPS and it is actually recommended in this case. Especially if you use WordPress our LSCache plugin for WordPress which will auto handle purging and caching that Private Cache cannot. Our developer is looking into the issue though of duplicate headers.

@darkspadez

This comment has been minimized.

Copy link

darkspadez commented Nov 14, 2017

The bug has been fixed and will be in the next release, if you want to use it now you can rebuild OLS with the following patch and just replace the binary.

diff --git a/src/http/httprespheaders.cpp b/src/http/httprespheaders.cpp
index 951362d..323faa3 100644
--- a/src/http/httprespheaders.cpp
+++ b/src/http/httprespheaders.cpp
@@ -254,8 +254,11 @@ static int hasValue(const char *existVal, int existValLen, const char *val,
 
 
 //method: 0 replace,1 append, 2 merge, 3 add
+//headerIndex may be a invalid index, so just for compare with Set-cookie,
+//              Can not use to retrive name and nameLen
 int HttpRespHeaders::_add(int kvOrderNum, const char *pName, int nameLen,
-                          const char *pVal, unsigned int valLen, int method)
+                          const char *pVal, unsigned int valLen, int method,
+                          INDEX headerIndex)
 {
     assert(kvOrderNum >= 0);
     resp_kvpair *pKv;
@@ -278,9 +281,11 @@ int HttpRespHeaders::_add(int kvOrderNum, const char *pName, int nameLen,
         return 0;
     }
 
-    if ((method == LSI_HEADEROP_MERGE) && (pKv->valLen > 0))
+    if ((method == LSI_HEADEROP_MERGE || method == LSI_HEADEROP_ADD) 
+        && (pKv->valLen > 0))
     {
-        if (hasValue(getVal(pKv), pKv->valLen, pVal, valLen))
+        if ( (headerIndex != H_SET_COOKIE || method != LSI_HEADEROP_ADD)
+            && hasValue(getVal(pKv), pKv->valLen, pVal, valLen))
             return 0;//if exist when merge, ignor, otherwise same as append
     }
 
@@ -311,7 +316,7 @@ int HttpRespHeaders::add(INDEX headerIndex, const char *pVal,
     if (m_KVPairindex[headerIndex] == 0xFF)
         m_KVPairindex[headerIndex] = getTotalCount();
     return _add(m_KVPairindex[headerIndex], m_sPresetHeaders[headerIndex],
-                s_iHeaderLen[headerIndex], pVal, valLen, method);
+                s_iHeaderLen[headerIndex], pVal, valLen, method, headerIndex);
 }
 
 
@@ -336,7 +341,7 @@ int HttpRespHeaders::add(const char *pName, int nameLen, const char *pVal,
     else
         kvOrderNum = getTotalCount();
 
-    return _add(kvOrderNum, pName, nameLen, pVal, valLen, method);
+    return _add(kvOrderNum, pName, nameLen, pVal, valLen, method, headerIndex);
 }
 
 
diff --git a/src/http/httprespheaders.h b/src/http/httprespheaders.h
index f967042..2b15d3a 100644
--- a/src/http/httprespheaders.h
+++ b/src/http/httprespheaders.h
@@ -250,7 +250,7 @@ private:
                     struct iovec *iov, int maxIovCount);
 
     int _add(int kvOrderNum, const char *pName, int nameLen, const char *pVal,
-             unsigned int valLen, int method);
+             unsigned int valLen, int method, INDEX headerIndex);
 
     void            _del(int kvOrderNum);
     void            replaceHeader(resp_kvpair *pKv, const char *pVal,
@LQ2-apostrophe

This comment has been minimized.

Copy link
Author

LQ2-apostrophe commented Nov 16, 2017

I am glad that I have seen this patch in recent commits. I cloned OpenLiteSpeed source code from this GitHub repository, applied your BoringSSL patch, then compiled and reinstalled OpenLiteSpeed.

Finally your patch has fixed this duplication bug. Thank you so much!

Additionally, there is also a new patch for GZIP in recent commits. So I am still regularly checking for a new moddecompress error in server log.

I am closing this issue for now. I will reopen it again if I get the same moddecompress error message:

[moddecompress+++] compressbuf in 0, inflate/deflate return -5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment