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

Only set SSLCompression when it is set to true. #1452

Merged
merged 1 commit into from May 16, 2016

Conversation

buzzdeee
Copy link
Contributor

@buzzdeee buzzdeee commented May 9, 2016

The default for SSLCompression in Apache is 'Off' anyways, see:
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcompression
therefore there is no real need to add that into the config file.

This will prevent problems on Apache versions that have
linked against SSL libraries that do not have SSLCompression
enabled. There, even SSLCompression Off will lead to:

H00526: Syntax error on line 14 of /etc/apache2/modules/ssl.conf:
Setting Compression mode unsupported; not implemented by the SSL library

and prevents Apache startup

Add some spec tests for that behaviour, depending on whether Apache
version >= 2.4 or not, since the template also differentiates.

The default for SSLCompression in Apache is 'Off' anyways, see:
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcompression
therefore there is no real need to add that into the config file.

This will prevent problems on Apache versions that have
linked against SSL libraries that do not have SSLCompression
enabled. There, even SSLCompression Off will lead to:

H00526: Syntax error on line 14 of /etc/apache2/modules/ssl.conf:
Setting Compression mode unsupported; not implemented by the SSL library

and prevents Apache startup

Add some spec tests for that behaviour, depending on whether Apache
version >= 2.4 or not, since the template also differentiates.
@buzzdeee
Copy link
Contributor Author

buzzdeee commented May 9, 2016

rebased

@@ -12,7 +12,9 @@
SSLSessionCacheTimeout <%= @ssl_sessioncachetimeout %>
<%- if scope.function_versioncmp([@_apache_version, '2.4']) >= 0 -%>
Mutex <%= @_ssl_mutex %>
<%- if @ssl_compression -%>
Copy link
Contributor

Choose a reason for hiding this comment

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

this checks for truthiness. if it evaluates to falsey, this option will not be set (to off)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and that's exactly what I wanted, and wrote the spec tests for it ;)

I'm on OpenBSD*, and there Apache 2.4 from packages is built against libressl, which doesn't support SSLCompression at all.

Therefore, when the module sets
SSLCompression off

I get this error message on apache startup, and it refuses to start:

H00526: Syntax error on line 14 of /etc/apache2/modules/ssl.conf:
Setting Compression mode unsupported; not implemented by the SSL library

The default value for SSLCompression, if not given in the configuration file, is 'off', see:
https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcompression

The puppet module defaults the $ssl_compression parameter of the class to false, following the
Apache default value, which is fine. But since it's the default value, it is not necessary to explicitly
set it to 'off'. That said, for every Apache/SSL combination, that supports SSLCompression, nothing changes, for Apache/SSL combination, that doesn't support SSLCompression, things are not broken anymore (:
People that want to have SSLCompression, have to enable it explicitly in any case anyways, and likely know if their Apache/SSL combination supports it. For those the statement will be added to the config file as it was done before.

Does that make sense?

  • yes, yes, OpenBSD is not (yet ;) supported by the module. But there may be other OS/Distros that run into the same issue...

@jonnytdevops jonnytdevops merged commit dea8471 into puppetlabs:master May 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants