362980 - Unable to verify the identity of [www.something.com] as a trusted site

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061206 Minefield/3.0a1 ID:2006120608 [cairo]

for every https site I open I get the above message

(screenshot next)

Comment 1

[Peter van der Woude [:Peter6]]

Attachment: screenshot

Comment 2

[Peter van der Woude [:Peter6]]

this broke in the 
20061206_0754_0820_firefox-3.0a1.en-US.win32 build

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1165414620&maxdate=1165420439

-> bug 176501

Comment 3

[Kai Engert (:KaiE:)]

I can confirm this bug.

After some initial debugging on Windows, our code to load the roots module nssckbi.dll runs without failures in NSS, but still, cert manager does not display any root certs.

On Linux the patch from bug 176501 seems to work correct, and I still get all the roots.

Backing out the patch from bug 176501 makes it work again.
If I don't find the reason shortly, I'll back it out from cvs.

Comment 4

[Kai Engert (:KaiE:)]

Although the call to SECMOD_LoadUserModule does not fail on Windows, it seems to be the culprit. Well, at least the way we call it does not work.

We call it with params
  name="Builtins Root Module" library="full-path-to-dll"

But when I change this to call use SECMOD_AddNewModule with the same full-path-to-dll, it works correctly.

Comment 5

[Kai Engert (:KaiE:)]

Attachment: Patch v1

We need to escape backslashes in the parameter string passed on to NSS!

Comment 6

[Robert Relyea]

Comment on attachment 247740 [details] [diff] [review]
Patch v1

r+= relyea

This will get the tree working as is correct.
Since you are in C++, you may want a version of addEscape that works with C++ strings and avoid the C alloc (optional).

Comment 7

[Wan-Teh Chang]

Comment on attachment 247740 [details] [diff] [review]
Patch v1

For better style, you should use the exact same expression (i.e.,
reorder one of the expressions) in both of these if statements:

>+        if ((*src == quote) || (*src == '\\')) escapes++;

>+        if ((*src == '\\') || (*src == quote)) {
>+            *dest++ = '\\';
>+        }

I don't understand why nss_addEscape needs the 'quote' argument because that
argument is always '\"'.  Also, do you really need to quote "?  I don't see
how 'fullLibraryPath' (the return value of PR_GetLibraryName) can contain ".

Comment 8

[Wan-Teh Chang]

I didn't mean to remove Bob's review+ on patch v1.  It's caused
by a bug in Bugzilla when two people review a patch at the same time.

Comment 9

[Kai Engert (:KaiE:)]

(In reply to comment #6)
> Since you are in C++, you may want a version of addEscape that works with C++
> strings and avoid the C alloc (optional).

Since we already have a function that is known to work, I think using it is fine. I think C++ code that inserts escape chars would require one additional allocation, too.


(In reply to comment #7)
> For better style, you should use the exact same expression (i.e.,
> reorder one of the expressions) in both of these if statements:
> 
> >+        if ((*src == quote) || (*src == '\\')) escapes++;
> 
> >+        if ((*src == '\\') || (*src == quote)) {
> >+            *dest++ = '\\';
> >+        }

ok, changed


> I don't understand why nss_addEscape needs the 'quote' argument because that
> argument is always '\"'.  Also, do you really need to quote "?  I don't see
> how 'fullLibraryPath' (the return value of PR_GetLibraryName) can contain ".

Wan-Teh, I did not write this function, I copied it over from NSS. This is a general purpose function.

Comment 10

[Kai Engert (:KaiE:)]

Comment on attachment 247740 [details] [diff] [review]
Patch v1

setting back r=rrelyea

Comment 11

[Kai Engert (:KaiE:)]

Attachment: Patch v2

Slightly improved patch as proposed by Wan-Teh, this patch has r=rrelyea and r=wtchang

Comment 12

[Kai Engert (:KaiE:)]

fixed on trunk

Comment 13

[Peter van der Woude [:Peter6]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061207 Minefield/3.0a1 ID:2006120705 [cairo]

VERIFIED/FIXED

thanks

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #13)
> Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061207
> Minefield/3.0a1 ID:2006120705 [cairo]
> 
> VERIFIED/FIXED
> 
> thanks

Still not working for me with build 2006-12-07-20 of SeaMonkey trunk on Windows XP (stub installer); why the disparity here? 

Comment 15

[Kai Engert (:KaiE:)]

(In reply to comment #14)
> Still not working for me with build 2006-12-07-20 of SeaMonkey trunk on Windows
> XP (stub installer); why the disparity here? 

I confirm this. But I hope the machine who builds that seamonkey package pulled to late, and did not yet include the patch. Can not think of any other reason currently.

While I'm writing this, the latest windows seamonkey stub installer in latest-trunk is still 2006-12-07-08, from yesterday.

Don't understand where you got a newer 2006-12-07-20 from!

Let's wait until a newer seamonkey package shows up in latest-trunk.

Comment 16

[Wan-Teh Chang]

(In reply to comment #9)

OK, I now understand why the double quote (") needs to be escaped.

Since you only call nss_addEscape with quote='\"', nss_addEscape
doesn't need the 'quote' parameter.  I did suspect that you copied
nss_addEscape from NSS.  When nss_addEscape is taken from its original
context, the 'quote' parameter serves no purpose and may confuse people.
This is just a nit, so feel free to ignore it.

Comment 17

[Kai Engert (:KaiE:)]

(In reply to comment #14)
> Still not working for me with build 2006-12-07-20 of SeaMonkey trunk on Windows
> XP (stub installer); why the disparity here? 

This is now working for me.
It looks like the earlier builds indeed had not picked up the patch.

Comment 18

[Dustin C. Hatch]

As of right now, this does not appear to be fixed for me. I checked out from trunk this morning and verified that this patch is applied to the sources, however I still get the listed error.

Gentoo Linux (2.6.19) AMD64

Comment 19

[Kai Engert (:KaiE:)]

Dustin, do you get this error message with a particular site only, or with any https sites?

You build yourself?
How do you run your build? cd dist/bin/; ./firefox ?
Does your dist/bin directory contain the libnssckbi.so file?

Comment 20

[Dustin C. Hatch]

(In reply to comment #19)
> Dustin, do you get this error message with a particular site only, or with any
> https sites?
> 
> You build yourself?
> How do you run your build? cd dist/bin/; ./firefox ?
> Does your dist/bin directory contain the libnssckbi.so file?
> 
This error comes up on all HTTPS websites.

I built Minefield myself from CVS code.
Gentoo uses a stub script to call firefox.  I modified it slightly to call minefield. See:

#!/bin/sh
#
# Stub script to run mozilla-launcher.  We used to use a symlink here
# but OOo brokenness makes it necessary to use a stub instead:
# http://bugs.gentoo.org/show_bug.cgi?id=78890

export MOZILLA_LAUNCHER=firefox
export MOZILLA_LIBDIR=/usr/lib64/minefield
export MOZ_PLUGIN_PATH=${MOZ_PLUGIN_PATH:-/usr/lib64/nsbrowser/plugins}
exec /usr/libexec/mozilla-launcher "$@"

I don't have a dist/bin directory, and nowhere else exists "libnssckbi.so" in my firefox installation directory.  I do however, have the file under /usr/lib64/nss/, though this belongs to a package called "dev-libs/nss-3.11.4," upon which firefox, thunderbird, and minefield all depend.

Comment 21

[Kai Engert (:KaiE:)]

This might be a packaging issue.

I'm not a Gentoo user, but the location you quote does not sound right.

Could you please verify the directory that contains file libnssckbi.so is in the library search path?

For example on Fedora Linux, which also packages the NSS library files in a package separately from the applications, the file libnssckbi.so gets installed in /usr/lib

Comment 22

[Dustin C. Hatch]

(In reply to comment #21)
> This might be a packaging issue.
> 
> I'm not a Gentoo user, but the location you quote does not sound right.
> 
> Could you please verify the directory that contains file libnssckbi.so is in
> the library search path?
> 
> For example on Fedora Linux, which also packages the NSS library files in a
> package separately from the applications, the file libnssckbi.so gets installed
> in /usr/lib
> 
Clearly, this is not an issue with Minefield. I symlinked /usr/lib/nss/libnssckbi.so into /usr/lib64/minefield and the problem was corrected. I will look into the problem with Gentoo. Thanks for your help.