libs/pjproject/patches/0111-ssl-premature-destroy.patch

   1 From 68c69f516f95df1faa42e5647e9ce7cfdc41ac38 Mon Sep 17 00:00:00 2001
   2 From: Nanang Izzuddin <nanang@teluu.com>
   3 Date: Wed, 16 Jun 2021 12:15:29 +0700
   4 Subject: [PATCH 2/2] - Fix silly mistake: accepted active socket created
   5  without group lock in SSL socket. - Replace assertion with normal validation
   6  check of SSL socket instance in OpenSSL verification callback (verify_cb())
   7  to avoid crash, e.g: if somehow race condition with SSL socket destroy
   8  happens or OpenSSL application data index somehow gets corrupted.
   9
  10 ---
  11  pjlib/src/pj/ssl_sock_imp_common.c |  3 +-
  12  pjlib/src/pj/ssl_sock_ossl.c       | 45 +++++++++++++++++++++++++-----
  13  2 files changed, 40 insertions(+), 8 deletions(-)
  14
  15 --- a/pjlib/src/pj/ssl_sock_imp_common.c
  16 +++ b/pjlib/src/pj/ssl_sock_imp_common.c
  17 @@ -949,6 +949,7 @@ static pj_bool_t asock_on_accept_complet
  18
  19      /* Create active socket */
  20      pj_activesock_cfg_default(&asock_cfg);
  21 +    asock_cfg.grp_lock = ssock->param.grp_lock;
  22      asock_cfg.async_cnt = ssock->param.async_cnt;
  23      asock_cfg.concurrency = ssock->param.concurrency;
  24      asock_cfg.whole_data = PJ_TRUE;
  25 @@ -964,7 +965,7 @@ static pj_bool_t asock_on_accept_complet
  26             goto on_return;
  27
  28         pj_grp_lock_add_ref(glock);
  29 -       asock_cfg.grp_lock = ssock->param.grp_lock = glock;
  30 +       ssock->param.grp_lock = glock;
  31         pj_grp_lock_add_handler(ssock->param.grp_lock, ssock->pool, ssock,
  32                                 ssl_on_destroy);
  33      }
  34 --- a/pjlib/src/pj/ssl_sock_ossl.c
  35 +++ b/pjlib/src/pj/ssl_sock_ossl.c
  36 @@ -327,7 +327,8 @@ static pj_status_t STATUS_FROM_SSL_ERR(c
  37         ERROR_LOG("STATUS_FROM_SSL_ERR", err, ssock);
  38      }
  39
  40 -    ssock->last_err = err;
  41 +    if (ssock)
  42 +       ssock->last_err = err;
  43      return GET_STATUS_FROM_SSL_ERR(err);
  44  }
  45
  46 @@ -344,7 +345,8 @@ static pj_status_t STATUS_FROM_SSL_ERR2(
  47      /* Dig for more from OpenSSL error queue */
  48      SSLLogErrors(action, ret, err, len, ssock);
  49
  50 -    ssock->last_err = ssl_err;
  51 +    if (ssock)
  52 +       ssock->last_err = ssl_err;
  53      return GET_STATUS_FROM_SSL_ERR(ssl_err);
  54  }
  55
  56 @@ -587,6 +589,13 @@ static pj_status_t init_openssl(void)
  57
  58      /* Create OpenSSL application data index for SSL socket */
  59      sslsock_idx = SSL_get_ex_new_index(0, "SSL socket", NULL, NULL, NULL);
  60 +       if (sslsock_idx == -1) {
  61 +               status = STATUS_FROM_SSL_ERR2("Init", NULL, -1, ERR_get_error(), 0);
  62 +               PJ_LOG(1,(THIS_FILE,
  63 +                                 "Fatal error: failed to get application data index for "
  64 +                                 "SSL socket"));
  65 +               return status;
  66 +       }
  67
  68      return status;
  69  }
  70 @@ -614,21 +623,36 @@ static int password_cb(char *buf, int nu
  71  }
  72
  73
  74 -/* SSL password callback. */
  75 +/* SSL certificate verification result callback.
  76 + * Note that this callback seems to be always called from library worker
  77 + * thread, e.g: active socket on_read_complete callback, which should have
  78 + * already been equipped with race condition avoidance mechanism (should not
  79 + * be destroyed while callback is being invoked).
  80 + */
  81  static int verify_cb(int preverify_ok, X509_STORE_CTX *x509_ctx)
  82  {
  83 -    pj_ssl_sock_t *ssock;
  84 -    SSL *ossl_ssl;
  85 +    pj_ssl_sock_t *ssock = NULL;
  86 +    SSL *ossl_ssl = NULL;
  87      int err;
  88
  89      /* Get SSL instance */
  90      ossl_ssl = X509_STORE_CTX_get_ex_data(x509_ctx,
  91                                     SSL_get_ex_data_X509_STORE_CTX_idx());
  92 -    pj_assert(ossl_ssl);
  93 +    if (!ossl_ssl) {
  94 +       PJ_LOG(1,(THIS_FILE,
  95 +                 "SSL verification callback failed to get SSL instance"));
  96 +       goto on_return;
  97 +    }
  98
  99      /* Get SSL socket instance */
 100      ssock = SSL_get_ex_data(ossl_ssl, sslsock_idx);
 101 -    pj_assert(ssock);
 102 +    if (!ssock) {
 103 +       /* SSL socket may have been destroyed */
 104 +       PJ_LOG(1,(THIS_FILE,
 105 +                 "SSL verification callback failed to get SSL socket "
 106 +                 "instance (sslsock_idx=%d).", sslsock_idx));
 107 +       goto on_return;
 108 +    }
 109
 110      /* Store verification status */
 111      err = X509_STORE_CTX_get_error(x509_ctx);
 112 @@ -706,6 +730,7 @@ static int verify_cb(int preverify_ok, X
 113      if (PJ_FALSE == ssock->param.verify_peer)
 114         preverify_ok = 1;
 115
 116 +on_return:
 117      return preverify_ok;
 118  }
 119
 120 @@ -1213,6 +1238,12 @@ static void ssl_destroy(pj_ssl_sock_t *s
 121  static void ssl_reset_sock_state(pj_ssl_sock_t *ssock)
 122  {
 123      ossl_sock_t *ossock = (ossl_sock_t *)ssock;
 124 +
 125 +    /* Detach from SSL instance */
 126 +    if (ossock->ossl_ssl) {
 127 +       SSL_set_ex_data(ossock->ossl_ssl, sslsock_idx, NULL);
 128 +    }
 129 +
 130      /**
 131       * Avoid calling SSL_shutdown() if handshake wasn't completed.
 132       * OpenSSL 1.0.2f complains if SSL_shutdown() is called during an