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

Uniformize TLS configuration #2792

Open
ppkarwasz opened this issue Aug 2, 2024 · 2 comments
Open

Uniformize TLS configuration #2792

ppkarwasz opened this issue Aug 2, 2024 · 2 comments
Labels
appenders Affects one or more Appender plugins enhancement Additions or updates to features

Comments

@ppkarwasz
Copy link
Contributor

Many Log4j components have a configuration attribute to enable the verification of the TLS server certificate:

  • Network appenders (Socket, SMTP) use the verifyHostname attribute of the SSL nested component to provide the same feature. Its default value is false.

  • The HTTP Appender has a configuration attribute verifyHostname. The value defaults to true. Note that the HTTP Appender can also have a nested SSL component, but the value of SSL.verifyHostname is ignored.

  • We also have a log4j2.sslVerifyHostName configuration property that is used if the SSL component is absent.

I understand that in the past only HTTP servers had a X509 certificate issued by a public CA. However nowadays most SMTP servers have also publicly verifiable X509 certificates, so we can switch both defaults to true.

Besides that a public X509 certificate was never required by our appenders: they only connect to a single host.

Proposed changes

  • Let us deprecate HTTP.verifyHostname in 2.x. In 3.x we can still keep it, but set its default value to SSL.verifyHostname.
  • We can switch the default value of SSL.verifyHostname to true. This might require some additional work: the SMTP and HTTP appenders only connect to the host, when there is a log event to send. In the case of SMTP this happens only for ERROR log events by default, so users might realize that they have a configuration problem much later.
  • Currently if the user configures an SSL element, all the log4j2.ssl* configuration properties are ignored. I think those properties should still be used to provide default values for SSL.
@vy vy added enhancement Additions or updates to features appenders Affects one or more Appender plugins labels Aug 2, 2024
@vy
Copy link
Member

vy commented Aug 2, 2024

@ppkarwasz, good examination. Agreed with the proposal. Some remarks:

Let us deprecate HTTP.verifyHostname in 2.x. In 3.x we can still keep it, but set its default value to SSL.verifyHostname.

I'd be in favor of removing HTTP.verifyHostname in 3.x.

Currently if the user configures an SSL element, all the log4j2.ssl* configuration properties are ignored. I think those properties should still be used to provide default values for SSL.

This is a good improvement anyway. I would rather implement this in a separate PR and avoid unnecessarily involving it in the discussions we will have for other proposed changes in this PR.

@ppkarwasz
Copy link
Contributor Author

Let us deprecate HTTP.verifyHostname in 2.x. In 3.x we can still keep it, but set its default value to SSL.verifyHostname.

I'd be in favor of removing HTTP.verifyHostname in 3.x.

I guess we have to agree how many breaking changes do we allow in the configuration format.

We recommend users to use ConfigurationBuilder instead of directly instantiating Log4j Plugins as a protection against breaking code changes. Maybe it would be wiser to keep HTTP.verifyHostname at least in 3.x.

For a similar change Tomcat introduced a new SSL configuration format in 8.5, deprecated the old one and removed the old one in 10.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
appenders Affects one or more Appender plugins enhancement Additions or updates to features
Projects
None yet
Development

No branches or pull requests

2 participants