https://bz.apache.org/bugzilla/show_bug.cgi?id=65181

Konstantin Kolinko <knst.koli...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|FIXED                       |---
             Status|RESOLVED                    |REOPENED

--- Comment #8 from Konstantin Kolinko <knst.koli...@gmail.com> ---
Looking at commit, that implemented this feature for Tomcat Native 1.2.27,
1. There is a bug.

https://github.com/apache/tomcat-native/commit/69e884a96a308a2bfdd91c7de3a9b301838031c8

in native/src/sslcontext.c, lines 1039-1040

> -        if ((c->keys[idx] = load_pem_key(c, key_file)) == NULL) {
> +        if ((c->keys[idx] = load_pem_key(c, key_file)) == NULL
> +#ifndef OPENSSL_NO_ENGINE
> +                && tcn_ssl_engine != NULL &&
> +                (c->keys[idx] = ENGINE_load_private_key(tcn_ssl_engine, 
> key_file,
> +                                                        NULL, NULL)) == NULL
> +#endif
> +                ) {

I think that when one uses the default configuration (when there is no custom
engine configured with SSLEngine attribute of AprLifecycleListener) and thus
tcn_ssl_engine is NULL, the behaviour should remain unchanged.

As such, the code above has to be
"&& (tcn_ssl_engine == NULL || ENGINE_load_private_key() fails)".

With the current code when load_pem_key call fails and tcn_ssl_engine == NULL
the error reporting block is skipped.


2. I am OK with the patch with the above bug fixed,
but for future improvements there are some thoughts:

1) I wonder, whether in case of a custom engine (tcn_ssl_engine != NULL) it
would be better to try a "ENGINE_load_private_key" call first.

2) As "load_pem_key" is our own method (implemented in the same sslcontext.c
file), I wonder whether it would be better to move the change there (and maybe
rename that method).

3) I see that "ENGINE_load_private_key" method will be deprecated in OpenSSL
3.0 (master)
https://www.openssl.org/docs/man1.1.1/man3/ENGINE_load_private_key.html
https://www.openssl.org/docs/manmaster/man3/ENGINE_load_private_key.html

4) How about a "ENGINE_load_public_key" method? There is no need to call it?



STEPS TO REPRODUCE (for 1. bug)

Using release candidate of Tomcat 9.0.45:
1) Install Tomcat, e.g. unzip apache-tomcat-9.0.45-windows-x64.zip on Windows.
2) Edit the conf/server.xml file.
- ~line 68: Commend out the default HTTP connector on port 8080
- ~line 102: Uncomment a connector that uses OpenSSL for TLS
3) Create two files with some dummy content (e.g. "foo bar"):
conf/localhost-rsa-cert.pem
conf/localhost-rsa-key.pem
4) Start Tomcat.

ACTUAL BEHAVIOUR:

With Tomcat 9.0.45 and Tomcat Native 1.2.27 a startup failure is reported as
follows:

[[[
30-Mar-2021 18:28:20.133 WARNING [main]
org.apache.tomcat.util.net.openssl.OpenSSLContext.init Error initializing SSL
context
        java.lang.Exception: Unable to load certificate
[censored]\conf/localhost-rsa-cert.pem (error:0D06B08E:asn1 encoding
routines:asn1_d2i_read_bio:not enough data)
                at org.apache.tomcat.jni.SSLContext.setCertificate(Native
Method)
                at
org.apache.tomcat.util.net.openssl.OpenSSLContext.addCertificate(OpenSSLContext.java:379)
                at
org.apache.tomcat.util.net.openssl.OpenSSLContext.init(OpenSSLContext.java:250)
                at
org.apache.tomcat.util.net.SSLUtilBase.createSSLContext(SSLUtilBase.java:246)
                at
org.apache.tomcat.util.net.AprEndpoint.createSSLContext(AprEndpoint.java:397)
                at
org.apache.tomcat.util.net.AprEndpoint.bind(AprEndpoint.java:363)
                at
org.apache.tomcat.util.net.AbstractEndpoint.bindWithCleanup(AbstractEndpoint.java:1204)
                at
org.apache.tomcat.util.net.AbstractEndpoint.init(AbstractEndpoint.java:1217)

]]]

EXPECTED BEHAVIOUR:

If I replace bin/tcnative-1.dll file with Tomcat Native 1.2.26 (e.g. the one
from Tomcat 9.0.44), the error is reported as follows:

[[[
30-Mar-2021 18:30:02.551 WARNING [main]
org.apache.tomcat.util.net.openssl.OpenSSLContext.init Error initializing SSL
context
        java.lang.Exception: Unable to load certificate key
[censored]\conf/localhost-rsa-key.pem (error:0909006C:PEM routines:get_name:no
start line)
                at org.apache.tomcat.jni.SSLContext.setCertificate(Native
Method)
                at
org.apache.tomcat.util.net.openssl.OpenSSLContext.addCertificate(OpenSSLContext.java:379)
                at
org.apache.tomcat.util.net.openssl.OpenSSLContext.init(OpenSSLContext.java:250)
                at
org.apache.tomcat.util.net.SSLUtilBase.createSSLContext(SSLUtilBase.java:246)
                at
org.apache.tomcat.util.net.AprEndpoint.createSSLContext(AprEndpoint.java:397)
                at
org.apache.tomcat.util.net.AprEndpoint.bind(AprEndpoint.java:363)
                at
org.apache.tomcat.util.net.AbstractEndpoint.bindWithCleanup(AbstractEndpoint.java:1204)
                at
org.apache.tomcat.util.net.AbstractEndpoint.init(AbstractEndpoint.java:1217)
]]]

As loading the key in SSLContext::setCertificate (in native/src/sslcontext.c)
happens earlier than loading the certificate, the expected error message is
"Unable to load certificate key".

Seeing the "Unable to load certificate" message means that error handling for
the former was skipped.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to