Uploaded image for project: 'MINA'
  1. MINA
  2. DIRMINA-1072

SslFilter does not account for SSLEngine runtime exceptions

Agile BoardAttach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.0.16
    • 2.0.17
    • SSL
    • None

    Description

      Mina's SslFilter wraps Mina's SslHandler, which itself wraps Java's SSLEngine.

      SslFilter does not catch runtime exceptions that are thrown by SSLEngine - I am unsure if this is by design.

      Ideally, we'd prevent the engine to get into a state where it can throw such exceptions, but I'm not sure if that's completely feasible.

      None-the-less, I'm here providing an improvement that prevents at least one occurrence of an unchecked exception from being thrown (instead, my patch preemptively throws an SSLException that is then caught by the exception handling that's already in place).

      An alternative to this fix could be an additional catch block, that handles unchecked exceptions.

      The scenario that is causing the unchecked exception that is caught by this patch, is this:

      • client connects, causes an SslFilter to be initialized, which causes the SSLEngine to begin its handshake
      • server shuts down the input (for instance, for inactivity, or as a side-effect of resource starvation)
      • client sends data

      The corresponding stack trace starts with this:

      java.lang.IllegalStateException: Internal error
              at sun.security.ssl.SSLEngineImpl.initHandshaker(SSLEngineImpl.java:470)
              at sun.security.ssl.SSLEngineImpl.readRecord(SSLEngineImpl.java:1007)
              at sun.security.ssl.SSLEngineImpl.readNetRecord(SSLEngineImpl.java:907)
              at sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:781)
              at javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:624)

      Inspiration for this fix was obtain from the Jetty project, notably, this change: https://github.com/eclipse/jetty.project/issues/1228

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            Unassigned Unassigned
            guus.der.kinderen Guus der Kinderen
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment