Bug 66670 - Add SSLHostConfig#certificateKeyPasswordFile and SSLHostConfig#certificateKeystorePasswordFile
Summary: Add SSLHostConfig#certificateKeyPasswordFile and SSLHostConfig#certificateKey...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 9
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 9.0.76
Hardware: All All
: P2 enhancement (vote)
Target Milestone: -----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-06-26 15:01 UTC by Michael Osipov
Modified: 2023-10-23 11:05 UTC (History)
1 user (show)



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Osipov 2023-06-26 15:01:13 UTC
This is somewhat expired by https://httpd.apache.org/docs/current/mod/mod_ssl.html#sslpassphrasedialog and its flexiblity, but I do not intend to request all of those features.

One of the current problems to use inline passwords (certificateKeyPassword) in the server.xml
* Multiple connectors for the same certificate/key pair on different ports
* Multiple Tomcat instances for the same hostname, but you need different JVM configs/version/etc. or need the physical separation between processes

All of these requires to touch every single certificateKeyPassword and update when you rotate the cert/key pair and password. In HTTPd you don't have that problem because you modify a single password file and done.
Note: I don't intend to put HTTPd in front of those Tomcat because I need to configure and update it, it adds overhead and it has several open issues with expect/continue support for some of our use cases.

I'd like to introduce this parameter only for OpenSSL or PEM-based cert keys, not Java keystores since certificateKeystorePassword has a default value which does not allow to make it mutually exclusive. This attribute will be mutually exclusive with certificateKeyPassword since only either one should be populated, an exception will be raised when both is provided.

This will require changes in a few Java files as well as Tomcat Native in Java and C.

Target config example:
> <Connector port="%%HTTPS_PORT%%" connectionTimeout="20000" keepAliveTimeout="300000" maxParameterCount="1000"
>   maxHttpHeaderSize="24576" maxThreads="250"
>   SSLEnabled="true" scheme="https" secure="true"
>   defaultSSLHostConfigName="%%VIRTUAL_HOSTNAME%%">
>   <SSLHostConfig hostName="%%VIRTUAL_HOSTNAME%%" protocols="TLSv1.2+TLSv1.3"
>     honorCipherOrder="true" disableSessionTickets="true"
>     ciphers="HIGH:!aNULL:!eNULL:!EXPORT:!DES:!RC4:!3DES:!MD5:!PSK:!DSS:!SHA1:!SHA256:!SHA384">
>     <Certificate certificateFile="/etc/ssl/%%VIRTUAL_HOSTNAME%%/cert.crt"
>       certificateKeyFile="/etc/ssl/%%VIRTUAL_HOSTNAME%%/key.crt"
>       certificateKeyPasswordFile="/etc/ssl/%%VIRTUAL_HOSTNAME%%/password" type="RSA" />
>   </SSLHostConfig>
> </Connector>

IMPORTANT: This is not a security concern or to avoid plaintext passwords in conf files, it is solely about reducing admin overhead.

Let me know what you think, I'd like to start implementing it this week.
Comment 1 Christopher Schultz 2023-06-26 19:46:25 UTC
Why not just use an XML entity?

<!DOCTYPE Context [
<!ENTITY certPassword "tiger">
]>
...
...
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>
<SSLHostConfig certificateKeyPassword="&certPassword;"
...>

Or, if you really want to use a separate file:

<!DOCTYPE Context [
<!ENTITY certPassword SYSTEM "file:///etc/ssl/%%VIRTUAL_HOSTNAME%%/password">
]>

?
Comment 2 Michael Osipov 2023-06-26 19:56:57 UTC
(In reply to Christopher Schultz from comment #1)
> Why not just use an XML entity?
> 
> <!DOCTYPE Context [
> <!ENTITY certPassword "tiger">
> ]>
> ...
> ...
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> 
> Or, if you really want to use a separate file:
> 
> <!DOCTYPE Context [
> <!ENTITY certPassword SYSTEM "file:///etc/ssl/%%VIRTUAL_HOSTNAME%%/password">
> ]>
> 
> ?

I haven't even thought about this. I see some logical problems to it:
* From code, this will not work, XML only
* Most people don't even know system identifiers or DTDs at all
* What will happen if the file contains a line separator? Will it be stripped automatically?

WDYT?
Comment 3 Remy Maucherat 2023-06-27 08:57:42 UTC
There are property sources to do property replacement in server.xml. For example: https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/util/digester/ServiceBindingPropertySource.java (that's the only exotic one that is included in the Tomcat sources, though ;) ).
Comment 4 Michael Osipov 2023-06-27 09:06:43 UTC
(In reply to Remy Maucherat from comment #3)
> There are property sources to do property replacement in server.xml. For
> example:
> https://github.com/apache/tomcat/blob/main/java/org/apache/tomcat/util/
> digester/ServiceBindingPropertySource.java (that's the only exotic one that
> is included in the Tomcat sources, though ;) ).

Yeah, but obviously not straight forward for the user and requires a change in the setup on the target server which is not desired in my case.
Comment 5 Christopher Schultz 2023-06-27 14:06:52 UTC
(In reply to Michael Osipov from comment #2)
> * From code, this will not work, XML only

True, but not really relevant. If you want to re-use passwords (or files), you can do that in your code pretty easily.

> * Most people don't even know system identifiers or DTDs at all

Also true, but this is standard XML and not some weird hand-wavy thing that Tomcat invented like "property sources".

> * What will happen if the file contains a line separator? Will it be
> stripped automatically?

I haven't done extensive testing, but I believe exotic whitespace will be preserved. So if you have a trailing newline in your file, then your password will also have a trailing newline. So be careful.

But you'd have to do that, anyway. If Tomcat were to implement a "use this file here" attribute resolver, then we would not want to be doing things like trimming whitespace or anything like that, either.

(In reply to Remy Maucherat from comment #3)
> .../ServiceBindingPropertySource

This was going to be my next suggestion. I only recently discovered this capability existed and I fully intend to start playing-around with it because our current build process requires us to build locally in each environment to get e.g. database credentials into the right place in context.xml.

But... I think the ServiceBindingPropertySource only works with applications, not with server.xml, right Rémy?
Comment 6 Remy Maucherat 2023-06-27 14:21:46 UTC
(In reply to Christopher Schultz from comment #5)
> But... I think the ServiceBindingPropertySource only works with
> applications, not with server.xml, right Rémy?

These property sources are first and foremost for server.xml. For example this is used for the vault that some customers wanted to have sensitive stuff placed these instead of directly in server.xml ( https://github.com/web-servers/tomcat-vault/blob/main/INSTALL.md ). (of course, it's not really more secure, I know)
Comment 7 Michael Osipov 2023-06-28 07:09:52 UTC
So, you guys don't see a need for such a feature? Yeah, we all know that are workarounds/solutions, but they (completely) lack documentation and ease of access.

Chris, of course in my I can read the file myself, but this can basically apply to everything which is text-based, no?
Comment 8 Michael Osipov 2023-07-01 20:25:56 UTC
(In reply to Christopher Schultz from comment #1)
> Why not just use an XML entity?
> 
> <!DOCTYPE Context [
> <!ENTITY certPassword "tiger">
> ]>
> ...
> ...
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> <SSLHostConfig certificateKeyPassword="&certPassword;"
> ...>
> 
> Or, if you really want to use a separate file:
> 
> <!DOCTYPE Context [
> <!ENTITY certPassword SYSTEM "file:///etc/ssl/%%VIRTUAL_HOSTNAME%%/password">
> ]>
> 
> ?

This does not work:
SCHWERWIEGEND: Parse Fatal Error at line 33 column 44: Externe Entityreferenz "&certPassword;" ist in einem Attributwert nicht zulässig.
org.xml.sax.SAXParseException; systemId: file:/var/opt/tomcat-services/conf/server.xml; lineNumber: 33; columnNumber: 44; Externe Entityreferenz "&certPassword;" ist in einem Attributwert nicht zulässig.
        at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.createSAXParseException(ErrorHandlerWrapper.java:204)
        at com.sun.org.apache.xerces.internal.util.ErrorHandlerWrapper.fatalError(ErrorHandlerWrapper.java:178)
        at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:400)
        at com.sun.org.apache.xerces.internal.impl.XMLErrorReporter.reportError(XMLErrorReporter.java:32

because of https://stackoverflow.com/a/65784393/696632

I will pursue a draft PR next week.
Comment 9 Mark Thomas 2023-07-01 21:04:42 UTC
(In reply to Michael Osipov from comment #8)

> because of https://stackoverflow.com/a/65784393/696632

Doesn't that link also offer a solution (an internal entity) that would work for this use case?
Comment 10 Michael Osipov 2023-07-01 21:21:27 UTC
(In reply to Mark Thomas from comment #9)
> (In reply to Michael Osipov from comment #8)
> 
> > because of https://stackoverflow.com/a/65784393/696632
> 
> Doesn't that link also offer a solution (an internal entity) that would work
> for this use case?

> Reference an internal entity from an attribute value

Partially, because it still would require me to touch every server.xml file. So it is a half-hearted solution, then I can keep vim's "s#old#new#c".
Comment 11 Christopher Schultz 2023-09-27 14:16:35 UTC
(In reply to Michael Osipov from comment #8)
> This does not work:
> SCHWERWIEGEND: Parse Fatal Error at line 33 column 44: Externe
> Entityreferenz "&certPassword;" ist in einem Attributwert nicht zulässig.
> org.xml.sax.SAXParseException; systemId:
> file:/var/opt/tomcat-services/conf/server.xml; lineNumber: 33; columnNumber:
> 44; Externe Entityreferenz "&certPassword;" ist in einem Attributwert nicht
> zulässig.

Hmm. I thought we explicitly allowed external entities to support this kind of thing.
Comment 12 Michael Osipov 2023-09-27 14:43:50 UTC
(In reply to Christopher Schultz from comment #11)
> (In reply to Michael Osipov from comment #8)
> > This does not work:
> > SCHWERWIEGEND: Parse Fatal Error at line 33 column 44: Externe
> > Entityreferenz "&certPassword;" ist in einem Attributwert nicht zulässig.
> > org.xml.sax.SAXParseException; systemId:
> > file:/var/opt/tomcat-services/conf/server.xml; lineNumber: 33; columnNumber:
> > 44; Externe Entityreferenz "&certPassword;" ist in einem Attributwert nicht
> > zulässig.
> 
> Hmm. I thought we explicitly allowed external entities to support this kind
> of thing.

Note: I am working on an impl now. Hope to complete it by next week or so. This will be ideal combination with the recent TLS cert reload listener.
Comment 13 Michael Osipov 2023-09-30 17:10:43 UTC
Folks,

I have done quite good progress yesterday, all combinations of connectors and TLS implementations work from file. Before I do more testing and documentation, I'd like to know your opinion on the following:
Should it be a new parameter/attribute or should I refit the current password parameter/attribute to the approach of OpenSSL: https://www.openssl.org/docs/man3.0/man1/openssl-passphrase-options.html#OPTIONS? I would limit to "pass:", "file:", "env:" for now. No prefix will mean the current behavior, just like "pass:" or is that too breaking for you? WDYT?

WIP:
* https://github.com/michael-o/tomcat/compare/0e7812b88390e2ca989596236b86492dcbb69d18...michael-o:tomcat:BZ-66670
* https://github.com/michael-o/tomcat-native/compare/1.2.x...michael-o:tomcat-native:BZ-66670

Hope to present the PR in a week or so.
Comment 14 Christopher Schultz 2023-10-05 11:55:42 UTC
Is there a particular reason to add support directly to tcnative for this? Why not read the file in Java and pass the password to libtctative as usual? This would not require any changes to libtcnative, no additional release of libtcnative, no additional upgrade of libtcnative for users, etc.?
Comment 15 Michael Osipov 2023-10-05 12:18:05 UTC
(In reply to Christopher Schultz from comment #14)
> Is there a particular reason to add support directly to tcnative for this?
> Why not read the file in Java and pass the password to libtctative as usual?
> This would not require any changes to libtcnative, no additional release of
> libtcnative, no additional upgrade of libtcnative for users, etc.?

While you are right, I am doing this for consistency with the rest. Consider that not only Tomcat might use libtcnative, they will benefit as well. Netty does use libtcnative as well.
Comment 16 Michael Osipov 2023-10-23 11:05:00 UTC
Fixed in:
- main for 11.0.0-M14 and onwards
- 10.1.x for 10.1.16 and onwards
- 9.0.x  for 9.0.83 and onwards
- 8.5.x for 8.5.96 and onwards