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

Hiding nginx from the response header #4397

Merged
merged 4 commits into from
Oct 13, 2020
Merged

Hiding nginx from the response header #4397

merged 4 commits into from
Oct 13, 2020

Conversation

lancewf
Copy link
Contributor

@lancewf lancewf commented Oct 10, 2020

🔩 Description: What code changed, and why?

Nginx should not be exposed in the response header for automate-ui requests.

This change is building Nginx with the headers-more module. This module allows removing the server field from the response.

⛓️ Related Resources

#4403

👟 How to Build and Test the Change

  1. Open the hab studio and run start_all_services
  2. Open the UI (https://a2-dev.test/)
  3. Open the network tab
  4. Refresh the back and select the first line "event-feed"
  5. Ensure there is a line under the Response Header with "server: nginx".
  6. In had studio run rebuild components/automate-load-balancer/
  7. Wait for a minute for the rebuild component to load and refresh the UI and ensure the "server: nginx" is not there.

✅ Checklist

📷 Screenshots, if applicable

Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
@netlify
Copy link

netlify bot commented Oct 10, 2020

Deploy preview for chef-automate processing.

Building with commit fd7536d

https://app.netlify.com/sites/chef-automate/deploys/5f862e5e4577b80007c6ff66

Lance Finfrock added 3 commits October 12, 2020 15:44
Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
Added the headers-more-nginx-module to the nginx build.
Also add the line 'more_set_headers 'server: ' that removes
the server nginx field from the response.

Signed-off-by: Lance Finfrock <lfinfrock@chef.io>
@lancewf lancewf marked this pull request as ready for review October 13, 2020 22:51
@@ -0,0 +1,89 @@

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is needed in the nginx.conf file

@@ -106,6 +106,8 @@ http {

{{#each cfg.frontend_tls as |tls| ~}}
server {
more_set_headers 'server: ';
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removes the server line from the response.

@@ -5,4 +5,4 @@ exec 2>&1

source {{pkg.svc_config_path}}/render-certs.sh

exec {{pkgPathFor "core/nginx"}}/bin/nginx -c {{pkg.svc_config_path}}/nginx.conf
exec nginx -c {{pkg.svc_config_path}}/nginx.conf
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nginx is in the path now.

--with-http_slice_module \
--with-cc-opt="${CFLAGS}" \
--with-ld-opt="${LDFLAGS}" \
--add-module=${HAB_CACHE_SRC_PATH}/headers-more-nginx-module-${headers_more_version}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the headers-more module

pkg_origin=chef
pkg_version="0.1.0"
pkg_description="internal and external load balancer and reverse proxy for Automate 2.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the changes here were copied from here https://github.com/chef-base-plans/nginx/blob/master/plan.sh

@lancewf lancewf changed the title Hiding nginx response header Hiding nginx from the response header Oct 13, 2020
@lancewf lancewf self-assigned this Oct 13, 2020
@lancewf lancewf requested a review from a team October 13, 2020 23:20
Copy link
Contributor

@danielsdeleo danielsdeleo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I think it would be worth adding the more headers module upstream in core plans and then using that package like we did before but let’s go with this for now.

@lancewf lancewf merged commit baa6465 into master Oct 13, 2020
@lancewf lancewf deleted the lancewf/hide_nginx branch October 13, 2020 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants