nss: select client certificates by DER (#733657)

This commit is contained in:
Kamil Dudka 2011-09-19 14:00:00 +02:00
parent eaba136aa1
commit 95558f1c9d
2 changed files with 570 additions and 1 deletions

View File

@ -0,0 +1,560 @@
From 923616695a20cf64adab724e2dfc129987570153 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 26 Aug 2011 14:38:18 +0200
Subject: [PATCH 1/4] nss: select client certificates by DER
... instead of nicknames, which are not unique.
---
lib/nss.c | 48 ++++++++++++++++++++++++++++++++----------------
lib/urldata.h | 1 +
2 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/lib/nss.c b/lib/nss.c
index fb52402..2d0f217 100644
--- a/lib/nss.c
+++ b/lib/nss.c
@@ -354,6 +354,10 @@ static CURLcode nss_create_object(struct ssl_connect_data *ssl,
return CURLE_OUT_OF_MEMORY;
}
+ if(!cacert && CKO_CERTIFICATE == obj_class)
+ /* store reference to a client certificate */
+ ssl->obj_clicert = obj;
+
return CURLE_OK;
}
@@ -398,6 +402,10 @@ static int nss_load_cert(struct ssl_connect_data *ssl,
nickname = strdup(filename);
if(!nickname)
return 0;
+
+ /* we are not going to use libnsspem.so to read the client cert */
+ ssl->obj_clicert = NULL;
+
goto done;
}
@@ -797,44 +805,51 @@ static SECStatus SelectClientCert(void *arg, PRFileDesc *sock,
struct CERTCertificateStr **pRetCert,
struct SECKEYPrivateKeyStr **pRetKey)
{
- static const char pem_nickname[] = "PEM Token #1";
- const char *pem_slotname = pem_nickname;
-
struct ssl_connect_data *connssl = (struct ssl_connect_data *)arg;
struct SessionHandle *data = connssl->data;
const char *nickname = connssl->client_nickname;
- if(mod && nickname &&
- 0 == strncmp(nickname, pem_nickname, /* length of "PEM Token" */ 9)) {
-
+#ifdef HAVE_PK11_CREATEGENERICOBJECT
+ if(connssl->obj_clicert) {
/* use the cert/key provided by PEM reader */
- PK11SlotInfo *slot;
+ static const char pem_slotname[] = "PEM Token #1";
+ SECItem cert_der = { 0, NULL, 0 };
void *proto_win = SSL_RevealPinArg(sock);
- *pRetKey = NULL;
- *pRetCert = PK11_FindCertFromNickname(nickname, proto_win);
- if(NULL == *pRetCert) {
- failf(data, "NSS: client certificate not found: %s", nickname);
+ PK11SlotInfo *slot = PK11_FindSlotByName(pem_slotname);
+ if(NULL == slot) {
+ failf(data, "NSS: PK11 slot not found: %s", pem_slotname);
return SECFailure;
}
- slot = PK11_FindSlotByName(pem_slotname);
- if(NULL == slot) {
- failf(data, "NSS: PK11 slot not found: %s", pem_slotname);
+ if(PK11_ReadRawAttribute(PK11_TypeGeneric, connssl->obj_clicert, CKA_VALUE,
+ &cert_der) != SECSuccess) {
+ failf(data, "NSS: CKA_VALUE not found in PK11 generic object");
+ PK11_FreeSlot(slot);
+ return SECFailure;
+ }
+
+ *pRetCert = PK11_FindCertFromDERCertItem(slot, &cert_der, proto_win);
+ SECITEM_FreeItem(&cert_der, PR_FALSE);
+ if(NULL == *pRetCert) {
+ failf(data, "NSS: client certificate from file not found");
+ PK11_FreeSlot(slot);
return SECFailure;
}
*pRetKey = PK11_FindPrivateKeyFromCert(slot, *pRetCert, NULL);
PK11_FreeSlot(slot);
if(NULL == *pRetKey) {
- failf(data, "NSS: private key not found for certificate: %s", nickname);
+ failf(data, "NSS: private key from file not found");
+ CERT_DestroyCertificate(*pRetCert);
return SECFailure;
}
- infof(data, "NSS: client certificate: %s\n", nickname);
+ infof(data, "NSS: client certificate from file\n");
display_cert_info(data, *pRetCert);
return SECSuccess;
}
+#endif
/* use the default NSS hook */
if(SECSuccess != NSS_GetClientAuthData((void *)nickname, sock, caNames,
@@ -1076,6 +1091,7 @@ void Curl_nss_close(struct connectdata *conn, int sockindex)
/* destroy all NSS objects in order to avoid failure of NSS shutdown */
Curl_llist_destroy(connssl->obj_list, NULL);
connssl->obj_list = NULL;
+ connssl->obj_clicert = NULL;
#endif
PR_Close(connssl->handle);
connssl->handle = NULL;
diff --git a/lib/urldata.h b/lib/urldata.h
index 8965c0b..5a4168c 100644
--- a/lib/urldata.h
+++ b/lib/urldata.h
@@ -273,6 +273,7 @@ struct ssl_connect_data {
struct SessionHandle *data;
#ifdef HAVE_PK11_CREATEGENERICOBJECT
struct curl_llist *obj_list;
+ PK11GenericObject *obj_clicert;
#endif
#endif /* USE_NSS */
#ifdef USE_QSOSSL
--
1.7.4.4
From 3daf67d9a819b52dfa156168e0971ddd6a4c56e6 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 26 Aug 2011 14:53:26 +0200
Subject: [PATCH 2/4] nss: refactor fmt_nickname() -> dup_nickname()
Do not use artificial nicknames for certificates from files.
---
lib/nss.c | 42 ++++++++++++++----------------------------
1 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/lib/nss.c b/lib/nss.c
index 2d0f217..3f8767f 100644
--- a/lib/nss.c
+++ b/lib/nss.c
@@ -278,17 +278,16 @@ static int is_file(const char *filename)
return 0;
}
-/* Return on heap allocated filename/nickname of a certificate. The returned
- * string should be later deallocated using free(). *is_nickname is set to
- * TRUE if the given string is treated as nickname; FALSE if the given string
- * is treated as file name.
+/* Check if the given string is filename or nickname of a certificate. If the
+ * given string is considered a filename, return NULL. If the given string is
+ * considered a nickname, return a duplicated string. The returned string
+ * should be later deallocated using free(). If the OOM state occurs, we
+ * return NULL, too.
*/
-static char *fmt_nickname(struct SessionHandle *data, enum dupstring cert_kind,
- bool *is_nickname)
+static char* dup_nickname(struct SessionHandle *data, enum dupstring cert_kind)
{
const char *str = data->set.str[cert_kind];
const char *n;
- *is_nickname = TRUE;
if(!is_file(str))
/* no such file exists, use the string as nickname */
@@ -303,10 +302,7 @@ static char *fmt_nickname(struct SessionHandle *data, enum dupstring cert_kind,
}
/* we'll use the PEM reader to read the certificate from file */
- *is_nickname = FALSE;
-
- n++; /* skip last slash */
- return aprintf("PEM Token #%d:%s", 1, n);
+ return NULL;
}
#ifdef HAVE_PK11_CREATEGENERICOBJECT
@@ -1355,17 +1351,11 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex)
}
if(data->set.str[STRING_CERT]) {
- bool is_nickname;
- char *nickname = fmt_nickname(data, STRING_CERT, &is_nickname);
- if(!nickname)
- return CURLE_OUT_OF_MEMORY;
-
- if(!is_nickname && !cert_stuff(conn, sockindex, data->set.str[STRING_CERT],
- data->set.str[STRING_KEY])) {
+ char *nickname = dup_nickname(data, STRING_CERT);
+ if(!nickname && !cert_stuff(conn, sockindex, data->set.str[STRING_CERT],
+ data->set.str[STRING_KEY]))
/* failf() is already done in cert_stuff() */
- free(nickname);
return CURLE_SSL_CERTPROBLEM;
- }
/* store the nickname for SelectClientCert() called during handshake */
connssl->client_nickname = nickname;
@@ -1423,16 +1413,12 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex)
if(data->set.str[STRING_SSL_ISSUERCERT]) {
SECStatus ret = SECFailure;
- bool is_nickname;
- char *nickname = fmt_nickname(data, STRING_SSL_ISSUERCERT, &is_nickname);
- if(!nickname)
- return CURLE_OUT_OF_MEMORY;
-
- if(is_nickname)
+ char *nickname = dup_nickname(data, STRING_SSL_ISSUERCERT);
+ if(nickname) {
/* we support only nicknames in case of STRING_SSL_ISSUERCERT for now */
ret = check_issuer_cert(connssl->handle, nickname);
-
- free(nickname);
+ free(nickname);
+ }
if(SECFailure == ret) {
infof(data,"SSL certificate issuer check failed\n");
--
1.7.4.4
From 5f492f4c139317e4efd36e6059f0328a42ac8044 Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Fri, 26 Aug 2011 15:43:48 +0200
Subject: [PATCH 3/4] nss: big cleanup in nss_load_cert() and cert_stuff()
---
lib/nss.c | 150 ++++++++++++++++++++++---------------------------------------
1 files changed, 54 insertions(+), 96 deletions(-)
diff --git a/lib/nss.c b/lib/nss.c
index 3f8767f..5746e01 100644
--- a/lib/nss.c
+++ b/lib/nss.c
@@ -319,6 +319,9 @@ static CURLcode nss_create_object(struct ssl_connect_data *ssl,
CK_BBOOL ckfalse = CK_FALSE;
CK_ATTRIBUTE attrs[/* max count of attributes */ 4];
int attr_cnt = 0;
+ CURLcode err = (cacert)
+ ? CURLE_SSL_CACERT_BADFILE
+ : CURLE_SSL_CERTPROBLEM;
const int slot_id = (cacert) ? 0 : 1;
char *slot_name = aprintf("PEM Token #%d", slot_id);
@@ -328,7 +331,7 @@ static CURLcode nss_create_object(struct ssl_connect_data *ssl,
slot = PK11_FindSlotByName(slot_name);
free(slot_name);
if(!slot)
- return CURLE_SSL_CERTPROBLEM;
+ return err;
PK11_SETATTRS(attrs, attr_cnt, CKA_CLASS, &obj_class, sizeof(obj_class));
PK11_SETATTRS(attrs, attr_cnt, CKA_TOKEN, &cktrue, sizeof(CK_BBOOL));
@@ -343,7 +346,7 @@ static CURLcode nss_create_object(struct ssl_connect_data *ssl,
obj = PK11_CreateGenericObject(slot, attrs, attr_cnt, PR_FALSE);
PK11_FreeSlot(slot);
if(!obj)
- return CURLE_SSL_CERTPROBLEM;
+ return err;
if(!Curl_llist_insert_next(ssl->obj_list, ssl->obj_list->tail, obj)) {
PK11_DestroyGenericObject(obj);
@@ -368,80 +371,20 @@ static void nss_destroy_object(void *user, void *ptr)
}
#endif
-static int nss_load_cert(struct ssl_connect_data *ssl,
- const char *filename, PRBool cacert)
+static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
+ const char *filename, PRBool cacert)
{
-#ifdef HAVE_PK11_CREATEGENERICOBJECT
- /* All CA and trust objects go into slot 0. Other slots are used
- * for storing certificates.
- */
- const int slot_id = (cacert) ? 0 : 1;
-#endif
- CERTCertificate *cert;
- char *nickname = NULL;
- char *n = NULL;
-
- /* If there is no slash in the filename it is assumed to be a regular
- * NSS nickname.
- */
- if(is_file(filename)) {
- n = strrchr(filename, '/');
- if(n)
- n++;
- if(!mod)
- return 1;
- }
- else {
- /* A nickname from the NSS internal database */
- if(cacert)
- return 0; /* You can't specify an NSS CA nickname this way */
- nickname = strdup(filename);
- if(!nickname)
- return 0;
-
- /* we are not going to use libnsspem.so to read the client cert */
- ssl->obj_clicert = NULL;
-
- goto done;
- }
+ CURLcode err = (cacert)
+ ? CURLE_SSL_CACERT_BADFILE
+ : CURLE_SSL_CERTPROBLEM;
#ifdef HAVE_PK11_CREATEGENERICOBJECT
- nickname = aprintf("PEM Token #%d:%s", slot_id, n);
- if(!nickname)
- return 0;
-
- if(CURLE_OK != nss_create_object(ssl, CKO_CERTIFICATE, filename, cacert)) {
- free(nickname);
- return 0;
- }
-
-#else
- /* We don't have PK11_CreateGenericObject but a file-based cert was passed
- * in. We need to fail.
- */
- return 0;
+ /* libnsspem.so leaks memory if the requested file does not exist */
+ if(is_file(filename))
+ return nss_create_object(ssl, CKO_CERTIFICATE, filename, cacert);
#endif
- done:
- /* Double-check that the certificate or nickname requested exists in
- * either the token or the NSS certificate database.
- */
- if(!cacert) {
- cert = PK11_FindCertFromNickname((char *)nickname, NULL);
-
- /* An invalid nickname was passed in */
- if(cert == NULL) {
- free(nickname);
- PR_SetError(SEC_ERROR_UNKNOWN_CERT, 0);
- return 0;
- }
-
- CERT_DestroyCertificate(cert);
- }
-
- free(nickname);
-
- return 1;
+ return err;
}
/* add given CRL to cache if it is not already there */
@@ -530,23 +473,23 @@ fail:
return SECFailure;
}
-static int nss_load_key(struct connectdata *conn, int sockindex,
- char *key_file)
+static CURLcode nss_load_key(struct connectdata *conn, int sockindex,
+ char *key_file)
{
#ifdef HAVE_PK11_CREATEGENERICOBJECT
PK11SlotInfo *slot;
SECStatus status;
struct ssl_connect_data *ssl = conn->ssl;
- (void)sockindex; /* unused */
- if(CURLE_OK != nss_create_object(ssl, CKO_PRIVATE_KEY, key_file, FALSE)) {
+ CURLcode rv = nss_create_object(ssl, CKO_PRIVATE_KEY, key_file, FALSE);
+ if(CURLE_OK != rv) {
PR_SetError(SEC_ERROR_BAD_KEY, 0);
- return 0;
+ return rv;
}
slot = PK11_FindSlotByName("PEM Token #1");
if(!slot)
- return 0;
+ return CURLE_SSL_CERTPROBLEM;
/* This will force the token to be seen as re-inserted */
SECMOD_WaitForAnyTokenEvent(mod, 0, 0);
@@ -555,16 +498,18 @@ static int nss_load_key(struct connectdata *conn, int sockindex,
status = PK11_Authenticate(slot, PR_TRUE,
conn->data->set.str[STRING_KEY_PASSWD]);
PK11_FreeSlot(slot);
- return (SECSuccess == status) ? 1 : 0;
+ return (SECSuccess == status)
+ ? CURLE_OK
+ : CURLE_SSL_CERTPROBLEM;
#else
/* If we don't have PK11_CreateGenericObject then we can't load a file-based
* key.
*/
(void)conn; /* unused */
(void)key_file; /* unused */
- (void)sockindex; /* unused */
- return 0;
+ return CURLE_SSL_CERTPROBLEM;
#endif
+ (void)sockindex; /* unused */
}
static int display_error(struct connectdata *conn, PRInt32 err,
@@ -583,34 +528,37 @@ static int display_error(struct connectdata *conn, PRInt32 err,
return 0; /* The caller will print a generic error */
}
-static int cert_stuff(struct connectdata *conn,
- int sockindex, char *cert_file, char *key_file)
+static CURLcode cert_stuff(struct connectdata *conn, int sockindex,
+ char *cert_file, char *key_file)
{
struct SessionHandle *data = conn->data;
- int rv = 0;
+ CURLcode rv;
if(cert_file) {
rv = nss_load_cert(&conn->ssl[sockindex], cert_file, PR_FALSE);
- if(!rv) {
+ if(CURLE_OK != rv) {
if(!display_error(conn, PR_GetError(), cert_file))
failf(data, "Unable to load client cert %d.", PR_GetError());
- return 0;
+
+ return rv;
}
}
+
if(key_file || (is_file(cert_file))) {
if(key_file)
rv = nss_load_key(conn, sockindex, key_file);
else
/* In case the cert file also has the key */
rv = nss_load_key(conn, sockindex, cert_file);
- if(!rv) {
+ if(CURLE_OK != rv) {
if(!display_error(conn, PR_GetError(), key_file))
failf(data, "Unable to load client key %d.", PR_GetError());
- return 0;
+ return rv;
}
}
- return 1;
+
+ return CURLE_OK;
}
static char * nss_get_password(PK11SlotInfo * slot, PRBool retry, void *arg)
@@ -777,7 +725,6 @@ static SECStatus check_issuer_cert(PRFileDesc *sock,
cert_issuer = CERT_FindCertIssuer(cert,PR_Now(),certUsageObjectSigner);
proto_win = SSL_RevealPinArg(sock);
- issuer = NULL;
issuer = PK11_FindCertFromNickname(issuer_nickname, proto_win);
if((!cert_issuer) || (!issuer))
@@ -1135,8 +1082,11 @@ static CURLcode nss_load_ca_certificates(struct connectdata *conn,
const char *cafile = data->set.ssl.CAfile;
const char *capath = data->set.ssl.CApath;
- if(cafile && !nss_load_cert(&conn->ssl[sockindex], cafile, PR_TRUE))
- return CURLE_SSL_CACERT_BADFILE;
+ if(cafile) {
+ CURLcode rv = nss_load_cert(&conn->ssl[sockindex], cafile, PR_TRUE);
+ if(CURLE_OK != rv)
+ return rv;
+ }
if(capath) {
struct_stat st;
@@ -1156,7 +1106,7 @@ static CURLcode nss_load_ca_certificates(struct connectdata *conn,
return CURLE_OUT_OF_MEMORY;
}
- if(!nss_load_cert(&conn->ssl[sockindex], fullpath, PR_TRUE))
+ if(CURLE_OK != nss_load_cert(&conn->ssl[sockindex], fullpath, PR_TRUE))
/* This is purposefully tolerant of errors so non-PEM files can
* be in the same directory */
infof(data, "failed to load '%s' from CURLOPT_CAPATH\n", fullpath);
@@ -1352,10 +1302,18 @@ CURLcode Curl_nss_connect(struct connectdata *conn, int sockindex)
if(data->set.str[STRING_CERT]) {
char *nickname = dup_nickname(data, STRING_CERT);
- if(!nickname && !cert_stuff(conn, sockindex, data->set.str[STRING_CERT],
- data->set.str[STRING_KEY]))
- /* failf() is already done in cert_stuff() */
- return CURLE_SSL_CERTPROBLEM;
+ if(nickname)
+ /* we are not going to use libnsspem.so to read the client cert */
+ connssl->obj_clicert = NULL;
+ else {
+ CURLcode rv = cert_stuff(conn, sockindex, data->set.str[STRING_CERT],
+ data->set.str[STRING_KEY]);
+ if(CURLE_OK != rv) {
+ /* failf() is already done in cert_stuff() */
+ curlerr = rv;
+ goto error;
+ }
+ }
/* store the nickname for SelectClientCert() called during handshake */
connssl->client_nickname = nickname;
--
1.7.4.4
From f95bd2e5bc739e39dedf1f5eba0f65d0aaa27d5f Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Tue, 6 Sep 2011 18:17:38 +0200
Subject: [PATCH 4/4] nss: avoid a SIGSEGV with immature version of NSS
Bug: https://bugzilla.redhat.com/733685
---
lib/nss.c | 24 +++++++++++++++++++++++-
1 files changed, 23 insertions(+), 1 deletions(-)
diff --git a/lib/nss.c b/lib/nss.c
index 5746e01..7a45e2b 100644
--- a/lib/nss.c
+++ b/lib/nss.c
@@ -381,7 +381,29 @@ static CURLcode nss_load_cert(struct ssl_connect_data *ssl,
#ifdef HAVE_PK11_CREATEGENERICOBJECT
/* libnsspem.so leaks memory if the requested file does not exist */
if(is_file(filename))
- return nss_create_object(ssl, CKO_CERTIFICATE, filename, cacert);
+ err = nss_create_object(ssl, CKO_CERTIFICATE, filename, cacert);
+
+ if(CURLE_OK == err && !cacert) {
+ /* we have successfully loaded a client certificate */
+ CERTCertificate *cert;
+ char *nickname = NULL;
+ char *n = strrchr(filename, '/');
+ if(n)
+ n++;
+
+ /* The following undocumented magic helps to avoid a SIGSEGV on call
+ * of PK11_ReadRawAttribute() from SelectClientCert() when using For
+ * an immature version of libnsspem.so. For more details, go to
+ * <https://bugzilla.redhat.com/733685>. */
+ nickname = aprintf("PEM Token #1:%s", n);
+ if(nickname) {
+ cert = PK11_FindCertFromNickname(nickname, NULL);
+ if(cert)
+ CERT_DestroyCertificate(cert);
+
+ free(nickname);
+ }
+ }
#endif
return err;
--
1.7.4.4

View File

@ -1,13 +1,16 @@
Summary: A utility for getting files from remote servers (FTP, HTTP, and others)
Name: curl
Version: 7.22.0
Release: 1%{?dist}
Release: 2%{?dist}
License: MIT
Group: Applications/Internet
Source: http://curl.haxx.se/download/%{name}-%{version}.tar.lzma
Source2: curlbuild.h
Source3: hide_selinux.c
# nss: select client certificates by DER (#733657)
Patch1: 0001-curl-7.22.0-bz733657.patch
# patch making libcurl multilib ready
Patch101: 0101-curl-7.21.1-multilib.patch
@ -106,6 +109,9 @@ for f in CHANGES README; do
mv -f ${f}.utf8 ${f}
done
# upstream patches (not yet applied)
%patch1 -p1
# Fedora patches
%patch101 -p1
%patch102 -p1
@ -218,6 +224,9 @@ rm -rf $RPM_BUILD_ROOT
%{_datadir}/aclocal/libcurl.m4
%changelog
* Mon Sep 19 2011 Kamil Dudka <kdudka@redhat.com> 7.22.0-2
- nss: select client certificates by DER (#733657)
* Tue Sep 13 2011 Kamil Dudka <kdudka@redhat.com> 7.22.0-1
- new upstream release
- curl-config now provides dummy --static-libs option (#733956)