Closed Bug 75049 Opened 23 years ago Closed 23 years ago

Changes needed for NSS to build under Carbon

Categories

(Core Graveyard :: Security: UI, defect)

Product:
Core Graveyard
Component:
Security: UI
Version:
1.0 Branch
Platform:
PowerPC
Mac System 9.x
Type:
defect
Priority:
Not set
Severity:
blocker

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: sdagley, Assigned: beard)

References

Details

(Whiteboard: OSX++, PDT+, r=mcgreer, sr=nelsonb)

Attachments

(4 files)

Patch to build mac_rand.c when getingCarbon APIs
2.58 KB, patch
Details | Diff | Splinter Review
Latest diffs to mac_rand.c
3.49 KB, patch
Details | Diff | Splinter Review
Patch to instrument NSS to dump raw entropy data.
1.18 KB, patch
Details | Diff | Splinter Review
analyze.c -- analyzes multiple entropy data files (cc -o analyze analyze.c builds just fine on Mac OS X).
8.39 KB, text/plain
Details 
Changes needed to build NSS for Mac OS when targeting Carbon APIs.
Blocks: 75019
nelsonb,

Could you review this patch?

We need this in the NSS 3.2 branch so that PSM builds on Mac OS X
I previously reviewed another patch quite similar to this one,
and sent out numerous review comments to someone.  sdagley,
did I send them to you?
Javi, Steve,  
Is this the same patch I reviewed for you back in February?
I'll email you another copy of my review comments from back then.
Nelson, yes, it's the same patch as originally submitted in February.  I recall 
some comments directed at beard regarding entropy but no detailed review of the 
patch.

Right.  My two comments were:

a) the patch ifdef's out lots of code whose purpose was to provide
badly-needed entropy to the PRNG.  If you're going to remove sources
of entropy, you really should provide new sources of (at least)
equivalent amounts of entropy.  Lack of adequate entropy is a good
way to fool yourself into thinking you've got security when you don't.
I cannot advise on where to find entropy on a MAC running OSX.

b) (minor) please use platform independent data types rather than 
platform-dependent types where they're equivalent, e.g. PRUint32
rather than Uint32.
would it be possible to use unix_rand.c for Carbon builds?  Or do we not have
access to the OS X unix level calls in Carbon builds?
The CFM based builds don't have access to the unix APIs (it is possible but it'd 
be more work than just coming up with an acceptable version of a Carbon compliant 
mac_rand.c)

Adding ddrinan to cc-list.

Nelson is going to write and publish a document that details the requirements 
that we think must be satisifed by an entropy implementation for MAC OS X (or 
any other platform). We should then solicit help for Apple and/or MAC OS X 
experts in mozilla to assist us in implementing a mac_rand.c that satisfies 
Nelsons requirements.
Blocks: 73390
Any ideas when this might see some development work? Bug 73390 (Fizzilla can't
connect via https) is a big gap in functionality.
We have asked the community of Carbon developers to contribute the code
needed to add the entropy that is lost by the patch shown above.
So far, we have not received any such contribution.  When we do receive
one, then ...
Whiteboard: Waiting for contribution from Carbon developers
If NSPR will be a Mach-O binary (bug 71718), could/should NSS be built based on
 UNIX system calls as well?
Should this be Platform: Mac OS X, whiteboard: OSX+ ?
Our situation with OSX is similar to our situation with OS/2.  
We depend on others to provide the platform-dependent parts.
AFAIK, there are no OSX/Carbon experts on the PSM and NSS teams.  
We also don't have a system and related build software with which
we can try things out.  That's why we're looking for that contribution.
There may be a number of simple solutions to this problem of which
we're simply unaware.  
just spoke with nelson. since iplanet doesn't have the resources, i'll pull this
back into CPD. 

changing product so i can set the milestone appropriate to rtm
Assignee: javi → pinkerton
Severity: major → blocker
Component: Libraries → Security: General
Keywords: nsmac1, rtm
Product: NSS → Browser
Whiteboard: Waiting for contribution from Carbon developers → OSX+
Target Milestone: --- → mozilla0.9.2
Version: 3.0 → other
Status: NEW → ASSIGNED
--> beard, now that i have a beta stopper on my list.
Assignee: pinkerton → beard
Status: ASSIGNED → NEW
beta stopper
Whiteboard: OSX+ → OSX++
BTW, Ian McGreer has developed some techniques for measuring the amount of 
entropy actually being fed into the PRNG.  I suggest that these techniques
be used to measure the amount being collected on the MAC, and on OSX with
the previously proposed patch, and then with any other proposed patches.
The solution for this problem should not result in less entopy being fed
into the PRNG than is now fed for MACs.  I suggest people try Ian's 
techniques before proposing new patches.
*** Bug 84680 has been marked as a duplicate of this bug. ***
Is there any update on this bug?
Blocks: 86915
I'm working on it, looking for additional sources of entryopy.
I have an updated patch to mac_rand.c, using Microseconds instead of 
TickCount() to get a faster running clock. With this patch, I'm getting the 
following entropy measurements when running Mozilla 10 times w/o 
browsing:

0.00 < entropy < 0.10 ] : 19788
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 2409
0.30 < entropy < 0.40 ] : 3874
0.40 < entropy < 0.50 ] : 267
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 5497
0.70 < entropy < 0.80 ] : 6227
0.80 < entropy < 0.90 ] : 1803
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 4775

Ian McGreer tells me that this is sufficiently random for our purposes.

    
  
Status: NEW → ASSIGNED

    
    

    
  
Ian and Nelson, can I get r/sr from you and then would you check this 
patch in?
Keywords: review

    
  
Whiteboard: OSX++ → OSX++, PDT+

    
    

    
  
I don't really know Mac API's, but for the RNG part r=mcgreer.

Note that the entropy statistics are a bit inflated in this case. I looked at
the actual data collected, and much of it is repeated within the sample, but
different across samples. this is the case where the analyzer is limited. I
believe that there is as much entropy being fed in now as there ever was with
the Mac, so we should go ahead with the patch, but I need to work on a better
estimate of what the entropy really is.

    
    

    
  
I agree with Ian that the new numbers are inflated.  IMO, we shouldn't
rely so heavily on a single source, especially when it is monotonic.

What are the numbers you get without the microsecond counter?
If they're good enough, then I'm not worried about the possible inflation.

    
  
Whiteboard: OSX++, PDT+ → OSX++, PDT+ patch available

    
    

    
  
Nelson, can you give this an sr= then?
Whiteboard: OSX++, PDT+ patch available → OSX++, PDT+, r=mcgreer, sr=nelsonb??

    
    

    
  
Good:
The change from TickCount() to Microseconds() is an improvement.

Bad:
For TARGET_CARBON build, the patch simply comments out the code
that uses the user name, current directory, heap, and event queue
to feed data into RNG_RandomUpdate().  Only the code that uses
scrap size and scrap count has been provided with an altenate
implementation using Carbon API.

    
    

    
  
Wan-Teh, 
Is this good enough for a beta shipment?  For Mac OS X the release of NS6.1 will
be a beta, with final to come in the fall.  So if this partial fix gets us close
enough for now, it will buy time for the full fix by fall.

    
    

    
  
Clayton, it's possible that this patch is good enough for a beta, but I'm afraid
that this distinction may be lost on people outside our team. Is there perhaps a
little more time Patrick could devote to make sure we don't get into trouble?




    
    

    
  
The entryopy measurements should be enough to decide this. I'm not qualified to 
choose other sources of randomness, short of trying other things and measuring 
the resulting entropy using Ian McGreer's tools. The guidelines he gave me said 
that roughly 500 bits of randomness are needed to properly seed the RNG when the 
browser starts up. The data I collected apparently generate a fair bit more than 
this.

A comment on the code that is not running when TARGET_CARBON is 1. The PPCToolbox 
API GetDefaultUser() returns the same value every time it is run. 
LMGetCurDirStore() returns the last directory that was accessed in the standard 
file dialog, which typically won't change from run to run of an application. 
LMGetTheZone() is a pointer to the base address where an application loads. This 
won't be terribly random when an application is run multiple times before the 
user reboots. The data values returned from GetMainDevice() are effectively 
constant on a given machine configuration, they only change when the user changes 
the device resolution.

Lastly, the OS event queue is no longer exposed. The interesting thing, is that 
it is probably almost always empty while the user is waiting for the application 
to launch. Only if the user has typed ahead, or has clicked the mouse a number of 
times will the OS event queue contain anything.


    
    

    
  
There are two types of entropy we are looking at. One is pure entropy (things
that will change from run to run), the other is machine dependent entropy
(things that may not change very often from run to run, but are different from
machine to machine). We need to get as much of the first as possible, but we
also can use as much of the second as we can get our hands on. For example, on
UNIX we mix in /etc/passwd. It doesn't change much from run to run, but is
likely to be different on different machines.

bob

    
    

    
  
wtc, so? we've demonstrated there is more than enough entropy with those
removed. why do we have to replace them?

    
    

    
  
The key is that the bits must be truely random.  There are many measures
of randomness.  

One is the average value of the bit.  Obviously a bit that is usually 1  
and rarely 0, or or usually 0 and rarely 1 offers little entropy.  The 
test that has been previously given here measures the probability that 
the bit is equally likely to be zero or 1.  That's a first approximation 
of entropy.   But it's only an approximation.  An input stream that is
0x5a5a in all odd numbered runs and 0xa5a5 in all even numbered runs 
will appear to this first test to be pure entropy, but there is very 
little entopy in it.

Another measure is the predictability of the bit given that all past bits
input are known.  From a security perspective, a predictable bit stream
offers no entropy, even if its bits are 1 and 0 in equal numbers. 
When Ian wrote that much of the data collected is repeated within the 
sample, that seriously lessens the unpredictability of the stream.  
That concerns me.  

Above, I asked for a test result that should show how much of the 
newly measured "entropy" comes (or does not come) from the microsecond 
function.  I'd still like to see that result.  

    
    

    
  
I will publish those results as soon as possible.


    
    

    
  
*** Bug 86915 has been marked as a duplicate of this bug. ***

    
    

    
  
Using TickCount():

0.00 < entropy < 0.10 ] : 25099
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 20147
0.30 < entropy < 0.40 ] : 36415
0.40 < entropy < 0.50 ] : 18129
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 65144
0.70 < entropy < 0.80 ] : 31027
0.80 < entropy < 0.90 ] : 17237
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 20530

Using Microseconds():

0.00 < entropy < 0.10 ] : 19788
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 2409
0.30 < entropy < 0.40 ] : 3874
0.40 < entropy < 0.50 ] : 267
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 5497
0.70 < entropy < 0.80 ] : 6227
0.80 < entropy < 0.90 ] : 1803
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 4775

So, TickCount() is better by a large factor. So, the first patch should be checked in rather than the second patch. Nelson, with these results, please sr.
.

    
    

    
  
Patrick-

Did you collect the data from TickCount() and Microseconds() as close to
simultaneously as possible?  If not, the other entropy collection functions
could have greatly affected the data.  For example, if your clipboard was empty
before running the build using Microseconds(), that would cause the large
difference in the results.

I ask because the difference is an order of magnitude, though the contribution
from either TickCount() or Microseconds() is at most a few bytes.  So that alone
cannot be the difference.

After reading the documentation at www.apple.com, I strongly believe that
Microseconds() is the call we should be making.  The resolution of TickCount()
is only 1/60 of a second, while Microseconds() is about 20ms.  Using
Microseconds() would be a significant improvement.

Thus, the second patch is the one we should consider.  I'm still willing to give
an r= to that patch.

    
    

    
  
Look at the huge differences in sample counts.  
This is not comparing apples to apples.  
Why does the example with tick count numbers have an order of
magnitude more samples?


    
    

    
  
OK, it looks like I'll need to build it both ways, and clone the builds so I can 
run them in similar initial conditions.


    
    

    
  
OK, these were done back to back, same clipboard, hopefully nearly the same 
screen. These measurements concur with your opinion, that Microseconds() is 
better than TickCount(). Here're the results:

Using Microseconds:

0.00 < entropy < 0.10 ] : 20689
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 5949
0.30 < entropy < 0.40 ] : 24490
0.40 < entropy < 0.50 ] : 31
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 21879
0.70 < entropy < 0.80 ] : 10334
0.80 < entropy < 0.90 ] : 304
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 2596

