Closed Bug 404340 Opened 17 years ago Closed 13 years ago

freebl3.chk & softokn3.chk differ between exe & complete mar for Win32 builds (and nssdbm3.chk when using NSS 3.12.4 or later)

Categories

(Release Engineering :: General, defect, P5)

Product:
Release Engineering
Component:
General
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: nthomas, Assigned: rail)

References

Details

(Whiteboard: [updates])

Attachments

(3 files)

patcher-configs
1.62 KB, patch
Details | Diff | Splinter Review
update-packaging
708 bytes, patch
Details | Diff | Splinter Review
tools
902 bytes, patch
Details | Diff | Splinter Review 
Noticed this while working on bug 403952. Here's some sha1sums on today's Firefox trunk nightlies:

SHA1(./win32-exe/nonlocalized/freebl3.chk)= c955656fc73ae71e47c41732014de279271e021e
SHA1(./win32-exe/nonlocalized/softokn3.chk)= 2f16a7b224e0d304ff07684b2d00b6c2ba4b04b8
SHA1(./win32-mar/freebl3.chk)= 46413df8198679b273fad0d4794f32258fd3f4bc
SHA1(./win32-mar/softokn3.chk)= 507da87629a69817caee781b6a381acd9c1777c1

SHA1(./linux-bz2/firefox/libfreebl3.chk)= 7ea7883746b01029518a9539b520c0b4b5e3d592
SHA1(./linux-bz2/firefox/libsoftokn3.chk)= f647e839679de3505a6e9c40e94a3dac3f1de33d
SHA1(./linux-mar/libfreebl3.chk)= 7ea7883746b01029518a9539b520c0b4b5e3d592
SHA1(./linux-mar/libsoftokn3.chk)= f647e839679de3505a6e9c40e94a3dac3f1de33d

SHA1(./mac-dmg/Minefield.app/Contents/MacOS/libfreebl3.chk)= f8653116a2682587a5a82e813dcd2fbb935f8d17
SHA1(./mac-dmg/Minefield.app/Contents/MacOS/libsoftokn3.chk)= 82a47a395afc0ea17ce1c8ff3d2d78914eff3eb4
SHA1(./mac-mar/Contents/MacOS/libfreebl3.chk)= f8653116a2682587a5a82e813dcd2fbb935f8d17
SHA1(./mac-mar/Contents/MacOS/libsoftokn3.chk)= 82a47a395afc0ea17ce1c8ff3d2d78914eff3eb4

Probably this is because we call "make installer" about 10 million times on win32. Strangely, doesn't seem to be a problem on the 1.8 branch.
Summary: freebl.chk & softokn3.chk differ between exe & complete mar for Trunk/Firefox/Win32 → freebl3.chk & softokn3.chk differ between exe & complete mar for Trunk/Firefox/Win32
We just saw this in the FF3.0beta2 update verify step.
Blocks: 407077
Priority: -- → P1
reassigning - is this the correct component?
Component: Build & Release → Build Config
Product: mozilla.org → Firefox
QA Contact: build → build.config
Version: other → Trunk
from irc with bsmedberg:

Previously we only ran shlibsign much earlier in the NSS build process, but now
we're running it (correctly, sorta) as part of the package process, after we
strip libraries and it's not idempotent, so we get different results when we
run "make installer" (used for the installer) and "make package" (used for the
zip builds, which end up as the MARs).

Also see related work in bug#370693.
Depends on: 408205
Seems this matters for FIPS crypto as part of Firefox. For additional info, see:
http://oss-institute.org/fips-faq.html#a1 and http://www.mozilla.org/projects/security/pki/nss/tech-notes/tn6.html.

To verify if FIPS is working:
1) Install FF3
2) then go to Options->Advanced->security devices. In the list on left-hand-side, pick "SoftwareSecurityDevice" and then "change password", to set password. Once that is set, and you are back in the security devices dialog box, click "Enable FIPS". 
3) then visit www.paypal.com, and login.
4) Verify that the SSL connection is established. Also, to the left of the URL bar, is a small icon for that site. If you click on that icon, you will see a popdown window, with the passport control graphic, and some text confirming the paypal certificate info.
I just ran through the test outlined in comment 4 with b2 (using installer.exe) and b1 upgraded to b2 via the betatest channel, using clean profile each time on Windows 2000. 

I was able to login to PayPal, and got the same "location verified" passport popdown each time, "Verified by: VeriSign, Inc.", "You connection is encrypted" with the lock graphic, etc.

