Bug 872124 - nss 3.14 breaks fedpkg new-sources
Summary: nss 3.14 breaks fedpkg new-sources
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: nss
Version: 18
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Elio Maldonado Batiz
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 872838
TreeView+ depends on / blocked
 
Reported: 2012-11-01 10:30 UTC by Dan Mashal
Modified: 2012-12-12 04:33 UTC (History)
11 users (show)

Fixed In Version: nss-3.15-8.fc19, nss-3.14-5.fc18
Clone Of:
: 872838 (view as bug list)
Environment:
Last Closed: 2012-11-27 05:04:10 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)
proposed fix (1.46 KB, patch)
2012-11-02 11:11 UTC, Kamil Dudka
rrelyea: review-
Details | Diff
Fix locking issue. Also authenticate on the correct session (15.46 KB, patch)
2012-11-02 18:17 UTC, Bob Relyea
emaldona: review+
Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Mozilla Foundation 357025 0 None None None 2012-11-02 18:26:40 UTC

Description Dan Mashal 2012-11-01 10:30:07 UTC
Description of problem:

fedpkg new-sources hangs

Version-Release number of selected component (if applicable):

3.14-1

How reproducible:

Always

Steps to Reproduce:

1. install nss 3.14-1
2. fedpkg clone <packagename>
3. cd packagename
4. fedpkg new-sources file.tar.gz
hang

Workaround: 

yum downgrade nss nss-utils fixes the problem instantly.

Comment 1 Kamil Dudka 2012-11-01 12:34:19 UTC
What version of libcurl are you using?

$ rpm -q libcurl

Comment 2 Yanko Kaneti 2012-11-01 12:45:58 UTC
libcurl-7.27.0-3.fc19.x86_64  for me on rawhide where I can observe the same problem.

Comment 3 Dan Mashal 2012-11-01 12:55:59 UTC
[dan@localhost ~]$ rpm -qa libcurl
libcurl-7.27.0-4.fc18.x86_64

Comment 4 Elio Maldonado Batiz 2012-11-01 16:09:27 UTC
I think I know the cause, when I nerged from master into f18 I din't to reenable a patch patch in spec file as I should have. 

$ grep cbc nss.spec
Patch29:          nss-ssl-cbc-random-iv-off-by-default.patch
$ grep patch29 nss.spec 
#%patch29 -p0 -b .770682

A build with the patch reactivated is coming soon.

Comment 5 Elio Maldonado Batiz 2012-11-01 19:16:34 UTC
I have tagged nss-3.14-3.fc18 into the f18 buildroot override. 
Dan, could you please confirm that 'fedpkg new-sources' now works? Thanks.

Comment 6 Elio Maldonado Batiz 2012-11-01 20:30:39 UTC
Bad news, though the changes are needed they don't fix the bug. I created a dummy sources test file so I can upload it and thus verify that the nss build no longer causes a hang in fedpkg 'new-sources' or 'fedpkg update'. When I tried it"

$ fedpkg upload dummy-sources-for-testing
Uploading: 90921e15a47f5c935a7c13bb4b3d2349  dummy-sources-for-testing
...
It still hangs! The problem is somewhere else.

Comment 7 Elio Maldonado Batiz 2012-11-01 21:54:20 UTC
Until I have a fix it's now unpushed from updates-testing.

Comment 8 Dan Mashal 2012-11-02 00:36:55 UTC
Thank you for your prompt attention to this matter.

