(tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[michaelo]

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

(tomcat-native) branch 1.2.x updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[michaelo]

This is an automated email from the ASF dual-hosted git repository.

michaelo pushed a commit to branch 1.2.x
in repository https://gitbox.apache.org/repos/asf/tomcat-native.git


The following commit(s) were added to refs/heads/1.2.x by this push:
 new 193c4e504 BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently 
set undocumented default verify paths
193c4e504 is described below

commit 193c4e504fc10f74737b062ddd1b34f54f38a268
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 31493e74f..ff716091f 100644
--- a/native/src/ssl.c
+++ b/native/src/ssl.c
@@ -1894,15 +1894,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 646577e72..36cf11ed0 100644
--- a/native/src/sslcontext.c
+++ b/native/src/sslcontext.c
@@ -36,6 +36,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;
@@ -968,15 +969,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 a7462ec00..cac6e1b2d 100644
--- a/xdocs/miscellaneous/changelog.xml
+++ b/xdocs/miscellaneous/changelog.xml
@@ -44,6 +44,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

Re: [PR] BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocum… [tomcat-native]

[via GitHub]

michael-o commented on PR #22:
URL: https://github.com/apache/tomcat-native/pull/22#issuecomment-1784891589

   Merged.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: [PR] BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocum… [tomcat-native]

[via GitHub]

michael-o closed pull request #22: BZ 67818: 
SSL#setVerify()/SSLContext#setVerify() silently set undocum…
URL: https://github.com/apache/tomcat-native/pull/22


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 67818] SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[bugzilla]

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

Michael Osipov  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEW |RESOLVED

--- Comment #2 from Michael Osipov  ---
Fixed in:
- main for 2.0.7 and onwards
- 1.2.x for 1.2.40 and onwards

-- 
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

[PR] BZ 67926: PEMFile prints unidentifiable string representation of ASN.1 OIDs [tomcat]

[via GitHub]

michael-o opened a new pull request, #675:
URL: https://github.com/apache/tomcat/pull/675

   (no comment)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
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

[Mark Thomas]

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

Re: (tomcat-native) branch main updated: BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[Michael Osipov]

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: [PR] BZ 67926: PEMFile prints unidentifiable string representation of ASN.1 OIDs [tomcat]

[via GitHub]

ChristopherSchultz commented on code in PR #675:
URL: https://github.com/apache/tomcat/pull/675#discussion_r1376435849


##
java/org/apache/tomcat/util/net/jsse/PEMFile.java:
##
@@ -56,6 +56,8 @@
 import org.apache.tomcat.util.codec.binary.Base64;
 import org.apache.tomcat.util.file.ConfigFileLoader;
 import org.apache.tomcat.util.res.StringManager;
+import org.ietf.jgss.GSSException;

Review Comment:
   Where is this package sourced? It appears to be present in the JRE since at 
least 1.7. Interpreting the OID bytes in dotted-notation isn't very difficult. 
If it avoids adding a dependency or relying on a non-mandatory JRE package, I'd 
prefer a private implementation. If all JREs are required to provide this 
package, than it's safe to use IMO.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: [PR] BZ 67926: PEMFile prints unidentifiable string representation of ASN.1 OIDs [tomcat]

[via GitHub]

michael-o commented on code in PR #675:
URL: https://github.com/apache/tomcat/pull/675#discussion_r1376474963


##
java/org/apache/tomcat/util/net/jsse/PEMFile.java:
##
@@ -56,6 +56,8 @@
 import org.apache.tomcat.util.codec.binary.Base64;
 import org.apache.tomcat.util.file.ConfigFileLoader;
 import org.apache.tomcat.util.res.StringManager;
+import org.ietf.jgss.GSSException;

Review Comment:
   It has been present for the past 15 years. I am using that class in my code 
for 10+ years. They are default. Azul, Temurin, OpenJDK, Oracle, HPE JVMs have 
it. The SPNEGO authenticator is using it as well: 
https://github.com/apache/tomcat/blob/9b3c28da7ec8ddbd721b127f7f3b0a7d475ab797/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java#L46
   
   I consider it safe to use.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Re: [PR] BZ 67926: PEMFile prints unidentifiable string representation of ASN.1 OIDs [tomcat]

[via GitHub]

michael-o commented on code in PR #675:
URL: https://github.com/apache/tomcat/pull/675#discussion_r1376474963


##
java/org/apache/tomcat/util/net/jsse/PEMFile.java:
##
@@ -56,6 +56,8 @@
 import org.apache.tomcat.util.codec.binary.Base64;
 import org.apache.tomcat.util.file.ConfigFileLoader;
 import org.apache.tomcat.util.res.StringManager;
+import org.ietf.jgss.GSSException;

Review Comment:
   It has been present for the past 15 years. I am using that class in my code 
for 10+ years. They are default. Azul, Temurin, OpenJDK, Oracle, HPE JVMs have 
it. The SPNEGO authenticator is using it as well: 
https://github.com/apache/tomcat/blob/9b3c28da7ec8ddbd721b127f7f3b0a7d475ab797/java/org/apache/catalina/authenticator/SpnegoAuthenticator.java#L42-L46
   I consider it safe to use.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
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

[Christopher Schultz]

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

(tomcat) branch main updated: Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new 15d7157053 Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() 
silently set undocumented default verify paths
15d7157053 is described below

commit 15d71570530adb5382809a86cad3fd1dfe6ee880
Author: remm 
AuthorDate: Mon Oct 30 17:32:44 2023 +0100

Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set
undocumented default verify paths
---
 java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java | 6 --
 1 file changed, 6 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java 
b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 1820db64ad..1742fb739c 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -603,12 +603,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 break;
 }
 
-// SSLContext.setVerify(state.ctx, value, 
sslHostConfig.getCertificateVerificationDepth());
-if (SSL_CTX_set_default_verify_paths(state.sslCtx) > 0) {
-var store = SSL_CTX_get_cert_store(state.sslCtx);
-X509_STORE_set_flags(store, 0);
-}
-
 // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
 var openSSLCallbackVerify =
 
Linker.nativeLinker().upcallStub(openSSLCallbackVerifyHandle,


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

(tomcat) branch main updated: Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/main by this push:
 new 07a14381e0 Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() 
silently set undocumented default verify paths
07a14381e0 is described below

commit 07a14381e0c4698d3a130eb13318ea4db1f0b5f1
Author: remm 
AuthorDate: Mon Oct 30 17:34:39 2023 +0100

Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set
undocumented default verify paths
---
 .../org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java   | 6 --
 1 file changed, 6 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 65de58247e..dff89bb23b 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -600,12 +600,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 break;
 }
 
-// SSLContext.setVerify(state.ctx, value, 
sslHostConfig.getCertificateVerificationDepth());
-if (SSL_CTX_set_default_verify_paths(state.sslCtx) > 0) {
-var store = SSL_CTX_get_cert_store(state.sslCtx);
-X509_STORE_set_flags(store, 0);
-}
-
 // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
 var openSSLCallbackVerify =
 
Linker.nativeLinker().upcallStub(openSSLCallbackVerifyHandle,


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

(tomcat) branch 10.1.x updated: Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
 new 72698641f6 Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() 
silently set undocumented default verify paths
72698641f6 is described below

commit 72698641f6dc4307dfac702d322d3d753c10c925
Author: remm 
AuthorDate: Mon Oct 30 17:34:39 2023 +0100

Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set
undocumented default verify paths
---
 .../org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java   | 6 --
 1 file changed, 6 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 81100fc323..90b2a97047 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -601,12 +601,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 break;
 }
 
-// SSLContext.setVerify(state.ctx, value, 
sslHostConfig.getCertificateVerificationDepth());
-if (SSL_CTX_set_default_verify_paths(state.sslCtx) > 0) {
-var store = SSL_CTX_get_cert_store(state.sslCtx);
-X509_STORE_set_flags(store, 0);
-}
-
 // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
 var openSSLCallbackVerify =
 
Linker.nativeLinker().upcallStub(openSSLCallbackVerifyHandle,


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

(tomcat) branch 9.0.x updated: Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set undocumented default verify paths

[remm]

This is an automated email from the ASF dual-hosted git repository.

remm pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
 new 90de35fa32 Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() 
silently set undocumented default verify paths
90de35fa32 is described below

commit 90de35fa32b927b82dfb6992161a54192a56963a
Author: remm 
AuthorDate: Mon Oct 30 17:34:39 2023 +0100

Port BZ 67818: SSL#setVerify()/SSLContext#setVerify() silently set
undocumented default verify paths
---
 .../org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java   | 6 --
 1 file changed, 6 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index 81100fc323..90b2a97047 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -601,12 +601,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 break;
 }
 
-// SSLContext.setVerify(state.ctx, value, 
sslHostConfig.getCertificateVerificationDepth());
-if (SSL_CTX_set_default_verify_paths(state.sslCtx) > 0) {
-var store = SSL_CTX_get_cert_store(state.sslCtx);
-X509_STORE_set_flags(store, 0);
-}
-
 // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
 var openSSLCallbackVerify =
 
