Bug 472489 - libcurl linked with nss is incompatible with librpm linked against nss
Summary: libcurl linked with nss is incompatible with librpm linked against nss
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: curl
Version: 9
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jindrich Novy
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-11-21 05:22 UTC by Toshio Ernie Kuratomi
Modified: 2013-07-02 23:32 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-21 08:31:00 UTC
Type: ---


Attachments (Terms of Use)
Check whether we initialized NSS separately (2.65 KB, patch)
2008-11-21 18:42 UTC, Toshio Ernie Kuratomi
no flags Details | Diff
See if any ciphers are enabled (3.07 KB, patch)
2008-12-03 23:47 UTC, Rob Crittenden
no flags Details | Diff

Description Toshio Ernie Kuratomi 2008-11-21 05:22:04 UTC
Description of problem:
When using libcurl linked against libnss, the python pycurl and rpm modules cannot be used together otherwise connections to https URLs fails.

I'm not sure whether this is a problem in nss, the patches to make 

Version-Release number of selected component (if applicable):
libcurl-7.18.2-6.fc9.i386
rpm-python-4.4.2.3-2.fc9.i386
python-pycurl-7.18.2-1.fc9.i386
nss-3.12.1.1-5.fc9.i386

How reproducible:
Everytime

Steps to Reproduce:
1. Attempt to run the following script:

#!/usr/bin/python -tt
import rpm
import pycurl
c = pycurl.Curl()
c.setopt(pycurl.URL, 'https://admin.fedoraproject.org/pkgdb')
c.perform()
  
Actual results:
Python traceback ending with 
pycurl.error: (35, 'SSL connect error')


Expected results:
html page returned from the method

Additional info:
I recompiled our curl package against openssl instead of libnss and was able to retrieve the page.  I'm not sure if there's a problem with libnss, the specific patches to get rpm to use nss or the patches to get curl to use nss but something is clashing.

I can do more debugging on this but need some help as to what tests would be helpful to diagnose this further.

Comment 1 Toshio Ernie Kuratomi 2008-11-21 15:34:14 UTC
I compiled a version of curl with nss but without our two nss patches and found that that version works.  Looking into which of our two nss patches is causing the issues now.

Comment 2 Toshio Ernie Kuratomi 2008-11-21 15:45:46 UTC
This is the patch causing the issue: curl-7.18.2-nss-thread-safety.patch

Comment 3 Toshio Ernie Kuratomi 2008-11-21 16:22:54 UTC
And here's the offending hunk in the patch::

@@ -808,7 +823,8 @@
     return CURLE_OK;
 
   /* FIXME. NSS doesn't support multiple databases open at the same time. */
-  if(!initialized) {
+  PR_Lock(nss_initlock);
+  if(!initialized && !NSS_IsInitialized()) {
     initialized = 1;

If I change that hunk to this the test case works::

@@ -808,7 +823,8 @@
     return CURLE_OK;
 
   /* FIXME. NSS doesn't support multiple databases open at the same time. */
-  if(!initialized) {
+  PR_Lock(nss_initlock);
+  if(!initialized) {
     initialized = 1;
 

At this point I really have reached the end of where I can effectively diagnose this.  Someone who's familiar with nss should look at whether the right thing to do is to remove the NSS_IsInitialized() check as I've done or to move some of the code from within the !initialized block out.  I can experiment more but I'll be largely in the dark about whether the fix is "correct", just whether or not it works.

Comment 4 Toshio Ernie Kuratomi 2008-11-21 16:42:19 UTC
This seems to be exactly the problem mentioned here:
  https://bugzilla.redhat.com/show_bug.cgi?id=459297#c9

Comment 5 Toshio Ernie Kuratomi 2008-11-21 18:42:12 UTC
Created attachment 324333 [details]
Check whether we initialized NSS separately

Here's the patch I'm using now.  It checks whether libcurl initialized nss and shuts down nss if so.  If not, it leaves it up to the program to shutdown nss separately.  It runs all of libcurl's initialization except for NSS_Initialize().  This should address all the comments I saw in bug #459297.

My previous comments about needing to have my work reviewed still stands, though :-)

It replaces the current curl-7.18.2-nss-thread-safety.patch

Comment 6 Toshio Ernie Kuratomi 2008-12-01 17:47:12 UTC
Hi Kai,

Not sure if you or rcrit would be the person to look at this revised patch.  The previous patch fails to properly initialize libcurl when NSS has already been initialized by some other code.  The revised patch attempts to be more fine grained but I'm not an NSS guru so a review would be appreciated.

Comment 7 Rob Crittenden 2008-12-03 22:10:51 UTC
Thanks doing so much of the research. I'm looking into this now.

Comment 8 Rob Crittenden 2008-12-03 23:13:37 UTC
Ok, I've figured out what the problem is but the solution isn't so obvious.

The real fix is the call to NSS_SetDomesticPolicy().

When NSS is initialized it disables all ciphers.

rpm is apparently not enabling any ciphers either directly or by calling NSS_Set*Policy().

So when it gets into libcurl and we detect that NSS is initialized we don't call NSS_SetDomesticPolicy(). This means that when NSS starts creating a client socket it finds that it has no ciphers enabled and gives up.

What we need is an AreAnyCiphersEnabled() call. If this returns FALSE then we can call NSS_SetDomesticPolicy(). Unfortunately no such call currently exists AFAIK.

We shouldn't simply call NSS_SetDomesticPolicy() because then any settings done prior to getting into libcurl will be silently dropped.

Comment 9 Rob Crittenden 2008-12-03 23:47:00 UTC
Created attachment 325609 [details]
See if any ciphers are enabled

New patch that adds a test to see if any ciphers have been enabled and if not, call NSS_SetDomesticPolicy().

Comment 10 Toshio Ernie Kuratomi 2008-12-06 00:10:11 UTC
Verified the new patch works here as a replacement for curl-7.18.2-nss-thread-safety.patch.

It seems like initializing globals in a library is prone to clashes, though (what if rpm did initialize the Policy but only enabled a single cipher, for instance?).  Does libnss not have a session or context abstraction?  So we could create an nss context and then initialize that context separately from the context that rpm creates?

Comment 11 Jindrich Novy 2008-12-08 14:07:34 UTC
Patch from comment #9 is now applied in rawhide.

Comment 12 Rob Crittenden 2008-12-08 14:18:06 UTC
You can only call NSS_Init* once in a program. Subsequent calls are ignored.

Calling NSS_Init* initializes some default values such as what ciphers are available (none by default).

One can override this on both the default level and the per-socket level. So globally you can enable cipher X but disable it in a given connection if you want.

Comment 13 Toshio Ernie Kuratomi 2008-12-08 18:19:00 UTC
(In reply to comment #11)
> Patch from comment #9 is now applied in rawhide.

Thanks jnovy.  If we could get this applied to Fedora 9 & 10 as well, it will fix some bugs we see when using koji's python bindings and pycurl together.

Comment 14 Luke Macken 2008-12-12 19:31:10 UTC
Jnovy, do you plan on pushing this fix to Fedora 8-10 ?

Comment 15 Fedora Update System 2008-12-15 09:16:56 UTC
curl-7.18.2-7.fc9 has been submitted as an update for Fedora 9.
http://admin.fedoraproject.org/updates/curl-7.18.2-7.fc9

Comment 16 Fedora Update System 2008-12-15 10:13:46 UTC
curl-7.18.2-7.fc8 has been submitted as an update for Fedora 8.
http://admin.fedoraproject.org/updates/curl-7.18.2-7.fc8

Comment 17 Fedora Update System 2008-12-15 13:52:59 UTC
curl-7.18.2-9.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/curl-7.18.2-9.fc10

Comment 18 Fedora Update System 2008-12-18 00:33:08 UTC
curl-7.18.2-7.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update curl'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F8/FEDORA-2008-11405

Comment 19 Fedora Update System 2008-12-18 00:39:51 UTC
curl-7.18.2-7.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing-newkey update curl'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-11375

Comment 20 Fedora Update System 2008-12-18 00:41:52 UTC
curl-7.18.2-9.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update curl'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11340

Comment 21 Fedora Update System 2008-12-21 08:30:56 UTC
curl-7.18.2-9.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2008-12-21 08:31:07 UTC
curl-7.18.2-7.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 23 Fedora Update System 2008-12-21 08:38:25 UTC
curl-7.18.2-7.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.