680878 - Support for building NSS for iOS

Status: RESOLVED FIXED

(NSS :: Build, defect)

Description

[(not currently active) Ted Mielczarek]

Attachment: patch (not for landing)

NSS needs only minor changes to support building for iOS (the Darwin kernel is virtually identical). This patch works, but it's not suitable for landing. The _NSGetEnviron function, while available in the CRT, is not defined in the header files. Per Apple's policy, that means it's not available for applications to use.

The usage here is for additional entropy. We could simply set environ=NULL and skip that as a source of entropy, but I don't know what effect that has on security.

Comment 1

[Wan-Teh Chang]

Comment on attachment 554837 [details] [diff] [review]
patch (not for landing)

Ted: thanks for the patch.  This patch is surprisingly small.

In security/coreconf/Darwin.mk:

>@@ -56,8 +56,12 @@
> OS_REL_CFLAGS	= -Di386
> endif
> else
>+ifeq (,$(filter-out arm%,$(CPU_ARCH)))
>+OS_REL_CFLAGS   = -Darm
>+else
> OS_REL_CFLAGS	= -Dppc
> endif
>+endif

Let's leave the "ifeq (,$(filter-out arm%,$(CPU_ARCH)))"
block empty, i.e., do not add -Darm.  I am trying to
remove the -Di386, -Dppc, etc. in the future.

In security/nss/lib/freebl/unix_rand.c:

> #ifdef DARWIN
>+#if !defined(__arm__)
> #include <crt_externs.h>
>+#else
>+extern char ***_NSGetEnviron();
>+#endif
> #endif

It should be OK to not include the environment variables
in the entropy for the pseudorandom number generator
provided that /dev/urandom works.  Can you verify that
in a debugger or with printf statements?

It would be better to include <TargetConditionals.h>
and test !TARGET_OS_IPHONE instead of !defined(__arm__)
around #include <crt_externs.h>.

Comment 2

[(not currently active) Ted Mielczarek]

Attachment: Support building NSS for iOS

Okay, this cleans up Darwin.mk as you suggested, and fixes unix_rand.c as well. NSS seems to build fine with this patch, except the build fails in shlibsign if I do a top-level build (since shlibsign is an arm binary). Is there a way to build everything except skip the shlibsign step? I haven't run any tests on device yet, but I built a functioning Firefox with the older patch a few months ago and it was able to load SSL content.

Comment 3

[(not currently active) Ted Mielczarek]

Attachment: make commandline

Here's the commandline I used to invoke make to target iOS. ~/build/obj-nspr-arm contains a prebuilt NSPR cross-compiled for iOS (with --prefix=~/build/obj-nspr-arm/install).

Comment 4

[Wan-Teh Chang]

Comment on attachment 555082 [details] [diff] [review]
Support building NSS for iOS

Ted: thanks for the patch.

In security/coreconf/Darwin.mk:

>+ifeq (,$(filter-out arm%,$(CPU_ARCH)))
>+override CPU_ARCH	= arm
>+else

Do we need the "override" keyword here?

In security/nss/lib/freebl/unix_rand.c:

>+#if TARGET_OS_IPHONE
>+    // iOS does not expose a way to access environ.
>+    char **environ = NULL;
>+#else

Please use C-style comment delimiters /* */.  I
verified that our code has a null check for 'environ',
so this will work.

Have you verified that /dev/urandom works on iOS?

I don't know of any NSS build variable that skips
building shlibsign.  mozilla/security/manager/Makefile.in
does that manually (with the SKIP_CHK variable in that
makefile) if CROSS_COMPILE is defined:
http://mxr.mozilla.org/mozilla-central/source/security/manager/Makefile.in

Comment 5

[(not currently active) Ted Mielczarek]

(In reply to Wan-Teh Chang from comment #4)
> >+ifeq (,$(filter-out arm%,$(CPU_ARCH)))
> >+override CPU_ARCH	= arm
> >+else
> 
> Do we need the "override" keyword here?

I was copying what the x86 and x86_64 blocks did.

> Have you verified that /dev/urandom works on iOS?

I tested it using "dd" from a shell and it looked like it produced random data. I'm not sure how to test it any more thoroughly than that.

Comment 6

[Wan-Teh Chang]

Please remove "override" and see if that works.
We may not need "override" for arm.

Your testing of /dev/urandom should be enough.  It
verified that /dev/urandom exists and you can read
from it.

Comment 7

[(not currently active) Ted Mielczarek]

I suspect the override in the x86 path is to support things like "make CPU_ARCH=i686", while still setting a predictable value for CPU_ARCH. On further thought, we shouldn't need to set CPU_ARCH at all, since it will be already set to "arm". The x86 blocks above do so because they're matching the value of `uname -p` and normalizing it.

Comment 8

[(not currently active) Ted Mielczarek]

Attachment: Support building NSS for iOS

Here's a patch against NSS HEAD with your comments addressed. Can you commit this for me?

Comment 9

[Wan-Teh Chang]

Comment on attachment 555378 [details] [diff] [review]
Support building NSS for iOS

r=wtc.

Patch checked in on the NSS trunk (NSS 3.13).

Checking in coreconf/Darwin.mk;
/cvsroot/mozilla/security/coreconf/Darwin.mk,v  <--  Darwin.mk
new revision: 1.30; previous revision: 1.29
done
Checking in nss/lib/freebl/unix_rand.c;
/cvsroot/mozilla/security/nss/lib/freebl/unix_rand.c,v  <--  unix_rand.c
new revision: 1.40; previous revision: 1.39
done