448792 - Debian patches for NSS 3.12

Status: RESOLVED INCOMPLETE

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

The attached files were downloaded from
http://ftp.debian.org/debian/pool/main/n/nss/nss_3.12.0-4.diff.gz
and unpacked for your convenience.  There may be a web
interface to browse these files, but I didn't look for
it.

The actual patches are in the 'nss-3.12.0/debian/patches' directory.

Comment 1

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/compat

Comment 2

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/control

Comment 3

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-tools.dirs

Comment 4

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-1d.links

Comment 5

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/rules

Comment 6

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/copyright

Comment 7

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-dev.links

Comment 8

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/nss.pc.in

Comment 9

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/watch

Comment 10

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/nss-config.in

Comment 11

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-1d.dirs

Comment 12

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-1d.symbols

Comment 13

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/libnss3-dev.dirs

Comment 14

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/changelog

Comment 15

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/00list

Comment 16

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/38_mips64_build.dpatch

Comment 17

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/80_security_build.dpatch

Comment 18

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/81_sonames.dpatch

Comment 19

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/90_realpath.dpatch

Comment 20

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/38_hurd.dpatch

Comment 21

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/80_security_tools.dpatch

Comment 22

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/91_build_pwdecrypt.dpatch

Comment 23

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/95_add_spi+cacert_ca_certs.dpatch

Comment 24

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/85_security_load.dpatch

Comment 25

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/25_entropy.dpatch

Comment 26

[Wan-Teh Chang]

Attachment: nss-3.12.0/debian/patches/38_kbsd.dpatch

Comment 27

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 331957 [details] [diff] [review]
nss-3.12.0/debian/patches/95_add_spi+cacert_ca_certs.dpatch

Not going to bypass Mozilla's root cert policy. sr-

Comment 28

[Eddy Nigg (StartCom)]

Can somebody tell me what's this all about? Why is WT submitting patches on behalf of Debian? And what's this 95_add_spi+cacert_ca_certs.dpatch ????

Comment 29

[Wan-Teh Chang]

This bug is for informational purposes only.  I'd like to
make it easy for the NSS team to understand how NSS 3.12
has been modified in Debian and Ubuntu, and help the Debian
nss package maintainer reduce the patches he has to apply.
Some of the patches are clearly Debian-specific and we
won't check them in.

We're especially interested in understanding the patch
nss-3.12.0/debian/patches/81_sonames.dpatch (attachment 331952 [details] [diff] [review]),
which makes NSS-based applications built on Debian or
Ubuntu unusable on other Linux distributions because
libnss3.so.1d, libssl3.so.1d, etc. cannot be found.  See
http://groups.google.com/group/mozilla.dev.tech.crypto/browse_thread/thread/6eec8be954d49451/619ddbdd266be877?lnk=raot
https://bugs.launchpad.net/ubuntu/+source/libnss-db/+bug/238500

Comment 30

[Eddy Nigg (StartCom)]

Thanks Wan-Teh! I saw the thread you mentioned obviously. Are any of those patches being applied eventually?

Comment 31

[Wan-Teh Chang]

Comment on attachment 331957 [details] [diff] [review]
nss-3.12.0/debian/patches/95_add_spi+cacert_ca_certs.dpatch

This file is not a proposed patch, so we don't need to
set the review or superreview flag.

If I propose any patches based on the Debian patches,
I will name the attachments clearly as proposed patches.

Comment 32

[Wan-Teh Chang]

This bug is for informational purposes only.  I'm closing
it because it has served the intended purpose.

Comment 33

[Wan-Teh Chang]

Comment on attachment 331958 [details] [diff] [review]
nss-3.12.0/debian/patches/85_security_load.dpatch

Mike, Alexander,

The last change in this patch (to pk11load.c) is
causing the problem I described in bug 495384.  It
causes NSS_InitReadWrite to load a nonexistent
"<configdir>/libnssckbi.so" successfully and write
that nonexistent pathname to NSS's secmod.db
(or pkcs11.txt) file.

Can you remove that change from Debian/Ubuntu's NSS
package?  Thanks.

Comment 34

[Mike Hommey [:glandium]]

(In reply to comment #33)
> It causes NSS_InitReadWrite to load a nonexistent
> "<configdir>/libnssckbi.so" successfully and write
> that nonexistent pathname to NSS's secmod.db
> (or pkcs11.txt) file.

Why does it?
 
> Can you remove that change from Debian/Ubuntu's NSS
> package?  Thanks.

The change is necessary to avoid a non-library to be in /usr/lib, as per debian policy.

Comment 35

[Nelson Bolyard (seldom reads bugmail)]

Mike,
NSS tries to load a shared library from a user-supplied string, and if the
load succeeds, it remembers the string.  The patch in question causes a 
library other than the one named in that string to be loaded successfully,
without changing the string to match the successfully loaded library string.
Consequently, the string that is remembered is incorrect.

Comment 36

[Wan-Teh Chang]

Comment on attachment 331958 [details] [diff] [review]
nss-3.12.0/debian/patches/85_security_load.dpatch

Mike,

This patch changes two files: genload.c and pk11load.c.

I understand the change to genload.c is required by
Debian policy.  I'm also asking you to remove the change
to pk11load.c from this patch.  Specifically, it's this
change:

--- nss/mozilla/security/nss/lib/pk11wrap/pk11load.c
+++ nss/mozilla/security/nss/lib/pk11wrap/pk11load.c
@@ -331,6 +331,14 @@
 #endif
 
 	if (library == NULL) {
+	    full_name = rindex(mod->dllName, PR_GetDirectorySeparator());
+	    if (full_name)
+	        full_name++;
+	    else
+	        full_name = mod->dllName;
+	    library = loader_LoadLibrary(full_name);
+	}
+	if (library == NULL) {
 	    return SECFailure;
 	}

When the user tells NSS to load a module with an absolute
pathname "/foo/bar/libmodule.so", he only wants that particular
file.  But this change causes NSS to also load "libmodule.so"
from some other directories (on the shared library search path
or /usr/lib and /usr/lib/nss), which is wrong.

Comment 37

[Wan-Teh Chang]

Comment on attachment 331958 [details] [diff] [review]
nss-3.12.0/debian/patches/85_security_load.dpatch

My previous comment has a typo.

The sentence
  I'm also asking you to remove the change
  to pk11load.c from this patch.
should read
  I'm asking you to remove only the change
  to pk11load.c from this patch.

Comment 38

[Mike Hommey [:glandium]]

IIRC, Firefox won't load libnssckbi.so without that. Moreover, if /usr/lib/nss/libnsckbi.so doesn't exist, it should "gracefully" fail. I still don't understand what the problem is, but maybe that is because I'm tired. I'm not sure I want to pollute bugzilla (and everyone CCed to this bug) with my ununderstanding of the issue. Maybe going to mail would be a good idea. Or IRC, when I'll have some time, which is probably not very soon :(

Comment 39

[Wan-Teh Chang]

Firefox will try to load libnssckbi.so using three pathnames.
The last pathname it tries is the simple filename "libnssckbi.so":
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/manager/ssl/src/nsNSSComponent.cpp&rev=1.168&mark=811,814-816#811

So you only need to make sure that NSS searches in the
/usr/lib/nss directory.  Two possible solutions are:

1. Link libnss3.so with the -Wl,-rpath,/usr/lib/nss or
-Wl,-rpath,'$$ORIGIN/nss' linker flag, if Debian's policy
allows shared libraries to be linked with -rpath.

This will allow the PR_LoadLibrary call in SECMOD_LoadPKCS11Module
(pk11load.c) to succeed when Firefox tries to load
"libnssckbi.so".

This is my preferred solution because it may make some of
your changes to loader_LoadLibrary in genload.c unnecessary.

2. Change SECMOD_LoadPKCS11Module in pk11load.c to just call
loader_LoadLibrary instead of PR_LoadLibrary.  This requires
your changes to loader_LoadLibrary in genload.c.

Basically, the problematic change is this part:

+	    full_name = rindex(mod->dllName, PR_GetDirectorySeparator());
+	    if (full_name)
+	        full_name++;
+	    else

Comment 40

[Mike Hommey [:glandium]]

Unfortunately, rpath applies to the library that calls dlload(), and since nss uses PR_LoadLibrary to do so, it's nspr's rpath that counts. I would have gone the rpath way if that were possible :(

Comment 41

[Nelson Bolyard (seldom reads bugmail)]

Mike, 
Do NSS's shared libraries and NSPR's shared libraries reside in different 
directories in Debian?  If so, please give their respective path names.

Comment 42

[Wan-Teh Chang]

Here are the path names of NSPR and NSS's shared libraries on Ubuntu:

/usr/lib/libnspr4.so
/usr/lib/libplc4.so
/usr/lib/libplds4.so

/usr/lib/nss/libfreebl3.so
/usr/lib/nss/libsoftokn3.so
/usr/lib/nss/libnssdbm3.so
/usr/lib/nss/libnssckbi.so
/usr/lib/nss/libsoftokn3.chk
/usr/lib/nss/libfreebl3.chk
/usr/lib/libssl3.so
/usr/lib/libsmime3.so
/usr/lib/libnssutil3.so
/usr/lib/libnss3.so

The shared libraries in /usr/lib/nss are only loaded with
dlopen().  Only the shared libraries in /usr/lib can be
linked with -lfoo.

Comment 43

[Mike Hommey [:glandium]]

(In reply to comment #39)
> 2. Change SECMOD_LoadPKCS11Module in pk11load.c to just call
> loader_LoadLibrary instead of PR_LoadLibrary.  This requires
> your changes to loader_LoadLibrary in genload.c.
> 
> Basically, the problematic change is this part:
> 
> +        full_name = rindex(mod->dllName, PR_GetDirectorySeparator());
> +        if (full_name)
> +            full_name++;
> +        else

The changes in 3.12.5 have been a good occasion to change the code above to the following:
+	if ((library == NULL) &&
+	    !rindex(full_name, PR_GetDirectorySeparator())) {
+            library = PORT_LoadLibraryFromOrigin(my_shlib_name,
+                                      (PRFuncPtr) &softoken_LoadDSO,
+                                      full_name);
+	}

(PORT_LoadLibraryFromOrigin inheriting our previous changes to genload.c)

i.e. if the file name given to SECMOD_LoadPKCS11Module doesn't contain a directory separator, load the module from origin if PR_LoadLibrary failed.

From my testing, this solves the issue you raised.