On 2023/10/30 15:47:20 Christopher Schultz wrote: > Michael, > > On 10/30/23 08:40, Michael Osipov wrote: > > On 2023/10/30 11:50:55 Mark Thomas wrote: > >> 30 Oct 2023 10:25:07 micha...@apache.org: > >> > >>> This is an automated email from the ASF dual-hosted git repository. > >>> > >>> michaelo pushed a commit to branch main > >>> in repository https://gitbox.apache.org/repos/asf/tomcat-native.git > >>> > >>> > >>> The following commit(s) were added to refs/heads/main by this push: > >>> new ccc6bfe99 BZ 67818: SSL#setVerify()/SSLContext#setVerify() > >>> silently set undocumented default verify paths > >>> ccc6bfe99 is described below > >>> > >>> commit ccc6bfe99d1981aabde6a3175866f99d38207f03 > >>> Author: Michael Osipov <micha...@apache.org> > >>> AuthorDate: Wed Oct 18 22:22:06 2023 +0200 > >>> > >>> BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set > >>> undocumented default verify paths > >>> --- > >>> native/src/ssl.c | 11 ++--------- > >>> native/src/sslcontext.c | 12 +++--------- > >>> xdocs/miscellaneous/changelog.xml | 4 ++++ > >>> 3 files changed, 9 insertions(+), 18 deletions(-) > >>> > >>> diff --git a/native/src/ssl.c b/native/src/ssl.c > >>> index e0b0461a9..7f4ca7e78 100644 > >>> --- a/native/src/ssl.c > >>> +++ b/native/src/ssl.c > >>> @@ -1177,15 +1177,8 @@ TCN_IMPLEMENT_CALL(void, SSL, > >>> setVerify)(TCN_STDARGS, jlong ssl, > >>> if ((c->verify_mode == SSL_CVERIFY_OPTIONAL) || > >>> (c->verify_mode == SSL_CVERIFY_OPTIONAL_NO_CA)) > >>> verify |= SSL_VERIFY_PEER; > >>> - if (!c->store) { > >>> - if (SSL_CTX_set_default_verify_paths(c->ctx)) { > >>> - c->store = SSL_CTX_get_cert_store(c->ctx); > >>> - X509_STORE_set_flags(c->store, 0); > >>> - } > >>> - else { > >>> - /* XXX: See if this is fatal */ > >>> - } > >>> - } > >>> + if (!c->store) > >>> + c->store = SSL_CTX_get_cert_store(c->ctx); > >>> > >>> SSL_set_verify(ssl_, verify, SSL_callback_SSL_verify); > >>> } > >>> diff --git a/native/src/sslcontext.c b/native/src/sslcontext.c > >>> index 34669ff70..f5b2b9831 100644 > >>> --- a/native/src/sslcontext.c > >>> +++ b/native/src/sslcontext.c > >>> @@ -35,6 +35,7 @@ static apr_status_t ssl_context_cleanup(void *data) > >>> if (c) { > >>> int i; > >>> c->crl = NULL; > >>> + c->store = NULL; > >>> if (c->ctx) > >>> SSL_CTX_free(c->ctx); > >>> c->ctx = NULL; > >>> @@ -861,15 +862,8 @@ TCN_IMPLEMENT_CALL(void, SSLContext, > >>> setVerify)(TCN_STDARGS, jlong ctx, > >>> if ((c->verify_mode == SSL_CVERIFY_OPTIONAL) || > >>> (c->verify_mode == SSL_CVERIFY_OPTIONAL_NO_CA)) > >>> verify |= SSL_VERIFY_PEER; > >>> - if (!c->store) { > >>> - if (SSL_CTX_set_default_verify_paths(c->ctx)) { > >>> - c->store = SSL_CTX_get_cert_store(c->ctx); > >>> - X509_STORE_set_flags(c->store, 0); > >>> - } > >>> - else { > >>> - /* XXX: See if this is fatal */ > >>> - } > >>> - } > >>> + if (!c->store) > >>> + c->store = SSL_CTX_get_cert_store(c->ctx); > >>> > >>> SSL_CTX_set_verify(c->ctx, verify, SSL_callback_SSL_verify); > >>> } > >>> diff --git a/xdocs/miscellaneous/changelog.xml > >>> b/xdocs/miscellaneous/changelog.xml > >>> index ffd0e10f5..0aedd8212 100644 > >>> --- a/xdocs/miscellaneous/changelog.xml > >>> +++ b/xdocs/miscellaneous/changelog.xml > >>> @@ -59,6 +59,10 @@ > >>> <update> > >>> Remove an unreachable if condition around CRLs in sslcontext.c. > >>> (michaelo) > >>> </update> > >>> + <fix> > >>> + <bug>67818</bug>: > >>> <code>SSL.setVerify()</code>/<code>SSLContext.setVerify()</code> > >>> + silently set undocumented default verify paths. (michaelo) > >>> + </fix> > >> > >> I think this needs a better change log entry. It isn't clear if the paths > >> were set and now are not set or vice versa. > > > > I see. Can you propose something which is worded better? I wasn't able to > > come up with anything better. At most: > > SSL#setVerify()/SSLContext#setVerify() unconditionally silently set > > undocumented default verify paths > > I think if you try to figure out how to get the words "now" and/or > "when" into the change-entry, it'll be more clear what's happening.
What about? When <code>SSL.setVerify()</code>/<code>SSLContext.setVerify()</code> are invoked they silently set undocumented default verify paths. Now, one needs to properly configure those paths according to documentation. --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
