PSM/NSS init (thus SSL/https) fails when no profile exists

RESOLVED FIXED

Status

()

Core
Security: PSM
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: BenB, Assigned: BenB)

Tracking

({crash})

Trunk
x86
Windows XP
crash
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [kerh-coz])

Attachments

(2 obsolete attachments)

(Assignee)

Description

13 years ago
Reproduction:
Either
1. Put profile on a CD and use SSL (IIRC)
Or
1. Put <browser type="content" src="https://freemail.web.de" /> in profile
wizard (createProfileWizard.xul).

Actual result:
Crash

The crash is in debug builds actually an assert, because
SECMOD_GetInternalModule() returns NULL. The stack is:
    PK11_GetInternalSlot() (in pk11slot.c)
    PK11_TokenExists()
    ssl3_config_match_init()
    ...
    ssl_Do1stHandshake()

Expected result:
Go on without persistent local databases. Normal SSL/https works.

Cause:
There is code in the NSS init in PSM to deal with the case, but earlier in the
same function it just exits with an error in the same case.

Fix:
Patch attached which is confirmed to work at least for the 2. reproduction way.
Need review from NSS guys to ensure that I didn't break security stuff by
missing some init or something.
(Assignee)

Comment 1

13 years ago
Created attachment 197266 [details] [diff] [review]
Fix, v1, against 1.7.x

Bob, could you please review?
Attachment #197266 - Flags: review?(rrelyea)
(Assignee)

Comment 2

13 years ago
Credits: Bob pointed me to the right function(s).

Comment 3

13 years ago
Comment on attachment 197266 [details] [diff] [review]
Fix, v1, against 1.7.x

r- relyea

The patch is actually pretty good. The only thing I would like to see is to
detect a failure of NSS_NoDB_Init(), both in your call and the one later in
PSM. If one of the shared libraries for NSS doesn't exist, it is possible of
NSS to fail, even if no profile is specified.

NSS_NoDBInit returns a SECStatus rather than a mozilla error status, so you
will have to convert.

Other than that the patch is fine.

bob
Attachment #197266 - Flags: review?(rrelyea) → review-

Updated

13 years ago
Whiteboard: [kerh-coz]

Comment 4

12 years ago
*** Bug 342411 has been marked as a duplicate of this bug. ***

Comment 5

12 years ago
Created attachment 226650 [details] [diff] [review]
like this?
Attachment #197266 - Attachment is obsolete: true
Attachment #226650 - Flags: review?(rrelyea)

Comment 6

12 years ago
Comment on attachment 226650 [details] [diff] [review]
like this?

r=relyea

this one looks good to me.

bob
Attachment #226650 - Flags: superreview?(kengert)
Attachment #226650 - Flags: review?(rrelyea)
Attachment #226650 - Flags: review+

Comment 7

12 years ago
Comment on attachment 226650 [details] [diff] [review]
like this?

does NS_GetSpecialDirectory(NS_APP_USER_PROFILE_50_DIR,...) fail when the profile is on cd? I think it does not, and your patch probably does not influence that scenario?

r=kengert
Attachment #226650 - Flags: superreview?(kengert) → superreview+

Comment 8

12 years ago
Hi Kaie,

I believe actual failures to open the database is handled in PSM elsewhere (there is code the falls back to NSS_Init rather than NSS_InitRW and then to NSS_NoDB_Init). I think the bug here was that if we couldn't get the profile we would just bail rather than openning NSS with no db.

bob

Comment 9

12 years ago
Comment on attachment 226650 [details] [diff] [review]
like this?

mozilla/security/manager/ssl/src/nsNSSComponent.cpp 	1.140
Attachment #226650 - Attachment is obsolete: true
(Assignee)

Comment 10

12 years ago
Fantastic. Thanks timeless for adopting the patch, and thanks Bob and Kai for the reviews! :) Marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.