Skip to content
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

Support either changing the Server header or removing it from responses #274

Closed
dontcrash opened this issue May 22, 2023 · 22 comments
Closed
Labels
enhancement New feature or request

Comments

@dontcrash
Copy link

When connecting to any servers, in the HTTP response headers you will see this:
Server: nginx-proxy-manager
This gives potential attackers more information that they deserve!
When compiling nginx you can customise the server string in /src/nginx/src/http/ngx_http_header_filter_module.c

static char ngx_http_server_string[] = "Server: nginx" CRLF;
static char ngx_http_server_full_string[] = "Server: " NGINX_VER CRLF;
@dontcrash dontcrash added the enhancement New feature or request label May 22, 2023
@Zoey2936
Copy link
Member

curl -I https://z0ey.de
HTTP/1.1 308 Permanent Redirect
Server: nginx-proxy-manager/1.23.4 (nginx-quic)

you mean nginx-proxy-manager/1.23.4 (nginx-quic) right?

@dontcrash
Copy link
Author

dontcrash commented May 22, 2023

Yep, I don't see that as I have server_tokens off; in my config!

server_tokens off;
static char ngx_http_server_string[] = "Server: nginx" CRLF;

server_tokens on; (default)
static char ngx_http_server_full_string[] = "Server: " NGINX_VER CRLF;

But in this case the compiled nginx binary would be for example:

static char ngx_http_server_string[] = "Server: nginx-proxy-manager" CRLF;
static char ngx_http_server_full_string[] = "Server: " NGINX_VER CRLF;

So the variable NGINX_VER would return "nginx-proxy-manager/1.23.4 (nginx-quic)"

The static chars can be found in the source file for nginx: /src/nginx/src/http/ngx_http_header_filter_module.c

@dontcrash
Copy link
Author

dontcrash commented May 22, 2023

So there is a few places it is referenced.

This first one can probably be left alone
/src/core/nginx.h

#define NGINX_VERSION      "1.25.0"
#define NGINX_VER          "nginx/" NGINX_VERSION

/src/http/ngx_http_header_filter_module.c

static u_char ngx_http_server_string[] = "Server: nginx" CRLF;
static u_char ngx_http_server_full_string[] = "Server: " NGINX_VER CRLF;
static u_char ngx_http_server_build_string[] = "Server: " NGINX_VER_BUILD CRLF;`

/src/http/v2/ngx_http_v2_filter_module.c

    if (r->headers_out.server == NULL) {

        if (clcf->server_tokens == NGX_HTTP_SERVER_TOKENS_ON) {
            len += 1 + nginx_ver_len;

        } else if (clcf->server_tokens == NGX_HTTP_SERVER_TOKENS_BUILD) {
            len += 1 + nginx_ver_build_len;

        } else {
            len += 1 + sizeof(nginx);
        }
    }

/src/http/ngx_http_special_response.c:

static u_char ngx_http_error_full_tail[] =
"<hr><center>" NGINX_VER "</center>" CRLF
"</body>" CRLF
"</html>" CRLF
;


static u_char ngx_http_error_build_tail[] =
"<hr><center>" NGINX_VER_BUILD "</center>" CRLF
"</body>" CRLF
"</html>" CRLF
;


static u_char ngx_http_error_tail[] =
"<hr><center>nginx</center>" CRLF
"</body>" CRLF
"</html>" CRLF
;

@Zoey2936
Copy link
Member

currently, I change nginx to the nginx-proxy-manager:
https://github.com/ZoeyVid/nginx-quic/blob/latest/Dockerfile#L29...L31

@Zoey2936
Copy link
Member

I think removing is not possible and the build would fail than?

@Zoey2936
Copy link
Member

and the option of changing the header by the user is impossible, since the values are set while building...

@dontcrash
Copy link
Author

Not sure there would be enough demand for something like this from the general userbase to warrant two builds hmm, wondering if I should just use a modified Dockerfile, need to think about this one more, don't see an easy way to make it a simple enabled/disabled flag

@Zoey2936
Copy link
Member

could you please try this: https://github.com/GetPageSpeed/ngx_security_headers#hide_server_tokens (this module is built in)

@Zoey2936
Copy link
Member

for me it worked

@dontcrash
Copy link
Author

dontcrash commented May 22, 2023

That definitely hides it from the headers so one step closer!!
It does still show nginx-proxy-manager in the error pages such as 404 hm
A partial solution is that you can overwrite most default error pages but one example where you cannot is if you say send a http request to the https port via CURL, you cannot overwrite that error and you will see the standard nginx page "400 bad request"

@Zoey2936
Copy link
Member

The thing is... Even if I remove the server name from the error pages, the design of them is still unique and they could be identified...

@Zoey2936
Copy link
Member

But I can add hide_server_tokens on; by default

@Zoey2936
Copy link
Member

or add an env option for this

@dontcrash
Copy link
Author

Yes I agree, the error page is unique enough to identify it as nginx at the very least.
I think hide_server_tokens on; by default is a good security setting, unsure if others will agree but anything that makes it harder for attackers is a win in my mind :)

@dontcrash
Copy link
Author

dontcrash commented May 22, 2023

Could replace the error page with the Apache one to troll haha, watch attackers try Apache exploits on your server! Waste their time!

@Zoey2936
Copy link
Member

Could replace the error page with the Apache one to troll haha, watch attackers try Apache exploits on your server! Waste their time!

This would reuqire to change https://hg.nginx.org/nginx/file/tip/src/http/ngx_http_special_response.c, even if it would be possible, it could to easily break

@Zoey2936
Copy link
Member

@Zoey2936
Copy link
Member

done in latest comment, sorry, but I wont change the error pages

@Zoey2936
Copy link
Member

will be pushed to latest, if modsec is finally done and I get darkmode fixed

@dontcrash
Copy link
Author

done in latest comment, sorry, but I wont change the error pages

I was joking about the Apache one 😆
Awesome, I definitely think obscuring server headers will at least stop some script kiddies!

Thank you

@Zoey2936
Copy link
Member

will keep this open until then

@Zoey2936 Zoey2936 reopened this May 22, 2023
@Zoey2936
Copy link
Member

I've pushed this now to latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

No branches or pull requests

2 participants