Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-23740

ssl connection fails when server and client certs signed by same CA

Details

    Description

      "vanilla" ssl connections work, but assigning a cert to the server and a client cert causes "ssl protocol failures", sometimes obscure version mismatch errors.

      On investigation it turns out the that problem is in viossl.c and how ssl errors are handled.
      ssl_should_retry calls SSL_get_error to check for the current error, but the documentation for SSL_get_error says:
      "The current thread's
      error queue must be empty before the TLS/SSL I/O operation is
      attempted, or SSL_get_error() will not work reliably."

      (attaching segment of mysqld.trace of the working, patched version...note how X509_R_CERT_ALREADY_IN_HASH_TABLE
      errors get dropped by the patched code; otherwise they would cause a fatal error in the SSL connection, even though they are harmless, coming from the X509 certs being taken from both the server and the client)

      It took a couple of iterations to clean up these errors, because there was some race conditions on when the errors came in vs. when they get handled in the code.

      Solution: add ERR_clear_error() before each SSL_(read|write|etc) calls. And check for whatever errors are in the "queue" in the ssl_should_retry routine. The mysqld.trace shows that sometimes several errors are queued up, so just dismissing the first one won't be enough.

      Patch attached; it has some extra whitespace changes, and extra DBUG_PRINT's for diagnostics

      NOTE: v10.6 viossl.c has the same code in the 10.2 viossl.c, with minor changes that do not alter the problem noted in this bugreport

      Attachments

        1. MariaDB.patch
          3 kB
        2. mariaSSL.tgz
          9 kB
        3. mysqld.trace
          2 kB

        Issue Links

          Activity

            celane Charles Lane created issue -
            celane Charles Lane made changes -
            Field Original Value New Value
            Attachment MariaDB.patch [ 53855 ]
            celane Charles Lane made changes -
            Attachment MariaDB.patch [ 53866 ]
            celane Charles Lane made changes -
            Description "vanilla" ssl connections work, but assigning a cert to the server and a client cert causes "ssl protocol failures", sometimes obscure version mismatch errors.

            On investigation it turns out the that problem is in viossl.c and how ssl errors are handled.
            ssl_should_retry calls SSL_get_error to check for the current error, but the documentation for SSL_get_error says:
                 "The current thread's
                   error queue must be empty before the TLS/SSL I/O operation is
                   attempted, or SSL_get_error() will not work reliably."

            What was happening was that the handshake left some X509 errors on the queue; that the X509 code new were harmless, but when doing an SSL_read, those leftover errors caused everything to fail badly.

            Solution: add ERR_clear_error() before each SSL_(read|write|etc) calls.
            Patch attached; it has some extra whitespace changes, and a DBUG_PRINT of ssl error.

            NOTE: v10.6 viossl.c has the same code in viossl.c, with minor changes that do not alter the problem noted in this bugreport
            "vanilla" ssl connections work, but assigning a cert to the server and a client cert causes "ssl protocol failures", sometimes obscure version mismatch errors.

            On investigation it turns out the that problem is in viossl.c and how ssl errors are handled.
            ssl_should_retry calls SSL_get_error to check for the current error, but the documentation for SSL_get_error says:
                 "The current thread's
                   error queue must be empty before the TLS/SSL I/O operation is
                   attempted, or SSL_get_error() will not work reliably."

            (attaching segment of mysqld.trace of the working, patched version...note how X509_R_CERT_ALREADY_IN_HASH_TABLE
            errors get dropped by the patched code; otherwise they would cause a fatal error in the SSL connection, even though they are harmless, coming from the X509 certs being taken from both the server and the client)

            It took a couple of iterations to clean up these errors, because there was some race conditions on when the errors came in vs. when they get handled in the code.

            Solution: add ERR_clear_error() before each SSL_(read|write|etc) calls. And check for whatever errors are in the "queue" in the ssl_should_retry routine. The mysqld.trace shows that sometimes several errors are queued up, so just dismissing the first one won't be enough.

            Patch attached; it has some extra whitespace changes, and extra DBUG_PRINT's for diagnostics

            NOTE: v10.6 viossl.c has the same code in the 10.2 viossl.c, with minor changes that do not alter the problem noted in this bugreport
            celane Charles Lane made changes -
            Attachment mysqld.trace [ 53867 ]
            celane Charles Lane made changes -
            elenst Elena Stepanova made changes -
            Fix Version/s 10.2 [ 14601 ]
            Affects Version/s 10.6 [ 24028 ]
            Assignee Sergei Golubchik [ serg ]
            Labels contribution
            serg Sergei Golubchik made changes -
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            serg Sergei Golubchik made changes -
            Status Open [ 1 ] Confirmed [ 10101 ]
            serg Sergei Golubchik made changes -
            Status Confirmed [ 10101 ] In Review [ 10002 ]
            serg Sergei Golubchik made changes -
            Assignee Sergei Golubchik [ serg ] Vladislav Vaintroub [ wlad ]
            celane Charles Lane made changes -
            Attachment mariaSSL.tgz [ 54343 ]
            wlad Vladislav Vaintroub made changes -
            Priority Critical [ 2 ] Major [ 3 ]
            wlad Vladislav Vaintroub made changes -
            Status In Review [ 10002 ] Stalled [ 10000 ]
            wlad Vladislav Vaintroub made changes -
            Status Stalled [ 10000 ] In Progress [ 3 ]
            wlad Vladislav Vaintroub made changes -
            issue.field.resolutiondate 2021-03-15 18:39:29.0 2021-03-15 18:39:29.631
            wlad Vladislav Vaintroub made changes -
            Fix Version/s 10.2.38 [ 25207 ]
            Fix Version/s 10.3.29 [ 25206 ]
            Fix Version/s 10.4.19 [ 25205 ]
            Fix Version/s 10.5.10 [ 25204 ]
            Fix Version/s 10.2 [ 14601 ]
            Fix Version/s 10.3 [ 22126 ]
            Fix Version/s 10.4 [ 22408 ]
            Fix Version/s 10.5 [ 23123 ]
            Resolution Fixed [ 1 ]
            Status In Progress [ 3 ] Closed [ 6 ]
            serg Sergei Golubchik made changes -
            Workflow MariaDB v3 [ 113533 ] MariaDB v4 [ 158373 ]

            People

              wlad Vladislav Vaintroub
              celane Charles Lane
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.