Using TickCount:

0.00 < entropy < 0.10 ] : 20449
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 10711
0.30 < entropy < 0.40 ] : 4105
0.40 < entropy < 0.50 ] : 302
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 7688
0.70 < entropy < 0.80 ] : 9935
0.80 < entropy < 0.90 ] : 5816
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 9954

Microseconds is clearly the winner then.


    
    

    
  
I emailed the raw data for these measurements to Ian and Nelson for them to 
confirm my conclusions.


    
    

    
  
The results shown above do not seem to support the conclusion that 
microseconds is "clearly the winner".

I tossed those numbers into a spreadsheet and got these results:

               sample bits    entropy bits  percentage (entropy/sample)
               -----------    ------------  ----------
TickCount         86272          31749.6      36.8
Microseconds      68960          29668.6      43.0

The change from TickCount to MicroSeconds does not account for the
difference in raw bits of input, does it?  

I conclude that there is some other factor, perhaps an uncontrolled
variable, that is causing the amount of input to the collector to
vary widely.  I believe the "noise" represented by that factor drowns
out the difference between tickCount and Microseconds.

However, that uncontrolled variable which affects the amount of data
collected, may itself be a large source of entropy.  The question in
my mind is whether another program running concurrently would see
essentially the same results (same number of samples).  If the cause
of this variance of amount collected is not observable or reproducible by 
another conncurrent process, then I'd say this entropy is sufficient, and 
the choice of TickCount or Microseconds is not significant (or better yet,
do both).


    
    

    
  
All,
we need to make a decision on this.  If you don't object, let's pick the
microsecond patch. The rationale is that it's not clearly worse (is it?), the
www.apple.doc has some information which tips the balance ever so slightly.

Let's check this in for RTM, and we can open a new bugs if we conclusively finds
a flaw in the solution.



    
    

    
  
Thanks to nelson for doing more than a cursory eyeball of the numbers (which is 
all I had time for). Yes, let's get this in for RTM, which also means let's 
nominate this for nsbranch (a manager from your group must add the nsbranch 
keyword) so it can be checked into the MOZILLA_0_9_2_BRANCH.

The appearance of various windows on the screen during my data collection could 
account for these changes. Since we're reading screen memory, as the pixels 
scroll in various windows this is probably affecting the data itself.


    
    

    
  
Isolating only the data supplied from the TickCount/Microseconds call:

Using TickCount(): 
0.00 < entropy < 0.10 ] : 16 
0.10 < entropy < 0.20 ] : 0 
0.20 < entropy < 0.30 ] : 1 
0.30 < entropy < 0.40 ] : 0 
0.40 < entropy < 0.50 ] : 2 
0.50 < entropy < 0.60 ] : 0 
0.60 < entropy < 0.70 ] : 3 
0.70 < entropy < 0.80 ] : 1 
0.80 < entropy < 0.90 ] : 3 
0.90 < entropy < 1.00 ] : 0 
1.00 < entropy < 1.10 ] : 6 

32 total bits (TickCount() returns a uint32) 

So, (0.0 * 16) + (.2 * 1) + (.4 * 2) + (.6 * 3) + (.7 * 1) + (.8 * 3) + (1.0 * 
6) = 11.9 

Using Microseconds(): 
0.00 < entropy < 0.10 ] : 34 
0.10 < entropy < 0.20 ] : 0 
0.20 < entropy < 0.30 ] : 0 
0.30 < entropy < 0.40 ] : 0 
0.40 < entropy < 0.50 ] : 0 
0.50 < entropy < 0.60 ] : 0 
0.60 < entropy < 0.70 ] : 9 
0.70 < entropy < 0.80 ] : 6 
0.80 < entropy < 0.90 ] : 6 
0.90 < entropy < 1.00 ] : 0 
1.00 < entropy < 1.10 ] : 9 

64 total bits (Microseconds() returns a struct of two 32-bit integers) 

(0.0 * 34) + (.6 * 9) + (.7 * 6) + (.8 * 6) + (1.0 * 9) = 23.4 

It appears Microseconds() is better.

    
    

    
  
Finally, a more conservative estimate of how much entropy is actually being 
supplied.  What I noticed in looking at the data is that the repeated calls to 
retrieve the clipboard vary from run to run, but not within each run.  That is, 
things like "FF" are repeated over and over in one sample, then in the next it 
will be "00" repeated.  That throws off my analyzer.  So I decided to remove 
*all* of that data, even though it may make some contribution.  With it removed, 
and using the Microseconds() data, I got:

0.00 < entropy < 0.10 ] : 51322
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 343
0.30 < entropy < 0.40 ] : 171
0.40 < entropy < 0.50 ] : 146
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 413
0.70 < entropy < 0.80 ] : 329
0.80 < entropy < 0.90 ] : 302
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 542

It is clear that much of the entropy measured here comes from collecting screen 
pixels, which seems like a good thing to do to me.  There are 4096 bytes from 
that call, which is much greater than anything else in the sample (after the 
clipboard stuff was removed).

    
    

    
  
Several comments:

First, thank you Ian for doing more in-depth investigation of the results.

Second, 11.9 and 23.4 are merely proverbial drips in the bucket.  So,
the important decision here is NOT tickcount vs microseconds.  

Third, If another process can collect the very same data that's being used
for input (or a large portion of the very same data), then the value of 
that data as being unpredictable and unrepeatable is essentially zero for
security purposes.  I'm guessing that the screen buffer is readable by all
processes, yes?

What numbers do you get if you remove the screen data as well?

    
    

    
  

Here is the Microseconds data with both clipboard and screen capture removed.

0.00 < entropy < 0.10 ] : 20366
0.10 < entropy < 0.20 ] : 0
0.20 < entropy < 0.30 ] : 19
0.30 < entropy < 0.40 ] : 34
0.40 < entropy < 0.50 ] : 31
0.50 < entropy < 0.60 ] : 0
0.60 < entropy < 0.70 ] : 71
0.70 < entropy < 0.80 ] : 81
0.80 < entropy < 0.90 ] : 47
0.90 < entropy < 1.00 ] : 0
1.00 < entropy < 1.10 ] : 151

    
    

    
  
The numbers in the table just above add up to 314.3 bits.
That number is more than 160, which is the absolute bare minimum.
And I believe it's sufficiently conservative to be believed. 
So, I approve this entropy change based on these numbers.

    
    

    
  
sr=nelsonb for the patch 
http://bugzilla.mozilla.org/showattachment.cgi?attach_id=39418
After obtaining the relevant drivers and PDT approvals, 
please check it into the appropriate branch(es).
Whiteboard: OSX++, PDT+, r=mcgreer, sr=nelsonb?? → OSX++, PDT+, r=mcgreer, sr=nelsonb

    
    

    
  
fizilla only, looks good. a= asa@mozilla.org for checkin to 0.9.2.
(on behalf of drivers)

    
    

    
  
Checked in on NSS tip, NSS_3_2_BRANCH, and MOZILLA_0_9_2_BRANCH, and updated 
NSS_CLIENT_TAG to point to the new revision.


    
    

    
  
meant to mark this fixed.

Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
Checked in on NSS_3_3_BRANCH.

    
    

    
  
NSS is part of Security: Crypto IIRC.  Changing module to reflect...
Component: Security: General → Security: Crypto

    
    

    
  
sonmi - can you pls verify.

Would I be able to verify this from any UI or going to any particular https:
page in the OS X build I have?

    
    

    
  
Lisa, if you can load <https://www.verisign.com/> and you get the lock icon on 
the bottom right of the window you've got a working NSS


    
    

    
  
Wonderful!  I've verified this on:
OS X:  2001-07-17 branch build.
Status: RESOLVED → VERIFIED

    
    

    
  
Mass changing Security:Crypto to PSM
Component: Security: Crypto → Client Library
Product: Browser → PSM
Target Milestone: mozilla0.9.2 → ---
Version: other → 2.1

    
    

    
  
Mass changing Security:Crypto to PSM

    
  
Product: PSM → Core

    
  
Version: psm2.1 → 1.0 Branch
Product: Core → Core Graveyard

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: