Remove packaged app signature verification code from mozilla-aurora due to incompatibility with system NSS

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: briansmith, Assigned: briansmith)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox19 unaffected, firefox20 unaffected, firefox21+ fixed, firefox22 fixed)

Details

Attachments

(2 attachments)

We do not need packaged app signature verification support in mozilla-aurora. We only need it for mozilla-central and the mozilla-b2g18 trees. That packaged app signature verification code uses a function that did not get added to the NSS release that mozilla-aurora will eventually ship with. This means that the Firefox/Gecko build is broken when the --use-system-nss option is used. We try to make sure that the --use-system-nss build option works for mozilla-beta and definition for releases. The way I've chosen to resolve this is to simply remove all the JAR signature verification code from mozilla-aurora, and leave just a stub for nsIX509CertDB.openSignedJARFileAsync that always fails.

In particular, I removed all the signed packaged app tests and I removed the support for the Firefox Marketplace root cert.
Attachment #728102 - Flags: review?(honzab.moz)
Note that the security/nss portion of the patch was created by doing these steps:

  python client.py update_nss NSS_3_14_3_RTM
  cd security/nss
  patch -p 1 < ../patches/bug-832942.patch

That is, NSS was reset to NSS 3.14.3, and then the patch for bug 832942 was applied on top of the stock NSS release. Note that the patch for bug 832942 does not affect compatibility with --use-system-nss.
Attachment #728104 - Flags: review?(honzab.moz)
Attachment #728104 - Attachment description: Remove the SEC_PKCS7VerifyDetachedSignatureAtTime patch to NSS and replace the code that called it with a stub tha always fails → Remove the SEC_PKCS7VerifyDetachedSignatureAtTime patch to NSS and replace the code that called it with a stub tha always fails, mozilla-aurora only
Is this the same code that verifies signatures of XPI addons?
Will this need to happen after every m-c->m-a merge?
(In reply to Kai Engert (:kaie) from comment #2)
> Is this the same code that verifies signatures of XPI addons?

No. There is an open bug to migrate the old verification logic to this new function (to get the certificate validation off of the main thread), but that hasn't happened yet.

(In reply to Alex Keybl [:akeybl] from comment #3)
> Will this need to happen after every m-c->m-a merge?

No. We're going to add the function to NSS upstream and/or we're going to change Gecko to use a different function so that this won't be an issue.
Attachment #728104 - Flags: review?(honzab.moz) → review+
Attachment #728102 - Flags: review?(honzab.moz) → review+
Comment on attachment 728102 [details] [diff] [review]
Backout the patch that adds support for the Firefox Marketplace root cert, mozilla-aurora only

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Linux distros cannot distribute Firefox that links to their system NSS libraries
Testing completed (on m-c, etc.): n/a
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: None
Attachment #728102 - Flags: approval-mozilla-aurora?
Comment on attachment 728104 [details] [diff] [review]
Remove the SEC_PKCS7VerifyDetachedSignatureAtTime patch to NSS and replace the code that called it with a stub tha always fails, mozilla-aurora only

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: Linux distros cannot distribute Firefox that links against their system NSS
Testing completed (on m-c, etc.): none (mozilla-aurora only patch)
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none
Attachment #728104 - Flags: approval-mozilla-aurora?
Attachment #728102 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #728104 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/7ba9d5f19980
https://hg.mozilla.org/releases/mozilla-aurora/rev/e39e3947febb

Thanks for the review, Honza.

NOTE that this will NOT land on any branches other than aurora, so marking it fixed.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Shouldn't that land on the now aurora 22?
still broken for me on mozilla-central.
clang++ -o nsCertOverrideService.o -c -I../../../../dist/stl_wrappers -I../../../../dist/system_wrappers -include /home/hussam/packages/firefox/src/firefox/config/gcc_hidden.h -DNSS_ENABLE_ECC -DDLL_PREFIX=\"lib\" -DDLL_SUFFIX=\".so\"  -DMOZ_GLUE_IN_PROGRAM -DXPCOM_TRANSLATE_NSGM_ENTRY_POINT=1 -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EXPORTABLE_JS_API -DNO_NSPR_10_SUPPORT  -I/home/hussam/packages/firefox/src/firefox/security/manager/ssl/src -I. -I../../../../dist/include  -I/usr/include/nspr -I/usr/include/nss      -fPIC -Qunused-arguments  -Qunused-arguments -Wall -Wpointer-arith -Woverloaded-virtual -Werror=return-type -Wtype-limits -Wempty-body -Wsign-compare -Wno-invalid-offsetof -Wno-c++0x-extensions -Wno-extended-offsetof -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-mismatched-tags -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector --param=ssp-buffer-size=4 -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -std=gnu++0x -I/home/hussam/packages/firefox/src/firefox/build/unix/headers -pthread -pipe  -DNDEBUG -DTRIMMED -g -Os -freorder-blocks  -fomit-frame-pointer  -Qunused-arguments  -DMOZILLA_CLIENT -include ../../../../mozilla-config.h -MD -MF .deps/nsCertOverrideService.o.pp  /home/hussam/packages/firefox/src/firefox/security/manager/ssl/src/nsCertOverrideService.cpp
nsRecentBadCerts.cpp
/home/hussam/packages/firefox/src/firefox/security/manager/ssl/src/JARSignatureVerification.cpp:589:8: error: use of
      undeclared identifier 'SEC_PKCS7VerifyDetachedSignatureAtTime'
  if (!SEC_PKCS7VerifyDetachedSignatureAtTime(p7_info, certUsageObjectSigner,
       ^
it stops building there. (sorry for the message spam).
Dear Hussam, I'm sorry, but at this time this is expected.

I just wrote an explanation of the situation and sent it to the dev-tech-crypto list, see
http://mozilla.6506.n7.nabble.com/Clarification-regarding-SEC-PKCS7VerifyDetachedSignatureAtTime-td276689.html
You need to log in before you can comment on or make changes to this bug.