You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2022/04/27 11:08:39 UTC

[tomcat] branch main updated: 66035: Add NULL check on the SSL session reference

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 c8ecaa44f6 66035: Add NULL check on the SSL session reference
c8ecaa44f6 is described below

commit c8ecaa44f6a110873bd7bf8b3c2f08354e2900d8
Author: remm <re...@apache.org>
AuthorDate: Wed Apr 27 13:08:08 2022 +0200

    66035: Add NULL check on the SSL session reference
    
    Add NULL check on the SSL session reference in the Panama code before
    accessing the session id and creation time.
---
 .../org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java   | 7 ++++++-
 webapps/docs/changelog.xml                                         | 4 ++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index e34759c913..52e0677144 100644
--- a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -1568,6 +1568,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
                     var allocator = SegmentAllocator.ofScope(engineScope);
                     MemorySegment lenPointer = allocator.allocate(CLinker.C_POINTER);
                     var session = SSL_get_session(state.ssl);
+                    if (MemoryAddress.NULL.equals(session)) {
+                        return new byte[0];
+                    }
                     MemoryAddress sessionId = SSL_SESSION_get_id(session, lenPointer);
                     int length = MemoryAccess.getInt(lenPointer);
                     id = (length == 0) ? new byte[0] : sessionId.asSegment(length, engineScope).toByteArray();
@@ -1589,7 +1592,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
             synchronized (OpenSSLEngine.this) {
                 if (!destroyed) {
                     var session = SSL_get_session(state.ssl);
-                    creationTime = SSL_SESSION_get_time(session);
+                    if (!MemoryAddress.NULL.equals(session)) {
+                        creationTime = SSL_SESSION_get_time(session);
+                    }
                 }
             }
             return creationTime * 1000L;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3df044a28f..702914aadd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -144,6 +144,10 @@
         Tomcat will not be running on a JRE where these issues are present.
         (markt)
       </scode>
+      <fix>
+        <bug>66035</bug>: Add NULL check on the SSL session reference in the
+        Panama code before accessing the session id and creation time. (remm)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">


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


Re: [tomcat] branch main updated: 66035: Add NULL check on the SSL session reference

Posted by Christopher Schultz <ch...@christopherschultz.net>.
Rémy,

On 4/27/22 07:08, remm@apache.org wrote:
> 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 c8ecaa44f6 66035: Add NULL check on the SSL session reference
> c8ecaa44f6 is described below
> 
> commit c8ecaa44f6a110873bd7bf8b3c2f08354e2900d8
> Author: remm <re...@apache.org>
> AuthorDate: Wed Apr 27 13:08:08 2022 +0200
> 
>      66035: Add NULL check on the SSL session reference
>      
>      Add NULL check on the SSL session reference in the Panama code before
>      accessing the session id and creation time.

Should this be done in tcnative as well? Or was this intended to be a 
belt-and-suspenders check to avoid the problem whether or not it exists 
in tcnative?

I think the attached patch ought to do it for tcnative.

-chris

=== CUT ===
diff --git a/native/src/ssl.c b/native/src/ssl.c
index d59246ea3..5329a93da 100644
--- a/native/src/ssl.c
+++ b/native/src/ssl.c
@@ -2001,8 +2001,12 @@ TCN_IMPLEMENT_CALL(jbyteArray, SSL, 
getSessionId)(TCN_STDARGS, jlong ssl)
      }
      UNREFERENCED(o);
      session = SSL_get_session(ssl_);
-    session_id = SSL_SESSION_get_id(session, &len);
+    if (NULL == session) {
+        tcn_ThrowException(e, "ssl session is null");
+        return NULL;
+    }

+    session_id = SSL_SESSION_get_id(session, &len);
      if (len == 0 || session_id == NULL) {
          return NULL;
      }
diff --git a/native/src/sslnetwork.c b/native/src/sslnetwork.c
index 6e5960f91..46b253ec8 100644
--- a/native/src/sslnetwork.c
+++ b/native/src/sslnetwork.c
@@ -689,7 +689,7 @@ TCN_IMPLEMENT_CALL(jint, SSLSocket, 
renegotiate)(TCN_STDARGS,

  #if defined(SSL_OP_NO_TLSv1_3)
      session  = SSL_get_session(con->ssl);
-    if (SSL_SESSION_get_protocol_version(session) == TLS1_3_VERSION) {
+    if (NULL != session && SSL_SESSION_get_protocol_version(session) == 
TLS1_3_VERSION) {
          // TLS 1.3 renegotiation
          retVal = SSL_verify_client_post_handshake(con->ssl);
          if (retVal <= 0) {

=== CUT ===


> ---
>   .../org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java   | 7 ++++++-
>   webapps/docs/changelog.xml                                         | 4 ++++
>   2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
> index e34759c913..52e0677144 100644
> --- a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
> +++ b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
> @@ -1568,6 +1568,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
>                       var allocator = SegmentAllocator.ofScope(engineScope);
>                       MemorySegment lenPointer = allocator.allocate(CLinker.C_POINTER);
>                       var session = SSL_get_session(state.ssl);
> +                    if (MemoryAddress.NULL.equals(session)) {
> +                        return new byte[0];
> +                    }
>                       MemoryAddress sessionId = SSL_SESSION_get_id(session, lenPointer);
>                       int length = MemoryAccess.getInt(lenPointer);
>                       id = (length == 0) ? new byte[0] : sessionId.asSegment(length, engineScope).toByteArray();
> @@ -1589,7 +1592,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
>               synchronized (OpenSSLEngine.this) {
>                   if (!destroyed) {
>                       var session = SSL_get_session(state.ssl);
> -                    creationTime = SSL_SESSION_get_time(session);
> +                    if (!MemoryAddress.NULL.equals(session)) {
> +                        creationTime = SSL_SESSION_get_time(session);
> +                    }
>                   }
>               }
>               return creationTime * 1000L;
> diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
> index 3df044a28f..702914aadd 100644
> --- a/webapps/docs/changelog.xml
> +++ b/webapps/docs/changelog.xml
> @@ -144,6 +144,10 @@
>           Tomcat will not be running on a JRE where these issues are present.
>           (markt)
>         </scode>
> +      <fix>
> +        <bug>66035</bug>: Add NULL check on the SSL session reference in the
> +        Panama code before accessing the session id and creation time. (remm)
> +      </fix>
>       </changelog>
>     </subsection>
>     <subsection name="Jasper">
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 

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