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

Remove deprecated ssl directive #1576

Open
kenyon opened this issue Oct 5, 2023 · 5 comments
Open

Remove deprecated ssl directive #1576

kenyon opened this issue Oct 5, 2023 · 5 comments

Comments

@kenyon
Copy link
Member

kenyon commented Oct 5, 2023

We need to remove the deprecated ssl directive, which was deprecated in version 1.15.0, and has been removed in nginx 1.25.1: https://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl

@kenyon
Copy link
Member Author

kenyon commented Oct 5, 2023

Actually, I see that the ssl directive gets removed on the second puppet run. So we just need to make sure it isn't added on the first puppet run. Not sure how that is happening (yet).

@smortex
Copy link
Member

smortex commented Oct 5, 2023

This comes from:

<% if scope.call_function('versioncmp', [scope['nginx::nginx_version'], '1.15.0']) < 0 -%>
ssl on;
<% end -%>

On first run, if nginx was not installed, the fact nginx_version has no value and the assumed version is 1.6.0:

String[1] $nginx_version = pick(fact('nginx_version'), '1.6.0'),

I am not sure if we can just remove this code fragment without consequences. On one hand, version 1.15.0 was released in 2020 and has known vulnerabilities, but on the other hand if we allow to set a specific version I am sure people use this to stick to a legacy release…

@kenyon
Copy link
Member Author

kenyon commented Oct 5, 2023

@smortex ahhh. Seems like the default value for $nginx_version should be the lowest version that we support. That would be the min() of what upstream supports, and what comes with OSes that are supported per metadata.json, I think. Not sure what that version is, but that has to be newer than 1.6.0 (released in 2014) by now.

@kenyon
Copy link
Member Author

kenyon commented Oct 5, 2023

I see now that this is documented too:

puppet-nginx/README.md

Lines 148 to 153 in e984c16

### Idempotency with nginx 1.15.0 and later
By default, this module might configure the deprecated `ssl on` directive. When
you next run puppet, this will be removed since the `nginx_version` fact will now
be available. To avoid this idempotency issue, you can manually set the base
class's `nginx_version` parameter.

Done in 0ff8265

@smortex
Copy link
Member

smortex commented Oct 6, 2023

According to https://www.nginx.com/support/:

We provide technical support for the current release, and releases that launched within two years of the launch date of the current release.

Latest release is from 2023-08-15 (1.25.2), so the first version after 2021-08-15 is 1.21.2 released on 2021-08-31. Version 1.15.0 is therefore not maintained anymore by NGINX.

But Debian oldoldstable ship version 1.14.2 and continue to backport security fixes for this version.

Maybe updating the default version would be enough for now: instead of having the 1st Puppet run produce a broken config with recent nginx and the 2nd run fix it, we can have a broken config on 1st run on legacy systems and a fix on the 2nd?

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

No branches or pull requests

2 participants