Comment 9 Vít Ondruch 2012-11-02 09:45:53 UTC
I am afraid that F18 is broken anyway :(

http://koji.fedoraproject.org/koji/taskinfo?taskID=4648975

Comment 11 Kamil Dudka 2012-11-02 10:14:24 UTC
(In reply to comment #4)
> Patch29:          nss-ssl-cbc-random-iv-off-by-default.patch

This could hardly take an effect because libcurl controls this explicitly by calling SSL_OptionSet(model, SSL_CBC_RANDOM_IV, ...).

(In reply to comment #6)
> Bad news, though the changes are needed they don't fix the bug. I created a
> dummy sources test file so I can upload it and thus verify that the nss
> build no longer causes a hang in fedpkg 'new-sources' or 'fedpkg update'.
> When I tried it"
> 
> $ fedpkg upload dummy-sources-for-testing
> Uploading: 90921e15a47f5c935a7c13bb4b3d2349  dummy-sources-for-testing

I am not sure if a real upload is necessary.  The following minimal example also fails with the new version of nss:

curl -svo/dev/null \
    --cert $HOME/.fedora.cert \
    --cacert $HOME/.fedora-upload-ca.cert \
    'https://koji.fedoraproject.org/koji/login'

I suspect that handling of client certificates is somehow broken.

Comment 12 Kamil Dudka 2012-11-02 10:52:02 UTC
This is a double lock PK11_Sign().  I am looking how to fix it...

 747│     session = pk11_GetNewSession(slot,&owner);
 748│     if (!owner || !(slot->isThreadSafe)) PK11_EnterSlotMonitor(slot);
 749│     crv = PK11_GETTAB(slot)->C_SignInit(session,&mech,key->pkcs11ID);
 750│     if (crv != CKR_OK) {
 751│         if (!owner || !(slot->isThreadSafe)) PK11_ExitSlotMonitor(slot);
 752│         pk11_CloseSession(slot,session,owner);
 753│         PORT_SetError( PK11_MapError(crv) );
 754│         return SECFailure;
 755│     }
 756│         /* PKCS11 2.20 says if CKA_ALWAYS_AUTHENTICATE then 
 757│          * do C_Login with CKU_CONTEXT_SPECIFIC 
 758│          * between C_SignInit and C_Sign */
 759├>        if (SECKEY_HAS_ATTRIBUTE_SET(key,CKA_ALWAYS_AUTHENTICATE)) {
 760│                 PK11_DoPassword(slot, PR_FALSE, key->wincx, PR_TRUE);
 761│         }

Comment 13 Kamil Dudka 2012-11-02 10:54:33 UTC
I meant a double _in_ PK11_Sign(), of course.  Is the indentation broken intentionally to highlight the location of the bug?

Comment 14 Vít Ondruch 2012-11-02 10:57:18 UTC
(In reply to comment #10)
I expired the override. I hope you don't mind. It should contain all of nss, nss-softokn and nss-utils. But since it seems that nss-3.14-3.fc18 is not the "right" version anyway, I hope it is OK and I'll be able to build my packages.

Comment 15 Kamil Dudka 2012-11-02 11:11:12 UTC
Created attachment 637038 [details]
proposed fix

Comment 16 Elio Maldonado Batiz 2012-11-02 16:05:33 UTC
Comment on attachment 637038 [details]
proposed fix

It makes a lot of sense to me, exit the monitor once  you sign always -not just just error and enter always before signing. Thank you Kamil.

Comment 17 Bob Relyea 2012-11-02 16:53:05 UTC
Comment on attachment 637038 [details]
proposed fix

r-

Kamil had definitely identified the real problem. PK11_EnterSlotMonitor is poorly named, since it implies a monitor, not a lock (monitors allows the same thread to enter it twice).

Unfortunately the fix needs to be more complex. Doug Ebert's patch (https://bugzilla.mozilla.org/show_bug.cgi?id=357025) appearently has 2 problems:

1) we need to login on the session used for signing.
2) we need to make sure we handle locking correctly.

I suggest removing the CKA_ALWAYS_AUTHENTICATE line (you can tell the newly added line because the indents are wrong.

We have the same problem in pk11_PrivDecryptRaw().

bob

Comment 18 Jeff Law 2012-11-02 17:46:26 UTC
In the mean time, what should someone do who is running F18 who needs to upload new bits into Fedora rawhide?

Comment 19 Jeff Law 2012-11-02 18:07:31 UTC
Nevermind.  Just saw the comment about downgrading nss things...  Which does the trick just fine.

Comment 20 Bob Relyea 2012-11-02 18:17:14 UTC
Created attachment 637191 [details]
Fix locking issue. Also authenticate on the correct session

Comment 21 Bruno Wolff III 2012-11-03 15:05:57 UTC
Note that this is also broken in rawhide. I am having the same issue with nss-3.14-7.fc19.

Comment 22 Elio Maldonado Batiz 2012-11-05 16:30:38 UTC
Comment on attachment 637191 [details]
Fix locking issue. Also authenticate on the correct session

Thank you Bob not only for the patch but also for the various explanations that help me a lot with the review. I have verified, with builds with this patch applied, that 'fedpkg new-sources' and 'fedpkg update' no longer hang on Rawhide and F-18.

Comment 23 Elio Maldonado Batiz 2012-11-05 16:34:55 UTC
Comment on attachment 637191 [details]
Fix locking issue. Also authenticate on the correct session

Thank you Bob, not just for the fix but for your explanations which helped a lot so I cauld review the patch. Besides the code review, I have verified that this patch does solve the reported problem on both rawhide and f18

Comment 24 Fedora Update System 2012-11-05 18:51:11 UTC
nss-softokn-3.14-1.fc18, nss-3.14-5.fc18, nss-util-3.14-1.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/FEDORA-2012-17351/nss-3.14-5.fc18,nss-softokn-3.14-1.fc18,nss-util-3.14-1.fc18

Comment 25 Fedora Update System 2012-11-05 22:46:05 UTC
Package nss-softokn-3.14-1.fc18, nss-3.14-5.fc18, nss-util-3.14-1.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing nss-softokn-3.14-1.fc18 nss-3.14-5.fc18 nss-util-3.14-1.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-17351/nss-3.14-5.fc18,nss-softokn-3.14-1.fc18,nss-util-3.14-1.fc18
then log in and leave karma (feedback).

Comment 26 Dan Mashal 2012-11-12 18:24:30 UTC
verified fixed

Comment 27 Fedora Update System 2012-11-13 19:06:19 UTC
Package nss-softokn-3.14-1.fc18, nss-util-3.14-1.fc18, nss-3.14-6.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing nss-softokn-3.14-1.fc18 nss-util-3.14-1.fc18 nss-3.14-6.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-17351/nss-3.14-6.fc18,nss-softokn-3.14-1.fc18,nss-util-3.14-1.fc18
then log in and leave karma (feedback).

Comment 28 Fedora Update System 2012-11-21 20:54:13 UTC
Package nss-util-3.14-1.fc18, nss-3.14-7.fc18, nss-softokn-3.14-5.fc18:
* should fix your issue,
* was pushed to the Fedora 18 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing nss-util-3.14-1.fc18 nss-3.14-7.fc18 nss-softokn-3.14-5.fc18'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-17351/nss-3.14-7.fc18,nss-softokn-3.14-5.fc18,nss-util-3.14-1.fc18
then log in and leave karma (feedback).

Comment 29 Fedora Update System 2012-11-24 03:42:32 UTC
nss-util-3.14-1.fc17,nss-softokn-3.14-5.fc17,nss-3.14-7.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/nss-util-3.14-1.fc17,nss-softokn-3.14-5.fc17,nss-3.14-7.fc17

Comment 30 Fedora Update System 2012-11-27 05:04:14 UTC
nss-util-3.14-1.fc18, nss-3.14-7.fc18, nss-softokn-3.14-5.fc18 has been pushed to the Fedora 18 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 31 Fedora Update System 2012-12-12 04:33:43 UTC
nss-util-3.14-1.fc17, nss-softokn-3.14-5.fc17, nss-3.14-7.fc17 has been pushed to the Fedora 17 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.