Re: (tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths
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
> >>> 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 @@
> >>>
> >>> Remove an unreachable if condition around CRLs in sslcontext.c.
> >>> (michaelo)
> >>>
> >>> +
> >>> + 67818:
> >>> SSL.setVerify()/SSLContext.setVerify()
> >>> + silently set undocumented default verify paths. (michaelo)
> >>> +
> >>
> >> 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 SSL.setVerify()/SSLContext.setVerify() 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
Re: (tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths
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
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 @@
Remove an unreachable if condition around CRLs in sslcontext.c.
(michaelo)
+
+ 67818:
SSL.setVerify()/SSLContext.setVerify()
+ silently set undocumented default verify paths. (michaelo)
+
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.
-chris
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: (tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths
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
> > 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 @@
> >
> > Remove an unreachable if condition around CRLs in sslcontext.c.
> > (michaelo)
> >
> > +
> > + 67818:
> > SSL.setVerify()/SSLContext.setVerify()
> > + silently set undocumented default verify paths. (michaelo)
> > +
>
> 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
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Re: (tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths
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
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 @@
Remove an unreachable if condition around CRLs in sslcontext.c.
(michaelo)
+
+ 67818:
SSL.setVerify()/SSLContext.setVerify()
+ silently set undocumented default verify paths. (michaelo)
+
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.
Same for 1.2.x
Mark
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
(tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths
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
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 @@
Remove an unreachable if condition around CRLs in sslcontext.c.
(michaelo)
+
+ 67818:
SSL.setVerify()/SSLContext.setVerify()
+ silently set undocumented default verify paths. (michaelo)
+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
