Closed Bug 489961 Opened 15 years ago Closed 13 years ago

freebl3.dll, softokn3.dll, and nssdmb3.dll aren't Authenticode signed

Categories

(Release Engineering :: General, defect, P5)

Product:
Release Engineering
Component:
General
Platform:
x86
Windows XP
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: pjemen, Assigned: catlee)

References

Details

(Whiteboard: [signing][oldbug][approved-patches-landed])

Attachments

(3 files, 3 obsolete files)

[checked in] in theory this would build modutil
769 bytes, patch
ted
: review+
benjamin
: approval2.0+
Details | Diff | Splinter Review
[unbitrotted] regenerate chk files after signing dlls
17.99 KB, patch
nthomas
: review+
catlee
: checked-in+
Details | Diff | Splinter Review
Fix tests
1.07 KB, patch
bhearsum
: review+
catlee
: checked-in+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.9) Gecko/2009040821 Firefox/3.0.9 (.NET CLR 3.5.30729) XPCOMViewer/1.0a1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.9) Gecko/2009040821 Firefox/3.0.9 (.NET CLR 3.5.30729) XPCOMViewer/1.0a1

freebl3.dll isn't digitally signed 

Reproducible: Always
See also bug 352555 comment 15, bug 483922 comment 9.
Status: UNCONFIRMED → NEW
Component: General → Release Engineering
Ever confirmed: true
Product: Firefox → mozilla.org
QA Contact: general → release
Version: unspecified → other
Component: Release Engineering → Release Engineering: Future
To the explanation given in bug 352555 comment 15 I would add that, beginning 
in NSS 3.12.4, there is one additional shared library that has a .chk file 
produced by shlibsign.  The additional chk file is nssdbm3.chk .
(In reply to comment #2)
Thanks for that information Nelson. 

RelEng: Firefox 3.5b4 is using NSS 3.12.3 so we're OK there; mozilla-central has 3.12.4 Beta.
Summary: freebl3.dll isn't digitally signed → freebl3.dll and softokn3.dll aren't digitally signed
For ideas on how to fix this, see bug 521878 comment 29.
nssdbm3.chk is required for FIPS using NSS 3.12.4, which is Firefox 3.5 and later (mozilla-1.9.2 repo), so that's another file we no longer Authenticode sign (bug 521878).
Summary: freebl3.dll and softokn3.dll aren't digitally signed → freebl3.dll, softokn3.dll, and nssdmb3.dll aren't Authenticode signed
(In reply to bug 521878 comment #29)
> In essence, you can regard shlibsign as just another tool, like another 
> compiler or post processor of some kind.  You can build it once, and keep 
> that one built copy and use it over and over for days, weeks, months, years
> on end.  shlibsign itself hasn't changed in a long time.  You could use a 
> copy built (say) a year ago and it would work just fine today.  

shlibsign changing on us is the only reservation I have to this plan. Are there any ways we could get notified if there are changes in the future ?
Let me say some things to assuage your reservations.
1. shlibsign exists for one reason only: to produce "signatures" required 
to make NSS binaries pass the FIPS 140 tests.  
2. The FIPS 140 evaluations are done about every 2 years, on average, and 
between evaluations, the code is frozen.  NSS just went into a code freeze,
having just been evaluated.  This means that shlibsign won't change until 
the next time NSS goes into FIPS evaluation, probably 1-2 years from now.
3. Even then, shlibsign doesn't necessarily change for each evaluation. 
It only changes if/when the FIPS 140 signature requirements change.  
They haven't changed in a LONG time.  They've been the same for several 
evaluations now.  
4. If and when it changes, even if we forget to tell you, you'll find out
real quick, because the signatures produced by your (then) old shlibsign
will no longer work, and the (then) new NSS will fail to come up in FIPS 
mode.  But the solution will be to simply replace the (then) old shlibsign
with the (then) new one.  But I think there will be lots of "heads up" 
about impending FIPS evaluations, so I wouldn't worry about it.
(In reply to comment #6)
> shlibsign changing on us is the only reservation I have to this plan. Are there
> any ways we could get notified if there are changes in the future ?

Sorry for eavesdropping.  Sounds like:

- you can use any recent-enough shlibsign
- shlibsign is built every build
- you're worried about whether the shlibsign used is, in fact, recent enough.

Why not upload shlibsign with the build, and download it with the build at signing time, and use that?
(In reply to comment #8)
> Why not upload shlibsign with the build, and download it with the build at
> signing time, and use that?

You're welcome to stick it in the test package, for example, if you'd like. (We already have a few other NSS utils in there.)
Guys: i have a changeset which I'm just about ready to add to a bug and try to land (obviously it'll need reviews, etc.). What it will do is cause Firefox (or equivalent) to refuse to load libraries which are not authenticode signed if Firefox is signed. As such, this means that nightlies would have https support until this bug is fixed, but releases would not.

Since this seems to be "simply" a matter of copying over shlibsign from a known good release, or from the release we're making. I'd like to see about pushing this eagerly.

Nelson's comments seem to indicate we shouldn't be very worried about updates to shlibsign, and I think that we could probably come to an agreement that *if* shlibsign changes interestingly, the NSS team could inform the build team so that they could deal with it.

Can we start by trying aki/luser's approach from comment 9.
If shlibsign isn't going to change often, I'm ok with having a pre-built version on the signing machine.
Assignee: nobody → catlee
Component: Release Engineering: Future → Release Engineering
Priority: -- → P3
Does shlibsign differ between 1.9.1, 1.9.2 and trunk?
Nelson says in comment 7 that it hasn't changed in a long time.
What is the proper invocation of shlibsign to regenerate the .chk files?
You can search for shlibsign in the Mozilla source tree for the
command line:
http://mxr.mozilla.org/mozilla-central/search?string=shlibsign

The existing makefile code to look at is here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/installer/packager.mk#213

The SIGN_NSS command is what you want.
Having problems getting shlibsign working on the signing machine.  Running without arguments gives the help message, running on a dll causes a crash.

What are the dependencies of this executable?
there would be, yes. depends from http://dependencywalker.com will tell you what they are (you can use depends on both a working system and your non working system for comparison).
shlibsign.exe depends on the following NSPR and NSS DLLs:
  nspr4.dll
  plc4.dll
  plds4.dll

  softokn3.dll
  freebl3.dll
  sqlite3.dll
  nssutil3.dll
how do I verify that the re-generated .chk files are valid?
http://people.mozilla.org/~catlee/Firefox%20Setup%203.6%20RC%202-chksigntest.exe

contains a 3.6 build where all of the dlls have been authenticode signed, and then the .chk files have been regenerated.

Can somebody verify the output is correct?
These are the changes I made to generate the package above.

Assuming the build has been signed correctly, I think this is done.
Attachment #423533 - Flags: review?(nrthomas)
Chris: shlibsign doesn't have a command-line option to verify
the generated chk files.  I believe the only way to verify
the generated chk files are correct is to actually run an
NSS-based program (such as Firefox) and see if you can put
it in FIPS mode.

You can consider doing some sanity checks, such as expected
file sizes and the first few bytes (which specify the version
of the file format and the type of the public key).  Here
are the first 12 bytes of a chk file:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/cmd/shlibsign/shlibsign.c&rev=1.18&mark=1029-1035#1021

Since we don't verify the generated chk files elsewhere, you
don't need to verify them.
Comment on attachment 423533 [details] [diff] [review]
regenerate chk files after signing dlls

This looks fine for normal case where we only sign once, but when resigning the hash storage is guarded by a shouldSign(), so we won't hash the chk files and fail later.

>diff --git a/release/signing/sign-release.py b/release/signing/sign-release.py
>+        try:
>+            command = ['shlibsign', '-v', '-i', basename]
>+            check_call(command, cwd=dirname, stdout=stdout, stderr=STDOUT)
>+            stdout.seek(0)
>+            data = stdout.read()
>+            # TODO: Check result

What did you have in mind for the TODO, given the 'except:' will catch a non-zero exit code ?
Attachment #423533 - Flags: review?(nrthomas) → review-

        Attachment #423533 -
        Attachment is obsolete: true

    
    

    
  
(In reply to comment #23)
> (From update of attachment 423533 [details] [diff] [review])
> This looks fine for normal case where we only sign once, but when resigning the
> hash storage is guarded by a shouldSign(), so we won't hash the chk files and
> fail later.

Good catch, that should be fixed now.

> >diff --git a/release/signing/sign-release.py b/release/signing/sign-release.py
> >+        try:
> >+            command = ['shlibsign', '-v', '-i', basename]
> >+            check_call(command, cwd=dirname, stdout=stdout, stderr=STDOUT)
> >+            stdout.seek(0)
> >+            data = stdout.read()
> >+            # TODO: Check result
> 
> What did you have in mind for the TODO, given the 'except:' will catch a
> non-zero exit code ?

Check the command's output.  signcode sometimes fails even though it returns 0, so I wanted to grab some string that shlibsign outputs and test for that.  Also fixed.

    
  

        Attachment #424257 -
        Flags: review?(nrthomas)

    
    

    
  
Comment on attachment 424257 [details] [diff] [review]
regenerate chk files after signing dlls

Looks good. We just need to document somewhere where shlibsign comes from and the deps it requires.

        Attachment #424257 -
        Flags: review?(nrthomas) → review+

    
    

    
  
(In reply to comment #22)
> Chris: shlibsign doesn't have a command-line option to verify
> the generated chk files.  I believe the only way to verify
> the generated chk files are correct is to actually run an
> NSS-based program (such as Firefox) and see if you can put
> it in FIPS mode.

It would be really awesome if there was a way to verify at the command-line. 

> Since we don't verify the generated chk files elsewhere, you
> don't need to verify them.

I disagree. This is exactly the sort of situation where an automated check would be extremely valuable.

    
    

    
  
> It would be really awesome if there was a way to verify at the command-line. 

NSS's modutil utility program could be used for this purpose.  
Is that even built in Firefox builds?  (I have no idea)

    
  
Assignee: catlee → nobody

    
    

    
  


  

        Attachment #430594 -
        Flags: review?(ted.mielczarek)

    
    

    
  
Comment on attachment 430594 [details] [diff] [review]
[checked in] in theory this would build modutil

Seems reasonable.

        Attachment #430594 -
        Flags: review?(ted.mielczarek) → review+
Whiteboard: [signing]

    
    

    
  
A user reported in bug 575117 that this made Firefox unusable - Norton Internet Security 2010 apparently quarantined those 3 dll's because they were not signed.

    
    

    
  
I have emailed symantec about this...

    
    

    
  
(In reply to comment #35)
> I have emailed symantec about this...

should be fixed now:

A Symantec Employee posted a Reply in a Forum (http://tinyurl.com/2ep3bun

> The issues with Firefox 3.6.6 should now be resolved.  You do NOT need to run LiveUpdate to get the fix - this was a back-end change that you should see reflected automatically in the next few hours. Thanks to everyone for alerting us to the problem and for being patient as we worked to fix it.  There were some technical irregularities with this Firefox release that caused Norton to make the detection, and we are following up with Mozilla to ensure this doesn't happen again.

    
    

    
  
LOL.  FF 3.6.6 was identical to all previous FF 3.x releases in this regard.
I look forward to seeing *ANY* signs that they're "following up" with Mozilla.

    
    

    
  
If this particular bug is a longstanding dup, the core issue has popped up before, suggesting the error/cause is recurrent.  While we're grateful that Mozilla folks contacted Symantec to expediently resolve the issue (again), this problem speaks of the lack of professional communication and testing between Symantec and Mozilla before update/revision release of browser AND security programs.

The impetus to prevent recurrence of similar bugs will probably require Mozilla volunteer bug-stampers to take the initiative, to forge and maintain a strong working liaison with Norton.  

Nelson's comment rings true: my own experience with the Norton technical support staff was bleak.  Three-for-3, I had to figure out and find fixes for OS-NIS conflicts in order to resolve them.

    
    

    
  
FWIW, now that this issue has been brought to my attention I am working directly with Symantec to resolve it so the same issue doesn't happen for the next release.

I already work with the AV vendors, notifying them of new releases, etc.

    
    

    
  
Nelson,

Please take a look at incident 575116 filed by me (and attached to this thread 6 post up), as proof that Symantec is "following up" on the issue.  Also we have had a longstanding and highly frustrating discussion with Kevin Needham at Mozilla on this topic, who frequently fails to respond to emails and phone calls.  Perhaps we have been speaking to the wrong person, but Symantec is HIGHLY motivated to resolve this problem ASAP.

    
    

    
  
Jeff, I can be a more responsive contact if needed. I manage Firefox branch releases, so I should probably be in the know in any case. Ping me via email and we'll talk specifics.

    
    

    
  
Nelson: you're not really helping here, and just putting well intentioned people like Jeff on-tilt. Please stop.

Jeff: we know that Adobe's interested, and I'm pretty sure that Kev and Christian have both been and will continue to be responsive. Christian is our branch release manager, and should be included in discussions about upcoming releases. Kev is, as you know, a dedicated friend to all companies that Mozilla interacts with, and can help us figure out how to smooth this process out in the future and how we can try to develop our products in harmony.

I'm glad to see that the connections have been made, and this issue will race towards resolution!

    
    

    
  
Thanks Christian and Mike, I will follow up via email to discuss the details.  Also, I'd like to specifically apologize to Kevin for my earlier comments.  My characterization was out of line and very unfair to him.  We have spoken via email and I hope we have cleared things up.

    
    

    
  
Totally wasn't clear, but I meant that I could be a second point of contact if Kev is not available (as people can be at times). I wasn't trying to take the place of Kev, merely provide a clear second communication channel when it comes to releases :-)

    
  
Assignee: nobody → catlee

    
    

    
  
Comment on attachment 430594 [details] [diff] [review]
[checked in] in theory this would build modutil

It does indeed.

How do I use modutil to verify that a .chk file is valid?

    
  
Priority: P3 → P2

    
  
Whiteboard: [signing] → [signing][oldbug]

    
  
Depends on: 590991

    
    

    
  
Waiting on dep bug
Priority: P2 → P5

    
    

    
  
AFAICT, it seems very unlikely that we will ever again ship a FIPS-140-validated Mozilla-Authenticode-signed set of NSS libraries with Firefox that requires these *.chk files.

My understanding is that the goal is to Authenticode-sign the NSS libraries for FF4.0 release. The *.chk files we will ship with the release will be completely worthless since we're not (and can't) ship a FIPS-validated version of these libraries with the release. We probably need to make some different changes to handle this gracefully, but AFAICT no matter what we do, these *.chk files will not be an issue for FF4.0 release.

Please let me know how I can assist you. FWIW, I am in 650 Castro every day.

    
    

    
  
Brian, can't?  
Is this because softoken will be rolled into the same shared lib as PSM in 
FF 4 (I thought it was too late to do that for FF 4 now) or for some other
reason, and if some other, what reason is that?

    
    

    
  
I'm also confused by the 'can't'. It clearly doesn't reflect what we where talking about last week.

> The *.chk files we will ship with the release will be completely
> worthless since we're not (and can't) ship a FIPS-validated version of these
> libraries with the release.

The *.chk files are not worthless. At this point 4.0 still uses shared libraries, and there has been no changes to softoken and freebl and nssdbm that prevents them from checking their .chk files. Whether or not the libraries are actually validated, the still do their integrity checks when in FIPS mode. Try breaking them in a beta an see how many bug reports you get.:)

Even if you remove the FIPS button in FF 4.0 (which would likely cause a support issue), Users who upgrade and already have FIPS on will wind up with a non-functioning browser.

> AFAICT no matter what we do, these *.chk files will not be an issue for FF4.0
> release.

They actually will be. I don't know why one things it's a problem for authenticode. If authenticode modifies the binaries, just create the .chk files after the authenticode signing. If you need the .chk to replace the authenticode, then it's relatively easy to turn them on always (one simple change to softoken will cause all three to check themselves).

bob

    
    

    
  
> The *.chk files we will ship with the release will be completely
> worthless since we're not (and can't) ship a FIPS-validated version of these
> libraries with the release.

Even if we don't ship a FIPS validated version of Softoken with the release, the .chk files aren't worthless because we might later validate the version we ship after we ship it. (Thank you Kai for pointing this out to me.)

Nelson and Robert: my understanding is/was that it won't be acceptable to ship with the currently-validated version of Softoken/FreeBL since we will require a version of Softoken with changes that haven't even been committed yet. I will email you both privately about it.

    
    

    
  
What's the new thinking?
Do Authenticode signing, then regenerate the .chk files?

    
    

    
  
(In reply to comment #52)
> What's the new thinking?
> Do Authenticode signing, then regenerate the .chk files?

Yes.  I'm seeing if I can adapt the steps you suggested in bug 590991 to our signing environment so we can make sure they're valid during the post-signing verification.

    
    

    
  
Comment on attachment 430594 [details] [diff] [review]
[checked in] in theory this would build modutil

This has review, it's been signed off by two build people. We wouldn't automatically ship this file so all we're doing is adding a minor amount of code to the build system w/o affecting the shipping product. (not quite npotb since it's built, but...)

        Attachment #430594 -
        Flags: approval2.0?

    
  

        Attachment #430594 -
        Flags: approval2.0? → approval2.0+

    
  
Keywords: checkin-needed

    
    

    
  
The checkin-needed keyword attracted me here, but it's not immediately clear which of the two patches needs to be pushed. If the second patch depends on the first, the first needs approval before either can be pushed.

Please use the whiteboard to answer these questions for potential volunteer "pushers". Requiring people to look through the comments to figure out this info (if it's even there) means this bug just gets passed over.

    
  
OS: Windows XP → All
Whiteboard: [signing][oldbug] → [signing][oldbug][push-attachment:430594]

    
    

    
  


  
Basically the same as before with the small addition of making sure we compress the re-generated chk file if we're working with a mar file.

Verification works by calling chktest.exe, which kaie has built for us at http://kuix.de/misc/nss-unofficial-fips-chktest.zip

Makefile targets have been updated to get rid of old targets.  Don't need to use USE_NEW any more.

        Attachment #424257 -
        Attachment is obsolete: true

        Attachment #496000 -
        Flags: review?(nrthomas)

    
    

    
  
Comment on attachment 430594 [details] [diff] [review]
[checked in] in theory this would build modutil

http://hg.mozilla.org/mozilla-central/rev/3ca5d6674feb

        Attachment #430594 -
        Attachment description: in theory this would build modutil → [checked in] in theory this would build modutil
Keywords: checkin-needed
Whiteboard: [signing][oldbug][push-attachment:430594] → [signing][oldbug]

    
    

    
  
Was this expected to have caused a ~140K code size increase?

    
    

    
  
Is it just measuring the fact that we're producing an extra binary? (modutil) Because that's all the patch that landed does.

    
    

    
  
Comment on attachment 496000 [details] [diff] [review]
regenerate chk files after signing dlls

>diff --git a/release/signing/Makefile b/release/signing/Makefile

You'll need to merge for changes from bug 618040 (broken context near sign: target).

>diff --git a/release/signing/sign-release.py b/release/signing/sign-release.py
>                             if remember:
>                                 logs.append("Caching compressed %s as %s" % (f, h))
>                                 self.rememberFile(h, f)
>+                                # Remember any regenerated chk files
>+                                if chk:
>+                                    logs.append("Caching %s as %s" % (chk, h))

Nit: 'Caching compressed %s...'

>diff --git a/release/signing/verify-signature.py b/release/signing/verify-signature.py
>+        # We need to wait for all the .chk and their dlls to be unpacked /
>+        # uncompressed before checking them
>+        for f in chkfiles:
>+            d = os.path.dirname(f)
>+            b = os.path.basename(f)
>+            nullfd = open(os.devnull, "w")
>+            cmd = ['chktest', b.replace(".chk", ".dll")]
>+            log.debug("Checking chk file %s" % cmd)

Can this go now you've finished debugging ?

Looks good otherwise.

        Attachment #496000 -
        Flags: review?(nrthomas) → review+

    
    

    
  
(In reply to comment #60)
> >+            log.debug("Checking chk file %s" % cmd)
> Can this go now you've finished debugging ?

Oops, missed that this is hidden unless you -v the call so that's fine. Initially it just looked more verbose than all the other verification calls.

    
  
OS: All → Windows XP

    
    

    
  
We should make sure that the CompanyName property of the executables (in the resource file) is consistent with the signature. See bug 489961. Right now, the resource files contain "Mozilla Foundation" whereas the cert will presumably be issued to "Mozilla Corporation."
Depends on: 632631
See Also:  → 632631

    
    

    
  


  
Needs another run through testing at this point, it's been a long time!

        Attachment #496000 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 513287 [details] [diff] [review]
[unbitrotted] regenerate chk files after signing dlls

I ran the output of this through a staging run of 3.6.14 build 2 updates and update_verify for all platforms.  updates got generated successfully, and update_verify worked, with some (ignorable) problems:

- mac update_verify failed to download 3.6.14 build 1 files for some locales. I believe this is due to the locales removed in https://bugzilla.mozilla.org/show_bug.cgi?id=629256

- win32 update_verify had lots of warnings about "WARN: partial updates are the same size as complete updates, this should only happen for major updates on my update_verify runs". This is due to releaseConfig['testOlderPartials'] = True

        Attachment #513287 -
        Flags: review?(nrthomas)

    
    

    
  
Comment on attachment 513287 [details] [diff] [review]
[unbitrotted] regenerate chk files after signing dlls

r+ based on review of attachment 496000 [details] [diff] [review] and the short interdiff with this patch.

        Attachment #513287 -
        Flags: review?(nrthomas) → review+

    
    

    
  
(bookkeeping)
Whiteboard: [signing][oldbug] → [signing][oldbug][approved-patches-landed]

    
    

    
  
Comment on attachment 513287 [details] [diff] [review]
[unbitrotted] regenerate chk files after signing dlls

http://hg.mozilla.org/build/tools/rev/8d3279cac2ed

        Attachment #513287 -
        Flags: checked-in+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Fix tests
        — 
          — Splinter Review
      

    


  

        Attachment #521863 -
        Flags: review?(bhearsum)

        Attachment #521863 -
        Flags: review?(bhearsum) → review+

    
  
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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: