Closed Bug 309877 Opened 19 years ago Closed 18 years ago

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

Categories

(Core :: Security: PSM, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: BenB, Assigned: BenB)

References

Details

(Keywords: crash, Whiteboard: [kerh-coz])

Attachments

(2 obsolete files)

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.
Attached patch Fix, v1, against 1.7.x (obsolete) — Splinter Review
Bob, could you please review?
Attachment #197266 - Flags: review?(rrelyea)
Credits: Bob pointed me to the right function(s).
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-
Whiteboard: [kerh-coz]
*** Bug 342411 has been marked as a duplicate of this bug. ***
Attached patch like this? (obsolete) — Splinter Review
Attachment #197266 - Attachment is obsolete: true
Attachment #226650 - Flags: review?(rrelyea)
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 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+
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 on attachment 226650 [details] [diff] [review]
like this?

mozilla/security/manager/ssl/src/nsNSSComponent.cpp 	1.140
Attachment #226650 - Attachment is obsolete: true
Fantastic. Thanks timeless for adopting the patch, and thanks Bob and Kai for the reviews! :) Marking FIXED.
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: