sous-chefs / nginx

Development repository for the nginx cookbook
https://supermarket.chef.io/cookbooks/nginx
Apache License 2.0
551 stars 810 forks source link

Security fixes to address #604 #605

Closed hrak closed 2 years ago

hrak commented 2 years ago

Fix nginx running as root on Debian family Fix webserver able to overwrite/delete config files Add Inspec tests

Description

This PR partly reverts #593 that caused nginx to run as root on Debian platform family. It also makes sure that all configuration files are written as root:root so the webserver has no permission to alter or delete its own configuration.

Issues Resolved

604

591

Check List

hrak commented 2 years ago

Seems good, but a few of the integration tests are failing. Any chance it’s something that can be fixed with this PR?

I briefly looked into it but it seems unrelated to the changes. Something with RHEL + nginx repo + missing libpcre2 dependency.

Edit:

Found something, i think it is related to this line in the install resource. Seems like the nginx package from the nginx.org repo now depends on pcre2 which comes from the base repo, which is disabled during installation:

package_install_opts = '--disablerepo=* --enablerepo=nginx' if platform_family?('amazon', 'fedora', 'rhel')
Dependencies Resolved

===============================================================================================================================================================================================================================================================
 Package                                                   Arch                                                       Version                                                                  Repository                                                 Size
===============================================================================================================================================================================================================================================================
Installing:
 nginx                                                     x86_64                                                     1:1.23.1-1.el7.ngx                                                       nginx                                                     799 k
Installing for dependencies:
 pcre2                                                     x86_64                                                     10.23-2.el7                                                              base
hrak commented 2 years ago

Maybe the easiest solution would be to install pcre2 before the installation of nginx, so turn it into something like:

if platform_family?('amazon', 'fedora', 'rhel') {
  package 'pcre2'
  package_install_opts = '--disablerepo=* --enablerepo=nginx' 
}

If you agree with this solution i will gladly make it part of this PR to make the build pass.

hrak commented 2 years ago

Looks like you’re right, these are unrelated failures. Still, you squashed a few!

I think this can be merged in, and these failures dealt with separately.

Yeah, i found the issue with fedora-latest already. It seems to require openssl1.1 to be installed. I will file a PR later to fix that.

bmhughes commented 2 years ago

I can't test this myself now but does using the CentOS 9 repo rather than 8 fix the Fedora install issue?

hrak commented 2 years ago

I can't test this myself now but does using the CentOS 9 repo rather than 8 fix the Fedora install issue?

I just tested this, and this works without openssl1.1 so thats probably the better solution. Will update PR.

kitchen-porter commented 2 years ago

Released as: 12.1.0

damacus commented 2 years ago

I'm a bit late to the party, but why do we need to run as root at all?

Running this as root seems pretty dangerous.

jeffbyrnes commented 2 years ago

Firefly meme GIF of character about to say something, pausing, starting again, then resting their chin in their hand in rueful agreement

I think I was losing my marbles when I forgot that.

hrak commented 2 years ago

I'm a bit late to the party, but why do we need to run as root at all?

Running this as root seems pretty dangerous.

That's what this PR fixed.

Running as root is indeed dangerous, and i don't want to know how many instances are running as root due to this issue in versions between 12.0.6 and <12.1.0 (in our case, it was quite a few).