Linker.nativeLinker().upcallStub(openSSLCallbackVerifyHandle,


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 68026] New: org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached

[bugzilla]

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

Bug ID: 68026
   Summary: org.apache.tomcat.buf.MessageBytes.toString() is no
longuer cached
   Product: Tomcat 10
   Version: 10.1.15
  Hardware: PC
OS: Linux
Status: NEW
  Severity: normal
  Priority: P2
 Component: Servlet
  Assignee: dev@tomcat.apache.org
  Reporter: lucas.pou...@lpoconseil.fr
  Target Milestone: --

Created attachment 39323
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=39323&action=edit
Java Flight Recording with spring boot application

Since fixing bug 66196, the org.apache.tomcat.buf.MessageBytes.toString() is no
longuer cached.

For one tomcat request, if a multiple calls (by example to this method,
org.apache.catalina.connector.RequestFacade.getMethod() by springframework) is
processed, a memory overconsumption is observed

Maybe just fix with this ? 


```
public String toString() {
switch (type) {
case T_NULL:
case T_STR:
// No conversion required
break;
case T_BYTES:
// strValue = byteC.toString(); #OLD
setString(byteC.toString());
break;
case T_CHARS:
// strValue = charC.toString(); #OLD
setString(charC.toString());
break;
}

return strValue;
}
```

Same problem with tomcat 
-  >= 10.1.0
-  >= 9.0.71
-  >= 8.5.85

-- 
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

[Bug 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached

[bugzilla]

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

lucas.pou...@lpoconseil.fr changed:

   What|Removed |Added

   Severity|normal  |regression
   Priority|P2  |P1

-- 
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

[Bug 67793] FORM authenticator does not remember original max inactive interval in all use-cases

[bugzilla]

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

--- Comment #3 from Channa  ---
Hi Tomcat Team,

Any update on solution or future release which resolves this issue. 

Thanks
Channa

-- 
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

Buildbot success in on tomcat-9.0.x

[buildbot]

Build status: Build succeeded!
Worker used: bb_worker2_ubuntu
URL: https://ci2.apache.org/#builders/37/builds/746
Blamelist: Mark Thomas , chenggwang 
<90715678+chenggw...@users.noreply.github.com>, lihan , remm 

Build Text: build successful
Status Detected: restored build
Build Source Stamp: [branch 9.0.x] 90de35fa32b927b82dfb6992161a54192a56963a


Steps:

  worker_preparation: 0

  git: 0

  shell: 0

  shell_1: 0

  shell_2: 0

  shell_3: 0

  shell_4: 0

  shell_5: 0

  compile: 1

  shell_6: 0

  shell_7: 0

  shell_8: 0

  shell_9: 0

  Rsync docs to nightlies.apache.org: 0

  shell_10: 0

  Rsync RAT to nightlies.apache.org: 0

  compile_1: 1

  shell_11: 0

  Rsync Logs to nightlies.apache.org: 0


-- ASF Buildbot


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

[Bug 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached

[bugzilla]

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

lucas.pou...@lpoconseil.fr changed:

   What|Removed |Added

  Component|Servlet |Catalina

-- 
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

[Bug 68026] org.apache.tomcat.buf.MessageBytes.toString() is no longuer cached

[bugzilla]

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

lucas.pou...@lpoconseil.fr changed:

   What|Removed |Added

 CC||lucas.pou...@lpoconseil.fr

-- 
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

Re: [PR] BZ 67926: PEMFile prints unidentifiable string representation of ASN.1 OIDs [tomcat]

[via GitHub]

aooohan commented on code in PR #675:
URL: https://github.com/apache/tomcat/pull/675#discussion_r1376990387


##
java/org/apache/tomcat/util/net/jsse/PEMFile.java:
##
@@ -624,6 +626,16 @@ private byte[] fromHex(String hexString) {
 }
 return bytes;
 }
+
+
+private String toDottedOidString(byte[] oidBytes) {
+try {
+Oid oid = new Oid(oidBytes);
+return oid.toString();
+} catch (GSSException e) {
+throw new IllegalArgumentException(e.getMajorString());

Review Comment:
   I think this only need to return a hex string of `oidBytes` if it does not 
follow format as the message in GSSException only show a tip 'Improperly 
formatted ASN.1 DER encoding for Oid' which not help.
   ```suggestion
   return HexUtils.toHexString(oidBytes)
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org