Closed Bug 75049 Opened 24 years ago Closed 23 years ago

Changes needed for NSS to build under Carbon

Categories

(Core Graveyard :: Security: UI, defect)

1.0 Branch
PowerPC
Mac System 9.x
defect
Not set
blocker

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: sdagley, Assigned: beard)

References

Details

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

Attachments

(4 files)

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.
Attached patch Latest diffs to mac_rand.c (deleted) — Splinter Review
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: