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