One minor note - on step #2 "security devices" is a button under the "encryption" tab, with a clean profile I think it defaults to the "general" tab.
Can you please test what happens when you delete the *.chk files and try that test on a clean profile?

I'm worried that the "enable fips" button would simply do nothing, and you remain in non-fips mode, and of course, the connection to paypal will work, too.


Bob, Wan-Teh, could you think of a better UI test to verify the application is correctly running in FIPS mode? (something that can be automated)


The only other idea I have is more complicated, because it would require to operate two test SSL server that disallows the FIPS ciphers. One (a) that is restricted to use FIPS ciphers, and another one (b) that is restricted to use non-FIPS ciphers. A test case could connect to both, see an expected success on (a), and an excepted failure on (b).


Maybe we should add a mode to shlibsign to test the NSS shared
libraries in FIPS mode.  We can also just use the pk11mode test
program (which is not being built in Mozilla builds right now)
to do that.
Any progress here? Not sure who is waiting for whom :)

Is this a packaging bug that should be fixed in the build system, so that installer contents are the same as MAR and ZIP, or something else? We've shipped the past 3 betas (at least) this way.
(In reply to comment #8)
> Is this a packaging bug that should be fixed in the build system, so that
> installer contents are the same as MAR and ZIP, or something else? We've
> shipped the past 3 betas (at least) this way.

I don't see a packaging bug.
If MAR and ZIP archives have different contents in their .chk files, because shlibsign get run again, that's perfectly fine.
On Fedora Linux, we even run the shlibsign as part of the package installation process, so every end user system will have different file contents.
The files are meant to help detect accidental modifications of the library files, as I understand it.

In other words, if you are sure the library files are not modified after generating the .chk files, you should be fine, even if you run shlibsign multiple times and ship different versions in different packaging formats.

Wan-Teh, if this is wrong, please slap me.


(In reply to comment #7)
> Maybe we should add a mode to shlibsign to test the NSS shared
> libraries in FIPS mode.

That's a realy good idea, but nothing we can done quickly, until someone steps up to do it.


> We can also just use the pk11mode test
> program (which is not being built in Mozilla builds right now)
> to do that.

This idea sounds simpler to do.
How would you execute pk11mode and what is the expected output?


pk11mode without any arguments uses the softoken in FIPS mode.
I think you can just check its exit status to determine if the tests pass or not.
We just hit this again in 3.0beta4. 
It is fine for freebl3.chk & softokn3.chk differ between exe & complete mar.

freebl3.chk & softokn3.chk are checksums for freebl3.dll and softokn3.dll.
What's special is that there are multiple (in fact, infinitely many) correct
checksums for each file.  shlibsign is the tool that generate these
checksums, and each time you run shlibsign, it outputs one of the
correct checksums at random.  This is why freebl3.chk & softokn3.chk
differ between exe & complete mar.
Thanks for the nice summary. We keep raising because it gives us false errors when we test updates for a new release (ie the chk files from "3.0b3 updated to 3.0b4" are different from those in an unpacked 3.0b4). 

I think this comes back to being a tinderbox/build packaging problem (boo!); we shouldn't be using the unshipped zip file to make the mar. Bug 313956 is closely related. Alternatively, we could make the signing code replace the chk files in the mar with the ones from the exe.
Component: Build Config → Build & Release
Priority: P1 → P3
Product: Firefox → mozilla.org
QA Contact: build.config → build
Version: Trunk → other
Component: Release Engineering → Release Engineering: Future
QA Contact: build → release
This continued to hit us in all releases from FF3.0b4 -> FF3.0rc3. :-(
(In reply to comment #12)
> It is fine for freebl3.chk & softokn3.chk differ between exe & complete mar.
How about in the partial mar case where app update tries to patch the existing .chk files?
No longer blocks: 470811
Should we just ignore the .chk files in the update verification step (i.e. in check_updates.sh)?
Blocks: 522943
NSS 3.12.4 added another check file - nssdbm3.chk - which has the same problem.
Summary: freebl3.chk & softokn3.chk differ between exe & complete mar for Trunk/Firefox/Win32 → freebl3.chk & softokn3.chk differ between exe & complete mar for Win32 builds (and nssdbm3.chk when using NSS 3.12.4 or later)
Mass move of bugs from Release Engineering:Future -> Release Engineering. See
http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Would this be solved by fixing bug 313956?
(In reply to comment #20)
> Would this be solved by fixing bug 313956?

Yeah, looks like it.
Though I am less concerned about users using zip builds getting a failed partial that causes them to have to download a complete update bug 313956 would just move this problem over to zip build users.
(In reply to comment #22)
> Though I am less concerned about users using zip builds getting a failed
> partial that causes them to have to download a complete update bug 313956 would
> just move this problem over to zip build users.

If we build the MAR out of the EXE we should seriously consider dropping the ZIP format altogether IMHO.
The zip is handy for doing packaged unittests and talos runs.
if we can extract the exe to generate the mar, couldn't we use the same tools to extract the exe for running tests?  Alternatively, could we use the mar file using our mar extractor instead of using unzip.exe?
The installer can be extracted with 7-Zip but the files are separated into non-localized, localized, and optional directories. If it is desirable I can add a command line option to the installer that can take a path to extract the files into single directory. If we were to keep the zip's it would be great if it could be generated from the installer.
A good number of users want to have the zips for automated deployment or for rapidly testing some version because it's much more handy to just unpack and run than to go through a number of dialogs (which is impossible to do automatically) and - in case of testers - to overwrite registry entries of your main installation.
If users of a .zip package are required to use a complete MAR for the first update, I don't think that's a large problem, though.
If our EXEs had a "extract me to a dir" mode wouldn't that get rid of the only advantage of ZIPs?
(In reply to comment #27)
> A good number of users want to have the zips for automated deployment or for
> rapidly testing some version because it's much more handy to just unpack and
> run than to go through a number of dialogs (which is impossible to do
> automatically)
Actually, it is possible to install Firefox (last I tested Thunderbird as well) silently and configure several options as well.
It probably will generate some public outcry until we have educated people, but as long as there is also an option to not touch anything like registry and start menus, i.e. to _only_ extract the files to some specified target, I guess it might work for those people, even if it's more work for them (right-clicking and selecting "Extract" is so much simpler).

And we need to continue to support the option to generate a zip the normal way even if we don't ship it, as it's the default package generation option from toolkit and Mozilla-based apps that don't have an NSIS installer need to have it available.

For Firefox, Thunderbird and SeaMonkey, we should be able to do it, but I'll make sure to direct all complaints to those people who make the decision (SeaMonkey users are usually those shouting loudest at such things and I hate repeatedly pointing out reasons to toolkit decisions I might not fully support myself).
(In reply to comment #30)
> For Firefox, Thunderbird and SeaMonkey, we should be able to do it, but I'll
> make sure to direct all complaints to those people who make the decision
> (SeaMonkey users are usually those shouting loudest at such things and I hate
> repeatedly pointing out reasons to toolkit decisions I might not fully support
> myself).

For the record, I don't think Firefox stopping shipping a ZIP means SeaMonkey or Thunderbird can't ship it anymore. I'm certainly not advocating ripping out all the ZIP code.
Whiteboard: [updates]
Priority: P3 → P5
(In reply to comment #27)
> A good number of users want to have the zips for automated deployment or for
> rapidly testing some version because it's much more handy to just unpack and
> run than to go through a number of dialogs (which is impossible to do
> automatically) 

Scriptable:

mkdir -p tmp/combined
cd tmp
7z x <package>
cp -rp nonlocalized/* combined/
cp -rp localized/* combined/
cp -rp optional/* combined/

and use the files in combined.
Nick, it would be awesome to have for the first time the exact same files in the zip as installed by the installer!
Assignee: nobody → rail
rs, zips are different again. We create the zip, then the installer, then the complete mar. Each time we get different chk files. Many sad faces.

Bug 313956 will unpack the exe to create the mar. It's really tempting to change 'make package' on windows to call 'make installer' then zip up DIST/installer-stage/core/, then stop calling make installer. I gave 0 seconds thought to the consequences of that so there are probably issues.
I briefly had that same temptation a couple weeks ago. I should be able to spend some time making this more sane after I finish my Firefox 4 work in a month or two.
(In reply to comment #35)
> It's really tempting to
> change 'make package' on windows to call 'make installer' then zip up
> DIST/installer-stage/core/

Well, that would mean that anyone wanting a zip needs to have NSIS installed, which might not be what we want for developers out there, even more those of apps outside the Firefox, Thunderbird, SeaMonkey spectrum where we are maintaining NSIS installers and buildbots well enough.
For our buildbot machines, I agree that it would be a good target to have zips, installers, and mars built from a common source without regenerating .chk files or calling packager.mk at all multiple times. Though I might wonder all the same why we can't use the $(DIST)/$(APPNAME) directory for all of them without regenerating it at any point, i.e. also without regenerating the .chk files.
(In reply to comment #37)
> (In reply to comment #35)
> > It's really tempting to
> > change 'make package' on windows to call 'make installer' then zip up
> > DIST/installer-stage/core/
> 
> Well, that would mean that anyone wanting a zip needs to have NSIS installed,
> which might not be what we want for developers out there, even more those of
> apps outside the Firefox, Thunderbird, SeaMonkey spectrum where we are
> maintaining NSIS installers and buildbots well enough.

tbqh, I don't see the problem with that. The NSIS installer is the supported package format for Windows. AFAIK the only reason we've bothered to keep .zip around is because the complete MAR depends on it.

Nothing would stop someone else from maintaining zip packaging code in their specific product.
(In reply to comment #38)
> AFAIK the only reason we've bothered to keep .zip
> around is because the complete MAR depends on it.

AFAIK other main reasons are because 1) we are not alone in the Mozilla universe and not every Mozilla-based app should need to have an NSIS installer fully configured, 2) it shares almost all code with .tar.bz generation anyhow, and 3) because nightly testers *love* installing ZIPs instead of going to more complicated fiddling with the installer (in the case of installing multiple builds and not wanting any registry entries, desktop icons or other - from their perspective - "crap").
MozillaBuild ships with NSIS, and it's the only supported build setup on Windows nowadays. There's no problem there.
(In reply to comment #40)
> MozillaBuild ships with NSIS, and it's the only supported build setup on
> Windows nowadays. There's no problem there.

Ah, right, thanks for the comment, didn't think of that. With that in mind, I guess we probably could safely depend on the installer at least as an intermediate format for generating the other two (MAR and ZIP) package formats.
Depends on: 313956
Bug 313956 landed on m-c and fixed this bug. However it would be better to remove *.chk files from "force" list.
Attached patch patcher-configs — — Splinter Review 
Something like this, but for the 5.0 patcher configs.
Attachment #525683 - Flags: feedback?(nrthomas)
Comment on attachment 525683 [details] [diff] [review]
patcher-configs

We should remove the files from the patcher config creator, too:
http://mxr.mozilla.org/build-central/source/tools/release/patcher-config-creator.pl#21

This means that the MU configs won't force them, but that's fine, because we only serve completes to MUs.
Attached patch update-packaging — — Splinter Review 
If I understood correctly, the current implementation of make_incremental_update.sh forces all *.chk files to be clobbered even if they aren't passed via -f parameter.

Both patches haven't been tested yet.
Attachment #525684 - Flags: feedback?(nrthomas)
See also bug 313956 comment 42. There is no diff between exe and complete mar!
(In reply to comment #44)
> We should remove the files from the patcher config creator, too:
> http://mxr.mozilla.org/build-central/source/tools/release/patcher-config-creator.pl#21

BTW, should I add a special case to ignore getChkFile() results in build/tools/release/signing/{signing.py,sign-release.py} for m-c based builds?
(In reply to comment #47)
> (In reply to comment #44)
> > We should remove the files from the patcher config creator, too:
> > http://mxr.mozilla.org/build-central/source/tools/release/patcher-config-creator.pl#21
> 
> BTW, should I add a special case to ignore getChkFile() results in
> build/tools/release/signing/{signing.py,sign-release.py} for m-c based builds?

Naw, just change it globally. 1.9.1, 1.9.2, and 2.0 don't use this script for minor updates. It only gets used for Major Update and new branch creation.
(In reply to comment #47)
> (In reply to comment #44)
> > We should remove the files from the patcher config creator, too:
> > http://mxr.mozilla.org/build-central/source/tools/release/patcher-config-creator.pl#21
> 
> BTW, should I add a special case to ignore getChkFile() results in
> build/tools/release/signing/{signing.py,sign-release.py} for m-c based builds?

No, that function is called to determine if we should be re-generating a .chk file after signing a .dll. We need to keep doing that.
Attached patch tools — — Splinter Review 

        Attachment #525704 -
        Flags: feedback?(nrthomas)

    
    

    
  
The patches go beyond the original bug. Closing it with bug 649779 as a followup.

Fixed by bug 313956.
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
  

        Attachment #525683 -
        Flags: feedback?(nrthomas)

    
  

        Attachment #525684 -
        Flags: feedback?(nrthomas)

    
  

        Attachment #525704 -
        Flags: feedback?(nrthomas)
Product: mozilla.org → Release Engineering

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: