From 664776a2f8b4574ab8c80e7bc6986ef62ef24b77 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

Date: Thu, 3 Jul 2014 23:53:44 +0200

Subject: [PATCH 1/5] nss: let nss_{cache,load}_crl return CURLcode

Upstream-commit: 2968f957aa025003d15a4fa42c3138e99c6d2e3f

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/nss.c | 21 ++++++++++-----------

 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/lib/nss.c b/lib/nss.c

index 86775b4..a82fc64 100644

--- a/lib/nss.c

+++ b/lib/nss.c

@@ -471,7 +471,7 @@ static SECStatus nss_cache_crl(SECItem *crlDER)

     /* CRL already cached */

     SEC_DestroyCrl(crl);

     SECITEM_FreeItem(crlDER, PR_FALSE);

-    return SECSuccess;

+    return CURLE_SSL_CRL_BADFILE;

   }

   /* acquire lock before call of CERT_CacheCRL() */

@@ -480,16 +480,16 @@ static SECStatus nss_cache_crl(SECItem *crlDER)

     /* unable to cache CRL */

     PR_Unlock(nss_crllock);

     SECITEM_FreeItem(crlDER, PR_FALSE);

-    return SECFailure;

+    return CURLE_SSL_CRL_BADFILE;

   }

   /* we need to clear session cache, so that the CRL could take effect */

   SSL_ClearSessionCache();

   PR_Unlock(nss_crllock);

-  return SECSuccess;

+  return CURLE_OK;

 }

-static SECStatus nss_load_crl(const char* crlfilename)

+static CURLcode nss_load_crl(const char* crlfilename)

 {

   PRFileDesc *infile;

   PRFileInfo  info;

@@ -499,7 +499,7 @@ static SECStatus nss_load_crl(const char* crlfilename)

   infile = PR_Open(crlfilename, PR_RDONLY, 0);

   if(!infile)

-    return SECFailure;

+    return CURLE_SSL_CRL_BADFILE;

   if(PR_SUCCESS != PR_GetOpenFileInfo(infile, &info))

     goto fail;

@@ -545,7 +545,7 @@ static SECStatus nss_load_crl(const char* crlfilename)

 fail:

   PR_Close(infile);

   SECITEM_FreeItem(&filedata, PR_FALSE);

-  return SECFailure;

+  return CURLE_SSL_CRL_BADFILE;

 }

 static CURLcode nss_load_key(struct connectdata *conn, int sockindex,

@@ -1463,13 +1463,12 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)

   }

   if(data->set.ssl.CRLfile) {

-    if(SECSuccess != nss_load_crl(data->set.ssl.CRLfile)) {

-      curlerr = CURLE_SSL_CRL_BADFILE;

+    const CURLcode rv = nss_load_crl(data->set.ssl.CRLfile);

+    if(CURLE_OK != rv) {

+      curlerr = rv;

       goto error;

     }

-    infof(data,

-          "  CRLfile: %s\n",

-          data->set.ssl.CRLfile ? data->set.ssl.CRLfile : "none");

+    infof(data, "  CRLfile: %s\n", data->set.ssl.CRLfile);

   }

   if(data->set.str[STRING_CERT]) {

-- 

2.13.5

From 9efc8373f8190581b5463ebcb38f52ddaa89db51 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

Date: Fri, 4 Jul 2014 00:36:21 +0200

Subject: [PATCH 2/5] nss: make crl_der allocated on heap

... and spell it as crl_der instead of crlDER

Upstream-commit: caa4db8a51e2b02e43ee85e63bc3fec232986699

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/nss.c | 23 ++++++++++++++---------

 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/nss.c b/lib/nss.c

index a82fc64..4e210bb 100644

--- a/lib/nss.c

+++ b/lib/nss.c

@@ -463,23 +463,23 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,

 }

 /* add given CRL to cache if it is not already there */

-static SECStatus nss_cache_crl(SECItem *crlDER)

+static CURLcode nss_cache_crl(SECItem *crl_der)

 {

   CERTCertDBHandle *db = CERT_GetDefaultCertDB();

-  CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crlDER, 0);

+  CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0);

   if(crl) {

     /* CRL already cached */

     SEC_DestroyCrl(crl);

-    SECITEM_FreeItem(crlDER, PR_FALSE);

+    SECITEM_FreeItem(crl_der, PR_TRUE);

     return CURLE_SSL_CRL_BADFILE;

   }

   /* acquire lock before call of CERT_CacheCRL() */

   PR_Lock(nss_crllock);

-  if(SECSuccess != CERT_CacheCRL(db, crlDER)) {

+  if(SECSuccess != CERT_CacheCRL(db, crl_der)) {

     /* unable to cache CRL */

     PR_Unlock(nss_crllock);

-    SECITEM_FreeItem(crlDER, PR_FALSE);

+    SECITEM_FreeItem(crl_der, PR_TRUE);

     return CURLE_SSL_CRL_BADFILE;

   }

@@ -494,7 +494,7 @@ static CURLcode nss_load_crl(const char* crlfilename)

   PRFileDesc *infile;

   PRFileInfo  info;

   SECItem filedata = { 0, NULL, 0 };

-  SECItem crlDER = { 0, NULL, 0 };

+  SECItem *crl_der = NULL;

   char *body;

   infile = PR_Open(crlfilename, PR_RDONLY, 0);

@@ -510,6 +510,10 @@ static CURLcode nss_load_crl(const char* crlfilename)

   if(info.size != PR_Read(infile, filedata.data, info.size))

     goto fail;

+  crl_der = SECITEM_AllocItem(NULL, NULL, 0U);

+  if(!crl_der)

+    goto fail;

+

   /* place a trailing zero right after the visible data */

   body = (char*)filedata.data;

   body[--filedata.len] = '\0';

@@ -530,20 +534,21 @@ static CURLcode nss_load_crl(const char* crlfilename)

     /* retrieve DER from ASCII */

     *trailer = '\0';

-    if(ATOB_ConvertAsciiToItem(&crlDER, begin))

+    if(ATOB_ConvertAsciiToItem(crl_der, begin))

       goto fail;

     SECITEM_FreeItem(&filedata, PR_FALSE);

   }

   else

     /* assume DER */

-    crlDER = filedata;

+    *crl_der = filedata;

   PR_Close(infile);

-  return nss_cache_crl(&crlDER);

+  return nss_cache_crl(crl_der);

 fail:

   PR_Close(infile);

+  SECITEM_FreeItem(crl_der, PR_TRUE);

   SECITEM_FreeItem(&filedata, PR_FALSE);

   return CURLE_SSL_CRL_BADFILE;

 }

-- 

2.13.5

From f2c35b7b7f50b691d3019783ce19cc6a8dd5b484 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

Date: Fri, 4 Jul 2014 00:39:23 +0200

Subject: [PATCH 3/5] nss: fix a memory leak when CURLOPT_CRLFILE is used

Upstream-commit: 52cd5ac21cdfdc0a6c016de97fe70d3a50baa526

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/nss.c     | 38 +++++++++++++++++++++++++++++++++-----

 lib/urldata.h |  1 +

 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/lib/nss.c b/lib/nss.c

index 4e210bb..c3247c8 100644

--- a/lib/nss.c

+++ b/lib/nss.c

@@ -425,6 +425,14 @@ static void nss_destroy_object(void *user, void *ptr)

   PK11_DestroyGenericObject(obj);

 }

+/* same as nss_destroy_object() but for CRL items */

+static void nss_destroy_crl_item(void *user, void *ptr)

+{

+  SECItem *crl_der = (SECItem *)ptr;

+  (void) user;

+  SECITEM_FreeItem(crl_der, PR_TRUE);

+}

+

 static CURLcode nss_load_cert(struct ssl_connect_data *ssl,

                               const char *filename, PRBool cacert)

 {

@@ -463,7 +471,7 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,

 }

 /* add given CRL to cache if it is not already there */

-static CURLcode nss_cache_crl(SECItem *crl_der)

+static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)

 {

   CERTCertDBHandle *db = CERT_GetDefaultCertDB();

   CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0);

@@ -474,12 +482,17 @@ static CURLcode nss_cache_crl(SECItem *crl_der)

     return CURLE_SSL_CRL_BADFILE;

   }

+  /* store the CRL item so that we can free it in Curl_nss_close() */

+  if(!Curl_llist_insert_next(ssl->crl_list, ssl->crl_list->tail, crl_der)) {

+    SECITEM_FreeItem(crl_der, PR_FALSE);

+    return CURLE_OUT_OF_MEMORY;

+  }

+

   /* acquire lock before call of CERT_CacheCRL() */

   PR_Lock(nss_crllock);

   if(SECSuccess != CERT_CacheCRL(db, crl_der)) {

     /* unable to cache CRL */

     PR_Unlock(nss_crllock);

-    SECITEM_FreeItem(crl_der, PR_TRUE);

     return CURLE_SSL_CRL_BADFILE;

   }

@@ -489,7 +502,8 @@ static CURLcode nss_cache_crl(SECItem *crl_der)

   return CURLE_OK;

 }

-static CURLcode nss_load_crl(const char* crlfilename)

+static CURLcode nss_load_crl(struct ssl_connect_data *connssl,

+                             const char* crlfilename)

 {

   PRFileDesc *infile;

   PRFileInfo  info;

@@ -544,7 +558,7 @@ static CURLcode nss_load_crl(const char* crlfilename)

     *crl_der = filedata;

   PR_Close(infile);

-  return nss_cache_crl(crl_der);

+  return nss_cache_crl(connssl, crl_der);

 fail:

   PR_Close(infile);

@@ -1147,6 +1161,10 @@ void Curl_nss_close(struct connectdata *conn, int sockindex)

     connssl->obj_list = NULL;

     connssl->obj_clicert = NULL;

+    /* destroy all CRL items */

+    Curl_llist_destroy(connssl->crl_list, NULL);

+    connssl->crl_list = NULL;

+

     PR_Close(connssl->handle);

     connssl->handle = NULL;

   }

@@ -1325,6 +1343,8 @@ static CURLcode nss_fail_connect(struct ssl_connect_data *connssl,

   /* cleanup on connection failure */

   Curl_llist_destroy(connssl->obj_list, NULL);

   connssl->obj_list = NULL;

+  Curl_llist_destroy(connssl->crl_list, NULL);

+  connssl->crl_list = NULL;

   return curlerr;

 }

@@ -1367,6 +1387,14 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)

   if(!connssl->obj_list)

     return CURLE_OUT_OF_MEMORY;

+  /* list of all CRL items we need to destroy in Curl_nss_close() */

+  connssl->crl_list = Curl_llist_alloc(nss_destroy_crl_item);

+  if(!connssl->crl_list) {

+    Curl_llist_destroy(connssl->obj_list, NULL);

+    connssl->obj_list = NULL;

+    return CURLE_OUT_OF_MEMORY;

+  }

+

   /* FIXME. NSS doesn't support multiple databases open at the same time. */

   PR_Lock(nss_initlock);

   curlerr = nss_init(conn->data);

@@ -1468,7 +1496,7 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)

   }

   if(data->set.ssl.CRLfile) {

-    const CURLcode rv = nss_load_crl(data->set.ssl.CRLfile);

+    const CURLcode rv = nss_load_crl(connssl, data->set.ssl.CRLfile);

     if(CURLE_OK != rv) {

       curlerr = rv;

       goto error;

diff --git a/lib/urldata.h b/lib/urldata.h

index f4c6222..3624af1 100644

--- a/lib/urldata.h

+++ b/lib/urldata.h

@@ -301,6 +301,7 @@ struct ssl_connect_data {

   PRFileDesc *handle;

   char *client_nickname;

   struct SessionHandle *data;

+  struct curl_llist *crl_list;

   struct curl_llist *obj_list;

   PK11GenericObject *obj_clicert;

   ssl_connect_state connecting_state;

-- 

2.13.5

From 6f93eefb3361e430274eb9e76ff84380289c6164 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

Date: Fri, 4 Jul 2014 12:41:53 +0200

Subject: [PATCH 4/5] nss: make the list of CRL items global

Otherwise NSS could use an already freed item for another connection.

Upstream-commit: ca2aa61b66d684a1076d43025048f1a43d5755b6

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/nss.c     | 46 ++++++++++++++++++++++------------------------

 lib/urldata.h |  1 -

 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/lib/nss.c b/lib/nss.c

index c3247c8..acbd09a 100644

--- a/lib/nss.c

+++ b/lib/nss.c

@@ -77,6 +77,7 @@ PRFileDesc *PR_ImportTCPSocket(PRInt32 osfd);

 static PRLock *nss_initlock = NULL;

 static PRLock *nss_crllock = NULL;

 static PRLock *nss_findslot_lock = NULL;

+struct curl_llist *nss_crl_list = NULL;

 NSSInitContext * nss_context = NULL;

 volatile int initialized = 0;

@@ -471,7 +472,7 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,

 }

 /* add given CRL to cache if it is not already there */

-static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)

+static CURLcode nss_cache_crl(SECItem *crl_der)

 {

   CERTCertDBHandle *db = CERT_GetDefaultCertDB();

   CERTSignedCrl *crl = SEC_FindCrlByDERCert(db, crl_der, 0);

@@ -482,14 +483,16 @@ static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)

     return CURLE_SSL_CRL_BADFILE;

   }

-  /* store the CRL item so that we can free it in Curl_nss_close() */

-  if(!Curl_llist_insert_next(ssl->crl_list, ssl->crl_list->tail, crl_der)) {

-    SECITEM_FreeItem(crl_der, PR_FALSE);

+  /* acquire lock before call of CERT_CacheCRL() and accessing nss_crl_list */

+  PR_Lock(nss_crllock);

+

+  /* store the CRL item so that we can free it in Curl_nss_cleanup() */

+  if(!Curl_llist_insert_next(nss_crl_list, nss_crl_list->tail, crl_der)) {

+    SECITEM_FreeItem(crl_der, PR_TRUE);

+    PR_Unlock(nss_crllock);

     return CURLE_OUT_OF_MEMORY;

   }

-  /* acquire lock before call of CERT_CacheCRL() */

-  PR_Lock(nss_crllock);

   if(SECSuccess != CERT_CacheCRL(db, crl_der)) {

     /* unable to cache CRL */

     PR_Unlock(nss_crllock);

@@ -502,8 +505,7 @@ static CURLcode nss_cache_crl(struct ssl_connect_data *ssl, SECItem *crl_der)

   return CURLE_OK;

 }

-static CURLcode nss_load_crl(struct ssl_connect_data *connssl,

-                             const char* crlfilename)

+static CURLcode nss_load_crl(const char* crlfilename)

 {

   PRFileDesc *infile;

   PRFileInfo  info;

@@ -558,7 +560,7 @@ static CURLcode nss_load_crl(struct ssl_connect_data *connssl,

     *crl_der = filedata;

   PR_Close(infile);

-  return nss_cache_crl(connssl, crl_der);

+  return nss_cache_crl(crl_der);

 fail:

   PR_Close(infile);

@@ -996,6 +998,11 @@ static CURLcode nss_init(struct SessionHandle *data)

   if(initialized)

     return CURLE_OK;

+  /* list of all CRL items we need to destroy in Curl_nss_cleanup() */

+  nss_crl_list = Curl_llist_alloc(nss_destroy_crl_item);

+  if(!nss_crl_list)

+    return CURLE_OUT_OF_MEMORY;

+

   /* First we check if $SSL_DIR points to a valid dir */

   cert_dir = getenv("SSL_DIR");

   if(cert_dir) {

@@ -1096,6 +1103,11 @@ void Curl_nss_cleanup(void)

     NSS_ShutdownContext(nss_context);

     nss_context = NULL;

   }

+

+  /* destroy all CRL items */

+  Curl_llist_destroy(nss_crl_list, NULL);

+  nss_crl_list = NULL;

+

   PR_Unlock(nss_initlock);

   PR_DestroyLock(nss_initlock);

@@ -1161,10 +1173,6 @@ void Curl_nss_close(struct connectdata *conn, int sockindex)

     connssl->obj_list = NULL;

     connssl->obj_clicert = NULL;

-    /* destroy all CRL items */

-    Curl_llist_destroy(connssl->crl_list, NULL);

-    connssl->crl_list = NULL;

-

     PR_Close(connssl->handle);

     connssl->handle = NULL;

   }

@@ -1343,8 +1351,6 @@ static CURLcode nss_fail_connect(struct ssl_connect_data *connssl,

   /* cleanup on connection failure */

   Curl_llist_destroy(connssl->obj_list, NULL);

   connssl->obj_list = NULL;

-  Curl_llist_destroy(connssl->crl_list, NULL);

-  connssl->crl_list = NULL;

   return curlerr;

 }

@@ -1387,14 +1393,6 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)

   if(!connssl->obj_list)

     return CURLE_OUT_OF_MEMORY;

-  /* list of all CRL items we need to destroy in Curl_nss_close() */

-  connssl->crl_list = Curl_llist_alloc(nss_destroy_crl_item);

-  if(!connssl->crl_list) {

-    Curl_llist_destroy(connssl->obj_list, NULL);

-    connssl->obj_list = NULL;

-    return CURLE_OUT_OF_MEMORY;

-  }

-

   /* FIXME. NSS doesn't support multiple databases open at the same time. */

   PR_Lock(nss_initlock);

   curlerr = nss_init(conn->data);

@@ -1496,7 +1494,7 @@ static CURLcode nss_setup_connect(struct connectdata *conn, int sockindex)

   }

   if(data->set.ssl.CRLfile) {

-    const CURLcode rv = nss_load_crl(connssl, data->set.ssl.CRLfile);

+    const CURLcode rv = nss_load_crl(data->set.ssl.CRLfile);

     if(CURLE_OK != rv) {

       curlerr = rv;

       goto error;

diff --git a/lib/urldata.h b/lib/urldata.h

index 3624af1..f4c6222 100644

--- a/lib/urldata.h

+++ b/lib/urldata.h

@@ -301,7 +301,6 @@ struct ssl_connect_data {

   PRFileDesc *handle;

   char *client_nickname;

   struct SessionHandle *data;

-  struct curl_llist *crl_list;

   struct curl_llist *obj_list;

   PK11GenericObject *obj_clicert;

   ssl_connect_state connecting_state;

-- 

2.13.5

From de0742d4141ede4d1849ff1ebffd820faea53ad7 Mon Sep 17 00:00:00 2001

From: Kamil Dudka <kdudka@redhat.com>

Date: Wed, 8 Oct 2014 17:13:59 +0200

Subject: [PATCH 5/5] nss: do not fail if a CRL is already cached

This fixes a copy-paste mistake from commit 2968f957.

Upstream-commit: 9e37a7f9a5cd141c717aa0262e8dee7713c25200

Signed-off-by: Kamil Dudka <kdudka@redhat.com>

---

 lib/nss.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/nss.c b/lib/nss.c

index acbd09a..1b8abd3 100644

--- a/lib/nss.c

+++ b/lib/nss.c

@@ -480,7 +480,7 @@ static CURLcode nss_cache_crl(SECItem *crl_der)

     /* CRL already cached */

     SEC_DestroyCrl(crl);

     SECITEM_FreeItem(crl_der, PR_TRUE);

-    return CURLE_SSL_CRL_BADFILE;

+    return CURLE_OK;

   }

   /* acquire lock before call of CERT_CacheCRL() and accessing nss_crl_list */

-- 

2.13.5