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

Catch all requests with wrong host and return 444 status #261

Closed
seocam opened this issue Feb 21, 2014 · 5 comments
Closed

Catch all requests with wrong host and return 444 status #261

seocam opened this issue Feb 21, 2014 · 5 comments
Labels
enhancement New feature or request

Comments

@seocam
Copy link
Contributor

seocam commented Feb 21, 2014

For security and cleanness reasons it's a good practice to deny requests without host or with hosts not configured in any vhost.

Nginx provides a pretty easy and simple way to that:

server {
    listen      80;
    server_name "";
    return      444;
}

Reference: http://nginx.org/en/docs/http/request_processing.html#how_to_prevent_undefined_server_names

So far I couldn't find a way to do that with this module.

I can implement that and send a pull request but I'd to discuss what's the best approach to get this done.

@seocam
Copy link
Contributor Author

seocam commented Feb 21, 2014

The closest I could get with the module was:

 nginx::resource::vhost { 'default':
    server_name         => ['_'],
    location_custom_cfg => {
      return => 444,
    },
    access_log       => '/dev/null',
    error_log        => '/dev/null',
    index_files      => [],
  }

Which results in:

server {
  listen                *:80;
  server_name           _;
  access_log            /dev/null;
  error_log             /dev/null;
  location / {
    return 444;
  }
}

The problem here is that the requests will get logged in /dev/null. That's not necessary and the IO could actually impact the performance.

Also the location doesn't need to be there.

@seocam
Copy link
Contributor Author

seocam commented Apr 4, 2014

@jfryman my goal here is to have an easy way to generate this:

server {
    listen      80;
    return      444;
}

One way to do that would be to make the log files and sever_name optional.

Another way would be to implement a variable $deny_unknown_vhosts in the nginx main class. Personally I would prefer this approach because it seems more clear what it does.

I'm just looking for feedback before doing a PR.

Thanks

@jfryman
Copy link
Contributor

jfryman commented Apr 4, 2014

I'd prefer to make the log files and server_name values optional. The primary aim is to model the definitions as close to the original mappings so migration from unmanaged files to Puppet managed is as easy as possible.

Maybe also consider a way to make the default location optional

@scottsb
Copy link
Contributor

scottsb commented Dec 16, 2014

FWIW, with the current code you can get a little closer to the goal with this:

nginx::resource::vhost { 'default':
  ensure              => present,
  server_name         => ['""'],
  access_log          => 'off',
  error_log           => '/dev/null emerg',
  index_files         => [],
  location_custom_cfg => {
    return => 444,
  }
}

Which generates:

server {
  listen                *:80;
  server_name           "";
  access_log            off;
  error_log             /dev/null emerg;
  location / {
    return 444;
  }
}

@3flex 3flex added the enhancement New feature or request label Apr 13, 2015
@wyardley
Copy link
Collaborator

wyardley commented Oct 7, 2016

I don't think it's a given that most people want a default vhost to return 4XX, depends on whether it's a system with many virtualhosts or just one (default), in which case one may very well want requests without a matching host to still hit that default vhost. I think the module does give the ability to configure things as you describe.

#888 does allow suppressing the error_log directive by setting it to 'absent'.

I'm going to close this one for now.

@wyardley wyardley closed this as completed Oct 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants