Could we get more context and details on how the problem manifests itself? I need to set up a bug reproduction environment locally.

here's what I do in python.

import pycurl

c = pycurl.Curl()
c.setopt(pycurl.SSLKEY, '/path/to/pemkey')
c.setopt(pycurl.SSLCERT, '/path/to/certfile')
c.setopt(pycurl.URL, 'https://someplace')
c.perform()
c.reset()
c.setopt(pycurl.SSLKEY, '/path/to/other/pemkey')
c.setopt(pycurl.SSLCERT, '/path/to/other/certfile')
c.setopt(pycurl.URL, 'https://someplace/otherpath')
c.perform()

the second one will fail to use the other key/certfile - it will only use the first one.

If you need a more detailed test I can get you one if you have a fedora 14 or a rhel6 box handy.

Thanks, this is very useful. Yes, please send me any test you think may help as I do have several f14 and rhel6 VMs handy. I do want to confirm certain suspicions I have based on my reading on the curl and the pem module sources.

Regarding "the second one will fail to use the other key/certfile - it will only use the first one" this could happen if the second one was never added to the in-memory database or if it was added but when searching for the key the second time nss (pem) returned the first one instead. I am looking at the curl code at https://github.com/bagder/curl/blob/master/lib/nss.c#L798
and wonder if one of those Find calls could be returning the first one. The nickname is based on the file name. I need to trace and see what happens when we add the second cert and key.

Bob said it was not supported to have two keys in a single slot and I suspect that's the main obstacle here:

https://bugzilla.mozilla.org/show_bug.cgi?id=402712#c54

I noticed the SSL caching game is also involved here -- bug 678580 .  When connecting the second time to the same server the SelectClientCert() callback is not called at all.  Elio, please try it with this patch for libcurl:

--- a/lib/nss.c
+++ b/lib/nss.c
@@ -1230,7 +1230,7 @@
     goto error;

   /* do not use SSL cache if we are not going to verify peer */
-  ssl_no_cache = (data->set.ssl.verifypeer) ? PR_FALSE : PR_TRUE;
+  ssl_no_cache = /*(data->set.ssl.verifypeer) ? PR_FALSE :*/ PR_TRUE;
   if(SSL_OptionSet(model, SSL_NO_CACHE, ssl_no_cache) != SECSuccess)
     goto error;

Note this seems to be an orthogonal problem to the one key per slot issue.

I'll gladly try it but I need help with a reproducer that's ssl capable and calls libcurl. I could try implementing what's enumareted on comment 3 but to save time, could I get the actual app that found the bug?

The tool I'm modifying isn't ssl capable. It was just indended to add certs one after the other to the in-memory database so I could trace through pem in the debugger.

Created attachment 486838 [details]
libcurl-based testing client

Elio, you need to link it with -lcurl.  Then give it a CA cert, two client certs and two https:// URLs.  I will try to connect both, one after one, writing some verbose output to stderr.  As for the server, you have three possibilities:

1) Use the server that Seth wants to get working with yum.

2) Use the testing server from NSS.

3) Use s_server from OpenSSL.

Created attachment 489323 [details]
this solves the issue #1

I'll need to do some additional debugging to see how to fix the SSL cache problem.

I've prepared scratch builds of nss and libcurl for testing.

https://brewweb.devel.redhat.com/taskinfo?taskID=3222298
https://brewweb.devel.redhat.com/taskinfo?taskID=3222305

Note the "fix" for libcurl is wrong and desperate.  This needs to be fixed properly, and preferably in nss.

Seth, could you please give it a try?

I think we should hook somewhere around the following condition:

        /* If we previously did client-auth, make sure that the token that
        ** holds the private key still exists, is logged in, hasn't been
        ** removed, etc.
        */
        if (sidOK && !ssl3_ClientAuthTokenPresent(sid)) {
            sidOK = PR_FALSE;
        }

If I return PR_FALSE from ssl3_ClientAuthTokenPresent() in a debugger, it connects fine with unpatched libcurl.  Any chance to invalidate the client authentication for a particular SSL socket?

Created attachment 489418 [details]
less intrusive patch for libcurl

I am not sure if there is a better way to force nss to use the right client cert on the second attempt...

I should mention that I saw something strange last night while tracing through pem's AddObjectIfNeeded where the ca cert was inserted multiple times into PEM's internal object list at different slots, but I didn't have either your patches applied.

Kamil, Before I review your patches let me test the second one (attachment 489418 [details]) with and without your patch for nss and trace in the debugger again.

Both of them need to be applied to get yum working.  attachment #489323 [details] makes it possible to use more than one client cert, while attachment #489418 [details] prevents nss from using the cached client cert on the subsequent request.

Here is a scratch build of libcurl with attachment #48941 applied:

https://brewweb.devel.redhat.com/taskinfo?taskID=3222960

I ran with both your patches applied and I get a connection.

The strange behaviour I had mentioned earlier is reproducible as is:

Breakpoint 2, AddObjectIfNeeded (objClass=3461563219, type=pemTrust, certDER=0x80d4978, keyDER=0x0, filename=0xbfffe74c "redhat-uep.pem - 2", objid=0, slotID=0) at pinst.c:338
338	                && (0 == strcmp(gobj[i]->nickname, nickname))) {
(gdb) print gobj[0]->nickname
$1 = 0x8104198 "redhat-uep.pem - 0" ---this is expected
(gdb) print gobj[1]->nickname
$2 = 0x8105498 "redhat-uep.pem - 0" --- unexpected
(gdb) print gobj[2]->nickname
$3 = 0x8105d78 "redhat-uep.pem - 1" --- unexpected
(gdb) print gobj[3]->nickname
$4 = 0x8105de0 "redhat-uep.pem - 1" --- unexpected
(gdb) print gobj[4]->nickname
$5 = 0x8109510 "redhat-uep.pem - 2" --- unexpected
(gdb) print gobj[5]->nickname

(In reply to comment #29)
> Breakpoint 2, AddObjectIfNeeded (objClass=3461563219, type=pemTrust,
> certDER=0x80d4978, keyDER=0x0, filename=0xbfffe74c "redhat-uep.pem - 2",
> objid=0, slotID=0) at pinst.c:338
> 338                 && (0 == strcmp(gobj[i]->nickname, nickname))) {
> (gdb) print gobj[0]->nickname
> $1 = 0x8104198 "redhat-uep.pem - 0" ---this is expected
> (gdb) print gobj[1]->nickname
> $2 = 0x8105498 "redhat-uep.pem - 0" --- unexpected

I spotted that nonsense a month ago -- see bug #674570 comment #12

All you need is attachment #479980 [details] I guess.

Wait a second, I'm the submitter of that patch and you it's reviewer :-)

> I spotted that nonsense a month ago -- see bug #674570 comment #12
> 
> All you need is attachment #479980 [details] I guess.
The patch in this attachment makes things worst. If applied the CA cert is then considered untrusted. Those, so called nonsensical, entries are needed. 
(In reply to comment #29)
> Breakpoint 2, AddObjectIfNeeded (objClass=3461563219, type=pemTrust,
> certDER=0x80d4978, keyDER=0x0, filename=0xbfffe74c "redhat-uep.pem - 2",
> objid=0, slotID=0) at pinst.c:338
> 338                 && (0 == strcmp(gobj[i]->nickname, nickname))) {
> (gdb) print gobj[0]->nickname
> $1 = 0x8104198 "redhat-uep.pem - 0" ---this is expected
> (gdb) print gobj[1]->nickname
> $2 = 0x8105498 "redhat-uep.pem - 0" --- unexpected
> (gdb) print gobj[2]->nickname
> $3 = 0x8105d78 "redhat-uep.pem - 1" --- unexpected
> (gdb) print gobj[3]->nickname
> $4 = 0x8105de0 "redhat-uep.pem - 1" --- unexpected
> (gdb) print gobj[4]->nickname
> $5 = 0x8109510 "redhat-uep.pem - 2" --- unexpected
> (gdb) print gobj[5]->nickname ---- has nothing
and the next continue ends the program

Do notice that in this list there are no entries for the client 1 nor client 2 certificates. I still haven't seen the attempt to add the client cert (with key) in the breakpoints at AddObjectIfNeeded.

That earlier remark, "I get a connection.", is mistaken. I tried to reproduce it and the http output coming back from the second URL had the "error 403" in it, so the server is rejecting the attempt for access. It worked for Kamil, Has anyone else tried it? I used Kamil's scratch build as well as one of my own. Mine had the same patches applied as kamil's but had debug symbols on and optimizations disabled.

(In reply to comment #32)
> > All you need is attachment #479980 [details] I guess.
> The patch in this attachment makes things worst. If applied the CA cert is then
> considered untrusted. Those, so called nonsensical, entries are needed. 

Right, that patch breaks everything.

> Do notice that in this list there are no entries for the client 1 nor client 2
> certificates. I still haven't seen the attempt to add the client cert (with
> key) in the breakpoints at AddObjectIfNeeded.

Did you try to revert the patch above?

> That earlier remark, "I get a connection.", is mistaken. I tried to reproduce
> it and the http output coming back from the second URL had the "error 403" in
> it, so the server is rejecting the attempt for access. It worked for Kamil, Has
> anyone else tried it? I used Kamil's scratch build as well as one of my own.
> Mine had the same patches applied as kamil's but had debug symbols on and
> optimizations disabled.

My scratch builds are also built without optimizations, the debug symbols are provided via *-debug packages as usually.  The breaking patch is not there, which means they should work fine.  In case anybody wants to give them a try, here they are:

https://brewweb.devel.redhat.com/taskinfo?taskID=3222298
https://brewweb.devel.redhat.com/taskinfo?taskID=3222960

> Did you try to revert the patch above?
Yes, reverting gave results like the ones quoted above which actually come from before even trying the bad patch.
> 
> > That earlier remark, "I get a connection.", is mistaken. I tried to reproduce
> > it and the http output coming back from the second URL had the "error 403" in
> > it, so the server is rejecting the attempt for access. It worked for Kamil, Has
> > anyone else tried it? I used Kamil's scratch build as well as one of my own.
> > Mine had the same patches applied as kamil's but had debug symbols on and
> > optimizations disabled.
> 
> My scratch builds are also built without optimizations, the debug symbols are
> provided via *-debug packages as usually.  The breaking patch is not there,
> which means they should work fine.  In case anybody wants to give them a try,
> here they are:

NSS has an unconventional build system. It's easy to get debug symbols but in order to truly disable optimizations I do as follows:
Uncomment these two lines:
RPM_OPT_FLAGS=`echo $RPM_OPT_FLAGS | sed -e 's/-O2/-O0/g'`
export RPM_OPT_FLAGS

> 
> https://brewweb.devel.redhat.com/taskinfo?taskID=3222298
> https://brewweb.devel.redhat.com/taskinfo?taskID=3222960
Yes, do try Kamil's builds. 
My own nss scratch build is at 
https://brewweb.devel.redhat.com/taskinfo?taskID=3224825

By bad, I had the client certificates backwards. Once I switched them it works. I got.

[emaldona@elio bz689031]$ ./bz689031 redhat-uep.pem ./c1.crt ./c2.crt https://cdn.redhat.com/content/beta/rhel/entitlement/6Server/x86_64/os/repodata/repomd.xml https://cdn.redhat.com/content/beta/rhel/server/6/6Server/x86_64/scalablefilesystem/os/repodata/repomd.xml
* About to connect() to cdn.redhat.com port 443 (#0)
*   Trying 184.85.236.251... * connected
* Connected to cdn.redhat.com (184.85.236.251) port 443 (#0)
* Initializing NSS with certpath: /etc/pki/nssdb
*   CAfile: redhat-uep.pem
  CApath: none
* NSS: client certificate: PEM Token #1:c1.crt
* 	subject: CN="admin/UID=deadbeef"
* 	start date: Feb 28 16:58:56 2011 GMT
* 	expire date: Feb 28 16:58:56 2012 GMT
* 	common name: admin/UID=deadbeef
* 	issuer: E=ca-support,CN=Red Hat Entitlement Operations Authority,OU=Red Hat Network,O="Red Hat, Inc.",ST=North Carolina,C=US
* SSL connection using TLS_RSA_WITH_AES_256_CBC_SHA
* Server certificate:
* 	subject: CN=cdn.redhat.com,OU=IT,O="Red Hat, Inc.",L=Raleigh,ST=NORTH CAROLINA,C=US
* 	start date: Jul 19 16:16:14 2010 GMT
* 	expire date: Jul 16 16:16:14 2020 GMT
* 	common name: cdn.redhat.com
* 	issuer: E=ca-support,CN=Red Hat Entitlement Operations Authority,OU=Red Hat Network,O="Red Hat, Inc.",ST=North Carolina,C=US
> GET /content/beta/rhel/entitlement/6Server/x86_64/os/repodata/repomd.xml HTTP/1.1
Host: cdn.redhat.com
Accept: */*

< HTTP/1.1 200 OK
< Server: Apache
< ETag: "93699391d297bf465bd708264a0d4edd:1301677616"
< Last-Modified: Fri, 01 Apr 2011 17:06:56 GMT
< Accept-Ranges: bytes
< Content-Length: 2201
< Content-Type: application/xml
< Date: Sun, 03 Apr 2011 18:27:29 GMT
< X-Cache: TCP_HIT from a63-233-112-196 (AkamaiGHost/6.3.4.1-7779626) (-)
< Connection: keep-alive
< X-Akamai-Request-ID: 7c4cdba
< 
* Connection #0 to host cdn.redhat.com left intact
* Closing connection #0
* About to connect() to cdn.redhat.com port 443 (#0)
*   Trying 184.85.236.251... * connected
* Connected to cdn.redhat.com (184.85.236.251) port 443 (#0)
*   CAfile: redhat-uep.pem
  CApath: none
* SSL connection using TLS_RSA_WITH_AES_256_CBC_SHA
* Server certificate:
* 	subject: CN=cdn.redhat.com,OU=IT,O="Red Hat, Inc.",L=Raleigh,ST=NORTH CAROLINA,C=US
* 	start date: Jul 19 16:16:14 2010 GMT
* 	expire date: Jul 16 16:16:14 2020 GMT
* 	common name: cdn.redhat.com
* 	issuer: E=ca-support,CN=Red Hat Entitlement Operations Authority,OU=Red Hat Network,O="Red Hat, Inc.",ST=North Carolina,C=US
> GET /content/beta/rhel/server/6/6Server/x86_64/scalablefilesystem/os/repodata/repomd.xml HTTP/1.1
Host: cdn.redhat.com
Accept: */*

< HTTP/1.1 200 OK
< Server: Apache
< ETag: "af21ea77ebafaa5e842e26530a0022c2:1300752586"
< Last-Modified: Tue, 22 Mar 2011 00:09:46 GMT
< Accept-Ranges: bytes
< Content-Length: 2472
< Content-Type: application/xml
< Date: Sun, 03 Apr 2011 18:27:30 GMT
< X-Cache: TCP_REFRESH_HIT from a63-233-112-196 (AkamaiGHost/6.3.4.1-7779626) (S)
< Connection: keep-alive
< X-Akamai-Request-ID: 7c4ce44
< 
* Connection #0 to host cdn.redhat.com left intact
* Closing connection #0

How do I get the CA cert to be trusted so the original reproducer works?

Keeping file names and pem files with their URL'ss straight, the sequence
cat 11297891535705194.pem key.pem > cc1.crt
cat rcm-debug.crt rcm-debug.key > cc2.crt
 
./bz689031 redhat-uep.pem ./cc1.crt ./cc2.crt \
> https://cdn.redhat.com/content/beta/rhel/entitlement/6Server/x86_64/os/repodata/repomd.xml \
> https://cdn.redhat.com/content/beta/rhel/server/6/6Server/x86_64/scalablefilesystem/os/repodata/repomd.xml
does work.

I have prepared a new scratch build of libcurl that includes also a patch for bug #690273 (default build with -O2, since we mostly debug at the NSS level anyway):

https://brewweb.devel.redhat.com/taskinfo?taskID=3226621

Comment on attachment 489323 [details]
this solves the issue #1

This is in fact  consistent with  AddObjectIfNeeded 

#if 0
                /* FIXME: is it safe to return object with different objid? */
                && (atoi(gobj[i]->id.data) == objid)
#endif

the usage of gobj[i]->id.data as criteria for detecting duplicates is disabled.

(In reply to comment #45)
> I have prepared a new scratch build of libcurl that includes also a patch for
> bug #690273 (default build with -O2, since we mostly debug at the NSS level
> anyway):
> 
> https://brewweb.devel.redhat.com/taskinfo?taskID=3226621

I was able to reproduce this by using the script in comment 3.  After updating the nss/curl/libcurl packages using the ones suggested in brew, the problem still exists.

(In reply to comment #47)
> I was able to reproduce this by using the script in comment 3.  After updating
> the nss/curl/libcurl packages using the ones suggested in brew, the problem
> still exists.

Could you please attach libcurl verbose output of the failure?

http://curl.haxx.se/libcurl/c/curl_easy_setopt.html#CURLOPTVERBOSE

As for the separate keys loading, I think the problem is caused by the additional call of PK11_CreateGenericObject() -- twice for the client cert, then for the key.  But I need to verify my hypothesis with a debugger.  Then I'll see if this is better to fix/workaround in libcurl or rather in nss.

Elio, have you tracked down the issue with CA cert verification in your testing setup?  Is it anyhow related to my patches?

Yes Denise we can help with a test environment on which this bug can be tested/reproduced.  As of today however, the Stage environment is being refreshed, so we lost all the users/product/subscriptions that were used by jmolet in comment 47.  Once the stage environment is rebuilt by the IT team, then we can help scaffold a test environment.

(In reply to comment #59)
> As for the separate keys loading, I think the problem is caused by the
> additional call of PK11_CreateGenericObject() -- twice for the client cert,
> then for the key.  But I need to verify my hypothesis with a debugger.  Then
> I'll see if this is better to fix/workaround in libcurl or rather in nss.

Nope, the problem with jmolet's reproducer was that both keys had the same basename.  I'll prepare a new patch for nss.

Created attachment 490534 [details]
proposed fix V2

Created attachment 490650 [details]
Supplemental Patch #1 - assertions

When we think that a new object (cert/key) has been seen already and plan to reuse what we think we have on the list, do an assertions based on the DER encoding of the cert or the private key as the case may be. The DER is a more dependable way of comparing to begin with. This patch would need adjustemts as the code has moved. I would like to include it.

Created attachment 490651 [details]
Supplemental patch #2 - Logging -- for informational purpose

This is not required but may prove useful and save headaches. I use while debugging because I have run into crashes when running tests as regular user and the log file had been previously created while running as root. I will log this as a separate bug. In the meantime, use it if you run into problems while tracing with a debug build.

Comment on attachment 490650 [details]
Supplemental Patch #1 - assertions

As far as I understand, PR_ASSERT() is noop when building production packages.  So this change is useful only for debug builds.

Giving r- as this may crash on SIGSEGV in case ->len of the new item is greater than length of the stored item.

Comment on attachment 490651 [details]
Supplemental patch #2 - Logging -- for informational purpose

see https://bugzilla.redhat.com/attachment.cgi?id=490990
and Bug 695018

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHBA-2011-0692.html

Comment on attachment 489418 [details]
less intrusive patch for libcurl

Cancelling as this patch or something like it was applied and this bug is closed.